From 344aec206c76ca04181f6ab369e0bffa59c19131 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 18 May 2019 23:57:23 -0400 Subject: [PATCH 1/5] Support special styling for wikidata-tagged features --- css/20_map.css | 17 ++++ modules/osm/entity.js | 3 + modules/svg/labels.js | 8 +- modules/svg/points.js | 8 +- modules/svg/tag_classes.js | 5 ++ test/spec/osm/entity.js | 163 +++++++++++++++++++---------------- test/spec/svg/tag_classes.js | 21 +++++ 7 files changed, 143 insertions(+), 82 deletions(-) diff --git a/css/20_map.css b/css/20_map.css index 15a9be96a..a2410d6fb 100644 --- a/css/20_map.css +++ b/css/20_map.css @@ -105,6 +105,7 @@ g.point .stroke { fill: #fff; } + g.qa_error .shadow, g.point .shadow, g.note .shadow { @@ -282,6 +283,22 @@ text.point { opacity: 0.8; } + +/* Wikidata-tagged */ +g.point.tag-wikidata .stroke { + fill: #8e9; +} +.labels-group.halo text.tag-wikidata { + stroke: #8e9; +} +.icon.areaicon-halo.tag-wikidata { + stroke: #9e9; +} +.icon.areaicon.tag-wikidata { + color: #050; +} + + /* Highlighting */ g.point.highlighted .shadow, path.shadow.highlighted { diff --git a/modules/osm/entity.js b/modules/osm/entity.js index 8663cac9e..a99f4bbc6 100644 --- a/modules/osm/entity.js +++ b/modules/osm/entity.js @@ -174,6 +174,9 @@ osmEntity.prototype = { return Object.keys(this.tags).some(osmIsInterestingTag); }, + hasWikidata: function() { + return !!this.tags.wikidata || !!this.tags['brand:wikidata']; + }, isHighwayIntersection: function() { return false; diff --git a/modules/svg/labels.js b/modules/svg/labels.js index d363396a0..e1a400783 100644 --- a/modules/svg/labels.js +++ b/modules/svg/labels.js @@ -156,7 +156,8 @@ export function svgLabels(projection, context) { texts.enter() .append('text') .attr('class', function(d, i) { - return classes + ' ' + labels[i].classes + ' ' + d.id; + var hasWd = d.hasWikidata() ? ' tag-wikidata' : ''; + return classes + ' ' + labels[i].classes + hasWd + ' ' + d.id; }) .merge(texts) .attr('x', get(labels, 'x')) @@ -192,7 +193,10 @@ export function svgLabels(projection, context) { // enter/update icons.enter() .append('use') - .attr('class', 'icon ' + classes) + .attr('class', function(d) { + var hasWd = d.hasWikidata() ? ' tag-wikidata' : ''; + return 'icon ' + classes + hasWd; + }) .attr('width', '17px') .attr('height', '17px') .merge(icons) diff --git a/modules/svg/points.js b/modules/svg/points.js index 3a37a0fbc..22e224d45 100644 --- a/modules/svg/points.js +++ b/modules/svg/points.js @@ -127,11 +127,9 @@ export function svgPoints(projection, context) { .attr('transform', svgPointTransform(projection)) .call(svgTagClasses()); - // Selecting the following implicitly - // sets the data (point entity) on the element - groups.select('.shadow'); - groups.select('.stroke'); - groups.select('.icon') + groups.select('.shadow'); // propagate bound data + groups.select('.stroke'); // propagate bound data + groups.select('.icon') // propagate bound data .attr('xlink:href', function(entity) { var preset = context.presets().match(entity, graph); var picon = preset && preset.icon; diff --git a/modules/svg/tag_classes.js b/modules/svg/tag_classes.js index 12fc4c14d..6f6d2f34c 100644 --- a/modules/svg/tag_classes.js +++ b/modules/svg/tag_classes.js @@ -139,6 +139,11 @@ export function svgTagClasses() { } } + // If this is a wikidata-tagged item, add a class for that.. + if (t.wikidata || t['brand:wikidata']) { + classes.push('tag-wikidata'); + } + return classes.join(' ').trim(); }; diff --git a/test/spec/osm/entity.js b/test/spec/osm/entity.js index 95746e2c5..6926b1afd 100644 --- a/test/spec/osm/entity.js +++ b/test/spec/osm/entity.js @@ -1,69 +1,69 @@ describe('iD.osmEntity', function () { it('returns a subclass of the appropriate type', function () { - expect(iD.Entity({type: 'node'})).be.an.instanceOf(iD.osmNode); - expect(iD.Entity({type: 'way'})).be.an.instanceOf(iD.osmWay); - expect(iD.Entity({type: 'relation'})).be.an.instanceOf(iD.osmRelation); - expect(iD.Entity({id: 'n1'})).be.an.instanceOf(iD.osmNode); - expect(iD.Entity({id: 'w1'})).be.an.instanceOf(iD.osmWay); - expect(iD.Entity({id: 'r1'})).be.an.instanceOf(iD.osmRelation); + expect(iD.osmEntity({type: 'node'})).be.an.instanceOf(iD.osmNode); + expect(iD.osmEntity({type: 'way'})).be.an.instanceOf(iD.osmWay); + expect(iD.osmEntity({type: 'relation'})).be.an.instanceOf(iD.osmRelation); + expect(iD.osmEntity({id: 'n1'})).be.an.instanceOf(iD.osmNode); + expect(iD.osmEntity({id: 'w1'})).be.an.instanceOf(iD.osmWay); + expect(iD.osmEntity({id: 'r1'})).be.an.instanceOf(iD.osmRelation); }); if (iD.debug) { it('is frozen', function () { - expect(Object.isFrozen(iD.Entity())).to.be.true; + expect(Object.isFrozen(iD.osmEntity())).to.be.true; }); it('freezes tags', function () { - expect(Object.isFrozen(iD.Entity().tags)).to.be.true; + expect(Object.isFrozen(iD.osmEntity().tags)).to.be.true; }); } describe('.id', function () { it('generates unique IDs', function () { - expect(iD.Entity.id('node')).not.to.equal(iD.Entity.id('node')); + expect(iD.osmEntity.id('node')).not.to.equal(iD.osmEntity.id('node')); }); describe('.fromOSM', function () { it('returns a ID string unique across entity types', function () { - expect(iD.Entity.id.fromOSM('node', '1')).to.equal('n1'); + expect(iD.osmEntity.id.fromOSM('node', '1')).to.equal('n1'); }); }); describe('.toOSM', function () { it('reverses fromOSM', function () { - expect(iD.Entity.id.toOSM(iD.Entity.id.fromOSM('node', '1'))).to.equal('1'); + expect(iD.osmEntity.id.toOSM(iD.osmEntity.id.fromOSM('node', '1'))).to.equal('1'); }); }); }); describe('#copy', function () { it('returns a new Entity', function () { - var n = iD.Entity({id: 'n'}), - result = n.copy(null, {}); - expect(result).to.be.an.instanceof(iD.Entity); + var n = iD.osmEntity({id: 'n'}); + var result = n.copy(null, {}); + expect(result).to.be.an.instanceof(iD.osmEntity); expect(result).not.to.equal(n); }); it('adds the new Entity to input object', function () { - var n = iD.Entity({id: 'n'}), - copies = {}, - result = n.copy(null, copies); + var n = iD.osmEntity({id: 'n'}); + var copies = {}; + var result = n.copy(null, copies); expect(Object.keys(copies)).to.have.length(1); expect(copies.n).to.equal(result); }); it('returns an existing copy in input object', function () { - var n = iD.Entity({id: 'n'}), - copies = {}, - result1 = n.copy(null, copies), - result2 = n.copy(null, copies); + var n = iD.osmEntity({id: 'n'}); + var copies = {}; + var result1 = n.copy(null, copies); + var result2 = n.copy(null, copies); expect(Object.keys(copies)).to.have.length(1); expect(result1).to.equal(result2); }); it('resets \'id\', \'user\', and \'version\' properties', function () { - var n = iD.Entity({id: 'n', version: 10, user: 'user'}), - copies = {}; + var n = iD.osmEntity({id: 'n', version: 10, user: 'user'}); + var copies = {}; n.copy(null, copies); expect(copies.n.isNew()).to.be.ok; expect(copies.n.version).to.be.undefined; @@ -71,8 +71,8 @@ describe('iD.osmEntity', function () { }); it('copies tags', function () { - var n = iD.Entity({id: 'n', tags: {foo: 'foo'}}), - copies = {}; + var n = iD.osmEntity({id: 'n', tags: {foo: 'foo'}}); + var copies = {}; n.copy(null, copies); expect(copies.n.tags).to.equal(n.tags); }); @@ -80,85 +80,85 @@ describe('iD.osmEntity', function () { describe('#update', function () { it('returns a new Entity', function () { - var a = iD.Entity(), - b = a.update({}); - expect(b instanceof iD.Entity).to.be.true; + var a = iD.osmEntity(); + var b = a.update({}); + expect(b instanceof iD.osmEntity).to.be.true; expect(a).not.to.equal(b); }); it('updates the specified attributes', function () { - var tags = {foo: 'bar'}, - e = iD.Entity().update({tags: tags}); + var tags = {foo: 'bar'}; + var e = iD.osmEntity().update({tags: tags}); expect(e.tags).to.equal(tags); }); it('preserves existing attributes', function () { - var e = iD.Entity({id: 'w1'}).update({}); + var e = iD.osmEntity({id: 'w1'}).update({}); expect(e.id).to.equal('w1'); }); it('doesn\'t modify the input', function () { var attrs = {tags: {foo: 'bar'}}; - iD.Entity().update(attrs); + iD.osmEntity().update(attrs); expect(attrs).to.eql({tags: {foo: 'bar'}}); }); it('doesn\'t copy prototype properties', function () { - expect(iD.Entity().update({})).not.to.have.ownProperty('update'); + expect(iD.osmEntity().update({})).not.to.have.ownProperty('update'); }); it('sets v to 1 if previously undefined', function() { - expect(iD.Entity().update({}).v).to.equal(1); + expect(iD.osmEntity().update({}).v).to.equal(1); }); it('increments v', function() { - expect(iD.Entity({v: 1}).update({}).v).to.equal(2); + expect(iD.osmEntity({v: 1}).update({}).v).to.equal(2); }); }); describe('#mergeTags', function () { it('returns self if unchanged', function () { - var a = iD.Entity({tags: {a: 'a'}}), - b = a.mergeTags({a: 'a'}); + var a = iD.osmEntity({tags: {a: 'a'}}); + var b = a.mergeTags({a: 'a'}); expect(a).to.equal(b); }); it('returns a new Entity if changed', function () { - var a = iD.Entity({tags: {a: 'a'}}), - b = a.mergeTags({a: 'b'}); - expect(b instanceof iD.Entity).to.be.true; + var a = iD.osmEntity({tags: {a: 'a'}}); + var b = a.mergeTags({a: 'b'}); + expect(b instanceof iD.osmEntity).to.be.true; expect(a).not.to.equal(b); }); it('merges tags', function () { - var a = iD.Entity({tags: {a: 'a'}}), - b = a.mergeTags({b: 'b'}); + var a = iD.osmEntity({tags: {a: 'a'}}); + var b = a.mergeTags({b: 'b'}); expect(b.tags).to.eql({a: 'a', b: 'b'}); }); it('combines non-conflicting tags', function () { - var a = iD.Entity({tags: {a: 'a'}}), - b = a.mergeTags({a: 'a'}); + var a = iD.osmEntity({tags: {a: 'a'}}); + var b = a.mergeTags({a: 'a'}); expect(b.tags).to.eql({a: 'a'}); }); it('combines conflicting tags with semicolons', function () { - var a = iD.Entity({tags: {a: 'a'}}), - b = a.mergeTags({a: 'b'}); + var a = iD.osmEntity({tags: {a: 'a'}}); + var b = a.mergeTags({a: 'b'}); expect(b.tags).to.eql({a: 'a;b'}); }); it('combines combined tags', function () { - var a = iD.Entity({tags: {a: 'a;b'}}), - b = iD.Entity({tags: {a: 'b'}}); + var a = iD.osmEntity({tags: {a: 'a;b'}}); + var b = iD.osmEntity({tags: {a: 'b'}}); expect(a.mergeTags(b.tags).tags).to.eql({a: 'a;b'}); expect(b.mergeTags(a.tags).tags).to.eql({a: 'b;a'}); }); it('combines combined tags with whitespace', function () { - var a = iD.Entity({tags: {a: 'a; b'}}), - b = iD.Entity({tags: {a: 'b'}}); + var a = iD.osmEntity({tags: {a: 'a; b'}}); + var b = iD.osmEntity({tags: {a: 'b'}}); expect(a.mergeTags(b.tags).tags).to.eql({a: 'a;b'}); expect(b.mergeTags(a.tags).tags).to.eql({a: 'b;a'}); @@ -167,24 +167,24 @@ describe('iD.osmEntity', function () { describe('#osmId', function () { it('returns an OSM ID as a string', function () { - expect(iD.Entity({id: 'w1234'}).osmId()).to.eql('1234'); - expect(iD.Entity({id: 'n1234'}).osmId()).to.eql('1234'); - expect(iD.Entity({id: 'r1234'}).osmId()).to.eql('1234'); + expect(iD.osmEntity({id: 'w1234'}).osmId()).to.eql('1234'); + expect(iD.osmEntity({id: 'n1234'}).osmId()).to.eql('1234'); + expect(iD.osmEntity({id: 'r1234'}).osmId()).to.eql('1234'); }); }); describe('#intersects', function () { it('returns true for a way with a node within the given extent', function () { - var node = iD.osmNode({loc: [0, 0]}), - way = iD.osmWay({nodes: [node.id]}), - graph = iD.coreGraph([node, way]); + var node = iD.osmNode({loc: [0, 0]}); + var way = iD.osmWay({nodes: [node.id]}); + var graph = iD.coreGraph([node, way]); expect(way.intersects([[-5, -5], [5, 5]], graph)).to.equal(true); }); it('returns false for way with no nodes within the given extent', function () { - var node = iD.osmNode({loc: [6, 6]}), - way = iD.osmWay({nodes: [node.id]}), - graph = iD.coreGraph([node, way]); + var node = iD.osmNode({loc: [6, 6]}); + var way = iD.osmWay({nodes: [node.id]}); + var graph = iD.coreGraph([node, way]); expect(way.intersects([[-5, -5], [5, 5]], graph)).to.equal(false); }); }); @@ -208,59 +208,72 @@ describe('iD.osmEntity', function () { describe('#hasParentRelations', function () { it('returns true for an entity that is a relation member', function () { - var node = iD.osmNode(), - relation = iD.osmRelation({members: [{id: node.id}]}), - graph = iD.coreGraph([node, relation]); + var node = iD.osmNode(); + var relation = iD.osmRelation({members: [{id: node.id}]}); + var graph = iD.coreGraph([node, relation]); expect(node.hasParentRelations(graph)).to.equal(true); }); it('returns false for an entity that is not a relation member', function () { - var node = iD.osmNode(), - graph = iD.coreGraph([node]); + var node = iD.osmNode(); + var graph = iD.coreGraph([node]); expect(node.hasParentRelations(graph)).to.equal(false); }); }); describe('#hasDeprecatedTags', function () { it('returns false if entity has no tags', function () { - expect(iD.Entity().deprecatedTags()).to.eql([]); + expect(iD.osmEntity().deprecatedTags()).to.eql([]); }); it('returns true if entity has deprecated tags', function () { - expect(iD.Entity({ tags: { amenity: 'toilet' } }).deprecatedTags()).to.eql([{ - old: { amenity: 'toilet' }, - replace: { amenity: 'toilets' } - }]); + expect(iD.osmEntity({ tags: { amenity: 'toilet' } }).deprecatedTags()).to.eql( + [{ old: { amenity: 'toilet' }, replace: { amenity: 'toilets' } }] + ); + }); + }); + + describe('#hasWikidata', function () { + it('returns false if entity has no tags', function () { + expect(iD.osmEntity().hasWikidata()).to.be.not.ok; + }); + + it('returns true if entity has a wikidata tag', function () { + expect(iD.osmEntity({ tags: { wikidata: 'Q18275868' } }).hasWikidata()).to.be.ok; + }); + + it('returns true if entity has a brand:wikidata tag', function () { + expect(iD.osmEntity({ tags: { 'brand:wikidata': 'Q18275868' } }).hasWikidata()).to.be.ok; }); }); describe('#hasInterestingTags', function () { it('returns false if the entity has no tags', function () { - expect(iD.Entity().hasInterestingTags()).to.equal(false); + expect(iD.osmEntity().hasInterestingTags()).to.equal(false); }); it('returns true if the entity has tags other than \'attribution\', \'created_by\', \'source\', \'odbl\' and tiger tags', function () { - expect(iD.Entity({tags: {foo: 'bar'}}).hasInterestingTags()).to.equal(true); + expect(iD.osmEntity({tags: {foo: 'bar'}}).hasInterestingTags()).to.equal(true); }); it('return false if the entity has only uninteresting tags', function () { - expect(iD.Entity({tags: {source: 'Bing'}}).hasInterestingTags()).to.equal(false); + expect(iD.osmEntity({tags: {source: 'Bing'}}).hasInterestingTags()).to.equal(false); }); it('return false if the entity has only tiger tags', function () { - expect(iD.Entity({tags: {'tiger:source': 'blah', 'tiger:foo': 'bar'}}).hasInterestingTags()).to.equal(false); + expect(iD.osmEntity({tags: {'tiger:source': 'blah', 'tiger:foo': 'bar'}}).hasInterestingTags()).to.equal(false); }); }); describe('#isHighwayIntersection', function () { it('returns false', function () { - expect(iD.Entity().isHighwayIntersection()).to.be.false; + expect(iD.osmEntity().isHighwayIntersection()).to.be.false; }); }); describe('#isDegenerate', function () { it('returns true', function () { - expect(iD.Entity().isDegenerate()).to.be.true; + expect(iD.osmEntity().isDegenerate()).to.be.true; }); }); diff --git a/test/spec/svg/tag_classes.js b/test/spec/svg/tag_classes.js index 2df7caeb1..267d9347d 100644 --- a/test/spec/svg/tag_classes.js +++ b/test/spec/svg/tag_classes.js @@ -185,6 +185,27 @@ describe('iD.svgTagClasses', function () { expect(selection.classed('tag-unpaved')).to.be.false; }); + it('does not add tag-wikidata if no wikidata tag', function() { + selection + .datum(iD.osmEntity()) + .call(iD.svgTagClasses()); + expect(selection.classed('tag-wikidata')).to.be.false; + }); + + it('adds tag-wikidata if entity has a wikidata tag', function() { + selection + .datum(iD.osmEntity({ tags: { wikidata: 'Q18275868' } })) + .call(iD.svgTagClasses()); + expect(selection.classed('tag-wikidata')).to.be.true; + }); + + it('adds tag-wikidata if entity has a brand:wikidata tag', function() { + selection + .datum(iD.osmEntity({ tags: { 'brand:wikidata': 'Q18275868' } })) + .call(iD.svgTagClasses()); + expect(selection.classed('tag-wikidata')).to.be.true; + }); + it('adds tags based on the result of the `tags` accessor', function() { var primary = function () { return { highway: 'primary'}; }; selection From e7b3354db714c918959b165ee34cbd50bbb2238e Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 20 May 2019 00:35:41 -0400 Subject: [PATCH 2/5] Render wikidata tagged items with green stroke only --- css/20_map.css | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/css/20_map.css b/css/20_map.css index a2410d6fb..ab36ca76e 100644 --- a/css/20_map.css +++ b/css/20_map.css @@ -285,17 +285,12 @@ text.point { /* Wikidata-tagged */ -g.point.tag-wikidata .stroke { - fill: #8e9; -} -.labels-group.halo text.tag-wikidata { - stroke: #8e9; +g.point.tag-wikidata path.stroke { + stroke-width: 3px; + stroke: #383; } .icon.areaicon-halo.tag-wikidata { - stroke: #9e9; -} -.icon.areaicon.tag-wikidata { - color: #050; + stroke: #6c6; } From 6eb73e7de6765fe434a6553b22273463ffabc38c Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 20 May 2019 10:55:23 -0400 Subject: [PATCH 3/5] Don't adjust map labels for wikidata tagged items --- modules/svg/labels.js | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/modules/svg/labels.js b/modules/svg/labels.js index e1a400783..d363396a0 100644 --- a/modules/svg/labels.js +++ b/modules/svg/labels.js @@ -156,8 +156,7 @@ export function svgLabels(projection, context) { texts.enter() .append('text') .attr('class', function(d, i) { - var hasWd = d.hasWikidata() ? ' tag-wikidata' : ''; - return classes + ' ' + labels[i].classes + hasWd + ' ' + d.id; + return classes + ' ' + labels[i].classes + ' ' + d.id; }) .merge(texts) .attr('x', get(labels, 'x')) @@ -193,10 +192,7 @@ export function svgLabels(projection, context) { // enter/update icons.enter() .append('use') - .attr('class', function(d) { - var hasWd = d.hasWikidata() ? ' tag-wikidata' : ''; - return 'icon ' + classes + hasWd; - }) + .attr('class', 'icon ' + classes) .attr('width', '17px') .attr('height', '17px') .merge(icons) From 06bdfbfb8ab5dc7fb05dc3f9197d2659782fb434 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 20 May 2019 15:36:10 -0400 Subject: [PATCH 4/5] Use gray stroke on marker and lock icon on field for locked item --- build_data.js | 1 + css/20_map.css | 6 ++--- css/80_app.css | 9 ++++++++ modules/ui/field.js | 24 +++++++++++--------- modules/ui/fields/input.js | 21 ++++++++++++----- modules/ui/fields/localized.js | 41 +++++++++++++++++++++++++--------- svg/fontawesome/fas-lock.svg | 1 + 7 files changed, 73 insertions(+), 30 deletions(-) create mode 100644 svg/fontawesome/fas-lock.svg diff --git a/build_data.js b/build_data.js index b55c8154b..e92125de8 100644 --- a/build_data.js +++ b/build_data.js @@ -55,6 +55,7 @@ module.exports = function buildData() { // Font Awesome icons used var faIcons = { 'fas-i-cursor': {}, + 'fas-lock': {}, 'fas-long-arrow-alt-right': {}, 'fas-th-list': {} }; diff --git a/css/20_map.css b/css/20_map.css index ab36ca76e..1c89fb588 100644 --- a/css/20_map.css +++ b/css/20_map.css @@ -287,10 +287,8 @@ text.point { /* Wikidata-tagged */ g.point.tag-wikidata path.stroke { stroke-width: 3px; - stroke: #383; -} -.icon.areaicon-halo.tag-wikidata { - stroke: #6c6; + stroke: #777; + fill: #ddd; } diff --git a/css/80_app.css b/css/80_app.css index 5c0e8a0c4..7850c7182 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -1528,6 +1528,15 @@ button.preset-favorite-button.active .icon { padding: 5px 10px 5px 0; } +.label-text .label-textannotation svg.icon { + margin: 0 10px; + color: #333; + opacity: 0.5; + width: 14px; + height: 14px; + vertical-align: text-top; +} + .field-label button { flex: 0 0 32px; border-left: 1px solid #ccc; diff --git a/modules/ui/field.js b/modules/ui/field.js index 3a6dc5aa8..7ba6c9743 100644 --- a/modules/ui/field.js +++ b/modules/ui/field.js @@ -1,9 +1,5 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; - -import { - event as d3_event, - select as d3_select -} from 'd3-selection'; +import { event as d3_event, select as d3_select } from 'd3-selection'; import { t } from '../util/locale'; import { textDirection } from '../util/locale'; @@ -117,18 +113,26 @@ export function uiField(context, presetField, entity, options) { .classed('nowrap', !options.wrap); if (options.wrap) { - var label = enter + var labelEnter = enter .append('label') .attr('class', 'field-label') .attr('for', function(d) { return 'preset-input-' + d.safeid; }); - label + var textEnter = labelEnter .append('span') - .attr('class', 'label-text') + .attr('class', 'label-text'); + + textEnter + .append('span') + .attr('class', 'label-textvalue') .text(function(d) { return d.label(); }); + textEnter + .append('span') + .attr('class', 'label-textannotation'); + if (options.remove) { - label + labelEnter .append('button') .attr('class', 'remove-icon') .attr('title', t('icons.remove')) @@ -137,7 +141,7 @@ export function uiField(context, presetField, entity, options) { } if (options.revert) { - label + labelEnter .append('button') .attr('class', 'modified-icon') .attr('title', t('icons.undo')) diff --git a/modules/ui/fields/input.js b/modules/ui/fields/input.js index 782875a95..1da4bc639 100644 --- a/modules/ui/fields/input.js +++ b/modules/ui/fields/input.js @@ -1,8 +1,5 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; -import { - select as d3_select, - event as d3_event -} from 'd3-selection'; +import { select as d3_select, event as d3_event } from 'd3-selection'; import { t, textDirection } from '../../util/locale'; import { dataPhoneFormats } from '../../../data'; @@ -113,7 +110,21 @@ export function uiFieldText(field, context) { .filter(function(k) { return k !== pKey && k !== 'name'; }); } - wrap.call(isSuggestion ? _brandTip : _brandTip.destroy); + selection.call(isSuggestion ? _brandTip : _brandTip.destroy); + + // add a label annotation + var annotation = selection.selectAll('.field-label .label-textannotation'); + var icon = annotation.selectAll('.icon') + .data(isSuggestion ? [0]: []); + + icon.exit() + .remove(); + + icon.enter() + .append('svg') + .attr('class', 'icon') + .append('use') + .attr('xlink:href', '#fas-lock'); } } diff --git a/modules/ui/fields/localized.js b/modules/ui/fields/localized.js index a0c6abc2f..009eb8bb3 100644 --- a/modules/ui/fields/localized.js +++ b/modules/ui/fields/localized.js @@ -1,10 +1,5 @@ import { dispatch as d3_dispatch } from 'd3-dispatch'; - -import { - select as d3_select, - event as d3_event -} from 'd3-selection'; - +import { select as d3_select, event as d3_event } from 'd3-selection'; import { t } from '../../util/locale'; import { dataWikipedia } from '../../../data'; @@ -97,9 +92,7 @@ export function uiFieldLocalized(field, context) { wrap = wrap.enter() .append('div') .attr('class', 'form-field-input-wrap form-field-input-' + field.type) - .merge(wrap) - .call(_isLocked ? _brandTip : _brandTip.destroy); - + .merge(wrap); input = wrap.selectAll('.localized-main') .data([0]); @@ -197,6 +190,24 @@ export function uiFieldLocalized(field, context) { .attr('readonly', _isLocked || null); + _selection.call(_isLocked ? _brandTip : _brandTip.destroy); + + // add a label annotations if locked + var annotation = selection.selectAll('.field-label .label-textannotation'); + var icon = annotation.selectAll('.icon') + .data(_isLocked ? [0]: []); + + icon.exit() + .remove(); + + icon.enter() + .append('svg') + .attr('class', 'icon') + .append('use') + .attr('xlink:href', '#fas-lock'); + + + // We are not guaranteed to get an `accept` or `cancel` when blurring the field. // (This can happen if the user actives the combo, arrows down, and then clicks off to blur) // So compare the current field value against the suggestions one last time. @@ -404,11 +415,19 @@ export function uiFieldLocalized(field, context) { .append('label') .attr('class', 'field-label'); - label + var text = label .append('span') - .attr('class', 'label-text') + .attr('class', 'label-text'); + + text + .append('span') + .attr('class', 'label-textvalue') .text(t('translate.localized_translation_label')); + text + .append('span') + .attr('class', 'label-textannotation'); + label .append('button') .attr('class', 'remove-icon-multilingual') diff --git a/svg/fontawesome/fas-lock.svg b/svg/fontawesome/fas-lock.svg new file mode 100644 index 000000000..19dfa22ee --- /dev/null +++ b/svg/fontawesome/fas-lock.svg @@ -0,0 +1 @@ + \ No newline at end of file From 69a25fd6aa6b4002d16667067943245d79f3338e Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 20 May 2019 17:50:50 -0400 Subject: [PATCH 5/5] Move locking code from input and localized up to uiField Also adjust styles some more for wikidata tagged items --- css/80_app.css | 8 +++-- modules/ui/field.js | 51 +++++++++++++++++++++++++---- modules/ui/fields/input.js | 41 +++-------------------- modules/ui/fields/localized.js | 56 +++++++++----------------------- test/spec/ui/fields/localized.js | 1 + 5 files changed, 70 insertions(+), 87 deletions(-) diff --git a/css/80_app.css b/css/80_app.css index 7850c7182..2c958b1bf 100644 --- a/css/80_app.css +++ b/css/80_app.css @@ -1557,11 +1557,13 @@ button.preset-favorite-button.active .icon { } .field-label .modified-icon, -.field-label .remove-icon { +.field-label .remove-icon, +.field-label .remove-icon-multilingual { display: none; } -.modified .field-label .modified-icon, -.present .field-label .remove-icon { +.modified:not(.locked) .field-label .modified-icon, +.present:not(.locked) .field-label .remove-icon, +.present:not(.locked) .field-label .remove-icon-multilingual { display: inline-block; } diff --git a/modules/ui/field.js b/modules/ui/field.js index 7ba6c9743..d38fa9450 100644 --- a/modules/ui/field.js +++ b/modules/ui/field.js @@ -4,6 +4,7 @@ import { event as d3_event, select as d3_select } from 'd3-selection'; import { t } from '../util/locale'; import { textDirection } from '../util/locale'; import { svgIcon } from '../svg/icon'; +import { tooltip } from '../util/tooltip'; import { uiFieldHelp } from './field_help'; import { uiFields } from './fields'; import { uiTagReference } from './tag_reference'; @@ -25,6 +26,12 @@ export function uiField(context, presetField, entity, options) { var _state = ''; var _tags = {}; + var _locked = false; + var _lockedTip = tooltip() + .title(t('inspector.lock.suggestion', { label: field.label })) + .placement('bottom'); + + field.keys = field.keys || [field.key]; // only create the fields that are actually being shown @@ -77,7 +84,7 @@ export function uiField(context, presetField, entity, options) { function revert(d) { d3_event.stopPropagation(); d3_event.preventDefault(); - if (!entity) return false; + if (!entity || _locked) return; var original = context.graph().base().entities[entity.id]; var t = {}; @@ -92,6 +99,7 @@ export function uiField(context, presetField, entity, options) { function remove(d) { d3_event.stopPropagation(); d3_event.preventDefault(); + if (_locked) return; var t = {}; d.keys.forEach(function(key) { @@ -162,9 +170,9 @@ export function uiField(context, presetField, entity, options) { .on('click', revert); container - .classed('modified', isModified()) - .classed('present', isPresent()) .each(function(d) { + var selection = d3_select(this); + if (!d.impl) { createField(); } @@ -189,12 +197,12 @@ export function uiField(context, presetField, entity, options) { } } - d3_select(this) + selection .call(d.impl); // add field help components if (help) { - d3_select(this) + selection .call(help.body) .select('.field-label') .call(help.button); @@ -202,7 +210,7 @@ export function uiField(context, presetField, entity, options) { // add tag reference components if (reference) { - d3_select(this) + selection .call(reference.body) .select('.field-label') .call(reference.button); @@ -210,6 +218,29 @@ export function uiField(context, presetField, entity, options) { d.impl.tags(_tags); }); + + + container + .classed('locked', _locked) + .classed('modified', isModified()) + .classed('present', isPresent()); + + + // show a tip and lock icon if the field is locked + var annotation = container.selectAll('.field-label .label-textannotation'); + var icon = annotation.selectAll('.icon') + .data(_locked ? [0]: []); + + icon.exit() + .remove(); + + icon.enter() + .append('svg') + .attr('class', 'icon') + .append('use') + .attr('xlink:href', '#fas-lock'); + + container.call(_locked ? _lockedTip : _lockedTip.destroy); }; @@ -227,6 +258,13 @@ export function uiField(context, presetField, entity, options) { }; + field.locked = function(val) { + if (!arguments.length) return _locked; + _locked = val; + return field; + }; + + field.show = function() { _show = true; if (!field.impl) { @@ -239,7 +277,6 @@ export function uiField(context, presetField, entity, options) { } }; - // A shown field has a visible UI, a non-shown field is in the 'Add field' dropdown field.isShown = function() { return _show || isPresent(); diff --git a/modules/ui/fields/input.js b/modules/ui/fields/input.js index 1da4bc639..7e2f12c80 100644 --- a/modules/ui/fields/input.js +++ b/modules/ui/fields/input.js @@ -21,18 +21,11 @@ export function uiFieldText(field, context) { var nominatim = services.geocoder; var input = d3_select(null); var _entity; - var _brandTip; - - if (field.id === 'brand') { - _brandTip = tooltip() - .title(t('inspector.lock.suggestion', { label: field.label })) - .placement('bottom'); - } - function i(selection) { var preset = _entity && context.presets().match(_entity, context.graph()); - var isSuggestion = preset && preset.suggestion && field.id === 'brand'; + var isLocked = preset && preset.suggestion && field.id === 'brand'; + field.locked(isLocked); var wrap = selection.selectAll('.form-field-input-wrap') .data([0]); @@ -57,8 +50,8 @@ export function uiFieldText(field, context) { .merge(input); input - .classed('disabled', !!isSuggestion) - .attr('readonly', isSuggestion || null) + .classed('disabled', !!isLocked) + .attr('readonly', isLocked || null) .on('input', change(true)) .on('blur', change()) .on('change', change()); @@ -99,32 +92,6 @@ export function uiFieldText(field, context) { input.node().value = vals.join(';'); change()(); }); - - } else if (preset && field.id === 'brand') { - var pTag = preset.id.split('/', 2); - var pKey = pTag[0]; - if (isSuggestion) { - // A "suggestion" preset (brand name) - // Put suggestion keys in `field.keys` so delete button can remove them all. - field.keys = Object.keys(preset.removeTags) - .filter(function(k) { return k !== pKey && k !== 'name'; }); - } - - selection.call(isSuggestion ? _brandTip : _brandTip.destroy); - - // add a label annotation - var annotation = selection.selectAll('.field-label .label-textannotation'); - var icon = annotation.selectAll('.icon') - .data(isSuggestion ? [0]: []); - - icon.exit() - .remove(); - - icon.enter() - .append('svg') - .attr('class', 'icon') - .append('use') - .attr('xlink:href', '#fas-lock'); } } diff --git a/modules/ui/fields/localized.js b/modules/ui/fields/localized.js index 009eb8bb3..20afe4df1 100644 --- a/modules/ui/fields/localized.js +++ b/modules/ui/fields/localized.js @@ -32,10 +32,6 @@ export function uiFieldLocalized(field, context) { var _selection = d3_select(null); var _multilingual = []; - var _isLocked = false; - var _brandTip = tooltip() - .title(t('inspector.lock.suggestion', { label: field.label })) - .placement('bottom'); var _buttonTip = tooltip() .title(t('translate.translate')) .placement('left'); @@ -45,13 +41,13 @@ export function uiFieldLocalized(field, context) { function calcLocked() { if (!_entity) { // the original entity - _isLocked = false; + field.locked(false); return; } var latest = context.hasEntity(_entity.id); if (!latest) { // get current entity, possibly edited - _isLocked = false; + field.locked(false); return; } @@ -62,8 +58,10 @@ export function uiFieldLocalized(field, context) { var showsBrand = preset && preset.fields .filter(function(d) { return d.id === 'brand'; }).length; - _isLocked = !!(field.id === 'name' && hasOriginalName && + var isLocked = !!(field.id === 'name' && hasOriginalName && (hasWikidata || (isSuggestion && !showsBrand))); + + field.locked(isLocked); } @@ -82,6 +80,7 @@ export function uiFieldLocalized(field, context) { function localized(selection) { _selection = selection; calcLocked(); + var isLocked = field.locked(); var entity = _entity && context.hasEntity(_entity.id); // get latest var preset = entity && context.presets().match(entity, context.graph()); @@ -112,13 +111,7 @@ export function uiFieldLocalized(field, context) { var pKey = pTag[0]; var pValue = pTag[1]; - if (preset.suggestion) { - // A "suggestion" preset (brand name) - // Put suggestion keys in `field.keys` so delete button can remove them all. - field.keys = Object.keys(preset.removeTags) - .filter(function(k) { return k !== pKey; }); - - } else { + if (!preset.suggestion) { // Not a suggestion preset - Add a suggestions dropdown if it makes sense to. // This code attempts to determine if the matched preset is the // kind of preset that even can benefit from name suggestions.. @@ -147,8 +140,8 @@ export function uiFieldLocalized(field, context) { } input - .classed('disabled', !!_isLocked) - .attr('readonly', _isLocked || null) + .classed('disabled', !!isLocked) + .attr('readonly', isLocked || null) .on('input', change(true)) .on('blur', change()) .on('change', change()); @@ -165,8 +158,8 @@ export function uiFieldLocalized(field, context) { .merge(translateButton); translateButton - .classed('disabled', !!_isLocked) - .call(_isLocked ? _buttonTip.destroy : _buttonTip) + .classed('disabled', !!isLocked) + .call(isLocked ? _buttonTip.destroy : _buttonTip) .on('click', addNew); @@ -186,25 +179,8 @@ export function uiFieldLocalized(field, context) { .call(renderMultilingual); localizedInputs.selectAll('button, input') - .classed('disabled', !!_isLocked) - .attr('readonly', _isLocked || null); - - - _selection.call(_isLocked ? _brandTip : _brandTip.destroy); - - // add a label annotations if locked - var annotation = selection.selectAll('.field-label .label-textannotation'); - var icon = annotation.selectAll('.icon') - .data(_isLocked ? [0]: []); - - icon.exit() - .remove(); - - icon.enter() - .append('svg') - .attr('class', 'icon') - .append('use') - .attr('xlink:href', '#fas-lock'); + .classed('disabled', !!isLocked) + .attr('readonly', isLocked || null); @@ -308,7 +284,7 @@ export function uiFieldLocalized(field, context) { function addNew() { d3_event.preventDefault(); - if (_isLocked) return; + if (field.locked()) return; var defaultLang = utilDetect().locale.toLowerCase().split('-')[0]; var langExists = _multilingual.find(function(datum) { return datum.lang === defaultLang; }); @@ -325,7 +301,7 @@ export function uiFieldLocalized(field, context) { function change(onInput) { return function() { - if (_isLocked) { + if (field.locked()) { d3_event.preventDefault(); return; } @@ -432,7 +408,7 @@ export function uiFieldLocalized(field, context) { .append('button') .attr('class', 'remove-icon-multilingual') .on('click', function(d){ - if (_isLocked) return; + if (field.locked()) return; d3_event.preventDefault(); var t = {}; t[key(d.lang)] = undefined; diff --git a/test/spec/ui/fields/localized.js b/test/spec/ui/fields/localized.js index 173d7e6f1..e4bd9316c 100644 --- a/test/spec/ui/fields/localized.js +++ b/test/spec/ui/fields/localized.js @@ -5,6 +5,7 @@ describe('iD.uiFieldLocalized', function() { context = iD.coreContext(); selection = d3.select(document.createElement('div')); field = iD.presetField('name', { key: 'name', type: 'localized' }); + field.locked = function() { return false; }; }); it('adds a blank set of fields when the + button is clicked', function() {