From 7ec122240226041a4c2dd6606cf8ebdba19c0586 Mon Sep 17 00:00:00 2001 From: John Firebaugh Date: Wed, 5 Jun 2013 14:58:02 -0700 Subject: [PATCH] Try to insert relation members at a sensible index (#1539) --- js/id/actions/add_member.js | 26 +++++++++- js/id/core/relation.js | 10 ++++ test/index.html | 2 + test/index_packaged.html | 1 + test/spec/actions/add_member.js | 90 +++++++++++++++++++++++++++++++++ test/spec/core/relation.js | 7 +++ 6 files changed, 135 insertions(+), 1 deletion(-) create mode 100644 test/spec/actions/add_member.js diff --git a/js/id/actions/add_member.js b/js/id/actions/add_member.js index 75961bd80..f3eac3d9c 100644 --- a/js/id/actions/add_member.js +++ b/js/id/actions/add_member.js @@ -1,5 +1,29 @@ iD.actions.AddMember = function(relationId, member, memberIndex) { return function(graph) { - return graph.replace(graph.entity(relationId).addMember(member, memberIndex)); + var relation = graph.entity(relationId); + + if (isNaN(memberIndex) && member.type === 'way') { + var members = relation.indexedMembers(); + members.push(member); + + var joined = iD.geo.joinMemberWays(members, graph); + for (var i = 0; i < joined.length; i++) { + var segment = joined[i]; + for (var j = 0; j < segment.length && segment.length >= 2; j++) { + if (segment[j] !== member) + continue; + + if (j === 0) { + memberIndex = segment[j + 1].index; + } else if (j === segment.length - 1) { + memberIndex = segment[j - 1].index + 1; + } else { + memberIndex = Math.min(segment[j - 1].index + 1, segment[j + 1].index + 1); + } + } + } + } + + return graph.replace(relation.addMember(member, memberIndex)); } }; diff --git a/js/id/core/relation.js b/js/id/core/relation.js index 1a174d9f4..4f527314c 100644 --- a/js/id/core/relation.js +++ b/js/id/core/relation.js @@ -31,6 +31,16 @@ _.extend(iD.Relation.prototype, { }); }, + // Return an array of members, each extended with an 'index' property whose value + // is the member index. + indexedMembers: function() { + var result = new Array(this.members.length); + for (var i = 0; i < this.members.length; i++) { + result[i] = _.extend({}, this.members[i], {index: i}) + } + return result; + }, + // Return the first member with the given role. A copy of the member object // is returned, extended with an 'index' property whose value is the member index. memberByRole: function(role) { diff --git a/test/index.html b/test/index.html index 15529d330..8fb428e32 100644 --- a/test/index.html +++ b/test/index.html @@ -102,6 +102,7 @@ + @@ -188,6 +189,7 @@ + diff --git a/test/index_packaged.html b/test/index_packaged.html index 45996b56a..038a1eaec 100644 --- a/test/index_packaged.html +++ b/test/index_packaged.html @@ -25,6 +25,7 @@ + diff --git a/test/spec/actions/add_member.js b/test/spec/actions/add_member.js new file mode 100644 index 000000000..1774f48df --- /dev/null +++ b/test/spec/actions/add_member.js @@ -0,0 +1,90 @@ +describe("iD.actions.AddMember", function() { + it("adds an member to a relation at the specified index", function() { + var r = iD.Relation({members: [{id: '1'}, {id: '3'}]}), + g = iD.actions.AddMember(r.id, {id: '2'}, 1)(iD.Graph([r])); + expect(g.entity(r.id).members).to.eql([{id: '1'}, {id: '2'}, {id: '3'}]); + }); + + describe("inserts way members at a sensible index", function() { + function members(graph) { + return _.pluck(graph.entity('r').members, 'id'); + } + + specify("no members", function() { + var graph = iD.Graph({ + 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'b': iD.Node({id: 'b', loc: [0, 0]}), + '-': iD.Way({id: '-', nodes: ['a', 'b']}), + 'r': iD.Relation({id: 'r'}) + }); + + graph = iD.actions.AddMember('r', {id: '-', type: 'way'})(graph); + expect(members(graph)).to.eql(['-']); + }); + + specify("not connecting", function() { + // a--->b c===>d + var graph = iD.Graph({ + 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'b': iD.Node({id: 'b', loc: [0, 0]}), + 'c': iD.Node({id: 'c', loc: [0, 0]}), + 'd': iD.Node({id: 'd', loc: [0, 0]}), + '-': iD.Way({id: '-', nodes: ['a', 'b']}), + '=': iD.Way({id: '=', nodes: ['c', 'd']}), + 'r': iD.Relation({id: 'r', members: [{id: '-', type: 'way'}]}) + }); + + graph = iD.actions.AddMember('r', {id: '=', type: 'way'})(graph); + expect(members(graph)).to.eql(['-', '=']); + }); + + specify("connecting at end", function() { + // a--->b===>c + var graph = iD.Graph({ + 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'b': iD.Node({id: 'b', loc: [0, 0]}), + 'c': iD.Node({id: 'c', loc: [0, 0]}), + '-': iD.Way({id: '-', nodes: ['a', 'b']}), + '=': iD.Way({id: '=', nodes: ['b', 'c']}), + 'r': iD.Relation({id: 'r', members: [{id: '-', type: 'way'}]}) + }); + + graph = iD.actions.AddMember('r', {id: '=', type: 'way'})(graph); + expect(members(graph)).to.eql(['-', '=']); + }); + + specify("connecting at beginning", function() { + // a===>b--->c~~~>d + var graph = iD.Graph({ + 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'b': iD.Node({id: 'b', loc: [0, 0]}), + 'c': iD.Node({id: 'c', loc: [0, 0]}), + 'd': iD.Node({id: 'd', loc: [0, 0]}), + '=': iD.Way({id: '=', nodes: ['a', 'b']}), + '-': iD.Way({id: '-', nodes: ['b', 'c']}), + '~': iD.Way({id: '~', nodes: ['c', 'd']}), + 'r': iD.Relation({id: 'r', members: [{id: '-', type: 'way'}, {id: '~', type: 'way'}]}) + }); + + graph = iD.actions.AddMember('r', {id: '=', type: 'way'})(graph); + expect(members(graph)).to.eql(['=', '-', '~']); + }); + + specify("connecting in middle", function() { + // a--->b===>c~~~>d + var graph = iD.Graph({ + 'a': iD.Node({id: 'a', loc: [0, 0]}), + 'b': iD.Node({id: 'b', loc: [0, 0]}), + 'c': iD.Node({id: 'c', loc: [0, 0]}), + 'd': iD.Node({id: 'd', loc: [0, 0]}), + '-': iD.Way({id: '-', nodes: ['a', 'b']}), + '=': iD.Way({id: '=', nodes: ['b', 'c']}), + '~': iD.Way({id: '~', nodes: ['c', 'd']}), + 'r': iD.Relation({id: 'r', members: [{id: '-', type: 'way'}, {id: '~', type: 'way'}]}) + }); + + graph = iD.actions.AddMember('r', {id: '=', type: 'way'})(graph); + expect(members(graph)).to.eql(['-', '=', '~']); + }); + }); +}); diff --git a/test/spec/core/relation.js b/test/spec/core/relation.js index 5bfb9e72b..e98217b3c 100644 --- a/test/spec/core/relation.js +++ b/test/spec/core/relation.js @@ -99,6 +99,13 @@ describe('iD.Relation', function () { }); }); + describe("#indexedMembers", function () { + it("returns an array of members extended with indexes", function () { + var r = iD.Relation({members: [{id: '1'}, {id: '3'}]}); + expect(r.indexedMembers()).to.eql([{id: '1', index: 0}, {id: '3', index: 1}]); + }); + }); + describe("#addMember", function () { it("adds a member at the end of the relation", function () { var r = iD.Relation();