From 7edebb897f5d943b53eb65d6544cd7e45e0d0dc6 Mon Sep 17 00:00:00 2001 From: J Guthrie Date: Mon, 8 Apr 2019 12:41:05 +0100 Subject: [PATCH 1/9] Only allow disconnect when selected ways relate to selected node --- modules/operations/disconnect.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/operations/disconnect.js b/modules/operations/disconnect.js index b9ede9fd3..fa3d78473 100644 --- a/modules/operations/disconnect.js +++ b/modules/operations/disconnect.js @@ -8,6 +8,10 @@ export function operationDisconnect(selectedIDs, context) { return context.geometry(id) === 'vertex'; }); + var ways = selectedIDs.filter(function(id) { + return context.geometry(id) !== 'vertex'; + }); + var entityID = vertices[0]; var action = actionDisconnect(entityID); @@ -23,7 +27,7 @@ export function operationDisconnect(selectedIDs, context) { operation.available = function() { - return vertices.length === 1; + return vertices.length === 1 && ways.every(function(way) { return context.graph().entity(way).nodes.includes(vertices[0]); }); }; From d7865ac4aa3b27b4980513c558dc90bbbcb81059 Mon Sep 17 00:00:00 2001 From: J Guthrie Date: Mon, 8 Apr 2019 12:42:16 +0100 Subject: [PATCH 2/9] don't unclose way if it is part of a larger disconnect operation --- modules/actions/disconnect.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/modules/actions/disconnect.js b/modules/actions/disconnect.js index 62a2162b0..b7ed93814 100644 --- a/modules/actions/disconnect.js +++ b/modules/actions/disconnect.js @@ -22,7 +22,7 @@ export function actionDisconnect(nodeId, newNodeId) { var action = function(graph) { var node = graph.entity(nodeId); var connections = action.connections(graph); - + connections.forEach(function(connection) { var way = graph.entity(connection.wayID); var newNode = osmNode({id: newNodeId, loc: node.loc, tags: node.tags}); @@ -59,6 +59,9 @@ export function actionDisconnect(nodeId, newNodeId) { } else { way.nodes.forEach(function(waynode, index) { if (waynode === nodeId) { + if (way.isClosed() && parentWays.length > 1 && wayIds && wayIds.includes(way.id) && index === way.nodes.length-1) { + return; + } candidates.push({ wayID: way.id, index: index }); } }); @@ -71,7 +74,7 @@ export function actionDisconnect(nodeId, newNodeId) { action.disabled = function(graph) { var connections = action.connections(graph); - if (connections.length === 0 || (wayIds && wayIds.length !== connections.length)) + if (connections.length === 0) return 'not_connected'; var parentWays = graph.parentWays(graph.entity(nodeId)); From 57a03580618d17f446737d00737019850f0b5116 Mon Sep 17 00:00:00 2001 From: J Guthrie Date: Mon, 8 Apr 2019 15:00:19 +0100 Subject: [PATCH 3/9] Add test cases --- test/index.html | 1 + test/spec/actions/disconnect.js | 29 +++++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/test/index.html b/test/index.html index 954589083..f8a71f073 100644 --- a/test/index.html +++ b/test/index.html @@ -88,6 +88,7 @@ + diff --git a/test/spec/actions/disconnect.js b/test/spec/actions/disconnect.js index a985933d0..e37dc178f 100644 --- a/test/spec/actions/disconnect.js +++ b/test/spec/actions/disconnect.js @@ -180,6 +180,35 @@ describe('iD.actionDisconnect', function () { expect(graph.entity('|').nodes).to.eql(['d', 'b']); }); + it('preserves the closed way when part of a larger disconnect operation', function () { + // Situation: + // a ---- bb === c + // = == + // = == + // d + // Disconnect - at b (whilst == is selected). + // + // Expected result: + // a ---- b ee === c + // = == + // = == + // d + // + var graph = iD.coreGraph([ + iD.osmNode({id: 'a'}), + iD.osmNode({id: 'b'}), + iD.osmNode({id: 'c'}), + iD.osmNode({id: 'd'}), + iD.osmWay({id: '-', nodes: ['a', 'b']}), + iD.osmWay({id: '=', nodes: ['e', 'c', 'd', 'e']}) + ]); + + graph = iD.actionDisconnect('b', 'e').limitWays(['='])(graph); + + expect(graph.entity('-').nodes).to.eql(['a', 'b']); + expect(graph.entity('=').nodes).to.eql(['e', 'c', 'd', 'e']); + }); + it('replaces later occurrences in a self-intersecting way', function() { // Situtation: // a --- b From d3d0a560eb967d10dee5988092921e3e73333eac Mon Sep 17 00:00:00 2001 From: Jamie Guthrie Date: Mon, 8 Apr 2019 16:01:08 +0100 Subject: [PATCH 4/9] Remove excess whitespace --- modules/actions/disconnect.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/actions/disconnect.js b/modules/actions/disconnect.js index b7ed93814..6359ff606 100644 --- a/modules/actions/disconnect.js +++ b/modules/actions/disconnect.js @@ -22,7 +22,7 @@ export function actionDisconnect(nodeId, newNodeId) { var action = function(graph) { var node = graph.entity(nodeId); var connections = action.connections(graph); - + connections.forEach(function(connection) { var way = graph.entity(connection.wayID); var newNode = osmNode({id: newNodeId, loc: node.loc, tags: node.tags}); From a29da38230dc8853ee024b31aa65a384a1a5ad94 Mon Sep 17 00:00:00 2001 From: Jamie Guthrie Date: Mon, 8 Apr 2019 16:02:31 +0100 Subject: [PATCH 5/9] Remove operationDisconnect test --- test/index.html | 1 - 1 file changed, 1 deletion(-) diff --git a/test/index.html b/test/index.html index f8a71f073..954589083 100644 --- a/test/index.html +++ b/test/index.html @@ -88,7 +88,6 @@ - From 4d24db597ec3934ed90952c8503243b4d9bc77d7 Mon Sep 17 00:00:00 2001 From: J Guthrie Date: Mon, 8 Apr 2019 16:28:01 +0100 Subject: [PATCH 6/9] Small refactor to improve efficiency --- modules/operations/disconnect.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/modules/operations/disconnect.js b/modules/operations/disconnect.js index fa3d78473..cf4fb82ad 100644 --- a/modules/operations/disconnect.js +++ b/modules/operations/disconnect.js @@ -4,12 +4,11 @@ import { behaviorOperation } from '../behavior/index'; export function operationDisconnect(selectedIDs, context) { - var vertices = selectedIDs.filter(function(id) { - return context.geometry(id) === 'vertex'; - }); + var vertices = [], + ways = []; - var ways = selectedIDs.filter(function(id) { - return context.geometry(id) !== 'vertex'; + selectedIDs.forEach(function(id) { + context.geometry(id) === 'vertex' ? vertices.push(id) : ways.push(id); }); var entityID = vertices[0]; From ac29803b6ffc932244d8bf361e8337641cd0e3df Mon Sep 17 00:00:00 2001 From: J Guthrie Date: Mon, 8 Apr 2019 16:38:10 +0100 Subject: [PATCH 7/9] Fix eslint error --- modules/operations/disconnect.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/modules/operations/disconnect.js b/modules/operations/disconnect.js index cf4fb82ad..55a12831c 100644 --- a/modules/operations/disconnect.js +++ b/modules/operations/disconnect.js @@ -8,7 +8,11 @@ export function operationDisconnect(selectedIDs, context) { ways = []; selectedIDs.forEach(function(id) { - context.geometry(id) === 'vertex' ? vertices.push(id) : ways.push(id); + if (context.geometry(id) === 'vertex') { + vertices.push(id); + } else { + ways.push(id); + } }); var entityID = vertices[0]; From bd44cec2e868472dd8a130a7e41096f431bd3f9f Mon Sep 17 00:00:00 2001 From: J Guthrie Date: Mon, 8 Apr 2019 16:48:27 +0100 Subject: [PATCH 8/9] Fix broken testcase! --- test/spec/actions/disconnect.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/spec/actions/disconnect.js b/test/spec/actions/disconnect.js index e37dc178f..703ffef88 100644 --- a/test/spec/actions/disconnect.js +++ b/test/spec/actions/disconnect.js @@ -200,7 +200,7 @@ describe('iD.actionDisconnect', function () { iD.osmNode({id: 'c'}), iD.osmNode({id: 'd'}), iD.osmWay({id: '-', nodes: ['a', 'b']}), - iD.osmWay({id: '=', nodes: ['e', 'c', 'd', 'e']}) + iD.osmWay({id: '=', nodes: ['b', 'c', 'd', 'b']}) ]); graph = iD.actionDisconnect('b', 'e').limitWays(['='])(graph); From 00c7eaddc2ac2533a9b6eb2f9ba8fbaa709b0562 Mon Sep 17 00:00:00 2001 From: J Guthrie Date: Mon, 8 Apr 2019 17:07:12 +0100 Subject: [PATCH 9/9] Fix to work with PhatomJS --- modules/actions/disconnect.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/actions/disconnect.js b/modules/actions/disconnect.js index 6359ff606..169dbf92e 100644 --- a/modules/actions/disconnect.js +++ b/modules/actions/disconnect.js @@ -59,7 +59,7 @@ export function actionDisconnect(nodeId, newNodeId) { } else { way.nodes.forEach(function(waynode, index) { if (waynode === nodeId) { - if (way.isClosed() && parentWays.length > 1 && wayIds && wayIds.includes(way.id) && index === way.nodes.length-1) { + if (way.isClosed() && parentWays.length > 1 && wayIds && wayIds.indexOf(way.id) !== -1 && index === way.nodes.length-1) { return; } candidates.push({ wayID: way.id, index: index });