From 3557e3c7115bc6af87d95840ccc3895b51e70e37 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Sat, 20 Apr 2019 00:02:46 -0400 Subject: [PATCH] Don't allow disconnecting a way if parts extend to undownloaded tiles (re: #2248, #4245) --- data/core.yaml | 1 + dist/locales/en.json | 1 + modules/actions/disconnect.js | 4 +- modules/operations/disconnect.js | 76 ++++++++++++++++++++------------ 4 files changed, 51 insertions(+), 31 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index de3f3da0e..a2b35eb7d 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -206,6 +206,7 @@ en: too_large: single: This can't be disconnected because not enough of it is currently visible. not_connected: There aren't enough lines/areas here to disconnect. + not_downloaded: This can't be disconnected because parts of it have not yet been downloaded. 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: diff --git a/dist/locales/en.json b/dist/locales/en.json index 3c6a95914..ac8f6dc4a 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -265,6 +265,7 @@ "single": "This can't be disconnected because not enough of it is currently visible." }, "not_connected": "There aren't enough lines/areas here to disconnect.", + "not_downloaded": "This can't be disconnected because parts of it have not yet been downloaded.", "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." }, diff --git a/modules/actions/disconnect.js b/modules/actions/disconnect.js index 587fc27c9..95a9250c0 100644 --- a/modules/actions/disconnect.js +++ b/modules/actions/disconnect.js @@ -104,9 +104,9 @@ export function actionDisconnect(nodeId, newNodeId) { }; - action.limitWays = function(_) { + action.limitWays = function(val) { if (!arguments.length) return wayIds; - wayIds = _; + wayIds = val; return action; }; diff --git a/modules/operations/disconnect.js b/modules/operations/disconnect.js index d7f4c501e..a45103d99 100644 --- a/modules/operations/disconnect.js +++ b/modules/operations/disconnect.js @@ -1,45 +1,50 @@ import { t } from '../util/locale'; import { actionDisconnect } from '../actions/index'; import { behaviorOperation } from '../behavior/index'; +import { utilGetAllNodes } from '../util/index'; export function operationDisconnect(selectedIDs, context) { var _disabled; - - var vertices = []; - var ways = []; - var others = []; - var _extent; + var vertexIDs = []; + var wayIDs = []; + var otherIDs = []; + var actions = []; selectedIDs.forEach(function(id) { if (context.geometry(id) === 'vertex') { - vertices.push(id); + vertexIDs.push(id); } else if (context.entity(id).type === 'way'){ - ways.push(id); + wayIDs.push(id); } else { - others.push(id); + otherIDs.push(id); } }); - var actions = []; - var disconnectingWay = vertices.length === 0 && ways.length === 1 && ways[0]; + var disconnectingWayID = (vertexIDs.length === 0 && wayIDs.length === 1 && wayIDs[0]); + var extent, nodes, coords; - if (disconnectingWay) { - _extent = context.entity(disconnectingWay).extent(context.graph()); + if (disconnectingWayID) { // disconnecting a way + var way = context.entity(disconnectingWayID); + extent = way.extent(context.graph()); + nodes = utilGetAllNodes([disconnectingWayID], context.graph()); + coords = nodes.map(function(n) { return n.loc; }); - context.entity(disconnectingWay).nodes.forEach(function(vertexID) { - var action = actionDisconnect(vertexID).limitWays(ways); + way.nodes.forEach(function(vertexID) { + var action = actionDisconnect(vertexID).limitWays(wayIDs); if (action.disabled(context.graph()) !== 'not_connected') { actions.push(action); } }); - } else { - vertices.forEach(function(vertexID) { + + } else { // disconnecting a vertex + vertexIDs.forEach(function(vertexID) { var action = actionDisconnect(vertexID); - if (ways.length > 0) { - var waysIDsForVertex = ways.filter(function(wayID) { - return context.graph().entity(wayID).nodes.includes(vertexID); + if (wayIDs.length > 0) { + var waysIDsForVertex = wayIDs.filter(function(wayID) { + var way = context.entity(wayID); + return way.nodes.indexOf(vertexID) !== -1; }); action.limitWays(waysIDsForVertex); } @@ -60,11 +65,12 @@ export function operationDisconnect(selectedIDs, context) { operation.available = function() { if (actions.length === 0) return false; - if (others.length !== 0) return false; + if (otherIDs.length !== 0) return false; - if (vertices.length !== 0 && ways.length !== 0 && !ways.every(function(way) { - return vertices.some(function(vertex) { - return context.graph().entity(way).nodes.includes(vertex); + if (vertexIDs.length !== 0 && wayIDs.length !== 0 && !wayIDs.every(function(wayID) { + return vertexIDs.some(function(vertexID) { + var way = context.entity(wayID); + return way.nodes.indexOf(vertexID) !== -1; }); })) return false; @@ -86,16 +92,28 @@ export function operationDisconnect(selectedIDs, context) { if (_disabled) { return _disabled; - } else if (_extent && _extent.area() && _extent.percentContainedIn(context.extent()) < 0.8) { + } else if (disconnectingWayID && extent.percentContainedIn(context.extent()) < 0.8) { return _disabled = 'too_large.single'; -// fixme- maybe needs? - // } else if (someMissing()) { - // return _disabled = 'not_downloaded'; + } else if (disconnectingWayID && someMissing()) { + return _disabled = 'not_downloaded'; } else if (selectedIDs.some(context.hasHiddenConnections)) { return _disabled = 'connected_to_hidden'; } return _disabled = false; + + + function someMissing() { + var osm = context.connection(); + if (osm) { + var missing = coords.filter(function(loc) { return !osm.isDataLoaded(loc); }); + if (missing.length) { + missing.forEach(function(loc) { context.loadTileAtLoc(loc); }); + return true; + } + } + return false; + } }; @@ -104,8 +122,8 @@ export function operationDisconnect(selectedIDs, context) { if (disable) { return t('operations.disconnect.' + disable); } - if (disconnectingWay) { - return t('operations.disconnect.' + context.geometry(disconnectingWay) + '.description'); + if (disconnectingWayID) { + return t('operations.disconnect.' + context.geometry(disconnectingWayID) + '.description'); } return t('operations.disconnect.description'); };