From 3d9009725e89ada1194bc760952be0773fae3c36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20=22Natsuyasumi=22=20Kuranowski?= Date: Fri, 9 Dec 2016 13:33:15 +0100 Subject: [PATCH 01/25] Address customization --- data/address-formats.json | 10 +++++++++- data/presets.yaml | 17 ++++++++++++++++- data/presets/fields.json | 21 ++++++++++++++++++++- data/presets/fields/address.json | 21 ++++++++++++++++++++- dist/locales/en.json | 17 ++++++++++++++++- modules/ui/fields/address.js | 26 ++++++++++++-------------- 6 files changed, 93 insertions(+), 19 deletions(-) diff --git a/data/address-formats.json b/data/address-formats.json index 973535963..2e8d164e3 100644 --- a/data/address-formats.json +++ b/data/address-formats.json @@ -25,7 +25,8 @@ }, { "countryCodes": ["vn"], - "format": [["housenumber", "street"], ["subdistrict"], ["district"], ["city"], ["province", "postcode"]] + "format": [["housenumber", "street"], ["subdistrict"], ["district"], ["city"], ["province", "postcode"]], + "customPlaceholders": ["subdistrict", "district", "city"] }, { "countryCodes": ["us"], @@ -38,6 +39,13 @@ { "countryCodes": ["tw"], "format": [["postcode", "city", "district"], ["place", "street"], ["housenumber", "floor"]] + }, + { + "countryCodes": ["jp"], + "format": [["postcode", "province", "county"], ["city", "suburb"], ["quarter", "neighbourhood"], ["block_number", "housenumber"]], + "customPlaceholders": ["province", "county", "city", "suburb", "quarter", "neighbourhood", "block_number", "housenumber"], + "dropdowns": ["postcode", "province", "county", "city", "suburb", "quarter", "neighbourhood", "block_number"], + "widths": {"postcode": 0.3, "province": 0.35, "county": 0.35, "city": 0.65, "suburb": 0.35, "quarter": 0.5, "neighbourhood": 0.5, "block_number": 0.5} } ] } diff --git a/data/presets.yaml b/data/presets.yaml index d536bc552..ab8460d10 100644 --- a/data/presets.yaml +++ b/data/presets.yaml @@ -78,24 +78,39 @@ en: # access=* label: Access address: - # 'addr:city=*, addr:conscriptionnumber=*, addr:country=*, addr:district=*, addr:floor=*, addr:hamlet=*, addr:housename=*, addr:housenumber=*, addr:place=*, addr:postcode=*, addr:province=*, addr:state=*, addr:street=*, addr:subdistrict=*, addr:suburb=*' + # 'addr:city=*, addr:block_number=*, addr:conscriptionnumber=*, addr:country=*, addr:county=*, addr:district=*, addr:floor=*, addr:hamlet=*, addr:housename=*, addr:housenumber=*, addr:neighbourhood=*, addr:place=*, addr:postcode=*, addr:province=*, addr:quarter=*, addr:state=*, addr:street=*, addr:subdistrict=*, addr:suburb=*' label: Address placeholders: + block_number: Block Number + block_number!jp: Block No. city: City + city!jp: City/Town/Village/Tokyo Special Ward + city!vn: City/Town conscriptionnumber: '123' country: Country + county: County + county!jp: District district: District + district!vn: Arrondissement/Town/District floor: Floor hamlet: Hamlet housename: Housename housenumber: '123' + housenumber!jp: Building No./Lot No. + neighbourhood: Neighbourhood + neighbourhood!jp: Chōme/Aza/Koaza place: Place postcode: Postcode province: Province + province!jp: Prefecture + quarter: Quarter + quarter!jp: Ōaza/Machi state: State street: Street subdistrict: Subdistrict + subdistrict!vn: Ward/Commune/Townlet suburb: Suburb + suburb!jp: Ward admin_level: # admin_level=* label: Admin Level diff --git a/data/presets/fields.json b/data/presets/fields.json index d7b964d76..50a340c84 100644 --- a/data/presets/fields.json +++ b/data/presets/fields.json @@ -80,16 +80,20 @@ "type": "address", "keys": [ "addr:city", + "addr:block_number", "addr:conscriptionnumber", "addr:country", + "addr:county", "addr:district", "addr:floor", "addr:hamlet", "addr:housename", "addr:housenumber", + "addr:neighbourhood", "addr:place", "addr:postcode", "addr:province", + "addr:quarter", "addr:state", "addr:street", "addr:subdistrict", @@ -104,20 +108,35 @@ "strings": { "placeholders": { "city": "City", + "block_number": "Block Number", "conscriptionnumber": "123", "country": "Country", + "county": "County", "district": "District", "floor": "Floor", "hamlet": "Hamlet", "housename": "Housename", "housenumber": "123", + "neighbourhood": "Neighbourhood", "place": "Place", "postcode": "Postcode", "province": "Province", + "quarter": "Quarter", "state": "State", "street": "Street", "subdistrict": "Subdistrict", - "suburb": "Suburb" + "suburb": "Suburb", + "subdistrict!vn": "Ward/Commune/Townlet", + "district!vn": "Arrondissement/Town/District", + "city!vn": "City/Town", + "province!jp": "Prefecture", + "county!jp": "District", + "city!jp": "City/Town/Village/Tokyo Special Ward", + "suburb!jp": "Ward", + "quarter!jp": "Ōaza/Machi", + "neighbourhood!jp": "Chōme/Aza/Koaza", + "block_number!jp": "Block No.", + "housenumber!jp": "Building No./Lot No." } } }, diff --git a/data/presets/fields/address.json b/data/presets/fields/address.json index 9d223b744..4f001c05d 100644 --- a/data/presets/fields/address.json +++ b/data/presets/fields/address.json @@ -2,16 +2,20 @@ "type": "address", "keys": [ "addr:city", + "addr:block_number", "addr:conscriptionnumber", "addr:country", + "addr:county", "addr:district", "addr:floor", "addr:hamlet", "addr:housename", "addr:housenumber", + "addr:neighbourhood", "addr:place", "addr:postcode", "addr:province", + "addr:quarter", "addr:state", "addr:street", "addr:subdistrict", @@ -24,20 +28,35 @@ "strings": { "placeholders": { "city": "City", + "block_number": "Block Number", "conscriptionnumber": "123", "country": "Country", + "county": "County", "district": "District", "floor": "Floor", "hamlet": "Hamlet", "housename": "Housename", "housenumber": "123", + "neighbourhood": "Neighbourhood", "place": "Place", "postcode": "Postcode", "province": "Province", + "quarter": "Quarter", "state": "State", "street": "Street", "subdistrict": "Subdistrict", - "suburb": "Suburb" + "suburb": "Suburb", + "subdistrict!vn": "Ward/Commune/Townlet", + "district!vn": "Arrondissement/Town/District", + "city!vn": "City/Town", + "province!jp": "Prefecture", + "county!jp": "District", + "city!jp": "City/Town/Village/Tokyo Special Ward", + "suburb!jp": "Ward", + "quarter!jp": "Ōaza/Machi", + "neighbourhood!jp": "Chōme/Aza/Koaza", + "block_number!jp": "Block No.", + "housenumber!jp": "Building No./Lot No." } } } diff --git a/dist/locales/en.json b/dist/locales/en.json index 44949de8f..e99dc0220 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -704,20 +704,35 @@ "label": "Address", "placeholders": { "city": "City", + "block_number": "Block Number", "conscriptionnumber": "123", "country": "Country", + "county": "County", "district": "District", "floor": "Floor", "hamlet": "Hamlet", "housename": "Housename", "housenumber": "123", + "neighbourhood": "Neighbourhood", "place": "Place", "postcode": "Postcode", "province": "Province", + "quarter": "Quarter", "state": "State", "street": "Street", "subdistrict": "Subdistrict", - "suburb": "Suburb" + "suburb": "Suburb", + "subdistrict!vn": "Ward/Commune/Townlet", + "district!vn": "Arrondissement/Town/District", + "city!vn": "City/Town", + "province!jp": "Prefecture", + "county!jp": "District", + "city!jp": "City/Town/Village/Tokyo Special Ward", + "suburb!jp": "Ward", + "quarter!jp": "Ōaza/Machi", + "neighbourhood!jp": "Chōme/Aza/Koaza", + "block_number!jp": "Block No.", + "housenumber!jp": "Building No./Lot No." } }, "admin_level": { diff --git a/modules/ui/fields/address.js b/modules/ui/fields/address.js index 76f6fcd3d..5fcb4aacb 100644 --- a/modules/ui/fields/address.js +++ b/modules/ui/fields/address.js @@ -19,17 +19,10 @@ export function uiFieldAddress(field, context) { nominatim = services.nominatim, wrap = d3.select(null), isInitialized = false, + widths, + addrTags, entity; - var widths = { - housenumber: 1/3, - street: 2/3, - city: 2/3, - state: 1/4, - postcode: 1/3 - }; - - function getNearStreets() { var extent = entity.extent(context.graph()), l = extent.center(), @@ -98,7 +91,6 @@ export function uiFieldAddress(field, context) { } } - function getNearValues(key) { var extent = entity.extent(context.graph()), l = extent.center(), @@ -130,6 +122,9 @@ export function uiFieldAddress(field, context) { return a && a.countryCodes && _.includes(a.countryCodes, countryCode); }) || _.first(dataAddressFormats); + if (typeof addressFormat.widths != 'undefined') { widths = addressFormat.widths; } + else { widths = {housenumber: 1/3, street: 2/3, city: 2/3, state: 1/4, postcode: 1/3}; } + function row(r) { // Normalize widths. var total = _.reduce(r, function(sum, field) { @@ -154,16 +149,19 @@ export function uiFieldAddress(field, context) { .enter() .append('input') .property('type', 'text') - .attr('placeholder', function (d) { return field.t('placeholders.' + d.id); }) + .attr('placeholder', function (d) { + var countryInserter = ''; + if (addressFormat.customPlaceholders.indexOf(d.id) !== -1) { countryInserter = '!' + countryCode; } + return field.t('placeholders.' + d.id + countryInserter); }) .attr('class', function (d) { return 'addr-' + d.id; }) .style('width', function (d) { return d.width * 100 + '%'; }); // Update // setup dropdowns for common address tags - var addrTags = [ + if (typeof addressFormat.dropdowns != "undefined") { addrTags = addressFormat.dropdowns; } + else { addrTags = [ 'street', 'city', 'state', 'province', 'district', - 'subdistrict', 'suburb', 'place', 'postcode' - ]; + 'subdistrict', 'suburb', 'place', 'postcode']; } addrTags.forEach(function(tag) { var nearValues = (tag === 'street') ? getNearStreets From aafb77c7ecec8a59e5ca6e11e874dd5867fe9c57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miko=C5=82aj=20=22Natsuyasumi=22=20Kuranowski?= Date: Fri, 9 Dec 2016 15:08:16 +0100 Subject: [PATCH 02/25] Small fixes --- modules/ui/fields/address.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/ui/fields/address.js b/modules/ui/fields/address.js index 5fcb4aacb..b3a90c7e6 100644 --- a/modules/ui/fields/address.js +++ b/modules/ui/fields/address.js @@ -122,7 +122,7 @@ export function uiFieldAddress(field, context) { return a && a.countryCodes && _.includes(a.countryCodes, countryCode); }) || _.first(dataAddressFormats); - if (typeof addressFormat.widths != 'undefined') { widths = addressFormat.widths; } + if (typeof addressFormat.widths !== 'undefined') { widths = addressFormat.widths; } else { widths = {housenumber: 1/3, street: 2/3, city: 2/3, state: 1/4, postcode: 1/3}; } function row(r) { @@ -158,7 +158,7 @@ export function uiFieldAddress(field, context) { // Update // setup dropdowns for common address tags - if (typeof addressFormat.dropdowns != "undefined") { addrTags = addressFormat.dropdowns; } + if (typeof addressFormat.dropdowns !== 'undefined') { addrTags = addressFormat.dropdowns; } else { addrTags = [ 'street', 'city', 'state', 'province', 'district', 'subdistrict', 'suburb', 'place', 'postcode']; } From fa70d79622ce9d1c24b542963b78bc6618b6ee14 Mon Sep 17 00:00:00 2001 From: slhh Date: Fri, 16 Dec 2016 15:38:59 +0100 Subject: [PATCH 03/25] Additional tests added for addNode, updateNode, and replaceNode. --- test/spec/osm/way.js | 116 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 115 insertions(+), 1 deletion(-) diff --git a/test/spec/osm/way.js b/test/spec/osm/way.js index a49ea4082..afc4fa90f 100644 --- a/test/spec/osm/way.js +++ b/test/spec/osm/way.js @@ -410,11 +410,21 @@ describe('iD.osmWay', function() { }); describe('#addNode', function () { - it('adds a node to the end of a way', function () { + it('adds a node to the end of a way when index is undefined', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('c').nodes).to.eql(['a', 'b', 'c']); + }); + + it('adds a node to an empty way', function () { var w = iD.Way(); expect(w.addNode('a').nodes).to.eql(['a']); }); + it('adds a node to the end of a way at a index greater than length', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('c',3).nodes).to.eql(['a', 'b', 'c']); + }); + it('adds a node to a way at index 0', function () { var w = iD.Way({nodes: ['a', 'b']}); expect(w.addNode('c', 0).nodes).to.eql(['c', 'a', 'b']); @@ -429,6 +439,36 @@ describe('iD.osmWay', function() { var w = iD.Way({nodes: ['a', 'b']}); expect(w.addNode('c', -1).nodes).to.eql(['a', 'c', 'b']); }); + + it('prevents duplicate consecutive nodes when adding in front of', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('b', 1).nodes).to.eql(['a', 'b']); + }); + + it('prevents duplicate consecutive nodes when adding behind', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('a', 1).nodes).to.eql(['a', 'b']); + }); + + it('prevents duplicate consecutive nodes at index 0', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('a', 0).nodes).to.eql(['a', 'b']); + }); + + it('prevents duplicate consecutive nodes at a negative index', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('a', -1).nodes).to.eql(['a', 'b']); + }); + + it('prevents duplicate consecutive nodes at a index equal to length', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('b', 2).nodes).to.eql(['a', 'b']); + }); + + it('prevents duplicate consecutive nodes when index is undefined', function () { + var w = iD.Way({nodes: ['a', 'b']}); + expect(w.addNode('b').nodes).to.eql(['a', 'b']); + }); }); describe('#updateNode', function () { @@ -436,8 +476,74 @@ describe('iD.osmWay', function() { var w = iD.Way({nodes: ['a', 'b', 'c']}); expect(w.updateNode('d', 1).nodes).to.eql(['a', 'd', 'c']); }); + + it('prevents duplicate consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); + expect(w.updateNode('b',2).nodes).to.eql(['a', 'b', 'd','e']); + w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); + expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd','e']); + w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.updateNode('b',2).nodes).to.eql(['a', 'b','e']); + }); + + it('preserves duplicate non-consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd', 'b','e']); + }); + + it('replaces a single one of duplicate nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.updateNode('d',1).nodes).to.eql(['a', 'd', 'c', 'b','e']); + w = iD.Way({nodes: ['a', 'b', 'b', 'c','e']}); + expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd', 'c','e']); + }); + + it('removes existing duplicate consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.updateNode('c',5).nodes).to.eql(['a', 'b', 'd', 'b','c']); + w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.updateNode('c',3).nodes).to.eql(['a', 'b', 'c', 'b', 'e']); + }); }); + describe('#replaceNode', function () { + it('replaces the node', function () { + var w = iD.Way({nodes: ['a']}); + expect(w.replaceNode('a','b').nodes).to.eql(['b']); + w = iD.Way({nodes: ['a', 'b', 'c']}); + expect(w.replaceNode('b', 'd').nodes).to.eql(['a', 'd', 'c']); + }); + + it('prevents duplicate consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); + expect(w.replaceNode('c','b').nodes).to.eql(['a', 'b', 'd','e']); + w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); + expect(w.replaceNode('c','d').nodes).to.eql(['a', 'b', 'd','e']); + w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.replaceNode('c','b').nodes).to.eql(['a', 'b','e']); + }); + + it('preserves duplicate non-consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.replaceNode('c','d').nodes).to.eql(['a', 'b', 'd', 'b','e']); + }); + + it('replaces duplicate non-consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); + expect(w.replaceNode('b','d').nodes).to.eql(['a', 'd', 'c', 'd','e']); + }); + + it('removes existing duplicate consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.replaceNode('e','c').nodes).to.eql(['a', 'b', 'd', 'b','c']); + w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.replaceNode('d','c').nodes).to.eql(['a', 'b', 'c', 'b', 'e']); + w = iD.Way({nodes: ['a', 'b', 'b', 'c', 'b', 'e']}); + expect(w.replaceNode('b','d').nodes).to.eql(['a', 'd', 'c', 'd', 'e']); + }); + }); + + describe('#removeNode', function () { it('removes the node', function () { var w = iD.Way({nodes: ['a']}); @@ -457,6 +563,14 @@ describe('iD.osmWay', function() { it('prevents duplicate consecutive nodes when preserving circularity', function () { var w = iD.Way({nodes: ['a', 'b', 'c', 'd', 'b', 'a']}); expect(w.removeNode('a').nodes).to.eql(['b', 'c', 'd', 'b']); + w = iD.Way({nodes: ['a', 'b', 'a']}); + expect(w.removeNode('b').nodes).to.eql(['a']); + }); + it('removes existing duplicate consecutive nodes', function () { + var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.removeNode('e').nodes).to.eql(['a', 'b', 'd', 'b']); + w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); + expect(w.removeNode('b').nodes).to.eql(['a', 'd', 'e']); }); }); From 7817d6dff113860a6d2425e725b600e58881dd89 Mon Sep 17 00:00:00 2001 From: slhh Date: Fri, 16 Dec 2016 15:43:29 +0100 Subject: [PATCH 04/25] Prevent duplicate consecutive nodes in addNode, updateNode, and replaceNode, and add some comments. --- modules/osm/way.js | 82 +++++++++++++++++++++++++++++++++------------- 1 file changed, 59 insertions(+), 23 deletions(-) diff --git a/modules/osm/way.js b/modules/osm/way.js index 7f328894c..e6c7a4c67 100644 --- a/modules/osm/way.js +++ b/modules/osm/way.js @@ -187,34 +187,70 @@ _.extend(osmWay.prototype, { }); }, - + // Adds a node (id) in front of the node which is currently at position index. + // If index is negative, it will be counted from the end of the way. + // If index is 0 or < -length, the node (id) will be added at the start of the way. + // If index is undefined or >= length, the node (id) will be added at the end of the way. + // Generating consecutive duplicates is silently prevented + addNode: function(id, index) { - var nodes = this.nodes.slice(); - nodes.splice(index === undefined ? nodes.length : index, 0, id); - return this.update({nodes: nodes}); - }, - - - updateNode: function(id, index) { - var nodes = this.nodes.slice(); - nodes.splice(index, 1, id); - return this.update({nodes: nodes}); - }, - - - replaceNode: function(needle, replacement) { - if (this.nodes.indexOf(needle) < 0) - return this; - - var nodes = this.nodes.slice(); - for (var i = 0; i < nodes.length; i++) { - if (nodes[i] === needle) { - nodes[i] = replacement; - } + var nodes = this.nodes.slice(), + spliceIndex = index === undefined ? nodes.length : index; + if (spliceIndex > nodes.length) spliceIndex = nodes.length; + if (spliceIndex < 0) spliceIndex = nodes.length + index; + if (spliceIndex < 0) spliceIndex = 0; + if (nodes[spliceIndex] !== id&& nodes[spliceIndex-1] !== id) { + nodes.splice(spliceIndex, 0, id); } return this.update({nodes: nodes}); }, + // Replaces the node which is currently at position index with the given node (id). + // If index is negative, it will be counted from the end of the way. + + // Consecutive duplicates are eliminated including existing ones. + + updateNode: function(id, index) { + var nodes = []; + + if (index < 0) index = this.nodes.length + index; + + for (var i = 0; i < this.nodes.length; i++) { + var node = this.nodes[i]; + if (i === index) { + if (nodes[nodes.length - 1] !== id) + nodes.push(id); + } else { + if (nodes[nodes.length - 1] !== node) + nodes.push(node); + } + } + + return this.update({nodes: nodes}); + }, + + // Replaces each occurrence of node id needle with replacement. + // Consecutive duplicates are eliminated including existing ones. + + replaceNode: function(needle, replacement) { + var nodes = []; + + for (var i = 0; i < this.nodes.length; i++) { + var node = this.nodes[i]; + if (node === needle) { + if (nodes[nodes.length - 1] !== replacement) + nodes.push(replacement); + } else { + if (nodes[nodes.length - 1] !== node) + nodes.push(node); + } + } + + return this.update({nodes: nodes}); + }, + + // Removes each occurrence of node id needle with replacement. + // Consecutive duplicates are eliminated. Circularity is preserved. removeNode: function(id) { var nodes = []; From d0727c2f366a2b23b3f876792592641a02792cbb Mon Sep 17 00:00:00 2001 From: popov Date: Wed, 28 Dec 2016 15:45:13 +1000 Subject: [PATCH 05/25] reflect-typo --- data/core.yaml | 4 ++-- dist/locales/en.json | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index e614c9fd0..5e0344b81 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -159,10 +159,10 @@ en: short: Y annotation: long: - area: Reflected an area across its long axis. + single: Reflected an object across its long axis. multiple: Reflected multiple objects across their long axis. short: - area: Reflected an area across its short axis. + single: Reflected an object across its short axis. multiple: Reflected multiple objects across their short axis. incomplete_relation: single: This object can't be reflected because it hasn't been fully downloaded. diff --git a/dist/locales/en.json b/dist/locales/en.json index 2e877fc5a..ef15e5fa9 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -205,11 +205,11 @@ }, "annotation": { "long": { - "area": "Reflected an area across its long axis.", + "single": "Reflected an object across its long axis.", "multiple": "Reflected multiple objects across their long axis." }, "short": { - "area": "Reflected an area across its short axis.", + "single": "Reflected an object across its short axis.", "multiple": "Reflected multiple objects across their short axis." } }, From f1cdde0f92c1b59fb7be3b87c04d81c87ca4fa42 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 6 Jan 2017 22:36:10 -0500 Subject: [PATCH 06/25] Changes to addNode and add tests --- modules/osm/way.js | 153 ++++++++++++---------- test/spec/osm/way.js | 295 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 334 insertions(+), 114 deletions(-) diff --git a/modules/osm/way.js b/modules/osm/way.js index e6c7a4c67..dcb10364f 100644 --- a/modules/osm/way.js +++ b/modules/osm/way.js @@ -187,86 +187,90 @@ _.extend(osmWay.prototype, { }); }, + // Adds a node (id) in front of the node which is currently at position index. - // If index is negative, it will be counted from the end of the way. - // If index is 0 or < -length, the node (id) will be added at the start of the way. - // If index is undefined or >= length, the node (id) will be added at the end of the way. - // Generating consecutive duplicates is silently prevented - + // If index is undefined, the node will be added to the end of the way for linear ways, + // or just before the final connecting node for circular ways. + // Consecutive duplicates are eliminated including existing ones. + // Circularity is always preserved when adding a node. addNode: function(id, index) { var nodes = this.nodes.slice(), - spliceIndex = index === undefined ? nodes.length : index; - if (spliceIndex > nodes.length) spliceIndex = nodes.length; - if (spliceIndex < 0) spliceIndex = nodes.length + index; - if (spliceIndex < 0) spliceIndex = 0; - if (nodes[spliceIndex] !== id&& nodes[spliceIndex-1] !== id) { - nodes.splice(spliceIndex, 0, id); - } - return this.update({nodes: nodes}); - }, + isClosed = this.isClosed(), + max = isClosed ? nodes.length - 1 : nodes.length; - // Replaces the node which is currently at position index with the given node (id). - // If index is negative, it will be counted from the end of the way. - - // Consecutive duplicates are eliminated including existing ones. - - updateNode: function(id, index) { - var nodes = []; - - if (index < 0) index = this.nodes.length + index; - - for (var i = 0; i < this.nodes.length; i++) { - var node = this.nodes[i]; - if (i === index) { - if (nodes[nodes.length - 1] !== id) - nodes.push(id); - } else { - if (nodes[nodes.length - 1] !== node) - nodes.push(node); - } + if (index === undefined) { + index = max; } - return this.update({nodes: nodes}); - }, - - // Replaces each occurrence of node id needle with replacement. - // Consecutive duplicates are eliminated including existing ones. - - replaceNode: function(needle, replacement) { - var nodes = []; - - for (var i = 0; i < this.nodes.length; i++) { - var node = this.nodes[i]; - if (node === needle) { - if (nodes[nodes.length - 1] !== replacement) - nodes.push(replacement); - } else { - if (nodes[nodes.length - 1] !== node) - nodes.push(node); - } + if (index < 0 || index > max) { + throw new RangeError('index ' + index + ' out of range 0..' + max); } - return this.update({nodes: nodes}); - }, - - // Removes each occurrence of node id needle with replacement. - // Consecutive duplicates are eliminated. Circularity is preserved. - - removeNode: function(id) { - var nodes = []; - - for (var i = 0; i < this.nodes.length; i++) { - var node = this.nodes[i]; - if (node !== id && nodes[nodes.length - 1] !== node) { - nodes.push(node); + // Will this change the connecting node? If so, pop the old connecting node(s). + if (isClosed && index === 0 && id !== this.first()) { + while (nodes.length > 1 && nodes[nodes.length - 1] === this.first()) { + nodes.pop(); } } - // Preserve circularity - if (this.nodes.length > 1 && this.first() === id && this.last() === id && nodes[nodes.length - 1] !== nodes[0]) { + nodes.splice(index, 0, id); + nodes = nodes.filter(noRepeatNodes); + + // If the way was closed before, add a connecting node to keep it closed.. + if (isClosed && (nodes.length === 1 || nodes[0] !== nodes[nodes.length - 1])) { nodes.push(nodes[0]); } + return this.update({ nodes: nodes }); + }, + + + // Replaces the node which is currently at position index with the given node (id). + // Consecutive duplicates are eliminated including existing ones. + // Circularity is not preserved when updating a node. + updateNode: function(id, index) { + var nodes = this.nodes.slice(), + isClosed = this.isClosed(), + max = nodes.length - 1; + + if (index === undefined || index < 0 || index > max) { + throw new RangeError('index ' + index + ' out of range 0..' + max); + } + + nodes.splice(index, 1, id); + nodes = nodes.filter(noRepeatNodes); + + return this.update({nodes: nodes}); + }, + + + // Replaces each occurrence of node id needle with replacement. + // Consecutive duplicates are eliminated including existing ones. + replaceNode: function(needle, replacement) { + var nodes = this.nodes.slice(); + + for (var i = 0; i < nodes.length; i++) { + if (nodes[i] === needle) { + nodes[i] = replacement; + } + } + + nodes = nodes.filter(noRepeatNodes); + checkCircular(this, nodes); + return this.update({nodes: nodes}); + }, + + + // Removes each occurrence of node id needle with replacement. + // Consecutive duplicates are eliminated including existing ones. + // Circularity is preserved. + removeNode: function(id) { + var nodes = this.nodes.slice(); + + nodes = nodes.filter(function(node, i, arr) { + return node !== id && noRepeatNodes(node, i, arr); + }); + checkCircular(this, nodes); return this.update({nodes: nodes}); }, @@ -284,7 +288,9 @@ _.extend(osmWay.prototype, { }) } }; - if (changeset_id) r.way['@changeset'] = changeset_id; + if (changeset_id) { + r.way['@changeset'] = changeset_id; + } return r; }, @@ -333,3 +339,16 @@ _.extend(osmWay.prototype, { }); } }); + + +// Filter function to eliminate consecutive duplicates. +function noRepeatNodes(node, i, arr) { + return i === 0 || node !== arr[i - 1]; +} + +// If the nodelist was circular before, keep it circular. +function keepCircular(nodes) { + if ((nodes.length === 1 || nodes[0] !== nodes[nodes.length - 1])) { + nodes.push(nodes[0]); + } +} diff --git a/test/spec/osm/way.js b/test/spec/osm/way.js index afc4fa90f..adb71fa87 100644 --- a/test/spec/osm/way.js +++ b/test/spec/osm/way.js @@ -410,73 +410,274 @@ describe('iD.osmWay', function() { }); describe('#addNode', function () { - it('adds a node to the end of a way when index is undefined', function () { - var w = iD.Way({nodes: ['a', 'b']}); - expect(w.addNode('c').nodes).to.eql(['a', 'b', 'c']); - }); - it('adds a node to an empty way', function () { var w = iD.Way(); expect(w.addNode('a').nodes).to.eql(['a']); }); - it('adds a node to the end of a way at a index greater than length', function () { - var w = iD.Way({nodes: ['a', 'b']}); - expect(w.addNode('c',3).nodes).to.eql(['a', 'b', 'c']); + it('adds a node to the end of a linear way when index is undefined', function () { + var w = iD.Way({ nodes: 'ab'.split('') }); + expect(w.addNode('c').nodes.join('')).to.eql('abc'); }); - it('adds a node to a way at index 0', function () { - var w = iD.Way({nodes: ['a', 'b']}); - expect(w.addNode('c', 0).nodes).to.eql(['c', 'a', 'b']); + it('adds a node before the end connector of a circular way when index is undefined', function () { + var w1 = iD.Way({ nodes: 'aba'.split('') }); + expect(w1.addNode('c').nodes.join('')).to.eql('abca', 'circular'); + var w2 = iD.Way({ nodes: 'aa'.split('') }); + expect(w2.addNode('c').nodes.join('')).to.eql('aca', 'single node'); }); - it('adds a node to a way at a positive index', function () { - var w = iD.Way({nodes: ['a', 'b']}); - expect(w.addNode('c', 1).nodes).to.eql(['a', 'c', 'b']); + it('adds a node to a linear way at a positive index', function () { + var w = iD.Way({ nodes: 'ab'.split('') }); + expect(w.addNode('c', 1).nodes.join('')).to.eql('acb'); }); - it('adds a node to a way at a negative index', function () { - var w = iD.Way({nodes: ['a', 'b']}); - expect(w.addNode('c', -1).nodes).to.eql(['a', 'c', 'b']); - }); - - it('prevents duplicate consecutive nodes when adding in front of', function () { - var w = iD.Way({nodes: ['a', 'b']}); - expect(w.addNode('b', 1).nodes).to.eql(['a', 'b']); + it('adds a node to a circular way at a positive index', function () { + var w1 = iD.Way({ nodes: 'aba'.split('') }); + expect(w1.addNode('c', 1).nodes.join('')).to.eql('acba', 'circular'); + var w2 = iD.Way({ nodes: 'aa'.split('') }); + expect(w2.addNode('c', 1).nodes.join('')).to.eql('aca', 'single node'); }); - it('prevents duplicate consecutive nodes when adding behind', function () { - var w = iD.Way({nodes: ['a', 'b']}); - expect(w.addNode('a', 1).nodes).to.eql(['a', 'b']); - }); - - it('prevents duplicate consecutive nodes at index 0', function () { - var w = iD.Way({nodes: ['a', 'b']}); - expect(w.addNode('a', 0).nodes).to.eql(['a', 'b']); - }); - - it('prevents duplicate consecutive nodes at a negative index', function () { - var w = iD.Way({nodes: ['a', 'b']}); - expect(w.addNode('a', -1).nodes).to.eql(['a', 'b']); - }); - - it('prevents duplicate consecutive nodes at a index equal to length', function () { - var w = iD.Way({nodes: ['a', 'b']}); - expect(w.addNode('b', 2).nodes).to.eql(['a', 'b']); + it('adds a node to a linear way at index 0', function () { + var w = iD.Way({ nodes: 'ab'.split('') }); + expect(w.addNode('c', 0).nodes.join('')).to.eql('cab'); }); - it('prevents duplicate consecutive nodes when index is undefined', function () { - var w = iD.Way({nodes: ['a', 'b']}); - expect(w.addNode('b').nodes).to.eql(['a', 'b']); + it('adds a node to a circular way at index 0, preserving circularity', function () { + var w1 = iD.Way({ nodes: 'aba'.split('') }); + expect(w1.addNode('c', 0).nodes.join('')).to.eql('cabc', 'circular'); + var w2 = iD.Way({ nodes: 'aa'.split('') }); + expect(w2.addNode('c', 0).nodes.join('')).to.eql('cac', 'single node'); }); + + it('throws RangeError if index outside of array range for linear way', function () { + var w = iD.Way({ nodes: 'ab'.split('') }); + expect(w.addNode.bind(w, 'c', 3)).to.throw(RangeError, /out of range 0\.\.2/, 'over range'); + expect(w.addNode.bind(w, 'c', -1)).to.throw(RangeError, /out of range 0\.\.2/, 'under range'); + }); + + it('throws RangeError if index outside of array range for circular way', function () { + var w = iD.Way({ nodes: 'aba'.split('') }); + expect(w.addNode.bind(w, 'c', 3)).to.throw(RangeError, /out of range 0\.\.2/, 'over range'); + expect(w.addNode.bind(w, 'c', -1)).to.throw(RangeError, /out of range 0\.\.2/, 'under range'); + }); + + it('eliminates duplicate consecutive nodes when adding to the end of a linear way', function () { + var w1 = iD.Way({ nodes: 'abb'.split('') }); + expect(w1.addNode('b').nodes.join('')).to.eql('ab', 'duplicate at end'); + var w2 = iD.Way({ nodes: 'abbc'.split('') }); + expect(w2.addNode('c').nodes.join('')).to.eql('abc', 'duplicate in middle'); + var w3 = iD.Way({ nodes: 'aabc'.split('') }); + expect(w3.addNode('c').nodes.join('')).to.eql('abc', 'duplicate at beginning'); + var w4 = iD.Way({ nodes: 'abbbcbb'.split('') }); + expect(w4.addNode('b').nodes.join('')).to.eql('abcb', 'duplicates multiple places'); + }); + + it('eliminates duplicate consecutive nodes when adding same node before the end connector of a circular way', function () { + var w1 = iD.Way({ nodes: 'abcca'.split('') }); + expect(w1.addNode('c').nodes.join('')).to.eql('abca', 'duplicate internal node at end'); + var w2 = iD.Way({ nodes: 'abbca'.split('') }); + expect(w2.addNode('c').nodes.join('')).to.eql('abca', 'duplicate internal node in middle'); + var w3 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w3.addNode('c').nodes.join('')).to.eql('abca', 'duplicate connector node at beginning'); + var w4 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w4.addNode('a').nodes.join('')).to.eql('abca', 'duplicate connector node at end'); + var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); + expect(w5.addNode('b').nodes.join('')).to.eql('abcba', 'duplicates multiple places'); + var w6 = iD.Way({ nodes: 'aa'.split('') }); + expect(w6.addNode('a').nodes.join('')).to.eql('aa', 'single node'); + var w7 = iD.Way({ nodes: 'aaa'.split('') }); + expect(w7.addNode('a').nodes.join('')).to.eql('aa', 'single node with duplicates'); + }); + + it('eliminates duplicate consecutive nodes when adding different node before the end connector of a circular way', function () { + var w1 = iD.Way({ nodes: 'abcca'.split('') }); + expect(w1.addNode('d').nodes.join('')).to.eql('abcda', 'duplicate internal node at end'); + var w2 = iD.Way({ nodes: 'abbca'.split('') }); + expect(w2.addNode('d').nodes.join('')).to.eql('abcda', 'duplicate internal node in middle'); + var w3 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w3.addNode('d').nodes.join('')).to.eql('abcda', 'duplicate connector node at beginning'); + var w4 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w4.addNode('d').nodes.join('')).to.eql('abcda', 'duplicate connector node at end'); + var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); + expect(w5.addNode('d').nodes.join('')).to.eql('abcbda', 'duplicates multiple places'); + var w6 = iD.Way({ nodes: 'aa'.split('') }); + expect(w6.addNode('d').nodes.join('')).to.eql('ada', 'single node'); + var w7 = iD.Way({ nodes: 'aaa'.split('') }); + expect(w7.addNode('d').nodes.join('')).to.eql('ada', 'single node with duplicates'); + }); + + it('eliminates duplicate consecutive nodes when adding to the beginning of a linear way', function () { + var w1 = iD.Way({ nodes: 'abb'.split('') }); + expect(w1.addNode('a', 0).nodes.join('')).to.eql('ab', 'duplicate at end'); + var w2 = iD.Way({ nodes: 'abbc'.split('') }); + expect(w2.addNode('a', 0).nodes.join('')).to.eql('abc', 'duplicate in middle'); + var w3 = iD.Way({ nodes: 'aabc'.split('') }); + expect(w3.addNode('a', 0).nodes.join('')).to.eql('abc', 'duplicate at beginning'); + var w4 = iD.Way({ nodes: 'abbbcbb'.split('') }); + expect(w4.addNode('a', 0).nodes.join('')).to.eql('abcb', 'duplicates multiple places'); + }); + + it('eliminates duplicate consecutive nodes when adding same node as beginning connector a circular way', function () { + var w1 = iD.Way({ nodes: 'abcca'.split('') }); + expect(w1.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate internal node at end'); + var w2 = iD.Way({ nodes: 'abbca'.split('') }); + expect(w2.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate internal node in middle'); + var w3 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w3.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate connector node at beginning'); + var w4 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w4.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate connector node at end'); + var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); + expect(w5.addNode('a', 0).nodes.join('')).to.eql('abcba', 'duplicates multiple places'); + var w6 = iD.Way({ nodes: 'aa'.split('') }); + expect(w6.addNode('a', 0).nodes.join('')).to.eql('aa', 'single node'); + var w7 = iD.Way({ nodes: 'aaa'.split('') }); + expect(w7.addNode('a', 0).nodes.join('')).to.eql('aa', 'single node with duplicates'); + }); + + it('eliminates duplicate consecutive nodes when adding different node as beginning connector of a circular way', function () { + var w1 = iD.Way({ nodes: 'abcca'.split('') }); + expect(w1.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate internal node at end'); + var w2 = iD.Way({ nodes: 'abbca'.split('') }); + expect(w2.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate internal node in middle'); + var w3 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w3.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate connector node at beginning'); + var w4 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w4.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate connector node at end'); + var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); + expect(w5.addNode('d', 0).nodes.join('')).to.eql('dabcbd', 'duplicates multiple places'); + var w6 = iD.Way({ nodes: 'aa'.split('') }); + expect(w6.addNode('d', 0).nodes.join('')).to.eql('dad', 'single node'); + var w7 = iD.Way({ nodes: 'aaa'.split('') }); + expect(w7.addNode('d', 0).nodes.join('')).to.eql('dad', 'single node with duplicates'); + }); + }); describe('#updateNode', function () { + it('throws RangeError if empty way', function () { + var w = iD.Way(); + expect(w.updateNode.bind(w, 'a', 0)).to.throw(RangeError, /out of range 0\.\.-1/); + }); + + it('updates a node on a linear way at a positive index', function () { + var w = iD.Way({ nodes: 'ab'.split('') }); + expect(w.updateNode('c', 1).nodes.join('')).to.eql('ac'); + }); + + it('updates a node on a circular way at a positive index', function () { + var w1 = iD.Way({ nodes: 'aba'.split('') }); + expect(w1.updateNode('c', 1).nodes.join('')).to.eql('aca', 'circular'); + var w2 = iD.Way({ nodes: 'aa'.split('') }); + expect(w2.updateNode('c', 1).nodes.join('')).to.eql('aca', 'single node'); + }); + + it('updates a node on a linear way at index 0', function () { + var w = iD.Way({ nodes: 'ab'.split('') }); + expect(w.updateNode('c', 0).nodes.join('')).to.eql('cab'); + }); + + it('updates a node on a circular way at index 0, preserving circularity', function () { + var w1 = iD.Way({ nodes: 'aba'.split('') }); + expect(w1.updateNode('c', 0).nodes.join('')).to.eql('cabc', 'circular'); + var w2 = iD.Way({ nodes: 'aa'.split('') }); + expect(w2.updateNode('c', 0).nodes.join('')).to.eql('cac', 'single node'); + }); + + it('throws RangeError if index outside of array range for linear way', function () { + var w = iD.Way({ nodes: 'ab'.split('') }); + expect(w.addNode.bind(w, 'c', 3)).to.throw(RangeError, /out of range 0\.\.2/, 'over range'); + expect(w.addNode.bind(w, 'c', -1)).to.throw(RangeError, /out of range 0\.\.2/, 'under range'); + }); + + it('throws RangeError if index outside of array range for circular way', function () { + var w = iD.Way({ nodes: 'aba'.split('') }); + expect(w.addNode.bind(w, 'c', 3)).to.throw(RangeError, /out of range 0\.\.2/, 'over range'); + expect(w.addNode.bind(w, 'c', -1)).to.throw(RangeError, /out of range 0\.\.2/, 'under range'); + }); + + it('eliminates duplicate consecutive nodes when adding to the end of a linear way', function () { + var w1 = iD.Way({ nodes: 'abb'.split('') }); + expect(w1.addNode('b').nodes.join('')).to.eql('ab', 'duplicate at end'); + var w2 = iD.Way({ nodes: 'abbc'.split('') }); + expect(w2.addNode('c').nodes.join('')).to.eql('abc', 'duplicate in middle'); + var w3 = iD.Way({ nodes: 'aabc'.split('') }); + expect(w3.addNode('c').nodes.join('')).to.eql('abc', 'duplicate at beginning'); + var w4 = iD.Way({ nodes: 'abbbcbb'.split('') }); + expect(w4.addNode('b').nodes.join('')).to.eql('abcb', 'duplicates multiple places'); + }); + + it('eliminates duplicate consecutive nodes when adding before the end connector of a circular way', function () { + var w1 = iD.Way({ nodes: 'abcca'.split('') }); + expect(w1.addNode('c').nodes.join('')).to.eql('abca', 'duplicate internal node at end'); + var w2 = iD.Way({ nodes: 'abbca'.split('') }); + expect(w2.addNode('c').nodes.join('')).to.eql('abca', 'duplicate internal node in middle'); + var w3 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w3.addNode('c').nodes.join('')).to.eql('abca', 'duplicate connector node at beginning'); + var w4 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w4.addNode('a').nodes.join('')).to.eql('abca', 'duplicate connector node at end'); + var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); + expect(w5.addNode('b').nodes.join('')).to.eql('abcba', 'duplicates multiple places'); + var w6 = iD.Way({ nodes: 'aa'.split('') }); + expect(w6.addNode('a').nodes.join('')).to.eql('aa', 'single node'); + var w7 = iD.Way({ nodes: 'aaa'.split('') }); + expect(w7.addNode('a').nodes.join('')).to.eql('aa', 'single node with duplicates'); + }); + + it('eliminates duplicate consecutive nodes when adding to the beginning of a linear way', function () { + var w1 = iD.Way({ nodes: 'abb'.split('') }); + expect(w1.addNode('a', 0).nodes.join('')).to.eql('ab', 'duplicate at end'); + var w2 = iD.Way({ nodes: 'abbc'.split('') }); + expect(w2.addNode('a', 0).nodes.join('')).to.eql('abc', 'duplicate in middle'); + var w3 = iD.Way({ nodes: 'aabc'.split('') }); + expect(w3.addNode('a', 0).nodes.join('')).to.eql('abc', 'duplicate at beginning'); + var w4 = iD.Way({ nodes: 'abbbcbb'.split('') }); + expect(w4.addNode('a', 0).nodes.join('')).to.eql('abcb', 'duplicates multiple places'); + }); + + it('eliminates duplicate consecutive nodes when adding same node as beginning connector a circular way', function () { + var w1 = iD.Way({ nodes: 'abcca'.split('') }); + expect(w1.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate internal node at end'); + var w2 = iD.Way({ nodes: 'abbca'.split('') }); + expect(w2.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate internal node in middle'); + var w3 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w3.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate connector node at beginning'); + var w4 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w4.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate connector node at end'); + var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); + expect(w5.addNode('a', 0).nodes.join('')).to.eql('abcba', 'duplicates multiple places'); + var w6 = iD.Way({ nodes: 'aa'.split('') }); + expect(w6.addNode('a', 0).nodes.join('')).to.eql('aa', 'single node'); + var w7 = iD.Way({ nodes: 'aaa'.split('') }); + expect(w7.addNode('a', 0).nodes.join('')).to.eql('aa', 'single node with duplicates'); + }); + + it('eliminates duplicate consecutive nodes when adding different node as beginning connector of a circular way', function () { + var w1 = iD.Way({ nodes: 'abcca'.split('') }); + expect(w1.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate internal node at end'); + var w2 = iD.Way({ nodes: 'abbca'.split('') }); + expect(w2.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate internal node in middle'); + var w3 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w3.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate connector node at beginning'); + var w4 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w4.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate connector node at end'); + var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); + expect(w5.addNode('d', 0).nodes.join('')).to.eql('dabcbd', 'duplicates multiple places'); + var w6 = iD.Way({ nodes: 'aa'.split('') }); + expect(w6.addNode('d', 0).nodes.join('')).to.eql('dad', 'single node'); + var w7 = iD.Way({ nodes: 'aaa'.split('') }); + expect(w7.addNode('d', 0).nodes.join('')).to.eql('dad', 'single node with duplicates'); + }); + + + +// old tests: it('updates the node id at the specified index', function () { var w = iD.Way({nodes: ['a', 'b', 'c']}); expect(w.updateNode('d', 1).nodes).to.eql(['a', 'd', 'c']); }); - + it('prevents duplicate consecutive nodes', function () { var w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); expect(w.updateNode('b',2).nodes).to.eql(['a', 'b', 'd','e']); @@ -513,7 +714,7 @@ describe('iD.osmWay', function() { w = iD.Way({nodes: ['a', 'b', 'c']}); expect(w.replaceNode('b', 'd').nodes).to.eql(['a', 'd', 'c']); }); - + it('prevents duplicate consecutive nodes', function () { var w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); expect(w.replaceNode('c','b').nodes).to.eql(['a', 'b', 'd','e']); @@ -542,8 +743,8 @@ describe('iD.osmWay', function() { expect(w.replaceNode('b','d').nodes).to.eql(['a', 'd', 'c', 'd', 'e']); }); }); - - + + describe('#removeNode', function () { it('removes the node', function () { var w = iD.Way({nodes: ['a']}); From f51003879196b67bd82d8a88e5083ba3c7a1547f Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 9 Jan 2017 17:33:46 -0500 Subject: [PATCH 07/25] Changes to updateNode and add tests --- modules/osm/way.js | 56 +++++++++++-- test/spec/osm/way.js | 195 ++++++++++++++++++++----------------------- 2 files changed, 140 insertions(+), 111 deletions(-) diff --git a/modules/osm/way.js b/modules/osm/way.js index dcb10364f..8f0f88e85 100644 --- a/modules/osm/way.js +++ b/modules/osm/way.js @@ -206,17 +206,31 @@ _.extend(osmWay.prototype, { throw new RangeError('index ' + index + ' out of range 0..' + max); } - // Will this change the connecting node? If so, pop the old connecting node(s). - if (isClosed && index === 0 && id !== this.first()) { - while (nodes.length > 1 && nodes[nodes.length - 1] === this.first()) { - nodes.pop(); + // If this is a closed way, remove all connector nodes except the first one + // (there may be duplicates) + if (isClosed) { + var connector = this.first(); + + // leading connectors.. + var i = 1; + while (i < nodes.length && nodes.length > 2 && nodes[i] === connector) { + nodes.splice(i, 1); + if (index > i) index--; + } + + // trailing connectors.. + i = nodes.length - 1; + while (i > 0 && nodes.length > 1 && nodes[i] === connector) { + nodes.splice(i, 1); + if (index > i) index--; + i = nodes.length - 1; } } nodes.splice(index, 0, id); nodes = nodes.filter(noRepeatNodes); - // If the way was closed before, add a connecting node to keep it closed.. + // If the way was closed before, append a connector node to keep it closed.. if (isClosed && (nodes.length === 1 || nodes[0] !== nodes[nodes.length - 1])) { nodes.push(nodes[0]); } @@ -227,7 +241,7 @@ _.extend(osmWay.prototype, { // Replaces the node which is currently at position index with the given node (id). // Consecutive duplicates are eliminated including existing ones. - // Circularity is not preserved when updating a node. + // Circularity is preserved when updating a node. updateNode: function(id, index) { var nodes = this.nodes.slice(), isClosed = this.isClosed(), @@ -237,9 +251,35 @@ _.extend(osmWay.prototype, { throw new RangeError('index ' + index + ' out of range 0..' + max); } + // If this is a closed way, remove all connector nodes except the first one + // (there may be duplicates) + if (isClosed) { + var connector = this.first(); + + // leading connectors.. + var i = 1; + while (i < nodes.length && nodes.length > 2 && nodes[i] === connector) { + nodes.splice(i, 1); + if (index > i) index--; + } + + // trailing connectors.. + i = nodes.length - 1; + while (i > 0 && nodes.length > 1 && nodes[i] === connector) { + nodes.splice(i, 1); + if (index === i) index = 0; // update leading connector instead + i = nodes.length - 1; + } + } + nodes.splice(index, 1, id); nodes = nodes.filter(noRepeatNodes); + // If the way was closed before, append a connector node to keep it closed.. + if (isClosed && (nodes.length === 1 || nodes[0] !== nodes[nodes.length - 1])) { + nodes.push(nodes[0]); + } + return this.update({nodes: nodes}); }, @@ -256,7 +296,7 @@ _.extend(osmWay.prototype, { } nodes = nodes.filter(noRepeatNodes); - checkCircular(this, nodes); + // checkCircular(this, nodes); return this.update({nodes: nodes}); }, @@ -270,7 +310,7 @@ _.extend(osmWay.prototype, { nodes = nodes.filter(function(node, i, arr) { return node !== id && noRepeatNodes(node, i, arr); }); - checkCircular(this, nodes); + // checkCircular(this, nodes); return this.update({nodes: nodes}); }, diff --git a/test/spec/osm/way.js b/test/spec/osm/way.js index adb71fa87..63d931b8b 100644 --- a/test/spec/osm/way.js +++ b/test/spec/osm/way.js @@ -424,31 +424,31 @@ describe('iD.osmWay', function() { var w1 = iD.Way({ nodes: 'aba'.split('') }); expect(w1.addNode('c').nodes.join('')).to.eql('abca', 'circular'); var w2 = iD.Way({ nodes: 'aa'.split('') }); - expect(w2.addNode('c').nodes.join('')).to.eql('aca', 'single node'); + expect(w2.addNode('c').nodes.join('')).to.eql('aca', 'single node circular'); }); - it('adds a node to a linear way at a positive index', function () { + it('adds an internal node to a linear way at a positive index', function () { var w = iD.Way({ nodes: 'ab'.split('') }); expect(w.addNode('c', 1).nodes.join('')).to.eql('acb'); }); - it('adds a node to a circular way at a positive index', function () { + it('adds an internal node to a circular way at a positive index', function () { var w1 = iD.Way({ nodes: 'aba'.split('') }); expect(w1.addNode('c', 1).nodes.join('')).to.eql('acba', 'circular'); var w2 = iD.Way({ nodes: 'aa'.split('') }); - expect(w2.addNode('c', 1).nodes.join('')).to.eql('aca', 'single node'); + expect(w2.addNode('c', 1).nodes.join('')).to.eql('aca', 'single node circular'); }); - it('adds a node to a linear way at index 0', function () { + it('adds a leading node to a linear way at index 0', function () { var w = iD.Way({ nodes: 'ab'.split('') }); expect(w.addNode('c', 0).nodes.join('')).to.eql('cab'); }); - it('adds a node to a circular way at index 0, preserving circularity', function () { + it('adds a leading node to a circular way at index 0, preserving circularity', function () { var w1 = iD.Way({ nodes: 'aba'.split('') }); expect(w1.addNode('c', 0).nodes.join('')).to.eql('cabc', 'circular'); var w2 = iD.Way({ nodes: 'aa'.split('') }); - expect(w2.addNode('c', 0).nodes.join('')).to.eql('cac', 'single node'); + expect(w2.addNode('c', 0).nodes.join('')).to.eql('cac', 'single node circular'); }); it('throws RangeError if index outside of array range for linear way', function () { @@ -486,9 +486,9 @@ describe('iD.osmWay', function() { var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); expect(w5.addNode('b').nodes.join('')).to.eql('abcba', 'duplicates multiple places'); var w6 = iD.Way({ nodes: 'aa'.split('') }); - expect(w6.addNode('a').nodes.join('')).to.eql('aa', 'single node'); + expect(w6.addNode('a').nodes.join('')).to.eql('aa', 'single node circular'); var w7 = iD.Way({ nodes: 'aaa'.split('') }); - expect(w7.addNode('a').nodes.join('')).to.eql('aa', 'single node with duplicates'); + expect(w7.addNode('a').nodes.join('')).to.eql('aa', 'single node circular with duplicates'); }); it('eliminates duplicate consecutive nodes when adding different node before the end connector of a circular way', function () { @@ -503,9 +503,9 @@ describe('iD.osmWay', function() { var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); expect(w5.addNode('d').nodes.join('')).to.eql('abcbda', 'duplicates multiple places'); var w6 = iD.Way({ nodes: 'aa'.split('') }); - expect(w6.addNode('d').nodes.join('')).to.eql('ada', 'single node'); + expect(w6.addNode('d').nodes.join('')).to.eql('ada', 'single node circular'); var w7 = iD.Way({ nodes: 'aaa'.split('') }); - expect(w7.addNode('d').nodes.join('')).to.eql('ada', 'single node with duplicates'); + expect(w7.addNode('d').nodes.join('')).to.eql('ada', 'single node circular with duplicates'); }); it('eliminates duplicate consecutive nodes when adding to the beginning of a linear way', function () { @@ -531,9 +531,9 @@ describe('iD.osmWay', function() { var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); expect(w5.addNode('a', 0).nodes.join('')).to.eql('abcba', 'duplicates multiple places'); var w6 = iD.Way({ nodes: 'aa'.split('') }); - expect(w6.addNode('a', 0).nodes.join('')).to.eql('aa', 'single node'); + expect(w6.addNode('a', 0).nodes.join('')).to.eql('aa', 'single node circular'); var w7 = iD.Way({ nodes: 'aaa'.split('') }); - expect(w7.addNode('a', 0).nodes.join('')).to.eql('aa', 'single node with duplicates'); + expect(w7.addNode('a', 0).nodes.join('')).to.eql('aa', 'single node circular with duplicates'); }); it('eliminates duplicate consecutive nodes when adding different node as beginning connector of a circular way', function () { @@ -548,9 +548,9 @@ describe('iD.osmWay', function() { var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); expect(w5.addNode('d', 0).nodes.join('')).to.eql('dabcbd', 'duplicates multiple places'); var w6 = iD.Way({ nodes: 'aa'.split('') }); - expect(w6.addNode('d', 0).nodes.join('')).to.eql('dad', 'single node'); + expect(w6.addNode('d', 0).nodes.join('')).to.eql('dad', 'single node circular'); var w7 = iD.Way({ nodes: 'aaa'.split('') }); - expect(w7.addNode('d', 0).nodes.join('')).to.eql('dad', 'single node with duplicates'); + expect(w7.addNode('d', 0).nodes.join('')).to.eql('dad', 'single node circular with duplicates'); }); }); @@ -558,155 +558,144 @@ describe('iD.osmWay', function() { describe('#updateNode', function () { it('throws RangeError if empty way', function () { var w = iD.Way(); - expect(w.updateNode.bind(w, 'a', 0)).to.throw(RangeError, /out of range 0\.\.-1/); + expect(w.updateNode.bind(w, 'd', 0)).to.throw(RangeError, /out of range 0\.\.-1/); }); - it('updates a node on a linear way at a positive index', function () { + it('updates an internal node on a linear way at a positive index', function () { var w = iD.Way({ nodes: 'ab'.split('') }); - expect(w.updateNode('c', 1).nodes.join('')).to.eql('ac'); + expect(w.updateNode('d', 1).nodes.join('')).to.eql('ad'); }); - it('updates a node on a circular way at a positive index', function () { - var w1 = iD.Way({ nodes: 'aba'.split('') }); - expect(w1.updateNode('c', 1).nodes.join('')).to.eql('aca', 'circular'); - var w2 = iD.Way({ nodes: 'aa'.split('') }); - expect(w2.updateNode('c', 1).nodes.join('')).to.eql('aca', 'single node'); + it('updates an internal node on a circular way at a positive index', function () { + var w = iD.Way({ nodes: 'aba'.split('') }); + expect(w.updateNode('d', 1).nodes.join('')).to.eql('ada', 'circular'); }); - it('updates a node on a linear way at index 0', function () { + it('updates a leading node on a linear way at index 0', function () { var w = iD.Way({ nodes: 'ab'.split('') }); - expect(w.updateNode('c', 0).nodes.join('')).to.eql('cab'); + expect(w.updateNode('d', 0).nodes.join('')).to.eql('db'); }); - it('updates a node on a circular way at index 0, preserving circularity', function () { + it('updates a leading node on a circular way at index 0, preserving circularity', function () { var w1 = iD.Way({ nodes: 'aba'.split('') }); - expect(w1.updateNode('c', 0).nodes.join('')).to.eql('cabc', 'circular'); + expect(w1.updateNode('d', 0).nodes.join('')).to.eql('dbd', 'circular'); var w2 = iD.Way({ nodes: 'aa'.split('') }); - expect(w2.updateNode('c', 0).nodes.join('')).to.eql('cac', 'single node'); + expect(w2.updateNode('d', 0).nodes.join('')).to.eql('dd', 'single node circular'); }); it('throws RangeError if index outside of array range for linear way', function () { var w = iD.Way({ nodes: 'ab'.split('') }); - expect(w.addNode.bind(w, 'c', 3)).to.throw(RangeError, /out of range 0\.\.2/, 'over range'); - expect(w.addNode.bind(w, 'c', -1)).to.throw(RangeError, /out of range 0\.\.2/, 'under range'); + expect(w.updateNode.bind(w, 'd', 2)).to.throw(RangeError, /out of range 0\.\.1/, 'over range'); + expect(w.updateNode.bind(w, 'd', -1)).to.throw(RangeError, /out of range 0\.\.1/, 'under range'); }); it('throws RangeError if index outside of array range for circular way', function () { var w = iD.Way({ nodes: 'aba'.split('') }); - expect(w.addNode.bind(w, 'c', 3)).to.throw(RangeError, /out of range 0\.\.2/, 'over range'); - expect(w.addNode.bind(w, 'c', -1)).to.throw(RangeError, /out of range 0\.\.2/, 'under range'); + expect(w.updateNode.bind(w, 'd', 3)).to.throw(RangeError, /out of range 0\.\.2/, 'over range'); + expect(w.updateNode.bind(w, 'd', -1)).to.throw(RangeError, /out of range 0\.\.2/, 'under range'); }); - it('eliminates duplicate consecutive nodes when adding to the end of a linear way', function () { - var w1 = iD.Way({ nodes: 'abb'.split('') }); - expect(w1.addNode('b').nodes.join('')).to.eql('ab', 'duplicate at end'); + it('eliminates duplicate consecutive nodes when updating the end of a linear way', function () { + var w1 = iD.Way({ nodes: 'abcc'.split('') }); + expect(w1.updateNode('c', 3).nodes.join('')).to.eql('abc', 'duplicate at end'); var w2 = iD.Way({ nodes: 'abbc'.split('') }); - expect(w2.addNode('c').nodes.join('')).to.eql('abc', 'duplicate in middle'); + expect(w2.updateNode('c', 3).nodes.join('')).to.eql('abc', 'duplicate in middle'); var w3 = iD.Way({ nodes: 'aabc'.split('') }); - expect(w3.addNode('c').nodes.join('')).to.eql('abc', 'duplicate at beginning'); + expect(w3.updateNode('c', 3).nodes.join('')).to.eql('abc', 'duplicate at beginning'); var w4 = iD.Way({ nodes: 'abbbcbb'.split('') }); - expect(w4.addNode('b').nodes.join('')).to.eql('abcb', 'duplicates multiple places'); + expect(w4.updateNode('b', 6).nodes.join('')).to.eql('abcb', 'duplicates multiple places'); }); - it('eliminates duplicate consecutive nodes when adding before the end connector of a circular way', function () { + it('eliminates duplicate consecutive nodes when updating same node before the end connector of a circular way', function () { var w1 = iD.Way({ nodes: 'abcca'.split('') }); - expect(w1.addNode('c').nodes.join('')).to.eql('abca', 'duplicate internal node at end'); + expect(w1.updateNode('c', 3).nodes.join('')).to.eql('abca', 'duplicate internal node at end'); var w2 = iD.Way({ nodes: 'abbca'.split('') }); - expect(w2.addNode('c').nodes.join('')).to.eql('abca', 'duplicate internal node in middle'); + expect(w2.updateNode('c', 3).nodes.join('')).to.eql('abca', 'duplicate internal node in middle'); var w3 = iD.Way({ nodes: 'aabca'.split('') }); - expect(w3.addNode('c').nodes.join('')).to.eql('abca', 'duplicate connector node at beginning'); + expect(w3.updateNode('c', 3).nodes.join('')).to.eql('abca', 'duplicate connector node at beginning'); var w4 = iD.Way({ nodes: 'abcaa'.split('') }); - expect(w4.addNode('a').nodes.join('')).to.eql('abca', 'duplicate connector node at end'); + expect(w4.updateNode('a', 3).nodes.join('')).to.eql('abca', 'duplicate connector node at end'); var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); - expect(w5.addNode('b').nodes.join('')).to.eql('abcba', 'duplicates multiple places'); - var w6 = iD.Way({ nodes: 'aa'.split('') }); - expect(w6.addNode('a').nodes.join('')).to.eql('aa', 'single node'); - var w7 = iD.Way({ nodes: 'aaa'.split('') }); - expect(w7.addNode('a').nodes.join('')).to.eql('aa', 'single node with duplicates'); + expect(w5.updateNode('b', 6).nodes.join('')).to.eql('abcba', 'duplicates multiple places'); }); - it('eliminates duplicate consecutive nodes when adding to the beginning of a linear way', function () { + it('eliminates duplicate consecutive nodes when updating different node before the end connector of a circular way', function () { + var w1 = iD.Way({ nodes: 'abcca'.split('') }); + expect(w1.updateNode('d', 3).nodes.join('')).to.eql('abcda', 'duplicate internal node at end'); + var w2 = iD.Way({ nodes: 'abbca'.split('') }); + expect(w2.updateNode('d', 3).nodes.join('')).to.eql('abda', 'duplicate internal node in middle'); + var w3 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w3.updateNode('d', 3).nodes.join('')).to.eql('abda', 'duplicate connector node at beginning'); + var w4 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w4.updateNode('d', 3).nodes.join('')).to.eql('dbcd', 'duplicate connector node at end'); + var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); + expect(w5.updateNode('d', 6).nodes.join('')).to.eql('abcbda', 'duplicates multiple places'); + }); + + it('eliminates duplicate consecutive nodes when updating the beginning of a linear way', function () { var w1 = iD.Way({ nodes: 'abb'.split('') }); - expect(w1.addNode('a', 0).nodes.join('')).to.eql('ab', 'duplicate at end'); + expect(w1.updateNode('b', 0).nodes.join('')).to.eql('b', 'duplicate at end'); var w2 = iD.Way({ nodes: 'abbc'.split('') }); - expect(w2.addNode('a', 0).nodes.join('')).to.eql('abc', 'duplicate in middle'); + expect(w2.updateNode('b', 0).nodes.join('')).to.eql('bc', 'duplicate in middle'); var w3 = iD.Way({ nodes: 'aabc'.split('') }); - expect(w3.addNode('a', 0).nodes.join('')).to.eql('abc', 'duplicate at beginning'); + expect(w3.updateNode('a', 0).nodes.join('')).to.eql('abc', 'duplicate at beginning'); var w4 = iD.Way({ nodes: 'abbbcbb'.split('') }); - expect(w4.addNode('a', 0).nodes.join('')).to.eql('abcb', 'duplicates multiple places'); + expect(w4.updateNode('a', 0).nodes.join('')).to.eql('abcb', 'duplicates multiple places'); }); - it('eliminates duplicate consecutive nodes when adding same node as beginning connector a circular way', function () { + it('eliminates duplicate consecutive nodes when updating same node as beginning connector a circular way', function () { var w1 = iD.Way({ nodes: 'abcca'.split('') }); - expect(w1.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate internal node at end'); + expect(w1.updateNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate internal node at end'); var w2 = iD.Way({ nodes: 'abbca'.split('') }); - expect(w2.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate internal node in middle'); + expect(w2.updateNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate internal node in middle'); var w3 = iD.Way({ nodes: 'aabca'.split('') }); - expect(w3.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate connector node at beginning'); + expect(w3.updateNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate connector node at beginning'); var w4 = iD.Way({ nodes: 'abcaa'.split('') }); - expect(w4.addNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate connector node at end'); + expect(w4.updateNode('a', 0).nodes.join('')).to.eql('abca', 'duplicate connector node at end'); var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); - expect(w5.addNode('a', 0).nodes.join('')).to.eql('abcba', 'duplicates multiple places'); + expect(w5.updateNode('a', 0).nodes.join('')).to.eql('abcba', 'duplicates multiple places'); var w6 = iD.Way({ nodes: 'aa'.split('') }); - expect(w6.addNode('a', 0).nodes.join('')).to.eql('aa', 'single node'); + expect(w6.updateNode('a', 0).nodes.join('')).to.eql('aa', 'single node circular'); var w7 = iD.Way({ nodes: 'aaa'.split('') }); - expect(w7.addNode('a', 0).nodes.join('')).to.eql('aa', 'single node with duplicates'); + expect(w7.updateNode('a', 0).nodes.join('')).to.eql('aa', 'single node circular with duplicates'); }); - it('eliminates duplicate consecutive nodes when adding different node as beginning connector of a circular way', function () { + it('eliminates duplicate consecutive nodes when updating different node as beginning connector of a circular way', function () { var w1 = iD.Way({ nodes: 'abcca'.split('') }); - expect(w1.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate internal node at end'); + expect(w1.updateNode('d', 0).nodes.join('')).to.eql('dbcd', 'duplicate internal node at end'); var w2 = iD.Way({ nodes: 'abbca'.split('') }); - expect(w2.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate internal node in middle'); + expect(w2.updateNode('d', 0).nodes.join('')).to.eql('dbcd', 'duplicate internal node in middle'); var w3 = iD.Way({ nodes: 'aabca'.split('') }); - expect(w3.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate connector node at beginning'); + expect(w3.updateNode('d', 0).nodes.join('')).to.eql('dbcd', 'duplicate connector node at beginning'); var w4 = iD.Way({ nodes: 'abcaa'.split('') }); - expect(w4.addNode('d', 0).nodes.join('')).to.eql('dabcd', 'duplicate connector node at end'); + expect(w4.updateNode('d', 0).nodes.join('')).to.eql('dbcd', 'duplicate connector node at end'); var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); - expect(w5.addNode('d', 0).nodes.join('')).to.eql('dabcbd', 'duplicates multiple places'); + expect(w5.updateNode('d', 0).nodes.join('')).to.eql('dbcbd', 'duplicates multiple places'); var w6 = iD.Way({ nodes: 'aa'.split('') }); - expect(w6.addNode('d', 0).nodes.join('')).to.eql('dad', 'single node'); + expect(w6.updateNode('d', 0).nodes.join('')).to.eql('dd', 'single node circular'); var w7 = iD.Way({ nodes: 'aaa'.split('') }); - expect(w7.addNode('d', 0).nodes.join('')).to.eql('dad', 'single node with duplicates'); + expect(w7.updateNode('d', 0).nodes.join('')).to.eql('dd', 'single node circular with duplicates'); }); - - -// old tests: - it('updates the node id at the specified index', function () { - var w = iD.Way({nodes: ['a', 'b', 'c']}); - expect(w.updateNode('d', 1).nodes).to.eql(['a', 'd', 'c']); - }); - - it('prevents duplicate consecutive nodes', function () { - var w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); - expect(w.updateNode('b',2).nodes).to.eql(['a', 'b', 'd','e']); - w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); - expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd','e']); - w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); - expect(w.updateNode('b',2).nodes).to.eql(['a', 'b','e']); - }); - - it('preserves duplicate non-consecutive nodes', function () { - var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); - expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd', 'b','e']); - }); - - it('replaces a single one of duplicate nodes', function () { - var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); - expect(w.updateNode('d',1).nodes).to.eql(['a', 'd', 'c', 'b','e']); - w = iD.Way({nodes: ['a', 'b', 'b', 'c','e']}); - expect(w.updateNode('d',2).nodes).to.eql(['a', 'b', 'd', 'c','e']); - }); - - it('removes existing duplicate consecutive nodes', function () { - var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); - expect(w.updateNode('c',5).nodes).to.eql(['a', 'b', 'd', 'b','c']); - w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); - expect(w.updateNode('c',3).nodes).to.eql(['a', 'b', 'c', 'b', 'e']); + it('eliminates duplicate consecutive nodes when updating different node as ending connector of a circular way', function () { + var w1 = iD.Way({ nodes: 'abcca'.split('') }); + expect(w1.updateNode('d', 4).nodes.join('')).to.eql('dbcd', 'duplicate internal node at end'); + var w2 = iD.Way({ nodes: 'abbca'.split('') }); + expect(w2.updateNode('d', 4).nodes.join('')).to.eql('dbcd', 'duplicate internal node in middle'); + var w3 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w3.updateNode('d', 4).nodes.join('')).to.eql('dbcd', 'duplicate connector node at beginning'); + var w4 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w4.updateNode('d', 4).nodes.join('')).to.eql('dbcd', 'duplicate connector node at end'); + var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); + expect(w5.updateNode('d', 7).nodes.join('')).to.eql('dbcbd', 'duplicates multiple places'); + var w6 = iD.Way({ nodes: 'aa'.split('') }); + expect(w6.updateNode('d', 1).nodes.join('')).to.eql('dd', 'single node circular'); + var w7 = iD.Way({ nodes: 'aaa'.split('') }); + expect(w7.updateNode('d', 2).nodes.join('')).to.eql('dd', 'single node circular with duplicates'); }); }); + describe('#replaceNode', function () { it('replaces the node', function () { var w = iD.Way({nodes: ['a']}); From 7b86afc9de12eac93319258ee4da9ac0ad499072 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 9 Jan 2017 18:16:30 -0500 Subject: [PATCH 08/25] Changes to replaceNode and add tests, also stricter isClosed --- modules/osm/way.js | 30 +++++++++------ test/spec/osm/way.js | 89 +++++++++++++++++++++++++++++--------------- 2 files changed, 78 insertions(+), 41 deletions(-) diff --git a/modules/osm/way.js b/modules/osm/way.js index 8f0f88e85..519ae45fb 100644 --- a/modules/osm/way.js +++ b/modules/osm/way.js @@ -122,7 +122,7 @@ _.extend(osmWay.prototype, { isClosed: function() { - return this.nodes.length > 0 && this.first() === this.last(); + return this.nodes.length > 1 && this.first() === this.last(); }, @@ -286,8 +286,10 @@ _.extend(osmWay.prototype, { // Replaces each occurrence of node id needle with replacement. // Consecutive duplicates are eliminated including existing ones. + // Circularity is preserved. replaceNode: function(needle, replacement) { - var nodes = this.nodes.slice(); + var nodes = this.nodes.slice(), + isClosed = this.isClosed(); for (var i = 0; i < nodes.length; i++) { if (nodes[i] === needle) { @@ -296,7 +298,12 @@ _.extend(osmWay.prototype, { } nodes = nodes.filter(noRepeatNodes); - // checkCircular(this, nodes); + + // If the way was closed before, append a connector node to keep it closed.. + if (isClosed && (nodes.length === 1 || nodes[0] !== nodes[nodes.length - 1])) { + nodes.push(nodes[0]); + } + return this.update({nodes: nodes}); }, @@ -305,12 +312,18 @@ _.extend(osmWay.prototype, { // Consecutive duplicates are eliminated including existing ones. // Circularity is preserved. removeNode: function(id) { - var nodes = this.nodes.slice(); + var nodes = this.nodes.slice(), + isClosed = this.isClosed(); nodes = nodes.filter(function(node, i, arr) { return node !== id && noRepeatNodes(node, i, arr); }); - // checkCircular(this, nodes); + + // If the way was closed before, append a connector node to keep it closed.. + if (isClosed && (nodes.length === 1 || nodes[0] !== nodes[nodes.length - 1])) { + nodes.push(nodes[0]); + } + return this.update({nodes: nodes}); }, @@ -385,10 +398,3 @@ _.extend(osmWay.prototype, { function noRepeatNodes(node, i, arr) { return i === 0 || node !== arr[i - 1]; } - -// If the nodelist was circular before, keep it circular. -function keepCircular(nodes) { - if ((nodes.length === 1 || nodes[0] !== nodes[nodes.length - 1])) { - nodes.push(nodes[0]); - } -} diff --git a/test/spec/osm/way.js b/test/spec/osm/way.js index 63d931b8b..c17165ae7 100644 --- a/test/spec/osm/way.js +++ b/test/spec/osm/way.js @@ -127,16 +127,24 @@ describe('iD.osmWay', function() { }); describe('#isClosed', function() { - it('returns false when the way has no nodes', function() { - expect(iD.Way().isClosed()).to.equal(false); + it('returns false when the way contains no nodes', function() { + expect(iD.Way().isClosed()).to.be.false; + }); + + it('returns false when the way contains a single node', function() { + expect(iD.Way({ nodes: 'a'.split('') }).isClosed()).to.be.false; }); it('returns false when the way ends are not equal', function() { - expect(iD.Way({nodes: ['n1', 'n2']}).isClosed()).to.equal(false); + expect(iD.Way({ nodes: 'abc'.split('') }).isClosed()).to.be.false; }); it('returns true when the way ends are equal', function() { - expect(iD.Way({nodes: ['n1', 'n2', 'n1']}).isClosed()).to.equal(true); + expect(iD.Way({ nodes: 'aba'.split('') }).isClosed()).to.be.true; + }); + + it('returns true when the way contains two of the same node', function() { + expect(iD.Way({ nodes: 'aa'.split('') }).isClosed()).to.be.true; }); }); @@ -697,39 +705,62 @@ describe('iD.osmWay', function() { describe('#replaceNode', function () { - it('replaces the node', function () { - var w = iD.Way({nodes: ['a']}); - expect(w.replaceNode('a','b').nodes).to.eql(['b']); - w = iD.Way({nodes: ['a', 'b', 'c']}); - expect(w.replaceNode('b', 'd').nodes).to.eql(['a', 'd', 'c']); + it('replaces a node', function () { + var w1 = iD.Way({ nodes: 'a'.split('') }); + expect(w1.replaceNode('a','b').nodes.join('')).to.eql('b', 'single replace, single node'); + var w2 = iD.Way({ nodes: 'abc'.split('') }); + expect(w2.replaceNode('b','d').nodes.join('')).to.eql('adc', 'single replace, linear'); + var w2 = iD.Way({ nodes: 'abca'.split('') }); + expect(w2.replaceNode('b','d').nodes.join('')).to.eql('adca', 'single replace, circular'); }); - it('prevents duplicate consecutive nodes', function () { - var w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); - expect(w.replaceNode('c','b').nodes).to.eql(['a', 'b', 'd','e']); - w = iD.Way({nodes: ['a', 'b', 'c', 'd','e']}); - expect(w.replaceNode('c','d').nodes).to.eql(['a', 'b', 'd','e']); - w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); - expect(w.replaceNode('c','b').nodes).to.eql(['a', 'b','e']); + it('replaces multiply occurring nodes', function () { + var w1 = iD.Way({ nodes: 'abcb'.split('') }); + expect(w1.replaceNode('b','d').nodes.join('')).to.eql('adcd', 'multiple replace, linear'); + var w2 = iD.Way({ nodes: 'abca'.split('') }); + expect(w2.replaceNode('a','d').nodes.join('')).to.eql('dbcd', 'multiple replace, circular'); + var w3 = iD.Way({ nodes: 'aa'.split('') }); + expect(w3.replaceNode('a','d').nodes.join('')).to.eql('dd', 'multiple replace, single node circular'); }); - it('preserves duplicate non-consecutive nodes', function () { - var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); - expect(w.replaceNode('c','d').nodes).to.eql(['a', 'b', 'd', 'b','e']); + it('eliminates duplicate consecutive nodes when replacing along a linear way', function () { + var w1 = iD.Way({ nodes: 'abbcd'.split('') }); + expect(w1.replaceNode('c','b').nodes.join('')).to.eql('abd', 'duplicate before'); + var w2 = iD.Way({ nodes: 'abcdd'.split('') }); + expect(w2.replaceNode('c','d').nodes.join('')).to.eql('abd', 'duplicate after'); + var w3 = iD.Way({ nodes: 'abbcbb'.split('')}); + expect(w3.replaceNode('c','b').nodes.join('')).to.eql('ab', 'duplicate before and after'); }); - it('replaces duplicate non-consecutive nodes', function () { - var w = iD.Way({nodes: ['a', 'b', 'c', 'b','e']}); - expect(w.replaceNode('b','d').nodes).to.eql(['a', 'd', 'c', 'd','e']); + it('eliminates duplicate consecutive nodes when replacing internal nodes along a circular way', function () { + var w1 = iD.Way({ nodes: 'abbcda'.split('') }); + expect(w1.replaceNode('c','b').nodes.join('')).to.eql('abda', 'duplicate before'); + var w2 = iD.Way({ nodes: 'abcdda'.split('') }); + expect(w2.replaceNode('c','d').nodes.join('')).to.eql('abda', 'duplicate after'); + var w3 = iD.Way({ nodes: 'abbcbba'.split('')}); + expect(w3.replaceNode('c','b').nodes.join('')).to.eql('aba', 'duplicate before and after'); }); - it('removes existing duplicate consecutive nodes', function () { - var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); - expect(w.replaceNode('e','c').nodes).to.eql(['a', 'b', 'd', 'b','c']); - w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); - expect(w.replaceNode('d','c').nodes).to.eql(['a', 'b', 'c', 'b', 'e']); - w = iD.Way({nodes: ['a', 'b', 'b', 'c', 'b', 'e']}); - expect(w.replaceNode('b','d').nodes).to.eql(['a', 'd', 'c', 'd', 'e']); + it('eliminates duplicate consecutive nodes when replacing adjacent to connecting nodes along a circular way', function () { + var w1 = iD.Way({ nodes: 'abcda'.split('') }); + expect(w1.replaceNode('d','a').nodes.join('')).to.eql('abca', 'before single end connector'); + var w2 = iD.Way({ nodes: 'abcda'.split('') }); + expect(w2.replaceNode('b','a').nodes.join('')).to.eql('acda', 'after single beginning connector'); + var w3 = iD.Way({ nodes: 'abcdaa'.split('') }); + expect(w3.replaceNode('d','a').nodes.join('')).to.eql('abca', 'before duplicate end connector'); + var w4 = iD.Way({ nodes: 'aabcda'.split('') }); + expect(w4.replaceNode('b','a').nodes.join('')).to.eql('acda', 'after duplicate beginning connector'); + }); + + it('eliminates duplicate consecutive nodes when replacing connecting nodes along a circular way', function () { + var w1 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w1.replaceNode('a','d').nodes.join('')).to.eql('dbcd', 'duplicate end connector'); + var w2 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w2.replaceNode('a','d').nodes.join('')).to.eql('dbcd', 'duplicate beginning connector'); + var w3 = iD.Way({ nodes: 'aabcaa'.split('') }); + expect(w3.replaceNode('a','d').nodes.join('')).to.eql('dbcd', 'duplicate beginning and end connectors'); + var w4 = iD.Way({ nodes: 'aabaacaa'.split('') }); + expect(w4.replaceNode('a','d').nodes.join('')).to.eql('dbdcd', 'duplicates multiple places'); }); }); From 8676dd6e4cd0e19fa83d276c9874abdf1fd1e38e Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 9 Jan 2017 18:38:28 -0500 Subject: [PATCH 09/25] Changes to removeNode and add tests --- modules/osm/way.js | 6 ++-- test/spec/osm/way.js | 69 ++++++++++++++++++++++++++++++-------------- 2 files changed, 51 insertions(+), 24 deletions(-) diff --git a/modules/osm/way.js b/modules/osm/way.js index 519ae45fb..5f71ca599 100644 --- a/modules/osm/way.js +++ b/modules/osm/way.js @@ -315,9 +315,9 @@ _.extend(osmWay.prototype, { var nodes = this.nodes.slice(), isClosed = this.isClosed(); - nodes = nodes.filter(function(node, i, arr) { - return node !== id && noRepeatNodes(node, i, arr); - }); + nodes = nodes + .filter(function(node, i, arr) { return node !== id }) + .filter(noRepeatNodes); // If the way was closed before, append a connector node to keep it closed.. if (isClosed && (nodes.length === 1 || nodes[0] !== nodes[nodes.length - 1])) { diff --git a/test/spec/osm/way.js b/test/spec/osm/way.js index c17165ae7..3b3e8b1ad 100644 --- a/test/spec/osm/way.js +++ b/test/spec/osm/way.js @@ -710,8 +710,8 @@ describe('iD.osmWay', function() { expect(w1.replaceNode('a','b').nodes.join('')).to.eql('b', 'single replace, single node'); var w2 = iD.Way({ nodes: 'abc'.split('') }); expect(w2.replaceNode('b','d').nodes.join('')).to.eql('adc', 'single replace, linear'); - var w2 = iD.Way({ nodes: 'abca'.split('') }); - expect(w2.replaceNode('b','d').nodes.join('')).to.eql('adca', 'single replace, circular'); + var w4 = iD.Way({ nodes: 'abca'.split('') }); + expect(w4.replaceNode('b','d').nodes.join('')).to.eql('adca', 'single replace, circular'); }); it('replaces multiply occurring nodes', function () { @@ -766,35 +766,62 @@ describe('iD.osmWay', function() { describe('#removeNode', function () { - it('removes the node', function () { - var w = iD.Way({nodes: ['a']}); - expect(w.removeNode('a').nodes).to.eql([]); + it('removes a node', function () { + var w1 = iD.Way({ nodes: 'a'.split('') }); + expect(w1.removeNode('a').nodes.join('')).to.eql('', 'single remove, single node'); + var w2 = iD.Way({ nodes: 'abc'.split('') }); + expect(w2.removeNode('b').nodes.join('')).to.eql('ac', 'single remove, linear'); + var w3 = iD.Way({ nodes: 'abca'.split('') }); + expect(w3.removeNode('b').nodes.join('')).to.eql('aca', 'single remove, circular'); + var w4 = iD.Way({ nodes: 'aa'.split('') }); + expect(w4.removeNode('a').nodes.join('')).to.eql('', 'multiple remove, single node circular'); }); - it('prevents duplicate consecutive nodes', function () { - var w = iD.Way({nodes: ['a', 'b', 'c', 'b']}); - expect(w.removeNode('c').nodes).to.eql(['a', 'b']); + it('removes multiply occurring nodes', function () { + var w1 = iD.Way({ nodes: 'abcb'.split('') }); + expect(w1.removeNode('b').nodes.join('')).to.eql('ac', 'multiple remove, linear'); + var w2 = iD.Way({ nodes: 'abcba'.split('') }); + expect(w2.removeNode('b').nodes.join('')).to.eql('aca', 'multiple remove, circular'); }); - it('preserves circularity', function () { - var w = iD.Way({nodes: ['a', 'b', 'c', 'd', 'a']}); - expect(w.removeNode('a').nodes).to.eql(['b', 'c', 'd', 'b']); + it('eliminates duplicate consecutive nodes when removing along a linear way', function () { + var w1 = iD.Way({ nodes: 'abbcd'.split('') }); + expect(w1.removeNode('c').nodes.join('')).to.eql('abd', 'duplicate before'); + var w2 = iD.Way({ nodes: 'abcdd'.split('') }); + expect(w2.removeNode('c').nodes.join('')).to.eql('abd', 'duplicate after'); + var w3 = iD.Way({ nodes: 'abbcbb'.split('')}); + expect(w3.removeNode('c').nodes.join('')).to.eql('ab', 'duplicate before and after'); }); - it('prevents duplicate consecutive nodes when preserving circularity', function () { - var w = iD.Way({nodes: ['a', 'b', 'c', 'd', 'b', 'a']}); - expect(w.removeNode('a').nodes).to.eql(['b', 'c', 'd', 'b']); - w = iD.Way({nodes: ['a', 'b', 'a']}); - expect(w.removeNode('b').nodes).to.eql(['a']); + it('eliminates duplicate consecutive nodes when removing internal nodes along a circular way', function () { + var w1 = iD.Way({ nodes: 'abbcda'.split('') }); + expect(w1.removeNode('c').nodes.join('')).to.eql('abda', 'duplicate before'); + var w2 = iD.Way({ nodes: 'abcdda'.split('') }); + expect(w2.removeNode('c').nodes.join('')).to.eql('abda', 'duplicate after'); + var w3 = iD.Way({ nodes: 'abbcbba'.split('')}); + expect(w3.removeNode('c').nodes.join('')).to.eql('aba', 'duplicate before and after'); }); - it('removes existing duplicate consecutive nodes', function () { - var w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); - expect(w.removeNode('e').nodes).to.eql(['a', 'b', 'd', 'b']); - w = iD.Way({nodes: ['a', 'b', 'b', 'd', 'b', 'e']}); - expect(w.removeNode('b').nodes).to.eql(['a', 'd', 'e']); + + it('eliminates duplicate consecutive nodes when removing adjacent to connecting nodes along a circular way', function () { + var w1 = iD.Way({ nodes: 'abcdaa'.split('') }); + expect(w1.removeNode('d').nodes.join('')).to.eql('abca', 'duplicate end connector'); + var w2 = iD.Way({ nodes: 'aabcda'.split('') }); + expect(w2.removeNode('b').nodes.join('')).to.eql('acda', 'duplicate beginning connector'); + }); + + it('eliminates duplicate consecutive nodes when removing connecting nodes along a circular way', function () { + var w1 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w1.removeNode('a').nodes.join('')).to.eql('bcb', 'duplicate end connector'); + var w2 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w2.removeNode('a').nodes.join('')).to.eql('bcb', 'duplicate beginning connector'); + var w3 = iD.Way({ nodes: 'aabcaa'.split('') }); + expect(w3.removeNode('a').nodes.join('')).to.eql('bcb', 'duplicate beginning and end connectors'); + var w4 = iD.Way({ nodes: 'aabaacaa'.split('') }); + expect(w4.removeNode('a').nodes.join('')).to.eql('bcb', 'duplicates multiple places'); }); }); + describe('#asJXON', function () { it('converts a way to jxon', function() { var node = iD.Way({id: 'w-1', nodes: ['n1', 'n2'], tags: {highway: 'residential'}}); From 73e27c96571aa7fa4470fbe8ed3528b4e8ef70ca Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 9 Jan 2017 19:41:38 -0500 Subject: [PATCH 10/25] Now that updateNode preserves circularity, provide close/unclose functions This lets us break closed ways at their connecting node in the few situations where we actually want that behavior (disconnect action for circular non-area ways) --- modules/actions/disconnect.js | 7 +++++-- modules/osm/way.js | 36 ++++++++++++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/modules/actions/disconnect.js b/modules/actions/disconnect.js index 85f9ce454..82274234f 100644 --- a/modules/actions/disconnect.js +++ b/modules/actions/disconnect.js @@ -31,6 +31,9 @@ export function actionDisconnect(nodeId, newNodeId) { if (connection.index === 0 && way.isArea()) { // replace shared node with shared node.. graph = graph.replace(way.replaceNode(way.nodes[0], newNode.id)); + } else if (way.isClosed() && connection.index === way.nodes.length - 1) { + // replace closing node with new new node.. + graph = graph.replace(way.unclose().addNode(newNode.id)); } else { // replace shared node with multiple new nodes.. graph = graph.replace(way.updateNode(newNode.id, connection.index)); @@ -52,11 +55,11 @@ export function actionDisconnect(nodeId, newNodeId) { return; } if (way.isArea() && (way.nodes[0] === nodeId)) { - candidates.push({wayID: way.id, index: 0}); + candidates.push({ wayID: way.id, index: 0 }); } else { way.nodes.forEach(function(waynode, index) { if (waynode === nodeId) { - candidates.push({wayID: way.id, index: index}); + candidates.push({ wayID: way.id, index: index }); } }); } diff --git a/modules/osm/way.js b/modules/osm/way.js index 5f71ca599..bd7a28b8c 100644 --- a/modules/osm/way.js +++ b/modules/osm/way.js @@ -188,6 +188,36 @@ _.extend(osmWay.prototype, { }, + // If this way is not closed, append the beginning node to the end of the nodelist to close it. + close: function() { + if (this.isClosed() || !this.nodes.length) return this; + + var nodes = this.nodes.slice(); + nodes.push(nodes[0]); + nodes = nodes.filter(noRepeatNodes); + return this.update({ nodes: nodes }); + }, + + + // If this way is closed, remove any connector nodes from the end of the nodelist to unclose it. + unclose: function() { + if (!this.isClosed()) return this; + + var nodes = this.nodes.slice(), + connector = this.first(), + i = nodes.length - 1; + + // remove trailing connectors.. + while (i > 0 && nodes.length > 1 && nodes[i] === connector) { + nodes.splice(i, 1); + i = nodes.length - 1; + } + + nodes = nodes.filter(noRepeatNodes); + return this.update({ nodes: nodes }); + }, + + // Adds a node (id) in front of the node which is currently at position index. // If index is undefined, the node will be added to the end of the way for linear ways, // or just before the final connecting node for circular ways. @@ -207,7 +237,7 @@ _.extend(osmWay.prototype, { } // If this is a closed way, remove all connector nodes except the first one - // (there may be duplicates) + // (there may be duplicates) and adjust index if necessary.. if (isClosed) { var connector = this.first(); @@ -252,7 +282,7 @@ _.extend(osmWay.prototype, { } // If this is a closed way, remove all connector nodes except the first one - // (there may be duplicates) + // (there may be duplicates) and adjust index if necessary.. if (isClosed) { var connector = this.first(); @@ -316,7 +346,7 @@ _.extend(osmWay.prototype, { isClosed = this.isClosed(); nodes = nodes - .filter(function(node, i, arr) { return node !== id }) + .filter(function(node) { return node !== id; }) .filter(noRepeatNodes); // If the way was closed before, append a connector node to keep it closed.. From 42a65307969841bc09a239ef60b0d6829c8822c0 Mon Sep 17 00:00:00 2001 From: popov Date: Tue, 10 Jan 2017 10:41:58 +1000 Subject: [PATCH 11/25] Do not attach dragNode behavior in browse mode --- modules/modes/browse.js | 5 +---- modules/modes/drag_node.js | 8 +++++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/modules/modes/browse.js b/modules/modes/browse.js index 5552d91d6..16ed974d7 100644 --- a/modules/modes/browse.js +++ b/modules/modes/browse.js @@ -7,8 +7,6 @@ import { behaviorSelect } from '../behavior/index'; -import { modeDragNode } from './index'; - export function modeBrowse(context) { var mode = { @@ -22,8 +20,7 @@ export function modeBrowse(context) { behaviorPaste(context), behaviorHover(context).on('hover', context.ui().sidebar.hover), behaviorSelect(context), - behaviorLasso(context), - modeDragNode(context).behavior + behaviorLasso(context) ]; diff --git a/modules/modes/drag_node.js b/modules/modes/drag_node.js index 0eea5533b..391f9ab0a 100644 --- a/modules/modes/drag_node.js +++ b/modules/modes/drag_node.js @@ -79,12 +79,16 @@ export function modeDragNode(context) { function start(entity) { + activeIDs = _.map(context.graph().parentWays(entity), 'id'); + activeIDs.push(entity.id); + wasMidpoint = entity.type === 'midpoint'; + isCancelled = d3.event.sourceEvent.shiftKey || + !(wasMidpoint || _.some(activeIDs, function (activeID) { return selectedIDs.indexOf(activeID) !== -1; })) || context.features().hasHiddenConnections(entity, context.graph()); if (isCancelled) return behavior.cancel(); - wasMidpoint = entity.type === 'midpoint'; if (wasMidpoint) { var midpoint = entity; entity = osmNode(); @@ -97,8 +101,6 @@ export function modeDragNode(context) { context.perform(actionNoop()); } - activeIDs = _.map(context.graph().parentWays(entity), 'id'); - activeIDs.push(entity.id); context.enter(mode); } From cadb38009a311764f7ff6fefe1649baee1204c3b Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 9 Jan 2017 19:58:18 -0500 Subject: [PATCH 12/25] Add tests for way.close, way.unclose --- test/spec/osm/way.js | 70 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 4 deletions(-) diff --git a/test/spec/osm/way.js b/test/spec/osm/way.js index 3b3e8b1ad..8c41b76f8 100644 --- a/test/spec/osm/way.js +++ b/test/spec/osm/way.js @@ -417,6 +417,72 @@ describe('iD.osmWay', function() { }); }); + describe('#close', function () { + it('returns self for empty way', function () { + var w = iD.Way(); + expect(w.close()).to.deep.equal(w); + }); + + it('returns self for already closed way', function () { + var w1 = iD.Way({ nodes: 'aba'.split('') }); + expect(w1.close()).to.deep.equal(w1); + var w2 = iD.Way({ nodes: 'aa'.split('') }); + expect(w2.close()).to.deep.equal(w2); + }); + + it('closes a way', function () { + var w = iD.Way({ nodes: 'ab'.split('') }); + expect(w.close().nodes.join('')).to.eql('aba'); + }); + + it('eliminates duplicate consecutive nodes when closing a linear way', function () { + var w1 = iD.Way({ nodes: 'abb'.split('') }); + expect(w1.close().nodes.join('')).to.eql('aba', 'duplicate at end'); + var w2 = iD.Way({ nodes: 'abbc'.split('') }); + expect(w2.close().nodes.join('')).to.eql('abca', 'duplicate in middle'); + var w3 = iD.Way({ nodes: 'aabc'.split('') }); + expect(w3.close().nodes.join('')).to.eql('abca', 'duplicate at beginning'); + var w4 = iD.Way({ nodes: 'abbbcbb'.split('') }); + expect(w4.close().nodes.join('')).to.eql('abcba', 'duplicates multiple places'); + }); + }); + + describe('#unclose', function () { + it('returns self for empty way', function () { + var w = iD.Way(); + expect(w.unclose()).to.deep.equal(w); + }); + + it('returns self for already unclosed way', function () { + var w1 = iD.Way({ nodes: 'a'.split('') }); + expect(w1.unclose()).to.deep.equal(w1); + var w2 = iD.Way({ nodes: 'ab'.split('') }); + expect(w2.unclose()).to.deep.equal(w2); + }); + + it('uncloses a circular way', function () { + var w = iD.Way({ nodes: 'aba'.split('') }); + expect(w.unclose().nodes.join('')).to.eql('ab'); + }); + + it('eliminates duplicate consecutive nodes when unclosing a circular way', function () { + var w1 = iD.Way({ nodes: 'abcca'.split('') }); + expect(w1.unclose().nodes.join('')).to.eql('abc', 'duplicate internal node at end'); + var w2 = iD.Way({ nodes: 'abbca'.split('') }); + expect(w2.unclose().nodes.join('')).to.eql('abc', 'duplicate internal node in middle'); + var w3 = iD.Way({ nodes: 'aabca'.split('') }); + expect(w3.unclose().nodes.join('')).to.eql('abc', 'duplicate connector node at beginning'); + var w4 = iD.Way({ nodes: 'abcaa'.split('') }); + expect(w4.unclose().nodes.join('')).to.eql('abc', 'duplicate connector node at end'); + var w5 = iD.Way({ nodes: 'abbbcbba'.split('') }); + expect(w5.unclose().nodes.join('')).to.eql('abcb', 'duplicates multiple places'); + var w6 = iD.Way({ nodes: 'aa'.split('') }); + expect(w6.unclose().nodes.join('')).to.eql('a', 'single node circular'); + var w7 = iD.Way({ nodes: 'aaa'.split('') }); + expect(w7.unclose().nodes.join('')).to.eql('a', 'single node circular with duplicates'); + }); + }); + describe('#addNode', function () { it('adds a node to an empty way', function () { var w = iD.Way(); @@ -560,7 +626,6 @@ describe('iD.osmWay', function() { var w7 = iD.Way({ nodes: 'aaa'.split('') }); expect(w7.addNode('d', 0).nodes.join('')).to.eql('dad', 'single node circular with duplicates'); }); - }); describe('#updateNode', function () { @@ -703,7 +768,6 @@ describe('iD.osmWay', function() { }); }); - describe('#replaceNode', function () { it('replaces a node', function () { var w1 = iD.Way({ nodes: 'a'.split('') }); @@ -764,7 +828,6 @@ describe('iD.osmWay', function() { }); }); - describe('#removeNode', function () { it('removes a node', function () { var w1 = iD.Way({ nodes: 'a'.split('') }); @@ -821,7 +884,6 @@ describe('iD.osmWay', function() { }); }); - describe('#asJXON', function () { it('converts a way to jxon', function() { var node = iD.Way({id: 'w-1', nodes: ['n1', 'n2'], tags: {highway: 'residential'}}); From f109efda7ba6dc6fed81966871c2de5cebf9d5ac Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 12 Jan 2017 15:56:46 +0530 Subject: [PATCH 13/25] Fix close() for single node ways --- modules/osm/way.js | 2 +- test/spec/osm/way.js | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/modules/osm/way.js b/modules/osm/way.js index bd7a28b8c..99ccc3716 100644 --- a/modules/osm/way.js +++ b/modules/osm/way.js @@ -193,8 +193,8 @@ _.extend(osmWay.prototype, { if (this.isClosed() || !this.nodes.length) return this; var nodes = this.nodes.slice(); - nodes.push(nodes[0]); nodes = nodes.filter(noRepeatNodes); + nodes.push(nodes[0]); return this.update({ nodes: nodes }); }, diff --git a/test/spec/osm/way.js b/test/spec/osm/way.js index 8c41b76f8..f45a98ae1 100644 --- a/test/spec/osm/way.js +++ b/test/spec/osm/way.js @@ -431,8 +431,10 @@ describe('iD.osmWay', function() { }); it('closes a way', function () { - var w = iD.Way({ nodes: 'ab'.split('') }); - expect(w.close().nodes.join('')).to.eql('aba'); + var w1 = iD.Way({ nodes: 'ab'.split('') }); + expect(w1.close().nodes.join('')).to.eql('aba', 'multiple'); + var w2 = iD.Way({ nodes: 'a'.split('') }); + expect(w2.close().nodes.join('')).to.eql('aa', 'single'); }); it('eliminates duplicate consecutive nodes when closing a linear way', function () { @@ -461,8 +463,10 @@ describe('iD.osmWay', function() { }); it('uncloses a circular way', function () { - var w = iD.Way({ nodes: 'aba'.split('') }); - expect(w.unclose().nodes.join('')).to.eql('ab'); + var w1 = iD.Way({ nodes: 'aba'.split('') }); + expect(w1.unclose().nodes.join('')).to.eql('ab', 'multiple'); + var w2 = iD.Way({ nodes: 'aa'.split('') }); + expect(w2.unclose().nodes.join('')).to.eql('a', 'single'); }); it('eliminates duplicate consecutive nodes when unclosing a circular way', function () { From 8beb943fa9c296c0ed0e63194099d823908f3f77 Mon Sep 17 00:00:00 2001 From: huangyingjie Date: Thu, 12 Jan 2017 20:24:44 +0800 Subject: [PATCH 14/25] fix: properties exported are redefined --- modules/ui/index.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/modules/ui/index.js b/modules/ui/index.js index 8a51fdf98..37bfc22cc 100644 --- a/modules/ui/index.js +++ b/modules/ui/index.js @@ -1,5 +1,4 @@ export { uiInit } from './init'; -export { uiFields } from './fields/index'; export { uiAccount } from './account'; export { uiAttribution } from './attribution'; export { uiBackground } from './background'; @@ -18,7 +17,6 @@ export { uiGeolocate } from './geolocate'; export { uiHelp } from './help'; export { uiInfo } from './info'; export { uiInspector } from './inspector'; -export { uiIntro } from './intro'; export { uiLasso } from './lasso'; export { uiLoading } from './loading'; export { uiMapData } from './map_data'; From 45b7a405579357ba18bcb3a8121d81b136ad755b Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 12 Jan 2017 19:30:07 +0530 Subject: [PATCH 15/25] Fix area drawing for #3676 These changes are needed now that `addNode` * wants to preserve circularity * automatically remove duplicates * range checks index argument (can't call it with -1 anymore) --- modules/behavior/draw_way.js | 70 ++++++++++++++++++++++-------------- modules/modes/add_area.js | 13 +++++-- modules/modes/draw_area.js | 11 +++--- 3 files changed, 59 insertions(+), 35 deletions(-) diff --git a/modules/behavior/draw_way.js b/modules/behavior/draw_way.js index db3081177..396b4a2a5 100644 --- a/modules/behavior/draw_way.js +++ b/modules/behavior/draw_way.js @@ -41,20 +41,24 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { annotation = t((way.isDegenerate() ? 'operations.start.annotation.' : 'operations.continue.annotation.') + context.geometry(wayId)), - draw = behaviorDraw(context); + draw = behaviorDraw(context), + end = osmNode({ loc: context.map().mouseCoordinates() }), + startIndex, start, segment; - var startIndex = typeof index === 'undefined' ? way.nodes.length - 1 : 0, - start = osmNode({loc: context.graph().entity(way.nodes[startIndex]).loc}), - end = osmNode({loc: context.map().mouseCoordinates()}), + if (!isArea) { + startIndex = typeof index === 'undefined' ? way.nodes.length - 1 : 0; + start = osmNode({ loc: context.entity(way.nodes[startIndex]).loc }); segment = osmWay({ nodes: typeof index === 'undefined' ? [start.id, end.id] : [end.id, start.id], tags: _.clone(way.tags) }); + } + var fn = context[way.isDegenerate() ? 'replace' : 'perform']; if (isArea) { fn(actionAddEntity(end), - actionAddVertex(wayId, end.id, index) + actionAddVertex(wayId, end.id) ); } else { fn(actionAddEntity(start), @@ -70,7 +74,7 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { if (datum.type === 'node' && datum.id !== end.id) { loc = datum.loc; - } else if (datum.type === 'way' && datum.id !== segment.id) { + } else if (datum.type === 'way') { // && (segment || datum.id !== segment.id)) { var dims = context.map().dimensions(), mouse = context.mouse(), pad = 5, @@ -145,9 +149,8 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { return function(graph) { if (isArea) { return graph - .replace(way.addNode(newNode.id, index)) + .replace(way.addNode(newNode.id)) .remove(end); - } else { return graph .replace(graph.entity(wayId).addNode(newNode.id, index)) @@ -165,13 +168,19 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { var last = context.hasEntity(way.nodes[way.nodes.length - (isArea ? 2 : 1)]); if (last && last.loc[0] === loc[0] && last.loc[1] === loc[1]) return; - var newNode = osmNode({loc: loc}); - - context.replace( - actionAddEntity(newNode), - ReplaceTemporaryNode(newNode), - annotation - ); + if (isArea) { + context.replace( + actionMoveNode(end.id, loc), + annotation + ); + } else { + var newNode = osmNode({loc: loc}); + context.replace( + actionAddEntity(newNode), + ReplaceTemporaryNode(newNode), + annotation + ); + } finished = true; context.enter(mode); @@ -180,21 +189,28 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { // Connect the way to an existing way. drawWay.addWay = function(loc, edge) { - var previousEdge = startIndex ? - [way.nodes[startIndex], way.nodes[startIndex - 1]] : - [way.nodes[0], way.nodes[1]]; - // Avoid creating duplicate segments - if (!isArea && geoEdgeEqual(edge, previousEdge)) - return; + if (isArea) { + context.perform( + actionAddMidpoint({ loc: loc, edge: edge}, end), + annotation + ); + } else { + var previousEdge = startIndex ? + [way.nodes[startIndex], way.nodes[startIndex - 1]] : + [way.nodes[0], way.nodes[1]]; - var newNode = osmNode({ loc: loc }); + // Avoid creating duplicate segments + if (geoEdgeEqual(edge, previousEdge)) + return; - context.perform( - actionAddMidpoint({ loc: loc, edge: edge}, newNode), - ReplaceTemporaryNode(newNode), - annotation - ); + var newNode = osmNode({ loc: loc }); + context.perform( + actionAddMidpoint({ loc: loc, edge: edge}, newNode), + ReplaceTemporaryNode(newNode), + annotation + ); + } finished = true; context.enter(mode); diff --git a/modules/modes/add_area.js b/modules/modes/add_area.js index cf92eab68..f36ebef16 100644 --- a/modules/modes/add_area.js +++ b/modules/modes/add_area.js @@ -27,6 +27,13 @@ export function modeAddArea(context) { defaultTags = { area: 'yes' }; + function actionClose(wayId) { + return function (graph) { + return graph.replace(graph.entity(wayId).close()); + }; + } + + function start(loc) { var graph = context.graph(), node = osmNode({ loc: loc }), @@ -36,7 +43,7 @@ export function modeAddArea(context) { actionAddEntity(node), actionAddEntity(way), actionAddVertex(way.id, node.id), - actionAddVertex(way.id, node.id) + actionClose(way.id) ); context.enter(modeDrawArea(context, way.id, graph)); @@ -52,7 +59,7 @@ export function modeAddArea(context) { actionAddEntity(node), actionAddEntity(way), actionAddVertex(way.id, node.id), - actionAddVertex(way.id, node.id), + actionClose(way.id), actionAddMidpoint({ loc: loc, edge: edge }, node) ); @@ -67,7 +74,7 @@ export function modeAddArea(context) { context.perform( actionAddEntity(way), actionAddVertex(way.id, node.id), - actionAddVertex(way.id, node.id) + actionClose(way.id) ); context.enter(modeDrawArea(context, way.id, graph)); diff --git a/modules/modes/draw_area.js b/modules/modes/draw_area.js index 763a45944..643ec9afe 100644 --- a/modules/modes/draw_area.js +++ b/modules/modes/draw_area.js @@ -11,17 +11,18 @@ export function modeDrawArea(context, wayId, baseGraph) { mode.enter = function() { - var way = context.entity(wayId), - headId = way.nodes[way.nodes.length - 2], - tailId = way.first(); + var way = context.entity(wayId); - behavior = behaviorDrawWay(context, wayId, -1, mode, baseGraph) + behavior = behaviorDrawWay(context, wayId, undefined, mode, baseGraph) .tail(t('modes.draw_area.tail')); var addNode = behavior.addNode; behavior.addNode = function(node) { - if (node.id === headId || node.id === tailId) { + var length = way.nodes.length, + penultimate = length > 2 ? way.nodes[length - 2] : null; + + if (node.id === way.first() || node.id === penultimate) { behavior.finish(); } else { addNode(node); From cb35873e29f6f2cc1a92d82d3189d23c63aafeb1 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Thu, 12 Jan 2017 20:08:42 +0530 Subject: [PATCH 16/25] Don't allow continue from the connecting vertex of a closed way Now that addNode tries to preserve circularity, this could cause the continued segment to loop back. And anyway it's confusing why one vertex allows it and none others do. --- modules/operations/continue.js | 1 + 1 file changed, 1 insertion(+) diff --git a/modules/operations/continue.js b/modules/operations/continue.js index 4feaab5ef..5d24f463a 100644 --- a/modules/operations/continue.js +++ b/modules/operations/continue.js @@ -15,6 +15,7 @@ export function operationContinue(selectedIDs, context) { function candidateWays() { return graph.parentWays(vertex).filter(function(parent) { return parent.geometry(graph) === 'line' && + !parent.isClosed() && parent.affix(vertex.id) && (geometries.line.length === 0 || geometries.line[0] === parent); }); From fd9b3931d83998d72efbb24c4107811c9786f49c Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Fri, 13 Jan 2017 18:44:42 +0530 Subject: [PATCH 17/25] Use temp names for temp segments (closes #1369) --- modules/behavior/draw_way.js | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/modules/behavior/draw_way.js b/modules/behavior/draw_way.js index 396b4a2a5..4e969058a 100644 --- a/modules/behavior/draw_way.js +++ b/modules/behavior/draw_way.js @@ -42,16 +42,19 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { 'operations.start.annotation.' : 'operations.continue.annotation.') + context.geometry(wayId)), draw = behaviorDraw(context), - end = osmNode({ loc: context.map().mouseCoordinates() }), - startIndex, start, segment; + startIndex, start, end, segment; + if (!isArea) { startIndex = typeof index === 'undefined' ? way.nodes.length - 1 : 0; - start = osmNode({ loc: context.entity(way.nodes[startIndex]).loc }); - segment = osmWay({ + start = osmNode({ id: 'nStart', loc: context.entity(way.nodes[startIndex]).loc }); + end = osmNode({ id: 'nEnd', loc: context.map().mouseCoordinates() }); + segment = osmWay({ id: 'wTemp', nodes: typeof index === 'undefined' ? [start.id, end.id] : [end.id, start.id], tags: _.clone(way.tags) }); + } else { + end = osmNode({ loc: context.map().mouseCoordinates() }); } From 70f74c4953953abc0dfde240eacc3921b3d1a483 Mon Sep 17 00:00:00 2001 From: Albin Larsson Date: Fri, 13 Jan 2017 21:22:58 +0100 Subject: [PATCH 18/25] do not suggest duplicate commit messages --- modules/ui/commit.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/modules/ui/commit.js b/modules/ui/commit.js index b6b7080fa..fbd74445a 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -59,13 +59,18 @@ export function uiCommit(context) { if (err) return; var comments = []; + var addedComments = []; for (var i = 0; i < changesets.length; i++) { if (changesets[i].tags.comment) { - comments.push({ - title: changesets[i].tags.comment, - value: changesets[i].tags.comment - }); + if (addedComments.indexOf(changesets[i].tags.comment) === -1) { + comments.push({ + title: changesets[i].tags.comment, + value: changesets[i].tags.comment + }); + + addedComments.push(changesets[i].tags.comment); + } } } From 6c63ea05d47d89095189d0e6309bed19de36eaed Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Sun, 15 Jan 2017 02:17:56 +0530 Subject: [PATCH 19/25] chore(package): update rollup to version 0.41.4 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a5d90d53c..4db9b4346 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,7 @@ "npm-run-all": "~4.0.0", "phantomjs-prebuilt": "~2.1.11", "request": "~2.79.0", - "rollup": "0.41.1", + "rollup": "0.41.4", "rollup-plugin-commonjs": "7.0.0", "rollup-plugin-json": "2.0.2", "rollup-plugin-node-resolve": "2.0.0", From dc0a6e3e5f3855ea32611d22028930d75d722258 Mon Sep 17 00:00:00 2001 From: greenkeeperio-bot Date: Sat, 14 Jan 2017 01:30:59 +0530 Subject: [PATCH 20/25] chore(package): update mapillary-js to version 2.3.0 https://greenkeeper.io/ --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4db9b4346..8bb4e2ade 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "js-yaml": "~3.7.0", "jsonschema": "~1.1.0", "json-stable-stringify": "~1.0.1", - "mapillary-js": "2.2.0", + "mapillary-js": "2.3.0", "minimist": "~1.2.0", "mocha": "~3.2.0", "mocha-phantomjs-core": "~2.1.0", From 132a5b56c340339d6e8da2f546398b0e7df2c453 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 16 Jan 2017 17:56:59 +0530 Subject: [PATCH 21/25] Made it a bit simpler with lodash _uniqBy --- modules/ui/commit.js | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/modules/ui/commit.js b/modules/ui/commit.js index fbd74445a..998b10d7f 100644 --- a/modules/ui/commit.js +++ b/modules/ui/commit.js @@ -58,24 +58,18 @@ export function uiCommit(context) { context.connection().userChangesets(function (err, changesets) { if (err) return; - var comments = []; - var addedComments = []; - - for (var i = 0; i < changesets.length; i++) { - if (changesets[i].tags.comment) { - if (addedComments.indexOf(changesets[i].tags.comment) === -1) { - comments.push({ - title: changesets[i].tags.comment, - value: changesets[i].tags.comment - }); - - addedComments.push(changesets[i].tags.comment); - } - } - } + var comments = changesets.map(function(changeset) { + return { + title: changeset.tags.comment, + value: changeset.tags.comment + }; + }); commentField - .call(d3combobox().caseSensitive(true).data(comments)); + .call(d3combobox() + .caseSensitive(true) + .data(_.uniqBy(comments, 'title')) + ); }); var clippyArea = commentSection.append('div') From 0fb506f461378f7299a8318cb5f93beabe76c2e5 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 16 Jan 2017 20:37:34 +0530 Subject: [PATCH 22/25] Make sure new midpoint also gets included in activeIDs --- modules/modes/drag_node.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/modes/drag_node.js b/modules/modes/drag_node.js index 391f9ab0a..a36402908 100644 --- a/modules/modes/drag_node.js +++ b/modules/modes/drag_node.js @@ -97,10 +97,14 @@ export function modeDragNode(context) { var vertex = context.surface().selectAll('.' + entity.id); behavior.target(vertex.node(), entity); + activeIDs = _.map(context.graph().parentWays(entity), 'id'); + activeIDs.push(entity.id); + } else { context.perform(actionNoop()); } + setActiveElements(); context.enter(mode); } From 94483af4c76ec8f151704e50c3b00d314932a439 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 17 Jan 2017 16:46:41 +0530 Subject: [PATCH 23/25] Fix service name nominatim -> geocoder --- modules/ui/fields/combo.js | 2 +- modules/ui/fields/input.js | 2 +- test/spec/services/nominatim.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/ui/fields/combo.js b/modules/ui/fields/combo.js index 669d40e4f..06b58fafa 100644 --- a/modules/ui/fields/combo.js +++ b/modules/ui/fields/combo.js @@ -15,7 +15,7 @@ export { export function uiFieldCombo(field, context) { var dispatch = d3.dispatch('change'), - nominatim = services.nominatim, + nominatim = services.geocoder, taginfo = services.taginfo, isMulti = (field.type === 'multiCombo'), isNetwork = (field.type === 'networkCombo'), diff --git a/modules/ui/fields/input.js b/modules/ui/fields/input.js index 1b343de8c..9faf7597f 100644 --- a/modules/ui/fields/input.js +++ b/modules/ui/fields/input.js @@ -15,7 +15,7 @@ export { export function uiFieldText(field, context) { var dispatch = d3.dispatch('change'), - nominatim = services.nominatim, + nominatim = services.geocoder, input, entity; diff --git a/test/spec/services/nominatim.js b/test/spec/services/nominatim.js index a64d4ddb1..df3a098e9 100644 --- a/test/spec/services/nominatim.js +++ b/test/spec/services/nominatim.js @@ -3,7 +3,7 @@ describe('iD.serviceNominatim', function() { beforeEach(function() { server = sinon.fakeServer.create(); - nominatim = iD.services.nominatim; + nominatim = iD.services.geocoder; nominatim.reset(); }); From 489a25fe74b8f28fcfc499d055062fb7134440be Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 17 Jan 2017 16:55:34 +0530 Subject: [PATCH 24/25] Cleanup addressFormats formatting --- data/address-formats.json | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/data/address-formats.json b/data/address-formats.json index 7ce41a038..e300cbc27 100644 --- a/data/address-formats.json +++ b/data/address-formats.json @@ -24,7 +24,12 @@ ] }, { - "countryCodes": ["ad", "at", "ba", "be", "ch", "cz", "de", "dk", "es", "fi", "gr", "hr", "is", "it", "li", "nl", "no", "pl", "pt", "se", "si", "sk", "sm", "va"], + "countryCodes": [ + "ad", "at", "ba", "be", "ch", "cz", + "de", "dk", "es", "fi", "gr", "hr", + "is", "it", "li", "nl", "no", "pl", + "pt", "se", "si", "sk", "sm", "va" + ], "format": [ ["street", "housenumber"], ["postcode", "city"] @@ -53,8 +58,7 @@ ["district"], ["city"], ["province", "postcode"] - ], - "customPlaceholders": ["subdistrict", "district", "city"] + ] }, { "countryCodes": ["us"], @@ -86,13 +90,17 @@ ["quarter", "neighbourhood"], ["block_number", "housenumber"] ], - "customPlaceholders": ["province", "county", "city", "suburb", "quarter", "neighbourhood", "block_number", "housenumber"], - "dropdowns": ["postcode", "province", "county", "city", "suburb", "quarter", "neighbourhood", "block_number"], + "dropdowns": [ + "postcode", "province", "county", + "city", "suburb", + "quarter", "neighbourhood", + "block_number" + ], "widths": { "postcode": 0.3, "province": 0.35, "county": 0.35, "city": 0.65, "suburb": 0.35, "quarter": 0.5, "neighbourhood": 0.5, - "block_number": 0.5 + "block_number": 0.5, "housenumber": 0.5 } }, { From d09fd15092c339b819b62261b52951b905caf43f Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Tue, 17 Jan 2017 16:56:23 +0530 Subject: [PATCH 25/25] Cleanups and rename service nominatim -> geocoder --- data/presets.yaml | 9 ++------- data/presets/fields.json | 26 ++++++++++++-------------- data/presets/fields/address.json | 26 ++++++++++++-------------- dist/locales/en.json | 26 ++++++++++++-------------- modules/ui/fields/address.js | 26 +++++++++++++------------- 5 files changed, 51 insertions(+), 62 deletions(-) diff --git a/data/presets.yaml b/data/presets.yaml index b92b2ae8f..456ca41a0 100644 --- a/data/presets.yaml +++ b/data/presets.yaml @@ -84,7 +84,7 @@ en: # access=* label: Access address: - # 'addr:block_number=*, addr:city=*, addr:conscriptionnumber=*, addr:county=*, addr:country=*, addr:district=*, addr:floor=*, addr:hamlet=*, addr:housename=*, addr:housenumber=*, addr:neighbourhood=*, addr:place=*, addr:postcode=*, addr:province=*, addr:quarter=*, addr:state=*, addr:street=*, addr:subdistrict=*, addr:suburb=*' + # 'addr:block_number=*, addr:city=*, addr:block_number=*, addr:conscriptionnumber=*, addr:county=*, addr:country=*, addr:county=*, addr:district=*, addr:floor=*, addr:hamlet=*, addr:housename=*, addr:housenumber=*, addr:neighbourhood=*, addr:place=*, addr:postcode=*, addr:province=*, addr:quarter=*, addr:state=*, addr:street=*, addr:subdistrict=*, addr:suburb=*' label: Address placeholders: block_number: Block Number @@ -110,12 +110,7 @@ en: province: Province province!jp: Prefecture quarter: Quarter - quarter!jp: Ōaza/Mach - neighbourhood: Neighbourhood - place: Place - postcode: Postcode - province: Province - quarter: Quarte + quarter!jp: Ōaza/Machi state: State street: Street subdistrict: Subdistrict diff --git a/data/presets/fields.json b/data/presets/fields.json index b58e25361..68ec1dbf4 100644 --- a/data/presets/fields.json +++ b/data/presets/fields.json @@ -109,38 +109,36 @@ "label": "Address", "strings": { "placeholders": { - "block_number": "Block number", - "city": "City", "block_number": "Block Number", + "block_number!jp": "Block No.", + "city": "City", + "city!jp": "City/Town/Village/Tokyo Special Ward", + "city!vn": "City/Town", "conscriptionnumber": "123", - "county": "County", "country": "Country", "county": "County", + "county!jp": "District", "district": "District", + "district!vn": "Arrondissement/Town/District", "floor": "Floor", "hamlet": "Hamlet", "housename": "Housename", "housenumber": "123", + "housenumber!jp": "Building No./Lot No.", "neighbourhood": "Neighbourhood", + "neighbourhood!jp": "Chōme/Aza/Koaza", "place": "Place", "postcode": "Postcode", "province": "Province", + "province!jp": "Prefecture", "quarter": "Quarter", + "quarter!jp": "Ōaza/Machi", "state": "State", "street": "Street", "subdistrict": "Subdistrict", - "suburb": "Suburb", "subdistrict!vn": "Ward/Commune/Townlet", - "district!vn": "Arrondissement/Town/District", - "city!vn": "City/Town", - "province!jp": "Prefecture", - "county!jp": "District", - "city!jp": "City/Town/Village/Tokyo Special Ward", - "suburb!jp": "Ward", - "quarter!jp": "Ōaza/Machi", - "neighbourhood!jp": "Chōme/Aza/Koaza", - "block_number!jp": "Block No.", - "housenumber!jp": "Building No./Lot No." + "suburb": "Suburb", + "suburb!jp": "Ward" } } }, diff --git a/data/presets/fields/address.json b/data/presets/fields/address.json index d8ad9ab39..198967c7d 100644 --- a/data/presets/fields/address.json +++ b/data/presets/fields/address.json @@ -29,38 +29,36 @@ "label": "Address", "strings": { "placeholders": { - "block_number": "Block number", - "city": "City", "block_number": "Block Number", + "block_number!jp": "Block No.", + "city": "City", + "city!jp": "City/Town/Village/Tokyo Special Ward", + "city!vn": "City/Town", "conscriptionnumber": "123", - "county": "County", "country": "Country", "county": "County", + "county!jp": "District", "district": "District", + "district!vn": "Arrondissement/Town/District", "floor": "Floor", "hamlet": "Hamlet", "housename": "Housename", "housenumber": "123", + "housenumber!jp": "Building No./Lot No.", "neighbourhood": "Neighbourhood", + "neighbourhood!jp": "Chōme/Aza/Koaza", "place": "Place", "postcode": "Postcode", "province": "Province", + "province!jp": "Prefecture", "quarter": "Quarter", + "quarter!jp": "Ōaza/Machi", "state": "State", "street": "Street", "subdistrict": "Subdistrict", - "suburb": "Suburb", "subdistrict!vn": "Ward/Commune/Townlet", - "district!vn": "Arrondissement/Town/District", - "city!vn": "City/Town", - "province!jp": "Prefecture", - "county!jp": "District", - "city!jp": "City/Town/Village/Tokyo Special Ward", - "suburb!jp": "Ward", - "quarter!jp": "Ōaza/Machi", - "neighbourhood!jp": "Chōme/Aza/Koaza", - "block_number!jp": "Block No.", - "housenumber!jp": "Building No./Lot No." + "suburb": "Suburb", + "suburb!jp": "Ward" } } } diff --git a/dist/locales/en.json b/dist/locales/en.json index ce48c0b10..9913d9127 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -789,38 +789,36 @@ "address": { "label": "Address", "placeholders": { - "block_number": "Block number", - "city": "City", "block_number": "Block Number", + "block_number!jp": "Block No.", + "city": "City", + "city!jp": "City/Town/Village/Tokyo Special Ward", + "city!vn": "City/Town", "conscriptionnumber": "123", - "county": "County", "country": "Country", "county": "County", + "county!jp": "District", "district": "District", + "district!vn": "Arrondissement/Town/District", "floor": "Floor", "hamlet": "Hamlet", "housename": "Housename", "housenumber": "123", + "housenumber!jp": "Building No./Lot No.", "neighbourhood": "Neighbourhood", + "neighbourhood!jp": "Chōme/Aza/Koaza", "place": "Place", "postcode": "Postcode", "province": "Province", + "province!jp": "Prefecture", "quarter": "Quarter", + "quarter!jp": "Ōaza/Machi", "state": "State", "street": "Street", "subdistrict": "Subdistrict", - "suburb": "Suburb", "subdistrict!vn": "Ward/Commune/Townlet", - "district!vn": "Arrondissement/Town/District", - "city!vn": "City/Town", - "province!jp": "Prefecture", - "county!jp": "District", - "city!jp": "City/Town/Village/Tokyo Special Ward", - "suburb!jp": "Ward", - "quarter!jp": "Ōaza/Machi", - "neighbourhood!jp": "Chōme/Aza/Koaza", - "block_number!jp": "Block No.", - "housenumber!jp": "Building No./Lot No." + "suburb": "Suburb", + "suburb!jp": "Ward" } }, "admin_level": { diff --git a/modules/ui/fields/address.js b/modules/ui/fields/address.js index 4baea3760..4ed7cb1b4 100644 --- a/modules/ui/fields/address.js +++ b/modules/ui/fields/address.js @@ -16,11 +16,9 @@ import { utilGetSetValue } from '../../util/get_set_value'; export function uiFieldAddress(field, context) { var dispatch = d3.dispatch('init', 'change'), - nominatim = services.nominatim, + nominatim = services.geocoder, wrap = d3.select(null), isInitialized = false, - widths, - addrTags, entity; function getNearStreets() { @@ -122,8 +120,10 @@ export function uiFieldAddress(field, context) { return a && a.countryCodes && _.includes(a.countryCodes, countryCode); }) || _.first(dataAddressFormats); - if (typeof addressFormat.widths !== 'undefined') { widths = addressFormat.widths; } - else { widths = {housenumber: 1/3, street: 2/3, city: 2/3, state: 1/4, postcode: 1/3}; } + var widths = addressFormat.widths || { + housenumber: 1/3, street: 2/3, + city: 2/3, state: 1/4, postcode: 1/3 + }; function row(r) { // Normalize widths. @@ -150,23 +150,24 @@ export function uiFieldAddress(field, context) { .append('input') .property('type', 'text') .attr('placeholder', function (d) { - var countryInserter = ''; - if (addressFormat.customPlaceholders.indexOf(d.id) !== -1) { countryInserter = '!' + countryCode; } - return field.t('placeholders.' + d.id + countryInserter); }) + var localkey = d.id + '!' + countryCode, + tkey = field.strings.placeholders[localkey] ? localkey : d.id; + return field.t('placeholders.' + tkey); + }) .attr('class', function (d) { return 'addr-' + d.id; }) .style('width', function (d) { return d.width * 100 + '%'; }); // Update + // setup dropdowns for common address tags - if (typeof addressFormat.dropdowns !== 'undefined') { addrTags = addressFormat.dropdowns; } - else { addrTags = [ + var dropdowns = addressFormat.dropdowns || [ 'city', 'county', 'country', 'district', 'hamlet', 'neighbourhood', 'place', 'postcode', 'province', 'quarter', 'state', 'street', 'subdistrict', 'suburb' - ]; } + ]; // If fields exist for any of these tags, create dropdowns to pick nearby values.. - addrTags.forEach(function(tag) { + dropdowns.forEach(function(tag) { var nearValues = (tag === 'street') ? getNearStreets : (tag === 'city') ? getNearCities : getNearValues; @@ -202,7 +203,6 @@ export function uiFieldAddress(field, context) { .attr('class', 'preset-input-wrap') .merge(wrap); - if (nominatim && entity) { var center = entity.extent(context.graph()).center(); nominatim.countryCode(center, initCallback);