From 651ec363f67854e9edcc2d55130cf6ae3293ebaa Mon Sep 17 00:00:00 2001 From: Bryan Housel Date: Wed, 3 May 2017 16:08:23 -0400 Subject: [PATCH] Undo/Redo while drawing line/area should keep the user in drawing mode (closes #3530) --- modules/behavior/draw_way.js | 192 ++++++++++++++++++++--------------- modules/modes/add_area.js | 12 +-- modules/modes/add_line.js | 12 +-- modules/modes/draw_area.js | 4 +- modules/modes/draw_line.js | 4 +- 5 files changed, 127 insertions(+), 97 deletions(-) diff --git a/modules/behavior/draw_way.js b/modules/behavior/draw_way.js index f8b2c1414..b5ab9d5b0 100644 --- a/modules/behavior/draw_way.js +++ b/modules/behavior/draw_way.js @@ -4,71 +4,54 @@ import { t } from '../util/locale'; import { actionAddEntity, actionAddMidpoint, - actionAddVertex, - actionMoveNode -} from '../actions/index'; + actionMoveNode, + actionNoop +} from '../actions'; -import { - modeBrowse, - modeSelect -} from '../modes/index'; - -import { - osmNode, - osmWay -} from '../osm/index'; - -import { - geoChooseEdge, - geoEdgeEqual -} from '../geo/index'; - -import { - behaviorDraw -} from './draw'; - -import { - utilEntitySelector, - utilFunctor -} from '../util/index'; +import { behaviorDraw } from './draw'; +import { geoChooseEdge, geoEdgeEqual } from '../geo'; +import { modeBrowse, modeSelect } from '../modes'; +import { osmNode, osmWay } from '../osm'; +import { utilEntitySelector } from '../util'; -export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { +export function behaviorDrawWay(context, wayId, index, mode, startGraph) { - var way = context.entity(wayId), + var origWay = context.entity(wayId), isArea = context.geometry(wayId) === 'area', - finished = false, - annotation = t((way.isDegenerate() ? + tempEdits = 0, + annotation = t((origWay.isDegenerate() ? 'operations.start.annotation.' : 'operations.continue.annotation.') + context.geometry(wayId)), draw = behaviorDraw(context), - startIndex, start, end, segment; + startIndex, + start, + end, + segment; + // initialize the temporary drawing entities if (!isArea) { - startIndex = typeof index === 'undefined' ? way.nodes.length - 1 : 0; - start = osmNode({ id: 'nStart', loc: context.entity(way.nodes[startIndex]).loc }); + startIndex = typeof index === 'undefined' ? origWay.nodes.length - 1 : 0; + start = osmNode({ id: 'nStart', loc: context.entity(origWay.nodes[startIndex]).loc }); end = osmNode({ id: 'nEnd', loc: context.map().mouseCoordinates() }); segment = osmWay({ id: 'wTemp', nodes: typeof index === 'undefined' ? [start.id, end.id] : [end.id, start.id], - tags: _.clone(way.tags) + tags: _.clone(origWay.tags) }); } else { end = osmNode({ loc: context.map().mouseCoordinates() }); } + // Push an annotated state for undo to return back to. + // We must make sure to remove this edit later. + context.perform(actionNoop(), annotation); + tempEdits++; - var fn = context[way.isDegenerate() ? 'replace' : 'perform']; - if (isArea) { - fn(actionAddEntity(end), - actionAddVertex(wayId, end.id) - ); - } else { - fn(actionAddEntity(start), - actionAddEntity(end), - actionAddEntity(segment) - ); - } + // Add the temporary drawing entities to the graph. + // We must make sure to remove this edit later. + context.perform(AddDrawEntities()); + tempEdits++; function move(datum) { @@ -77,7 +60,7 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { if (datum.type === 'node' && datum.id !== end.id) { loc = datum.loc; - } else if (datum.type === 'way') { // && (segment || datum.id !== segment.id)) { + } else if (datum.type === 'way') { var dims = context.map().dimensions(), mouse = context.mouse(), pad = 5, @@ -94,12 +77,21 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { } context.replace(actionMoveNode(end.id, loc)); + end = context.entity(end.id); } function undone() { - finished = true; - context.enter(modeBrowse(context)); + // Undo popped the history back to the initial annotated no-op edit. + // Remove initial no-op edit and whatever edit happened immediately before it. + context.pop(2); + tempEdits = 0; + + if (context.hasEntity(wayId)) { + context.enter(mode); + } else { + context.enter(modeBrowse(context)); + } } @@ -133,8 +125,8 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { drawWay.off = function(surface) { - if (!finished) - context.pop(); + context.pop(tempEdits); + tempEdits = 0; context.map() .on('drawn.draw', null); @@ -148,15 +140,40 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { }; - function ReplaceTemporaryNode(newNode) { + function AddDrawEntities() { return function(graph) { if (isArea) { + // For area drawing, there is no need for a temporary node. + // `end` gets inserted into the way as the penultimate node. return graph - .replace(way.addNode(newNode.id)) + .replace(end) + .replace(origWay.addNode(end.id)); + } else { + // For line drawing, add a temporary start, end, and segment to the graph. + // This allows us to class the new segment as `active`, but still + // connect it back to parts of the way that have already been drawn. + return graph + .replace(start) + .replace(end) + .replace(segment); + } + }; + } + + + function ReplaceDrawEntities(newNode) { + return function(graph) { + if (isArea) { + // For area drawing, we didn't create a temporary node. + // `newNode` gets inserted into the _original_ way as the penultimate node. + return graph + .replace(origWay.addNode(newNode.id)) .remove(end); } else { + // For line drawing, add the `newNode` to the way at specified index, + // and remove the temporary start, end, and segment. return graph - .replace(graph.entity(wayId).addNode(newNode.id, index)) + .replace(origWay.addNode(newNode.id, index)) .remove(end) .remove(segment) .remove(start); @@ -168,54 +185,60 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { // Accept the current position of the temporary node and continue drawing. drawWay.add = function(loc) { // prevent duplicate nodes - var last = context.hasEntity(way.nodes[way.nodes.length - (isArea ? 2 : 1)]); + var last = context.hasEntity(origWay.nodes[origWay.nodes.length - (isArea ? 2 : 1)]); if (last && last.loc[0] === loc[0] && last.loc[1] === loc[1]) return; + context.pop(tempEdits); + if (isArea) { - context.replace( - actionMoveNode(end.id, loc), + context.perform( + AddDrawEntities(), annotation ); } else { var newNode = osmNode({loc: loc}); - context.replace( + context.perform( actionAddEntity(newNode), - ReplaceTemporaryNode(newNode), + ReplaceDrawEntities(newNode), annotation ); } - finished = true; + tempEdits = 0; context.enter(mode); }; // Connect the way to an existing way. drawWay.addWay = function(loc, edge) { - if (isArea) { + context.pop(tempEdits); + context.perform( + AddDrawEntities(), actionAddMidpoint({ loc: loc, edge: edge}, end), annotation ); } else { var previousEdge = startIndex ? - [way.nodes[startIndex], way.nodes[startIndex - 1]] : - [way.nodes[0], way.nodes[1]]; + [origWay.nodes[startIndex], origWay.nodes[startIndex - 1]] : + [origWay.nodes[0], origWay.nodes[1]]; // Avoid creating duplicate segments if (geoEdgeEqual(edge, previousEdge)) return; + context.pop(tempEdits); + var newNode = osmNode({ loc: loc }); context.perform( actionAddMidpoint({ loc: loc, edge: edge}, newNode), - ReplaceTemporaryNode(newNode), + ReplaceDrawEntities(newNode), annotation ); } - finished = true; + tempEdits = 0; context.enter(mode); }; @@ -223,47 +246,54 @@ export function behaviorDrawWay(context, wayId, index, mode, baseGraph) { // Connect the way to an existing node and continue drawing. drawWay.addNode = function(node) { // Avoid creating duplicate segments - if (way.areAdjacent(node.id, way.nodes[way.nodes.length - 1])) return; + if (origWay.areAdjacent(node.id, origWay.nodes[origWay.nodes.length - 1])) return; + + context.pop(tempEdits); context.perform( - ReplaceTemporaryNode(node), + ReplaceDrawEntities(node), annotation ); - finished = true; + tempEdits = 0; context.enter(mode); }; - // Finish the draw operation, removing the temporary node. If the way has enough - // nodes to be valid, it's selected. Otherwise, return to browse mode. + // Finish the draw operation, removing the temporary edits. + // If the way has enough nodes to be valid, it's selected. + // Otherwise, delete everything and return to browse mode. drawWay.finish = function() { - context.pop(); - finished = true; + context.pop(tempEdits); + tempEdits = 0; + + var way = context.hasEntity(wayId); + if (!way || origWay.isDegenerate()) { + drawWay.cancel(); + return; + } window.setTimeout(function() { context.map().dblclickEnable(true); }, 1000); - if (context.hasEntity(wayId)) { - context.enter(modeSelect(context, [wayId]).newFeature(true)); - } else { - context.enter(modeBrowse(context)); - } + context.enter(modeSelect(context, [wayId]).newFeature(true)); }; - // Cancel the draw operation and return to browse, deleting everything drawn. + // Cancel the draw operation, delete everything, and return to browse mode. drawWay.cancel = function() { - context.perform( - utilFunctor(baseGraph), - t('operations.cancel_draw.annotation')); + context.pop(tempEdits); + tempEdits = 0; + + while (context.graph() !== startGraph) { + context.pop(); + } window.setTimeout(function() { context.map().dblclickEnable(true); }, 1000); - finished = true; context.enter(modeBrowse(context)); }; diff --git a/modules/modes/add_area.js b/modules/modes/add_area.js index f36ebef16..2160a8b97 100644 --- a/modules/modes/add_area.js +++ b/modules/modes/add_area.js @@ -35,7 +35,7 @@ export function modeAddArea(context) { function start(loc) { - var graph = context.graph(), + var startGraph = context.graph(), node = osmNode({ loc: loc }), way = osmWay({ tags: defaultTags }); @@ -46,12 +46,12 @@ export function modeAddArea(context) { actionClose(way.id) ); - context.enter(modeDrawArea(context, way.id, graph)); + context.enter(modeDrawArea(context, way.id, startGraph)); } function startFromWay(loc, edge) { - var graph = context.graph(), + var startGraph = context.graph(), node = osmNode({ loc: loc }), way = osmWay({ tags: defaultTags }); @@ -63,12 +63,12 @@ export function modeAddArea(context) { actionAddMidpoint({ loc: loc, edge: edge }, node) ); - context.enter(modeDrawArea(context, way.id, graph)); + context.enter(modeDrawArea(context, way.id, startGraph)); } function startFromNode(node) { - var graph = context.graph(), + var startGraph = context.graph(), way = osmWay({ tags: defaultTags }); context.perform( @@ -77,7 +77,7 @@ export function modeAddArea(context) { actionClose(way.id) ); - context.enter(modeDrawArea(context, way.id, graph)); + context.enter(modeDrawArea(context, way.id, startGraph)); } diff --git a/modules/modes/add_line.js b/modules/modes/add_line.js index 9faf30f4b..64bd36382 100644 --- a/modules/modes/add_line.js +++ b/modules/modes/add_line.js @@ -27,7 +27,7 @@ export function modeAddLine(context) { function start(loc) { - var baseGraph = context.graph(), + var startGraph = context.graph(), node = osmNode({ loc: loc }), way = osmWay(); @@ -37,12 +37,12 @@ export function modeAddLine(context) { actionAddVertex(way.id, node.id) ); - context.enter(modeDrawLine(context, way.id, baseGraph)); + context.enter(modeDrawLine(context, way.id, startGraph)); } function startFromWay(loc, edge) { - var baseGraph = context.graph(), + var startGraph = context.graph(), node = osmNode({ loc: loc }), way = osmWay(); @@ -53,12 +53,12 @@ export function modeAddLine(context) { actionAddMidpoint({ loc: loc, edge: edge }, node) ); - context.enter(modeDrawLine(context, way.id, baseGraph)); + context.enter(modeDrawLine(context, way.id, startGraph)); } function startFromNode(node) { - var baseGraph = context.graph(), + var startGraph = context.graph(), way = osmWay(); context.perform( @@ -66,7 +66,7 @@ export function modeAddLine(context) { actionAddVertex(way.id, node.id) ); - context.enter(modeDrawLine(context, way.id, baseGraph)); + context.enter(modeDrawLine(context, way.id, startGraph)); } diff --git a/modules/modes/draw_area.js b/modules/modes/draw_area.js index 643ec9afe..3eb062e37 100644 --- a/modules/modes/draw_area.js +++ b/modules/modes/draw_area.js @@ -1,7 +1,7 @@ import { t } from '../util/locale'; import { behaviorDrawWay } from '../behavior/index'; -export function modeDrawArea(context, wayId, baseGraph) { +export function modeDrawArea(context, wayId, startGraph) { var mode = { button: 'area', id: 'draw-area' @@ -13,7 +13,7 @@ export function modeDrawArea(context, wayId, baseGraph) { mode.enter = function() { var way = context.entity(wayId); - behavior = behaviorDrawWay(context, wayId, undefined, mode, baseGraph) + behavior = behaviorDrawWay(context, wayId, undefined, mode, startGraph) .tail(t('modes.draw_area.tail')); var addNode = behavior.addNode; diff --git a/modules/modes/draw_line.js b/modules/modes/draw_line.js index 27058594a..c4fd68268 100644 --- a/modules/modes/draw_line.js +++ b/modules/modes/draw_line.js @@ -1,7 +1,7 @@ import { t } from '../util/locale'; import { behaviorDrawWay } from '../behavior/index'; -export function modeDrawLine(context, wayId, baseGraph, affix) { +export function modeDrawLine(context, wayId, startGraph, affix) { var mode = { button: 'line', id: 'draw-line' @@ -15,7 +15,7 @@ export function modeDrawLine(context, wayId, baseGraph, affix) { index = (affix === 'prefix') ? 0 : undefined, headId = (affix === 'prefix') ? way.first() : way.last(); - behavior = behaviorDrawWay(context, wayId, index, mode, baseGraph) + behavior = behaviorDrawWay(context, wayId, index, mode, startGraph) .tail(t('modes.draw_line.tail')); var addNode = behavior.addNode;