diff --git a/data/core.yaml b/data/core.yaml index c236dae97..0bed7af65 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -105,6 +105,7 @@ en: annotation: Disconnected lines/areas. not_connected: There aren't enough lines/areas here to disconnect. connected_to_hidden: This can't be disconnected because it is connected to a hidden feature. + relation: This can't be disconnected because it connects members of a relation. merge: title: Merge description: Merge these features. diff --git a/dist/locales/en.json b/dist/locales/en.json index 85f710944..ad4ddff19 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -133,7 +133,8 @@ "key": "D", "annotation": "Disconnected lines/areas.", "not_connected": "There aren't enough lines/areas here to disconnect.", - "connected_to_hidden": "This can't be disconnected because it is connected to a hidden feature." + "connected_to_hidden": "This can't be disconnected because it is connected to a hidden feature.", + "relation": "This can't be disconnected because it connects members of a relation." }, "merge": { "title": "Merge", diff --git a/js/id/actions/disconnect.js b/js/id/actions/disconnect.js index 069e5919a..d038faeba 100644 --- a/js/id/actions/disconnect.js +++ b/js/id/actions/disconnect.js @@ -64,6 +64,27 @@ iD.actions.Disconnect = function(nodeId, newNodeId) { var connections = action.connections(graph); if (connections.length === 0 || (wayIds && wayIds.length !== connections.length)) return 'not_connected'; + + var parentWays = graph.parentWays(graph.entity(nodeId)), + seenRelationIds = {}, + sharedRelation; + + parentWays.forEach(function(way) { + if (wayIds && wayIds.indexOf(way.id) === -1) + return; + + var relations = graph.parentRelations(way); + relations.forEach(function(relation) { + if (relation.id in seenRelationIds) { + sharedRelation = relation; + } else { + seenRelationIds[relation.id] = true; + } + }); + }); + + if (sharedRelation) + return 'relation'; }; action.limitWays = function(_) { diff --git a/test/spec/actions/disconnect.js b/test/spec/actions/disconnect.js index f8851e4cc..846f16428 100644 --- a/test/spec/actions/disconnect.js +++ b/test/spec/actions/disconnect.js @@ -81,6 +81,45 @@ describe("iD.actions.Disconnect", function () { expect(iD.actions.Disconnect('b', ['|']).disabled(graph)).not.to.be.ok; }); + it("returns 'relation' for a node connecting any two members of the same relation", function () { + // Covers restriction relations, routes, multipolygons. + // a ---- b ---- c + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Way({id: 'f', nodes: ['a', 'b']}), + iD.Way({id: 't', nodes: ['b', 'c']}), + iD.Relation({id: 'r', members: [ + { id: 'f' }, + { id: 't' } + ]}) + ]); + + expect(iD.actions.Disconnect('b').disabled(graph)).to.eql('relation'); + }); + + it("returns falsy for a node connecting two members of an unaffected relation", function () { + // a ---- b ---- c + // | + // d + + var graph = iD.Graph([ + iD.Node({id: 'a'}), + iD.Node({id: 'b'}), + iD.Node({id: 'c'}), + iD.Node({id: 'd'}), + iD.Way({id: 'f', nodes: ['a', 'b']}), + iD.Way({id: 't', nodes: ['b', 'c']}), + iD.Way({id: '|', nodes: ['b', 'd']}), + iD.Relation({id: 'r', members: [ + { id: 'f' }, + { id: 't' } + ]}) + ]); + + expect(iD.actions.Disconnect('b').limitWays(['|']).disabled(graph)).not.to.be.ok; + }); }); it("replaces the node with a new node in all but the first way", function () {