From df1a2ea361ae8da62f9c771fcf74f70ce3460c94 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Mon, 8 Apr 2019 21:26:58 -0400 Subject: [PATCH] Prevent some actions on features that extend beyond the loaded map (closes #2248) --- data/core.yaml | 15 ++++++++ dist/locales/en.json | 25 ++++++++++-- modules/actions/circularize.js | 3 +- modules/core/context.js | 6 +-- modules/core/validator.js | 4 +- modules/operations/circularize.js | 19 ++++++--- modules/operations/continue.js | 9 +++-- modules/operations/delete.js | 54 ++++++++++++++------------ modules/operations/detach_node.js | 13 ++++--- modules/operations/disconnect.js | 16 ++++---- modules/operations/move.js | 19 +++++---- modules/operations/orthogonalize.js | 18 ++++++--- modules/operations/reflect.js | 25 ++++++------ modules/operations/reverse.js | 6 +-- modules/operations/rotate.js | 25 ++++++------ modules/operations/split.js | 19 +++++---- modules/operations/straighten.js | 60 +++++++++++++++-------------- test/spec/operations/straighten.js | 40 +++++++++---------- 18 files changed, 218 insertions(+), 158 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index 027b8a8c6..2fb4af24a 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -99,6 +99,7 @@ en: not_closed: This can't be made circular because it's not a loop. too_large: This can't be made circular because not enough of it is currently visible. connected_to_hidden: This can't be made circular because it is connected to a hidden feature. + not_downloaded: This can't be made circular because parts of it have not yet been downloaded. orthogonalize: title: Square description: @@ -115,6 +116,7 @@ en: not_squarish: This can't be made square because it is not squarish. too_large: This can't be made square because not enough of it is currently visible. connected_to_hidden: This can't be made square because it is connected to a hidden feature. + not_downloaded: This can't be made square because parts of it have not yet been downloaded. straighten: title: Straighten description: Straighten this line. @@ -122,6 +124,7 @@ en: annotation: Straightened a line. too_bendy: This can't be straightened because it bends too much. connected_to_hidden: This line can't be straightened because it is connected to a hidden feature. + not_downloaded: This line can't be straightened because parts of it have not yet been downloaded. delete: title: Delete description: @@ -146,6 +149,9 @@ en: connected_to_hidden: single: This feature can't be deleted because it is connected to a hidden feature. multiple: These features can't be deleted because some are connected to hidden features. + not_downloaded: + single: This feature can't be deleted because parts of it have not yet been downloaded. + multiple: These features can't be deleted because parts of them have not yet been downloaded. has_wikidata_tag: single: This feature can't be deleted because it has a Wikidata tag. multiple: These features can't be deleted because some have Wikidata tags. @@ -229,6 +235,9 @@ en: connected_to_hidden: single: This feature can't be moved because it is connected to a hidden feature. multiple: These features can't be moved because some are connected to hidden features. + not_downloaded: + single: This feature can't be moved because parts of it have not yet been downloaded. + multiple: These features can't be moved because parts of them have not yet been downloaded. reflect: title: long: Reflect Long @@ -259,6 +268,9 @@ en: connected_to_hidden: single: This feature can't be reflected because it is connected to a hidden feature. multiple: These features can't be reflected because some are connected to hidden features. + not_downloaded: + single: This feature can't be reflected because parts of it have not yet been downloaded. + multiple: These features can't be reflected because parts of them have not yet been downloaded. rotate: title: Rotate description: @@ -278,6 +290,9 @@ en: connected_to_hidden: single: This feature can't be rotated because it is connected to a hidden feature. multiple: These features can't be rotated because some are connected to hidden features. + not_downloaded: + single: This feature can't be rotated because parts of it have not yet been downloaded. + multiple: These features can't be rotated because parts of them have not yet been downloaded. reverse: title: Reverse description: Make this line go in the opposite direction. diff --git a/dist/locales/en.json b/dist/locales/en.json index 1effc790d..c6b877115 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -127,7 +127,8 @@ }, "not_closed": "This can't be made circular because it's not a loop.", "too_large": "This can't be made circular because not enough of it is currently visible.", - "connected_to_hidden": "This can't be made circular because it is connected to a hidden feature." + "connected_to_hidden": "This can't be made circular because it is connected to a hidden feature.", + "not_downloaded": "This can't be made circular because parts of it have not yet been downloaded." }, "orthogonalize": { "title": "Square", @@ -146,7 +147,8 @@ "square_enough": "This can't be made more square than it already is.", "not_squarish": "This can't be made square because it is not squarish.", "too_large": "This can't be made square because not enough of it is currently visible.", - "connected_to_hidden": "This can't be made square because it is connected to a hidden feature." + "connected_to_hidden": "This can't be made square because it is connected to a hidden feature.", + "not_downloaded": "This can't be made square because parts of it have not yet been downloaded." }, "straighten": { "title": "Straighten", @@ -154,7 +156,8 @@ "key": "S", "annotation": "Straightened a line.", "too_bendy": "This can't be straightened because it bends too much.", - "connected_to_hidden": "This line can't be straightened because it is connected to a hidden feature." + "connected_to_hidden": "This line can't be straightened because it is connected to a hidden feature.", + "not_downloaded": "This line can't be straightened because parts of it have not yet been downloaded." }, "delete": { "title": "Delete", @@ -186,6 +189,10 @@ "single": "This feature can't be deleted because it is connected to a hidden feature.", "multiple": "These features can't be deleted because some are connected to hidden features." }, + "not_downloaded": { + "single": "This feature can't be deleted because parts of it have not yet been downloaded.", + "multiple": "These features can't be deleted because parts of them have not yet been downloaded." + }, "has_wikidata_tag": { "single": "This feature can't be deleted because it has a Wikidata tag.", "multiple": "These features can't be deleted because some have Wikidata tags." @@ -290,6 +297,10 @@ "connected_to_hidden": { "single": "This feature can't be moved because it is connected to a hidden feature.", "multiple": "These features can't be moved because some are connected to hidden features." + }, + "not_downloaded": { + "single": "This feature can't be moved because parts of it have not yet been downloaded.", + "multiple": "These features can't be moved because parts of them have not yet been downloaded." } }, "reflect": { @@ -332,6 +343,10 @@ "connected_to_hidden": { "single": "This feature can't be reflected because it is connected to a hidden feature.", "multiple": "These features can't be reflected because some are connected to hidden features." + }, + "not_downloaded": { + "single": "This feature can't be reflected because parts of it have not yet been downloaded.", + "multiple": "These features can't be reflected because parts of them have not yet been downloaded." } }, "rotate": { @@ -357,6 +372,10 @@ "connected_to_hidden": { "single": "This feature can't be rotated because it is connected to a hidden feature.", "multiple": "These features can't be rotated because some are connected to hidden features." + }, + "not_downloaded": { + "single": "This feature can't be rotated because parts of it have not yet been downloaded.", + "multiple": "These features can't be rotated because parts of them have not yet been downloaded." } }, "reverse": { diff --git a/modules/actions/circularize.js b/modules/actions/circularize.js index 7f564593a..cafb24193 100644 --- a/modules/actions/circularize.js +++ b/modules/actions/circularize.js @@ -225,8 +225,9 @@ export function actionCircularize(wayId, projection, maxAngle) { action.disabled = function(graph) { - if (!graph.entity(wayId).isClosed()) + if (!graph.entity(wayId).isClosed()) { return 'not_closed'; + } }; diff --git a/modules/core/context.js b/modules/core/context.js index e45f46825..a83750617 100644 --- a/modules/core/context.js +++ b/modules/core/context.js @@ -27,7 +27,9 @@ export function setAreaKeys(value) { export function coreContext() { - var context = {}; + var dispatch = d3_dispatch('enter', 'exit', 'change'); + var context = utilRebind({}, dispatch, 'on'); + context.version = '2.14.3'; // create a special translation that contains the keys in place of the strings @@ -54,8 +56,6 @@ export function coreContext() { addTranslation('en', dataEn); setLocale('en'); - var dispatch = d3_dispatch('enter', 'exit', 'change'); - context = utilRebind(context, dispatch, 'on'); // https://github.com/openstreetmap/iD/issues/772 // http://mathiasbynens.be/notes/localstorage-pattern#comment-9 diff --git a/modules/core/validator.js b/modules/core/validator.js index 206cd817c..9f95a4eb5 100644 --- a/modules/core/validator.js +++ b/modules/core/validator.js @@ -9,7 +9,7 @@ import * as Validations from '../validations/index'; export function coreValidator(context) { var dispatch = d3_dispatch('validated'); - var validator = {}; + var validator = utilRebind({}, dispatch, 'on'); var _rules = {}; var _disabledRules = {}; @@ -305,7 +305,7 @@ export function coreValidator(context) { }); - return utilRebind(validator, dispatch, 'on'); + return validator; } diff --git a/modules/operations/circularize.js b/modules/operations/circularize.js index a0ba14bd1..7698fb05b 100644 --- a/modules/operations/circularize.js +++ b/modules/operations/circularize.js @@ -1,6 +1,7 @@ import { t } from '../util/locale'; import { actionCircularize } from '../actions'; import { behaviorOperation } from '../behavior'; +import { utilGetAllNodes } from '../util'; export function operationCircularize(selectedIDs, context) { @@ -9,7 +10,8 @@ export function operationCircularize(selectedIDs, context) { var extent = entity.extent(context.graph()); var geometry = context.geometry(entityID); var action = actionCircularize(entityID, context.projection); - + var nodes = utilGetAllNodes(selectedIDs, context.graph()); + var coords = nodes.map(function(n) { return n.loc; }); var operation = function() { context.perform(action, operation.annotation()); @@ -24,13 +26,18 @@ export function operationCircularize(selectedIDs, context) { operation.disabled = function() { - var reason; - if (extent.percentContainedIn(context.extent()) < 0.8) { - reason = 'too_large'; + var osm = context.connection(); + var reason = action.disabled(context.graph()); + if (reason) { + return reason; + } else if (extent.percentContainedIn(context.extent()) < 0.8) { + return 'too_large'; + } else if (osm && !coords.every(osm.isDataLoaded)) { + return 'not_downloaded'; } else if (context.hasHiddenConnections(entityID)) { - reason = 'connected_to_hidden'; + return 'connected_to_hidden'; } - return action.disabled(context.graph()) || reason; + return false; }; diff --git a/modules/operations/continue.js b/modules/operations/continue.js index 560003fcb..37a467469 100644 --- a/modules/operations/continue.js +++ b/modules/operations/continue.js @@ -33,17 +33,20 @@ export function operationContinue(selectedIDs, context) { operation.available = function() { - return geometries.vertex.length === 1 && geometries.line.length <= 1 && + return geometries.vertex.length === 1 && + geometries.line.length <= 1 && !context.features().hasHiddenConnections(vertex, context.graph()); }; operation.disabled = function() { var candidates = candidateWays(); - if (candidates.length === 0) + if (candidates.length === 0) { return 'not_eligible'; - if (candidates.length > 1) + } else if (candidates.length > 1) { return 'multiple'; + } + return false; }; diff --git a/modules/operations/delete.js b/modules/operations/delete.js index d1152b191..2dd00a57c 100644 --- a/modules/operations/delete.js +++ b/modules/operations/delete.js @@ -4,14 +4,17 @@ import { behaviorOperation } from '../behavior'; import { geoExtent, geoSphericalDistance } from '../geo'; import { modeBrowse, modeSelect } from '../modes'; import { uiCmd } from '../ui'; +import { utilGetAllNodes } from '../util'; export function operationDelete(selectedIDs, context) { - var multi = (selectedIDs.length === 1 ? 'single' : 'multiple'), - action = actionDeleteMultiple(selectedIDs), - extent = selectedIDs.reduce(function(extent, id) { - return extent.extend(context.entity(id).extent(context.graph())); - }, geoExtent()); + var multi = (selectedIDs.length === 1 ? 'single' : 'multiple'); + var action = actionDeleteMultiple(selectedIDs); + var nodes = utilGetAllNodes(selectedIDs, context.graph()); + var coords = nodes.map(function(n) { return n.loc; }); + var extent = nodes.reduce(function(extent, node) { + return extent.extend(node.extent(context.graph())); + }, geoExtent()); var operation = function() { @@ -19,24 +22,24 @@ export function operationDelete(selectedIDs, context) { var nextSelectedLoc; if (selectedIDs.length === 1) { - var id = selectedIDs[0], - entity = context.entity(id), - geometry = context.geometry(id), - parents = context.graph().parentWays(entity), - parent = parents[0]; + var id = selectedIDs[0]; + var entity = context.entity(id); + var geometry = context.geometry(id); + var parents = context.graph().parentWays(entity); + var parent = parents[0]; // Select the next closest node in the way. if (geometry === 'vertex') { - var nodes = parent.nodes, - i = nodes.indexOf(id); + var nodes = parent.nodes; + var i = nodes.indexOf(id); if (i === 0) { i++; } else if (i === nodes.length - 1) { i--; } else { - var a = geoSphericalDistance(entity.loc, context.entity(nodes[i - 1]).loc), - b = geoSphericalDistance(entity.loc, context.entity(nodes[i + 1]).loc); + var a = geoSphericalDistance(entity.loc, context.entity(nodes[i - 1]).loc); + var b = geoSphericalDistance(entity.loc, context.entity(nodes[i + 1]).loc); i = a < b ? i - 1 : i + 1; } @@ -67,19 +70,21 @@ export function operationDelete(selectedIDs, context) { operation.disabled = function() { - var reason; + var osm = context.connection(); if (extent.area() && extent.percentContainedIn(context.extent()) < 0.8) { - reason = 'too_large'; + return 'too_large'; + } else if (osm && !coords.every(osm.isDataLoaded)) { + return 'not_downloaded'; } else if (selectedIDs.some(context.hasHiddenConnections)) { - reason = 'connected_to_hidden'; + return 'connected_to_hidden'; } else if (selectedIDs.some(protectedMember)) { - reason = 'part_of_relation'; + return 'part_of_relation'; } else if (selectedIDs.some(incompleteRelation)) { - reason = 'incomplete_relation'; + return 'incomplete_relation'; } else if (selectedIDs.some(hasWikidataTag)) { - reason = 'has_wikidata_tag'; + return 'has_wikidata_tag'; } - return reason; + return false; function hasWikidataTag(id) { var entity = context.entity(id); @@ -97,16 +102,15 @@ export function operationDelete(selectedIDs, context) { var parents = context.graph().parentRelations(entity); for (var i = 0; i < parents.length; i++) { - var parent = parents[i], - type = parent.tags.type, - role = parent.memberById(id).role || 'outer'; + var parent = parents[i]; + var type = parent.tags.type; + var role = parent.memberById(id).role || 'outer'; if (type === 'route' || type === 'boundary' || (type === 'multipolygon' && role === 'outer')) { return true; } } return false; } - }; diff --git a/modules/operations/detach_node.js b/modules/operations/detach_node.js index 5b03e731d..a578ed211 100644 --- a/modules/operations/detach_node.js +++ b/modules/operations/detach_node.js @@ -48,11 +48,13 @@ export function operationDetachNode(selectedIDs, context) { operation.disabled = function () { - var reason; - if (selectedIDs.some(context.hasHiddenConnections)) { - reason = 'connected_to_hidden'; + var reason = action.disabled(context.graph()); + if (reason) { + return reason; + } else if (selectedIDs.some(context.hasHiddenConnections)) { + return 'connected_to_hidden'; } - return action.disabled(context.graph()) || reason; + return false; }; @@ -60,7 +62,8 @@ export function operationDetachNode(selectedIDs, context) { var disableReason = operation.disabled(); if (disableReason) { return t('operations.detach_node.' + disableReason, - { relation: context.presets().item('type/restriction').name() }); + { relation: context.presets().item('type/restriction').name() } + ); } else { return t('operations.detach_node.description'); } diff --git a/modules/operations/disconnect.js b/modules/operations/disconnect.js index b9ede9fd3..62d526a93 100644 --- a/modules/operations/disconnect.js +++ b/modules/operations/disconnect.js @@ -4,10 +4,8 @@ import { behaviorOperation } from '../behavior/index'; export function operationDisconnect(selectedIDs, context) { - var vertices = selectedIDs.filter(function(id) { - return context.geometry(id) === 'vertex'; - }); - + var vertices = selectedIDs + .filter(function(id) { return context.geometry(id) === 'vertex'; }); var entityID = vertices[0]; var action = actionDisconnect(entityID); @@ -28,11 +26,13 @@ export function operationDisconnect(selectedIDs, context) { operation.disabled = function() { - var reason; - if (selectedIDs.some(context.hasHiddenConnections)) { - reason = 'connected_to_hidden'; + var reason = action.disabled(context.graph()); + if (reason) { + return reason; + } else if (selectedIDs.some(context.hasHiddenConnections)) { + return 'connected_to_hidden'; } - return action.disabled(context.graph()) || reason; + return false; }; diff --git a/modules/operations/move.js b/modules/operations/move.js index d5c587767..d9506b806 100644 --- a/modules/operations/move.js +++ b/modules/operations/move.js @@ -2,12 +2,15 @@ import { t } from '../util/locale'; import { behaviorOperation } from '../behavior'; import { geoExtent } from '../geo'; import { modeMove } from '../modes'; +import { utilGetAllNodes } from '../util'; export function operationMove(selectedIDs, context) { var multi = (selectedIDs.length === 1 ? 'single' : 'multiple'); - var extent = selectedIDs.reduce(function(extent, id) { - return extent.extend(context.entity(id).extent(context.graph())); + var nodes = utilGetAllNodes(selectedIDs, context.graph()); + var coords = nodes.map(function(n) { return n.loc; }); + var extent = nodes.reduce(function(extent, node) { + return extent.extend(node.extent(context.graph())); }, geoExtent()); @@ -23,15 +26,17 @@ export function operationMove(selectedIDs, context) { operation.disabled = function() { - var reason; + var osm = context.connection(); if (extent.area() && extent.percentContainedIn(context.extent()) < 0.8) { - reason = 'too_large'; + return 'too_large'; + } else if (osm && !coords.every(osm.isDataLoaded)) { + return 'not_downloaded'; } else if (selectedIDs.some(context.hasHiddenConnections)) { - reason = 'connected_to_hidden'; + return 'connected_to_hidden'; } else if (selectedIDs.some(incompleteRelation)) { - reason = 'incomplete_relation'; + return 'incomplete_relation'; } - return reason; + return false; function incompleteRelation(id) { var entity = context.entity(id); diff --git a/modules/operations/orthogonalize.js b/modules/operations/orthogonalize.js index eec9fee43..7935d439b 100644 --- a/modules/operations/orthogonalize.js +++ b/modules/operations/orthogonalize.js @@ -1,6 +1,7 @@ import { t } from '../util/locale'; import { actionOrthogonalize } from '../actions/index'; import { behaviorOperation } from '../behavior/index'; +import { utilGetAllNodes } from '../util'; export function operationOrthogonalize(selectedIDs, context) { @@ -8,6 +9,8 @@ export function operationOrthogonalize(selectedIDs, context) { var _entity; var _geometry; var action = chooseAction(); + var nodes = utilGetAllNodes(selectedIDs, context.graph()); + var coords = nodes.map(function(n) { return n.loc; }); function chooseAction() { @@ -51,15 +54,20 @@ export function operationOrthogonalize(selectedIDs, context) { operation.disabled = function() { if (!action) return ''; + var osm = context.connection(); var extent = _entity.extent(context.graph()); - var reason; + var reason = action.disabled(context.graph()); - if (_geometry !== 'vertex' && extent.percentContainedIn(context.extent()) < 0.8) { - reason = 'too_large'; + if (reason) { + return reason; + } else if (_geometry !== 'vertex' && extent.percentContainedIn(context.extent()) < 0.8) { + return 'too_large'; + } else if (osm && !coords.every(osm.isDataLoaded)) { + return 'not_downloaded'; } else if (context.hasHiddenConnections(_entityID)) { - reason = 'connected_to_hidden'; + return 'connected_to_hidden'; } - return action.disabled(context.graph()) || reason; + return false; }; diff --git a/modules/operations/reflect.js b/modules/operations/reflect.js index 9ee35f25c..8f9fe5a3f 100644 --- a/modules/operations/reflect.js +++ b/modules/operations/reflect.js @@ -18,8 +18,10 @@ export function operationReflectLong(selectedIDs, context) { export function operationReflect(selectedIDs, context, axis) { axis = axis || 'long'; var multi = (selectedIDs.length === 1 ? 'single' : 'multiple'); - var extent = selectedIDs.reduce(function(extent, id) { - return extent.extend(context.entity(id).extent(context.graph())); + var nodes = utilGetAllNodes(selectedIDs, context.graph()); + var coords = nodes.map(function(n) { return n.loc; }); + var extent = nodes.reduce(function(extent, node) { + return extent.extend(node.extent(context.graph())); }, geoExtent()); @@ -31,25 +33,22 @@ export function operationReflect(selectedIDs, context, axis) { operation.available = function() { - var nodes = utilGetAllNodes(selectedIDs, context.graph()); - var uniqeLocs = nodes.reduce(function(acc, node) { - return acc.add(node.loc); - }, new Set()); - - return uniqeLocs.size >= 3; + return nodes.length >= 3; }; operation.disabled = function() { - var reason; + var osm = context.connection(); if (extent.area() && extent.percentContainedIn(context.extent()) < 0.8) { - reason = 'too_large'; + return 'too_large'; + } else if (osm && !coords.every(osm.isDataLoaded)) { + return 'not_downloaded'; } else if (selectedIDs.some(context.hasHiddenConnections)) { - reason = 'connected_to_hidden'; + return 'connected_to_hidden'; } else if (selectedIDs.some(incompleteRelation)) { - reason = 'incomplete_relation'; + return 'incomplete_relation'; } - return reason; + return false; function incompleteRelation(id) { var entity = context.entity(id); diff --git a/modules/operations/reverse.js b/modules/operations/reverse.js index 3320c6902..c14c64643 100644 --- a/modules/operations/reverse.js +++ b/modules/operations/reverse.js @@ -4,15 +4,15 @@ import { behaviorOperation } from '../behavior/index'; export function operationReverse(selectedIDs, context) { - var entityId = selectedIDs[0]; + var entityID = selectedIDs[0]; var operation = function() { - context.perform(actionReverse(entityId), operation.annotation()); + context.perform(actionReverse(entityID), operation.annotation()); }; operation.available = function() { - return selectedIDs.length === 1 && context.geometry(entityId) === 'line'; + return selectedIDs.length === 1 && context.geometry(entityID) === 'line'; }; diff --git a/modules/operations/rotate.js b/modules/operations/rotate.js index 61335aaef..2371dfcbf 100644 --- a/modules/operations/rotate.js +++ b/modules/operations/rotate.js @@ -7,8 +7,10 @@ import { utilGetAllNodes } from '../util'; export function operationRotate(selectedIDs, context) { var multi = (selectedIDs.length === 1 ? 'single' : 'multiple'); - var extent = selectedIDs.reduce(function(extent, id) { - return extent.extend(context.entity(id).extent(context.graph())); + var nodes = utilGetAllNodes(selectedIDs, context.graph()); + var coords = nodes.map(function(n) { return n.loc; }); + var extent = nodes.reduce(function(extent, node) { + return extent.extend(node.extent(context.graph())); }, geoExtent()); @@ -18,25 +20,22 @@ export function operationRotate(selectedIDs, context) { operation.available = function() { - var nodes = utilGetAllNodes(selectedIDs, context.graph()); - var uniqeLocs = nodes.reduce(function(acc, node) { - return acc.add(node.loc); - }, new Set()); - - return uniqeLocs.size >= 2; + return nodes.size >= 2; }; operation.disabled = function() { - var reason; + var osm = context.connection(); if (extent.area() && extent.percentContainedIn(context.extent()) < 0.8) { - reason = 'too_large'; + return 'too_large'; + } else if (osm && !coords.every(osm.isDataLoaded)) { + return 'not_downloaded'; } else if (selectedIDs.some(context.hasHiddenConnections)) { - reason = 'connected_to_hidden'; + return 'connected_to_hidden'; } else if (selectedIDs.some(incompleteRelation)) { - reason = 'incomplete_relation'; + return 'incomplete_relation'; } - return reason; + return false; function incompleteRelation(id) { var entity = context.entity(id); diff --git a/modules/operations/split.js b/modules/operations/split.js index 05acba0da..d7ca65806 100644 --- a/modules/operations/split.js +++ b/modules/operations/split.js @@ -5,10 +5,8 @@ import { modeSelect } from '../modes/index'; export function operationSplit(selectedIDs, context) { - var vertices = selectedIDs.filter(function(id) { - return context.geometry(id) === 'vertex'; - }); - + var vertices = selectedIDs + .filter(function(id) { return context.geometry(id) === 'vertex'; }); var entityID = vertices[0]; var action = actionSplit(entityID); var ways = []; @@ -34,11 +32,13 @@ export function operationSplit(selectedIDs, context) { operation.disabled = function() { - var reason; - if (selectedIDs.some(context.hasHiddenConnections)) { - reason = 'connected_to_hidden'; + var reason = action.disabled(context.graph()); + if (reason) { + return reason; + } else if (selectedIDs.some(context.hasHiddenConnections)) { + return 'connected_to_hidden'; } - return action.disabled(context.graph()) || reason; + return false; }; @@ -46,8 +46,7 @@ export function operationSplit(selectedIDs, context) { var disable = operation.disabled(); if (disable) { return t('operations.split.' + disable); - } - if (ways.length === 1) { + } else if (ways.length === 1) { return t('operations.split.description.' + context.geometry(ways[0].id)); } else { return t('operations.split.description.multiple'); diff --git a/modules/operations/straighten.js b/modules/operations/straighten.js index 70087c07c..74d20f681 100644 --- a/modules/operations/straighten.js +++ b/modules/operations/straighten.js @@ -1,11 +1,14 @@ import { t } from '../util/locale'; import { actionStraighten } from '../actions/index'; import { behaviorOperation } from '../behavior/index'; -import { utilArrayDifference } from '../util/index'; +import { utilArrayDifference, utilGetAllNodes } from '../util/index'; export function operationStraighten(selectedIDs, context) { var action = actionStraighten(selectedIDs, context.projection); + var wayIDs = selectedIDs.filter(function(id) { return id.charAt(0) === 'w'; }); + var nodes = utilGetAllNodes(wayIDs, context.graph()); + var coords = nodes.map(function(n) { return n.loc; }); function operation() { @@ -14,49 +17,45 @@ export function operationStraighten(selectedIDs, context) { operation.available = function() { - var nodes = []; - var startNodes = []; - var endNodes = []; - var selectedNodes = []; + var nodeIDs = nodes.map(function(node) { return node.id; }); + var startNodeIDs = []; + var endNodeIDs = []; + var selectedNodeIDs = []; - // collect nodes along selected ways for (var i = 0; i < selectedIDs.length; i++) { - if (!context.hasEntity(selectedIDs[i])) return false; - var entity = context.entity(selectedIDs[i]); if (entity.type === 'node') { - selectedNodes.push(entity.id); + selectedNodeIDs.push(entity.id); continue; } else if (entity.type !== 'way' || entity.isClosed()) { return false; // exit early, can't straighten these } - nodes = nodes.concat(entity.nodes); - startNodes.push(entity.nodes[0]); - endNodes.push(entity.nodes[entity.nodes.length-1]); + startNodeIDs.push(entity.first()); + endNodeIDs.push(entity.last()); } - // Remove duplicate end/startNodes (duplicate nodes cannot be at the line end) - startNodes = startNodes.filter(function(n) { - return startNodes.indexOf(n) === startNodes.lastIndexOf(n); + // Remove duplicate end/startNodeIDs (duplicate nodes cannot be at the line end) + startNodeIDs = startNodeIDs.filter(function(n) { + return startNodeIDs.indexOf(n) === startNodeIDs.lastIndexOf(n); }); - endNodes = endNodes.filter(function(n) { - return endNodes.indexOf(n) === endNodes.lastIndexOf(n); + endNodeIDs = endNodeIDs.filter(function(n) { + return endNodeIDs.indexOf(n) === endNodeIDs.lastIndexOf(n); }); // Return false if line is only 2 nodes long - if (new Set(nodes).size <= 2) return false; + if (nodeIDs.length <= 2) return false; // Return false unless exactly 0 or 2 specific start/end nodes are selected - if (!(selectedNodes.length === 0 || selectedNodes.length === 2)) return false; + if (!(selectedNodeIDs.length === 0 || selectedNodeIDs.length === 2)) return false; // Ensure all ways are connected (i.e. only 2 unique endpoints/startpoints) - if (utilArrayDifference(startNodes, endNodes).length + - utilArrayDifference(endNodes, startNodes).length !== 2) return false; + if (utilArrayDifference(startNodeIDs, endNodeIDs).length + + utilArrayDifference(endNodeIDs, startNodeIDs).length !== 2) return false; // Ensure both start/end selected nodes lie on the selected path - if (selectedNodes.length === 2 && ( - nodes.indexOf(selectedNodes[0]) === -1 || nodes.indexOf(selectedNodes[1]) === -1 + if (selectedNodeIDs.length === 2 && ( + nodeIDs.indexOf(selectedNodeIDs[0]) === -1 || nodeIDs.indexOf(selectedNodeIDs[1]) === -1 )) return false; return true; @@ -64,13 +63,16 @@ export function operationStraighten(selectedIDs, context) { operation.disabled = function() { - var reason; - for (var i = 0; i < selectedIDs.length; i++) { - if (context.hasHiddenConnections(selectedIDs[i])) { - reason = 'connected_to_hidden'; - } + var osm = context.connection(); + var reason = action.disabled(context.graph()); + if (reason) { + return reason; + } else if (osm && !coords.every(osm.isDataLoaded)) { + return 'not_downloaded'; + } else if (selectedIDs.some(context.hasHiddenConnections)) { + return 'connected_to_hidden'; } - return action.disabled(context.graph()) || reason; + return false; }; diff --git a/test/spec/operations/straighten.js b/test/spec/operations/straighten.js index 8f699d9b8..5d62cf229 100644 --- a/test/spec/operations/straighten.js +++ b/test/spec/operations/straighten.js @@ -4,8 +4,9 @@ describe('iD.operationStraighten', function () { // Set up the fake context fakeContext = {}; - fakeContext.graph = function () { return graph; }; - fakeContext.hasHiddenConnections = function () { return false; }; + fakeContext.graph = function() { return graph; }; + fakeContext.entity = function(id) { return graph.entity(id); }; + fakeContext.hasHiddenConnections = function() { return false; }; describe('#available', function () { beforeEach(function () { @@ -41,82 +42,77 @@ describe('iD.operationStraighten', function () { }); it('is not available for no selected ids', function () { - var result = iD.operationStraighten([], fakeContext.graph()).available(); + var result = iD.operationStraighten([], fakeContext).available(); expect(result).to.be.not.ok; }); it('is not available for way with only 2 nodes', function () { - var result = iD.operationStraighten(['w1'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w1'], fakeContext).available(); expect(result).to.be.not.ok; }); it('is available for way with only 2 nodes connected to another 2-node way', function () { - var result = iD.operationStraighten(['w1', 'w1-2'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w1', 'w1-2'], fakeContext).available(); expect(result).to.be.ok; }); - it('is not available for unknown selected id', function () { - var result = iD.operationStraighten(['w0'], fakeContext.graph()).available(); - expect(result).to.be.not.ok; - }); - it('is not available for non-continuous ways', function () { - var result = iD.operationStraighten(['w2', 'w4'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w2', 'w4'], fakeContext).available(); expect(result).to.be.not.ok; }); it('is available for selected way with more than 2 nodes', function () { - var result = iD.operationStraighten(['w2'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w2'], fakeContext).available(); expect(result).to.be.ok; }); it('is available for selected, ordered, continuous ways', function () { - var result = iD.operationStraighten(['w1', 'w2', 'w3'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w1', 'w2', 'w3'], fakeContext).available(); expect(result).to.be.ok; }); it('is available for selected, un-ordered, continuous ways', function () { - var result = iD.operationStraighten(['w1', 'w3', 'w2'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w1', 'w3', 'w2'], fakeContext).available(); expect(result).to.be.ok; }); it('is available for selected, continuous ways with different way-directions', function () { - var result = iD.operationStraighten(['w1', 'w3', 'w2-2'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w1', 'w3', 'w2-2'], fakeContext).available(); expect(result).to.be.ok; }); it('is available for 2 selected nodes in the same way, more than one node apart', function () { - var result = iD.operationStraighten(['w5', 'n9', 'n11'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w5', 'n9', 'n11'], fakeContext).available(); expect(result).to.be.ok; }); it('is available for 2 selected nodes in adjacent ways, more than one node apart', function () { - var result = iD.operationStraighten(['w2', 'w3', 'n5', 'n3'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w2', 'w3', 'n5', 'n3'], fakeContext).available(); expect(result).to.be.ok; }); it('is available for 2 selected nodes in non-adjacent ways, providing inbetween ways are selected', function () { - var result = iD.operationStraighten(['n2', 'n7', 'w4', 'w1', 'w3', 'w2'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['n2', 'n7', 'w4', 'w1', 'w3', 'w2'], fakeContext).available(); expect(result).to.be.ok; }); it('is available for 2 selected nodes in non-adjacent, non-same-directional ways, providing inbetween ways are selected', function () { - var result = iD.operationStraighten(['n2', 'n7', 'w4', 'w1', 'w3', 'w2-2'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['n2', 'n7', 'w4', 'w1', 'w3', 'w2-2'], fakeContext).available(); expect(result).to.be.ok; }); it('is not available for nodes not on selected ways', function () { - var result = iD.operationStraighten(['w5', 'n4', 'n11'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w5', 'n4', 'n11'], fakeContext).available(); expect(result).to.be.not.ok; }); it('is not available for one selected node', function () { - var result = iD.operationStraighten(['w5', 'n9'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w5', 'n9'], fakeContext).available(); expect(result).to.be.not.ok; }); it('is not available for more than two selected nodes', function () { - var result = iD.operationStraighten(['w5', 'n9', 'n11', 'n12'], fakeContext.graph()).available(); + var result = iD.operationStraighten(['w5', 'n9', 'n11', 'n12'], fakeContext).available(); expect(result).to.be.not.ok; }); });