From 37534aed0e9af6f456c8b0df31547bbc8fdc4089 Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 21 Dec 2016 23:58:13 -0500 Subject: [PATCH] More cleanup of operations and post-paste behavior * Support move, rotate, reflect, delete post paste on multiselection * Improve text and error msgs for singular vs multi selections * Move `disabled` checks from actions to operations * Reproject center of rotation (closes #3667) * Cleanup tests --- data/core.yaml | 75 +++++++++++++++++----- dist/locales/en.json | 96 ++++++++++++++++++++++------ modules/actions/delete_multiple.js | 9 --- modules/actions/delete_node.js | 5 -- modules/actions/delete_relation.js | 6 -- modules/actions/delete_way.js | 15 ----- modules/actions/move.js | 11 ---- modules/actions/reflect.js | 25 +++----- modules/actions/rotate.js | 11 ++-- modules/modes/rotate.js | 2 +- modules/operations/delete.js | 36 +++++++++-- modules/operations/move.js | 16 +++-- modules/operations/reflect.js | 41 +++++++----- modules/operations/rotate.js | 9 +-- test/spec/actions/delete_multiple.js | 19 +++--- test/spec/actions/delete_relation.js | 17 ++--- test/spec/actions/delete_way.js | 51 +++++++-------- test/spec/actions/move.js | 43 +++++++------ test/spec/actions/reflect.js | 25 ++------ 19 files changed, 298 insertions(+), 214 deletions(-) diff --git a/data/core.yaml b/data/core.yaml index afc6b6344..e614c9fd0 100644 --- a/data/core.yaml +++ b/data/core.yaml @@ -77,7 +77,9 @@ en: connected_to_hidden: This line can't be straightened because it is connected to a hidden feature. delete: title: Delete - description: Delete object permanently. + description: + single: Delete this object permanently. + multiple: Delete these objects permanently. annotation: point: Deleted a point. vertex: Deleted a node from a way. @@ -85,9 +87,15 @@ en: area: Deleted an area. relation: Deleted a relation. multiple: "Deleted {n} objects." - incomplete_relation: This feature can't be deleted because it hasn't been fully downloaded. - part_of_relation: This feature can't be deleted because it's part of a larger relation. You must remove it from the relation first. - connected_to_hidden: This can't be deleted because it is connected to a hidden feature. + incomplete_relation: + single: This object can't be deleted because it hasn't been fully downloaded. + multiple: These objects can't be deleted because they haven't been fully downloaded. + part_of_relation: + single: This object can't be deleted because it is part of a larger relation. You must remove it from the relation first. + multiple: These objects can't be deleted because they are part of larger relations. You must remove them from the relations first. + connected_to_hidden: + single: This object can't be deleted because it is connected to a hidden feature. + multiple: These objects can't be deleted because some are connected to hidden features. add_member: annotation: Added a member to a relation. delete_member: @@ -118,7 +126,9 @@ en: conflicting_tags: These features can't be merged because some of their tags have conflicting values. move: title: Move - description: Move this to a different location. + description: + single: Move this object to a different location. + multiple: Move these objects to a different location. key: M annotation: point: Moved a point. @@ -126,31 +136,62 @@ en: line: Moved a line. area: Moved an area. multiple: Moved multiple objects. - incomplete_relation: This feature can't be moved because it hasn't been fully downloaded. - too_large: This can't be moved because not enough of it is currently visible. - connected_to_hidden: This can't be moved because it is connected to a hidden feature. + incomplete_relation: + single: This object can't be moved because it hasn't been fully downloaded. + multiple: These objects can't be moved because they haven't been fully downloaded. + too_large: + single: This object can't be moved because not enough of it is currently visible. + multiple: These objects can't be moved because not enough of them are currently visible. + connected_to_hidden: + single: This object can't be moved because it is connected to a hidden feature. + multiple: These objects can't be moved because some are connected to hidden features. reflect: title: reflect description: - long: Reflect this object across its long axis. - short: Reflect this object across its short axis. + long: + single: Reflect this object across its long axis. + multiple: Reflect these objects across their long axis. + short: + single: Reflect this object across its short axis. + multiple: Reflect these objects across their short axis. key: long: T short: Y annotation: - long: Reflected an area across its long axis. - short: Reflected an area across its short axis. - too_large: This can't be reflected because not enough of it is currently visible. - connected_to_hidden: This can't be reflected because it is connected to a hidden feature. + long: + area: Reflected an area across its long axis. + multiple: Reflected multiple objects across their long axis. + short: + area: Reflected an area across its short axis. + multiple: Reflected multiple objects across their short axis. + incomplete_relation: + single: This object can't be reflected because it hasn't been fully downloaded. + multiple: These objects can't be reflected because they haven't been fully downloaded. + too_large: + single: This object can't be reflected because not enough of it is currently visible. + multiple: These objects can't be reflected because not enough of them are currently visible. + connected_to_hidden: + single: This object can't be reflected because it is connected to a hidden feature. + multiple: These objects can't be reflected because some are connected to hidden features. rotate: title: Rotate - description: Rotate this object around its center point. + description: + single: Rotate this object around its center point. + multiple: Rotate these objects around their center point. key: R annotation: line: Rotated a line. area: Rotated an area. - too_large: This can't be rotated because not enough of it is currently visible. - connected_to_hidden: This can't be rotated because it is connected to a hidden feature. + multiple: Rotated multiple objects. + incomplete_relation: + single: This object can't be rotated because it hasn't been fully downloaded. + multiple: These objects can't be rotated because they haven't been fully downloaded. + too_large: + single: This object can't be rotated because not enough of it is currently visible. + multiple: These objects can't be rotated because not enough of them are currently visible. + connected_to_hidden: + single: This object can't be rotated because it is connected to a hidden feature. + multiple: These objects can't be rotated because some are connected to hidden features. 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 40322b142..b50995a57 100644 --- a/dist/locales/en.json +++ b/dist/locales/en.json @@ -101,7 +101,10 @@ }, "delete": { "title": "Delete", - "description": "Delete object permanently.", + "description": { + "single": "Delete this object permanently.", + "multiple": "Delete these objects permanently." + }, "annotation": { "point": "Deleted a point.", "vertex": "Deleted a node from a way.", @@ -110,9 +113,18 @@ "relation": "Deleted a relation.", "multiple": "Deleted {n} objects." }, - "incomplete_relation": "This feature can't be deleted because it hasn't been fully downloaded.", - "part_of_relation": "This feature can't be deleted because it's part of a larger relation. You must remove it from the relation first.", - "connected_to_hidden": "This can't be deleted because it is connected to a hidden feature." + "incomplete_relation": { + "single": "This object can't be deleted because it hasn't been fully downloaded.", + "multiple": "These objects can't be deleted because they haven't been fully downloaded." + }, + "part_of_relation": { + "single": "This object can't be deleted because it is part of a larger relation. You must remove it from the relation first.", + "multiple": "These objects can't be deleted because they are part of larger relations. You must remove them from the relations first." + }, + "connected_to_hidden": { + "single": "This object can't be deleted because it is connected to a hidden feature.", + "multiple": "These objects can't be deleted because some are connected to hidden features." + } }, "add_member": { "annotation": "Added a member to a relation." @@ -150,7 +162,10 @@ }, "move": { "title": "Move", - "description": "Move this to a different location.", + "description": { + "single": "Move this object to a different location.", + "multiple": "Move these objects to a different location." + }, "key": "M", "annotation": { "point": "Moved a point.", @@ -159,37 +174,82 @@ "area": "Moved an area.", "multiple": "Moved multiple objects." }, - "incomplete_relation": "This feature can't be moved because it hasn't been fully downloaded.", - "too_large": "This can't be moved because not enough of it is currently visible.", - "connected_to_hidden": "This can't be moved because it is connected to a hidden feature." + "incomplete_relation": { + "single": "This object can't be moved because it hasn't been fully downloaded.", + "multiple": "These objects can't be moved because they haven't been fully downloaded." + }, + "too_large": { + "single": "This object can't be moved because not enough of it is currently visible.", + "multiple": "These objects can't be moved because not enough of them are currently visible." + }, + "connected_to_hidden": { + "single": "This object can't be moved because it is connected to a hidden feature.", + "multiple": "These objects can't be moved because some are connected to hidden features." + } }, "reflect": { "title": "reflect", "description": { - "long": "Reflect this object across its long axis.", - "short": "Reflect this object across its short axis." + "long": { + "single": "Reflect this object across its long axis.", + "multiple": "Reflect these objects across their long axis." + }, + "short": { + "single": "Reflect this object across its short axis.", + "multiple": "Reflect these objects across their short axis." + } }, "key": { "long": "T", "short": "Y" }, "annotation": { - "long": "Reflected an area across its long axis.", - "short": "Reflected an area across its short axis." + "long": { + "area": "Reflected an area across its long axis.", + "multiple": "Reflected multiple objects across their long axis." + }, + "short": { + "area": "Reflected an area across its short axis.", + "multiple": "Reflected multiple objects across their short axis." + } }, - "too_large": "This can't be reflected because not enough of it is currently visible.", - "connected_to_hidden": "This can't be reflected because it is connected to a hidden feature." + "incomplete_relation": { + "single": "This object can't be reflected because it hasn't been fully downloaded.", + "multiple": "These objects can't be reflected because they haven't been fully downloaded." + }, + "too_large": { + "single": "This object can't be reflected because not enough of it is currently visible.", + "multiple": "These objects can't be reflected because not enough of them are currently visible." + }, + "connected_to_hidden": { + "single": "This object can't be reflected because it is connected to a hidden feature.", + "multiple": "These objects can't be reflected because some are connected to hidden features." + } }, "rotate": { "title": "Rotate", - "description": "Rotate this object around its center point.", + "description": { + "single": "Rotate this object around its center point.", + "multiple": "Rotate these objects around their center point." + }, "key": "R", "annotation": { "line": "Rotated a line.", - "area": "Rotated an area." + "area": "Rotated an area.", + "multiple": "Rotated multiple objects." }, - "too_large": "This can't be rotated because not enough of it is currently visible.", - "connected_to_hidden": "This can't be rotated because it is connected to a hidden feature." + "incomplete_relation": { + "single": "This object can't be rotated because it hasn't been fully downloaded.", + "multiple": "These objects can't be rotated because they haven't been fully downloaded." + }, + "too_large": { + "single": "This object can't be rotated because not enough of it is currently visible.", + "multiple": "These objects can't be rotated because not enough of them are currently visible." + }, + "connected_to_hidden": { + "single": "This object can't be rotated because it is connected to a hidden feature.", + "multiple": "These objects can't be rotated because some are connected to hidden features." + } }, "reverse": { "title": "Reverse", diff --git a/modules/actions/delete_multiple.js b/modules/actions/delete_multiple.js index db90d53ae..c9208d8c1 100644 --- a/modules/actions/delete_multiple.js +++ b/modules/actions/delete_multiple.js @@ -22,14 +22,5 @@ export function actionDeleteMultiple(ids) { }; - action.disabled = function(graph) { - for (var i = 0; i < ids.length; i++) { - var id = ids[i], - disabled = actions[graph.entity(id).type](id).disabled(graph); - if (disabled) return disabled; - } - }; - - return action; } diff --git a/modules/actions/delete_node.js b/modules/actions/delete_node.js index f8d9d99c3..402fe0cfe 100644 --- a/modules/actions/delete_node.js +++ b/modules/actions/delete_node.js @@ -31,10 +31,5 @@ export function actionDeleteNode(nodeId) { }; - action.disabled = function() { - return false; - }; - - return action; } diff --git a/modules/actions/delete_relation.js b/modules/actions/delete_relation.js index cb74101a6..0e0072682 100644 --- a/modules/actions/delete_relation.js +++ b/modules/actions/delete_relation.js @@ -39,11 +39,5 @@ export function actionDeleteRelation(relationId) { }; - action.disabled = function(graph) { - if (!graph.entity(relationId).isComplete(graph)) - return 'incomplete_relation'; - }; - - return action; } diff --git a/modules/actions/delete_way.js b/modules/actions/delete_way.js index 1917cd765..77905ae63 100644 --- a/modules/actions/delete_way.js +++ b/modules/actions/delete_way.js @@ -39,20 +39,5 @@ export function actionDeleteWay(wayId) { }; - action.disabled = function(graph) { - var disabled = false; - - graph.parentRelations(graph.entity(wayId)).forEach(function(parent) { - var type = parent.tags.type, - role = parent.memberById(wayId).role || 'outer'; - if (type === 'route' || type === 'boundary' || (type === 'multipolygon' && role === 'outer')) { - disabled = 'part_of_relation'; - } - }); - - return disabled; - }; - - return action; } diff --git a/modules/actions/move.js b/modules/actions/move.js index 70c203f36..fd44076bc 100644 --- a/modules/actions/move.js +++ b/modules/actions/move.js @@ -286,17 +286,6 @@ export function actionMove(moveIds, tryDelta, projection, cache) { }; - action.disabled = function(graph) { - function incompleteRelation(id) { - var entity = graph.entity(id); - return entity.type === 'relation' && !entity.isComplete(graph); - } - - if (_.some(moveIds, incompleteRelation)) - return 'incomplete_relation'; - }; - - action.delta = function() { return delta; }; diff --git a/modules/actions/reflect.js b/modules/actions/reflect.js index 056932c58..6382f83c8 100644 --- a/modules/actions/reflect.js +++ b/modules/actions/reflect.js @@ -1,4 +1,3 @@ -import _ from 'lodash'; import { polygonHull as d3polygonHull, polygonCentroid as d3polygonCentroid @@ -10,17 +9,18 @@ import { geoRotate } from '../geo'; +import { utilGetAllNodes } from '../util'; + /* Reflect the given area around its axis of symmetry */ -export function actionReflect(wayId, projection) { +export function actionReflect(reflectIds, projection) { var useLongAxis = true; // http://gis.stackexchange.com/questions/22895/finding-minimum-area-rectangle-for-given-points // http://gis.stackexchange.com/questions/3739/generalisation-strategies-for-building-outlines/3756#3756 - function getSmallestSurroundingRectangle(graph, way) { - var nodes = _.uniq(graph.childNodes(way)), - points = nodes.map(function(n) { return projection(n.loc); }), + function getSmallestSurroundingRectangle(graph, nodes) { + var points = nodes.map(function(n) { return projection(n.loc); }), hull = d3polygonHull(points), centroid = d3polygonCentroid(hull), minArea = Infinity, @@ -52,14 +52,9 @@ export function actionReflect(wayId, projection) { } - var action = function (graph) { - var targetWay = graph.entity(wayId); - if (!targetWay.isArea()) { - return graph; - } - - var ssr = getSmallestSurroundingRectangle(graph, targetWay), - nodes = targetWay.nodes; + var action = function(graph) { + var nodes = utilGetAllNodes(reflectIds, graph), + ssr = getSmallestSurroundingRectangle(graph, nodes); // Choose line pq = axis of symmetry. // The shape's surrounding rectangle has 2 axes of symmetry. @@ -85,8 +80,8 @@ export function actionReflect(wayId, projection) { var dy = q[1] - p[1]; var a = (dx * dx - dy * dy) / (dx * dx + dy * dy); var b = 2 * dx * dy / (dx * dx + dy * dy); - for (var i = 0; i < nodes.length - 1; i++) { - var node = graph.entity(nodes[i]); + for (var i = 0; i < nodes.length; i++) { + var node = nodes[i]; var c = projection(node.loc); var c2 = [ a * (c[0] - p[0]) + b * (c[1] - p[1]) + p[0], diff --git a/modules/actions/rotate.js b/modules/actions/rotate.js index 8667876f7..751b998c6 100644 --- a/modules/actions/rotate.js +++ b/modules/actions/rotate.js @@ -1,15 +1,12 @@ -import _ from 'lodash'; import { geoRotate } from '../geo'; +import { utilGetAllNodes } from '../util'; +export function actionRotate(rotateIds, pivot, angle, projection) { -export function actionRotate(wayId, pivot, angle, projection) { var action = function(graph) { return graph.update(function(graph) { - var way = graph.entity(wayId); - _.uniq(way.nodes).forEach(function(id) { - var node = graph.entity(id), - point = geoRotate([projection(node.loc)], angle, pivot)[0]; - + utilGetAllNodes(rotateIds, graph).forEach(function(node) { + var point = geoRotate([projection(node.loc)], angle, pivot)[0]; graph = graph.replace(node.move(projection.invert(point))); }); }); diff --git a/modules/modes/rotate.js b/modules/modes/rotate.js index 00e0e70f3..51a8fff57 100644 --- a/modules/modes/rotate.js +++ b/modules/modes/rotate.js @@ -45,7 +45,7 @@ export function modeRotate(context, entityIDs) { ], annotation = entityIDs.length === 1 ? t('operations.rotate.annotation.' + context.geometry(entityIDs[0])) : - t('operations.move.annotation.multiple'), + t('operations.rotate.annotation.multiple'), prevGraph, prevAngle, prevTransform, diff --git a/modules/operations/delete.js b/modules/operations/delete.js index ffdcb5a27..ead998bb2 100644 --- a/modules/operations/delete.js +++ b/modules/operations/delete.js @@ -8,7 +8,9 @@ import { uiCmd } from '../ui/index'; export function operationDelete(selectedIDs, context) { - var action = actionDeleteMultiple(selectedIDs); + var multi = (selectedIDs.length === 1 ? 'single' : 'multiple'), + action = actionDeleteMultiple(selectedIDs); + var operation = function() { var annotation, @@ -67,16 +69,42 @@ export function operationDelete(selectedIDs, context) { var reason; if (_.some(selectedIDs, context.hasHiddenConnections)) { reason = 'connected_to_hidden'; + } else if (_.some(selectedIDs, protectedMember)) { + reason = 'part_of_relation'; + } else if (_.some(selectedIDs, incompleteRelation)) { + reason = 'incomplete_relation'; } - return action.disabled(context.graph()) || reason; + return reason; + + function incompleteRelation(id) { + var entity = context.entity(id); + return entity.type === 'relation' && !entity.isComplete(context.graph()); + } + + function protectedMember(id) { + var entity = context.entity(id); + if (entity.type !== 'way') return false; + + 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'; + if (type === 'route' || type === 'boundary' || (type === 'multipolygon' && role === 'outer')) { + return true; + } + } + return false; + } + }; operation.tooltip = function() { var disable = operation.disabled(); return disable ? - t('operations.delete.' + disable) : - t('operations.delete.description'); + t('operations.delete.' + disable + '.' + multi) : + t('operations.delete.description' + '.' + multi); }; diff --git a/modules/operations/move.js b/modules/operations/move.js index 2c8fb58d0..7c9a9277a 100644 --- a/modules/operations/move.js +++ b/modules/operations/move.js @@ -1,13 +1,13 @@ import _ from 'lodash'; import { t } from '../util/locale'; -import { actionMove } from '../actions/index'; import { behaviorOperation } from '../behavior/index'; import { geoExtent } from '../geo/index'; import { modeMove } from '../modes/index'; export function operationMove(selectedIDs, context) { - var extent = selectedIDs.reduce(function(extent, id) { + var multi = (selectedIDs.length === 1 ? 'single' : 'multiple'), + extent = selectedIDs.reduce(function(extent, id) { return extent.extend(context.entity(id).extent(context.graph())); }, geoExtent()); @@ -29,17 +29,23 @@ export function operationMove(selectedIDs, context) { reason = 'too_large'; } else if (_.some(selectedIDs, context.hasHiddenConnections)) { reason = 'connected_to_hidden'; + } else if (_.some(selectedIDs, incompleteRelation)) { + reason = 'incomplete_relation'; } + return reason; - return actionMove(selectedIDs).disabled(context.graph()) || reason; + function incompleteRelation(id) { + var entity = context.entity(id); + return entity.type === 'relation' && !entity.isComplete(context.graph()); + } }; operation.tooltip = function() { var disable = operation.disabled(); return disable ? - t('operations.move.' + disable) : - t('operations.move.description'); + t('operations.move.' + disable + '.' + multi) : + t('operations.move.description.' + multi); }; diff --git a/modules/operations/reflect.js b/modules/operations/reflect.js index 7656e6495..413d8226e 100644 --- a/modules/operations/reflect.js +++ b/modules/operations/reflect.js @@ -1,6 +1,8 @@ +import _ from 'lodash'; import { t } from '../util/locale'; import { actionReflect } from '../actions/index'; import { behaviorOperation } from '../behavior/index'; +import { geoExtent } from '../geo/index'; export function operationReflectShort(selectedIDs, context) { @@ -15,30 +17,39 @@ export function operationReflectLong(selectedIDs, context) { export function operationReflect(selectedIDs, context, axis) { axis = axis || 'long'; - var entityId = selectedIDs[0]; - var entity = context.entity(entityId); - var extent = entity.extent(context.graph()); - var action = actionReflect(entityId, context.projection) - .useLongAxis(Boolean(axis === 'long')); + var multi = (selectedIDs.length === 1 ? 'single' : 'multiple'), + extent = selectedIDs.reduce(function(extent, id) { + return extent.extend(context.entity(id).extent(context.graph())); + }, geoExtent()); var operation = function() { - context.perform(action, t('operations.reflect.annotation.' + axis)); + var action = actionReflect(selectedIDs, context.projection) + .useLongAxis(Boolean(axis === 'long')); + context.perform(action, t('operations.reflect.annotation.' + axis + '.' + multi)); }; operation.available = function() { - return selectedIDs.length === 1 && context.geometry(entityId) === 'area'; + return selectedIDs.length > 1 || + context.entity(selectedIDs[0]).type !== 'node'; }; operation.disabled = function() { - if (extent.percentContainedIn(context.extent()) < 0.8) { - return 'too_large'; - } else if (context.hasHiddenConnections(entityId)) { - return 'connected_to_hidden'; - } else { - return false; + var reason; + if (extent.area() && extent.percentContainedIn(context.extent()) < 0.8) { + reason = 'too_large'; + } else if (_.some(selectedIDs, context.hasHiddenConnections)) { + reason = 'connected_to_hidden'; + } else if (_.some(selectedIDs, incompleteRelation)) { + reason = 'incomplete_relation'; + } + return reason; + + function incompleteRelation(id) { + var entity = context.entity(id); + return entity.type === 'relation' && !entity.isComplete(context.graph()); } }; @@ -46,8 +57,8 @@ export function operationReflect(selectedIDs, context, axis) { operation.tooltip = function() { var disable = operation.disabled(); return disable ? - t('operations.reflect.' + disable) : - t('operations.reflect.description.' + axis); + t('operations.reflect.' + disable + '.' + multi) : + t('operations.reflect.description.' + axis + '.' + multi); }; diff --git a/modules/operations/rotate.js b/modules/operations/rotate.js index 115e8510e..78e42a9a8 100644 --- a/modules/operations/rotate.js +++ b/modules/operations/rotate.js @@ -6,7 +6,8 @@ import { modeRotate } from '../modes/index'; export function operationRotate(selectedIDs, context) { - var extent = selectedIDs.reduce(function(extent, id) { + var multi = (selectedIDs.length === 1 ? 'single' : 'multiple'), + extent = selectedIDs.reduce(function(extent, id) { return extent.extend(context.entity(id).extent(context.graph())); }, geoExtent()); @@ -35,7 +36,7 @@ export function operationRotate(selectedIDs, context) { function incompleteRelation(id) { var entity = context.entity(id); - return entity.type === 'relation' && !entity.isComplete(graph); + return entity.type === 'relation' && !entity.isComplete(context.graph()); } }; @@ -43,8 +44,8 @@ export function operationRotate(selectedIDs, context) { operation.tooltip = function() { var disable = operation.disabled(); return disable ? - t('operations.rotate.' + disable) : - t('operations.rotate.description.' + (selectedIDs.length === 1 ? 'single' : 'multiple')); + t('operations.rotate.' + disable + '.' + multi) : + t('operations.rotate.description.' + multi); }; diff --git a/test/spec/actions/delete_multiple.js b/test/spec/actions/delete_multiple.js index 9e834f1d5..4de9a077c 100644 --- a/test/spec/actions/delete_multiple.js +++ b/test/spec/actions/delete_multiple.js @@ -19,13 +19,14 @@ describe('iD.actionDeleteMultiple', function () { expect(graph.hasEntity(n.id)).to.be.undefined; }); - describe('#disabled', function () { - it('returns the result of the first action that is disabled', function () { - var node = iD.Node(), - relation = iD.Relation({members: [{id: 'w'}]}), - graph = iD.Graph([node, relation]), - action = iD.actionDeleteMultiple([node.id, relation.id]); - expect(action.disabled(graph)).to.equal('incomplete_relation'); - }); - }); + // This was moved to operationDelete. We should test operations and move this test there. + // describe('#disabled', function () { + // it('returns the result of the first action that is disabled', function () { + // var node = iD.Node(), + // relation = iD.Relation({members: [{id: 'w'}]}), + // graph = iD.Graph([node, relation]), + // action = iD.actionDeleteMultiple([node.id, relation.id]); + // expect(action.disabled(graph)).to.equal('incomplete_relation'); + // }); + // }); }); diff --git a/test/spec/actions/delete_relation.js b/test/spec/actions/delete_relation.js index 9d0b615b0..acf3fb241 100644 --- a/test/spec/actions/delete_relation.js +++ b/test/spec/actions/delete_relation.js @@ -82,12 +82,13 @@ describe('iD.actionDeleteRelation', function () { expect(graph.hasEntity(parent.id)).to.be.undefined; }); - describe('#disabled', function() { - it('returns \'incomplete_relation\' if the relation is incomplete', function() { - var relation = iD.Relation({members: [{id: 'w'}]}), - graph = iD.Graph([relation]), - action = iD.actionDeleteRelation(relation.id); - expect(action.disabled(graph)).to.equal('incomplete_relation'); - }); - }); + // This was moved to operationDelete. We should test operations and move this test there. + // describe('#disabled', function() { + // it('returns \'incomplete_relation\' if the relation is incomplete', function() { + // var relation = iD.Relation({members: [{id: 'w'}]}), + // graph = iD.Graph([relation]), + // action = iD.actionDeleteRelation(relation.id); + // expect(action.disabled(graph)).to.equal('incomplete_relation'); + // }); + // }); }); diff --git a/test/spec/actions/delete_way.js b/test/spec/actions/delete_way.js index 3bb83b88e..889a08f48 100644 --- a/test/spec/actions/delete_way.js +++ b/test/spec/actions/delete_way.js @@ -69,31 +69,32 @@ describe('iD.actionDeleteWay', function() { expect(graph.hasEntity(relation.id)).to.be.undefined; }); - describe('#disabled', function () { - it('returns \'part_of_relation\' for members of route and boundary relations', function () { - var a = iD.Way({id: 'a'}), - b = iD.Way({id: 'b'}), - route = iD.Relation({members: [{id: 'a'}], tags: {type: 'route'}}), - boundary = iD.Relation({members: [{id: 'b'}], tags: {type: 'boundary'}}), - graph = iD.Graph([a, b, route, boundary]); - expect(iD.actionDeleteWay('a').disabled(graph)).to.equal('part_of_relation'); - expect(iD.actionDeleteWay('b').disabled(graph)).to.equal('part_of_relation'); - }); + // This was moved to operationDelete. We should test operations and move this test there. + // describe('#disabled', function () { + // it('returns \'part_of_relation\' for members of route and boundary relations', function () { + // var a = iD.Way({id: 'a'}), + // b = iD.Way({id: 'b'}), + // route = iD.Relation({members: [{id: 'a'}], tags: {type: 'route'}}), + // boundary = iD.Relation({members: [{id: 'b'}], tags: {type: 'boundary'}}), + // graph = iD.Graph([a, b, route, boundary]); + // expect(iD.actionDeleteWay('a').disabled(graph)).to.equal('part_of_relation'); + // expect(iD.actionDeleteWay('b').disabled(graph)).to.equal('part_of_relation'); + // }); - it('returns \'part_of_relation\' for outer members of multipolygons', function () { - var way = iD.Way({id: 'w'}), - relation = iD.Relation({members: [{id: 'w', role: 'outer'}], tags: {type: 'multipolygon'}}), - graph = iD.Graph([way, relation]), - action = iD.actionDeleteWay(way.id); - expect(action.disabled(graph)).to.equal('part_of_relation'); - }); + // it('returns \'part_of_relation\' for outer members of multipolygons', function () { + // var way = iD.Way({id: 'w'}), + // relation = iD.Relation({members: [{id: 'w', role: 'outer'}], tags: {type: 'multipolygon'}}), + // graph = iD.Graph([way, relation]), + // action = iD.actionDeleteWay(way.id); + // expect(action.disabled(graph)).to.equal('part_of_relation'); + // }); - it('returns falsy for inner members of multipolygons', function () { - var way = iD.Way({id: 'w'}), - relation = iD.Relation({members: [{id: 'w', role: 'inner'}], tags: {type: 'multipolygon'}}), - graph = iD.Graph([way, relation]), - action = iD.actionDeleteWay(way.id); - expect(action.disabled(graph)).not.ok; - }); - }); + // it('returns falsy for inner members of multipolygons', function () { + // var way = iD.Way({id: 'w'}), + // relation = iD.Relation({members: [{id: 'w', role: 'inner'}], tags: {type: 'multipolygon'}}), + // graph = iD.Graph([way, relation]), + // action = iD.actionDeleteWay(way.id); + // expect(action.disabled(graph)).not.ok; + // }); + // }); }); diff --git a/test/spec/actions/move.js b/test/spec/actions/move.js index 15230424a..554a44d35 100644 --- a/test/spec/actions/move.js +++ b/test/spec/actions/move.js @@ -1,29 +1,30 @@ describe('iD.actionMove', function() { var projection = d3.geoMercator().scale(250 / Math.PI); - describe('#disabled', function() { - it('returns falsy by default', function() { - var node = iD.Node({loc: [0, 0]}), - action = iD.actionMove([node.id], [0, 0], projection), - graph = iD.Graph([node]); - expect(action.disabled(graph)).not.to.be.ok; - }); + // This was moved to operationMove. We should test operations and move this test there. + // describe('#disabled', function() { + // it('returns falsy by default', function() { + // var node = iD.Node({loc: [0, 0]}), + // action = iD.actionMove([node.id], [0, 0], projection), + // graph = iD.Graph([node]); + // expect(action.disabled(graph)).not.to.be.ok; + // }); - it('returns \'incomplete_relation\' for an incomplete relation', function() { - var relation = iD.Relation({members: [{id: 1}]}), - action = iD.actionMove([relation.id], [0, 0], projection), - graph = iD.Graph([relation]); - expect(action.disabled(graph)).to.equal('incomplete_relation'); - }); + // it('returns \'incomplete_relation\' for an incomplete relation', function() { + // var relation = iD.Relation({members: [{id: 1}]}), + // action = iD.actionMove([relation.id], [0, 0], projection), + // graph = iD.Graph([relation]); + // expect(action.disabled(graph)).to.equal('incomplete_relation'); + // }); - it('returns falsy for a complete relation', function() { - var node = iD.Node({loc: [0, 0]}), - relation = iD.Relation({members: [{id: node.id}]}), - action = iD.actionMove([relation.id], [0, 0], projection), - graph = iD.Graph([node, relation]); - expect(action.disabled(graph)).not.to.be.ok; - }); - }); + // it('returns falsy for a complete relation', function() { + // var node = iD.Node({loc: [0, 0]}), + // relation = iD.Relation({members: [{id: node.id}]}), + // action = iD.actionMove([relation.id], [0, 0], projection), + // graph = iD.Graph([node, relation]); + // expect(action.disabled(graph)).not.to.be.ok; + // }); + // }); it('moves all nodes in a way by the given amount', function() { var node1 = iD.Node({loc: [0, 0]}), diff --git a/test/spec/actions/reflect.js b/test/spec/actions/reflect.js index aaf1b022d..1a8b22061 100644 --- a/test/spec/actions/reflect.js +++ b/test/spec/actions/reflect.js @@ -2,19 +2,6 @@ describe('iD.actionReflect', function() { var projection = d3.geoMercator(); it('does not create or remove nodes', function () { - var graph = iD.Graph([ - iD.Node({id: 'a', loc: [0, 0]}), - iD.Node({id: 'b', loc: [4, 0]}), - iD.Node({id: 'c', loc: [4, 2]}), - iD.Node({id: 'd', loc: [1, 2]}), - iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a'], tags: { area: 'yes'}}) - ]); - graph = iD.actionReflect('-', projection)(graph); - expect(graph.entity('-').nodes).to.have.length(5); - }); - - - it('only operates on areas', function () { var graph = iD.Graph([ iD.Node({id: 'a', loc: [0, 0]}), iD.Node({id: 'b', loc: [4, 0]}), @@ -22,8 +9,8 @@ describe('iD.actionReflect', function() { iD.Node({id: 'd', loc: [1, 2]}), iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}) ]); - var graph2 = iD.actionReflect('-', projection)(graph); - expect(graph2).to.deep.equal(graph); + graph = iD.actionReflect(['-'], projection)(graph); + expect(graph.entity('-').nodes).to.have.length(5); }); @@ -38,9 +25,9 @@ describe('iD.actionReflect', function() { iD.Node({id: 'b', loc: [4, 0]}), iD.Node({id: 'c', loc: [4, 2]}), iD.Node({id: 'd', loc: [1, 2]}), - iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a'], tags: { area: 'yes'}}) + iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}) ]); - graph = iD.actionReflect('-', projection)(graph); + graph = iD.actionReflect(['-'], projection)(graph); expect(graph.entity('a').loc[0]).to.be.closeTo(0, 1e-6); expect(graph.entity('a').loc[1]).to.be.closeTo(2, 1e-6); expect(graph.entity('b').loc[0]).to.be.closeTo(4, 1e-6); @@ -63,9 +50,9 @@ describe('iD.actionReflect', function() { iD.Node({id: 'b', loc: [4, 0]}), iD.Node({id: 'c', loc: [4, 2]}), iD.Node({id: 'd', loc: [1, 2]}), - iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a'], tags: { area: 'yes'}}) + iD.Way({id: '-', nodes: ['a', 'b', 'c', 'd', 'a']}) ]); - graph = iD.actionReflect('-', projection).useLongAxis(false)(graph); + graph = iD.actionReflect(['-'], projection).useLongAxis(false)(graph); expect(graph.entity('a').loc[0]).to.be.closeTo(4, 1e-6); expect(graph.entity('a').loc[1]).to.be.closeTo(0, 1e-6); expect(graph.entity('b').loc[0]).to.be.closeTo(0, 1e-6);