diff --git a/js/iD/actions/AddNodeToWayAction.js b/js/iD/actions/AddNodeToWayAction.js index 0be001040..441f2dd37 100644 --- a/js/iD/actions/AddNodeToWayAction.js +++ b/js/iD/actions/AddNodeToWayAction.js @@ -1,6 +1,6 @@ // iD/actions/AddNodeToWayAction.js -define(['dojo/_base/declare','iD/actions/UndoableEntityAction'], function(declare){ +define(['dojo/_base/declare','iD/actions/UndoableAction'], function(declare){ // ---------------------------------------------------------------------- // AddNodeToWayAction class @@ -13,12 +13,13 @@ declare("iD.actions.AddNodeToWayAction", [iD.actions.UndoableEntityAction], { firstNode: null, autoDelete: true, - constructor:function(_way, _node, _nodeList, _index, _autoDelete) { - this.entity = _way; - this.node = _node; - this.nodeList = _nodeList; - this.index = _index; - this.autoDelete = _autoDelete; + constructor:function(way, node, nodeList, index, autoDelete) { + // summary: Add a node to a way at a specified index, or -1 for the end of the way. + this.entity = way; + this.node = node; + this.nodeList = nodeList; + this.index = index; + this.autoDelete = autoDelete; }, doAction:function() { @@ -44,6 +45,8 @@ declare("iD.actions.AddNodeToWayAction", [iD.actions.UndoableEntityAction], { }, undoAction:function() { + // summary: Remove the added node. Fixme: if the way is now 1-length, we should + // do something like deleting it and converting the remaining node to a POI. var way=this.entity; // shorthand if (this.autoDelete && way.length()==2 && way.parentRelations().length()) return this.FAIL; @@ -53,8 +56,6 @@ declare("iD.actions.AddNodeToWayAction", [iD.actions.UndoableEntityAction], { this.markClean(); way.refresh(); - // ** if the way is now 1-length, we should do something like deleting it and - // converting the remaining node to a POI (see P2) return this.SUCCESS; }, }); diff --git a/js/iD/actions/CreateEntityAction.js b/js/iD/actions/CreateEntityAction.js index c8f721437..be8701101 100644 --- a/js/iD/actions/CreateEntityAction.js +++ b/js/iD/actions/CreateEntityAction.js @@ -10,16 +10,15 @@ declare("iD.actions.CreateEntityAction", [iD.actions.UndoableEntityAction], { setCreate:null, deleteAction:null, - // When undo is called, instead of simply removing the entity, we work through - // to make a Delete[Entity]Action, call that, and store it for later - // Then, when this action is called again (i.e. a redo), instead of creating yet another entity, we call the deleteAction.undoAction - constructor:function(entity,setCreate) { + // summary: Create a new entity - way, node or relation. this.setCreate = setCreate; this.setName("Create "+entity.entityType); }, doAction:function() { + // summary: Call out to the specified method (in the Controller) to create the entity. + // See undoAction for explanation of special redo handling. if (this.deleteAction!=null) { this.deleteAction.undoAction(); // redo } else { @@ -30,8 +29,10 @@ declare("iD.actions.CreateEntityAction", [iD.actions.UndoableEntityAction], { }, undoAction:function() { - // if the undo is called for the first time, call for a deletion, and (via setAction) store the - // deletion action for later. We'll undo the deletion if we get asked to redo this action + // summary: Special handling for undoing a create. When undo is called, instead + // of simply removing the entity, we work through to make a Delete[Entity]Action, + // call that, and store it for later. Then, when this action is called again + // (i.e. a redo), instead of creating yet another entity, we call the deleteAction.undoAction. if (this.deleteAction==null) { this.entity.remove(this.setAction); } this.deleteAction.doAction(); this.markClean(); @@ -39,6 +40,7 @@ declare("iD.actions.CreateEntityAction", [iD.actions.UndoableEntityAction], { }, setAction:function(action) { + // summary: Set the associated delete action (see undoAction for explanation). deleteAction = action; }, diff --git a/js/iD/actions/CreatePOIAction.js b/js/iD/actions/CreatePOIAction.js index 88db731a7..58e8ff063 100644 --- a/js/iD/actions/CreatePOIAction.js +++ b/js/iD/actions/CreatePOIAction.js @@ -14,6 +14,8 @@ declare("iD.actions.CreatePOIAction", [iD.actions.CompositeUndoableAction], { connection: null, constructor:function(connection,tags,lat,lon) { + // summary: Create a new node and set it as a POI. Used by drag-and-drop. Note that the + // node is remembered, so that on redo we can just reinstate it. this.setName("Create POI"); this.connection = connection; this.tags = tags; diff --git a/js/iD/actions/MoveNodeAction.js b/js/iD/actions/MoveNodeAction.js index c42591ffc..d5baa9fa3 100644 --- a/js/iD/actions/MoveNodeAction.js +++ b/js/iD/actions/MoveNodeAction.js @@ -15,6 +15,7 @@ declare("iD.actions.MoveNodeAction", [iD.actions.UndoableEntityAction], { setLatLon: null, constructor:function(node, newLat, newLon, setLatLon) { + // summary: Move a node to a new position. this.entity = node; this.newLat = newLat; this.newLon = newLon; diff --git a/js/iD/actions/UndoStack.js b/js/iD/actions/UndoStack.js index 107b32c1d..6ad169a99 100644 --- a/js/iD/actions/UndoStack.js +++ b/js/iD/actions/UndoStack.js @@ -1,4 +1,5 @@ // iD/actions/UndoStack.js +// ** FIXME: a couple of AS3-isms in undoIfAction/removeLastIfAction define(['dojo/_base/declare'], function(declare){ @@ -15,11 +16,15 @@ declare("iD.actions.UndoStack", null, { NO_CHANGE: 2, constructor:function() { + // summary: An undo stack. There can be any number of these, but almost all operations will + // take place on the global undo stack - implemented as a singleton-like property of + // the Controller. this.undoActions=[]; this.redoActions=[]; }, addAction:function(_action) { + // summary: Do an action, and add it to the undo stack if it succeeded. var result = _action.doAction(); switch (result) { case this.FAIL: @@ -46,26 +51,31 @@ declare("iD.actions.UndoStack", null, { }, breakUndo:function() { + // summary: Wipe the undo stack - typically used after saving. this.undoActions = []; this.redoActions = []; }, canUndo:function() { + // summary: Are there any items on the undo stack? return this.undoActions.length > 0; }, canRedo:function() { + // summary: Are there any redoable actions? return this.redoActions.length > 0; }, - // Undo the most recent action, and add it to the top of the redo stack undo:function() { + // summary: Undo the most recent action, and add it to the top of the redo stack. if (!this.undoActions.length) { return; } var action = undoActions.pop(); action.undoAction(); redoActions.push(action); }, undoIfAction:function(_action) { + // summary: Undo the most recent action _only_ if it was of a certain type. + // Fixme: isInstanceOf needs to be made into JavaScript. if (!this.undoActions.length) { return; } if (this.undoActions[this.undoActions.length-1].isInstanceOf(_action)) { this.undo(); @@ -74,12 +84,15 @@ declare("iD.actions.UndoStack", null, { return false; }, removeLastIfAction:function(_action) { + // summary: Remove the most recent action from the stack _only_ if it was of a certain type. + // Fixme: isInstanceOf needs to be made into JavaScript. if (this.undoActions.length && this.undoActions[this.undoActions.length-1].isInstanceOf(_action)) { this.undoActions.pop(); } }, getUndoDescription:function() { + // summary: Get the name of the topmost item on the undo stack. if (this.undoActions.length==0) return null; if (this.undoActions[this.undoActions.length-1].name) { return this.undoActions[this.undoActions.length-1].name; @@ -88,6 +101,7 @@ declare("iD.actions.UndoStack", null, { }, getRedoDescription:function() { + // summary: Get the name of the topmost item on the redo stack. if (this.redoActions.length==0) return null; if (this.redoActions[this.redoActions.length-1].name) { return this.redoActions[this.redoActions.length-1].name; @@ -95,8 +109,8 @@ declare("iD.actions.UndoStack", null, { return null; }, - // Takes the action most recently undone, does it, and adds it to the undo stack redo:function() { + // summary: Takes the action most recently undone, does it, and adds it to the undo stack. if (!this.redoActions.length) { return; } var action = this.redoActions.pop(); action.doAction(); diff --git a/js/iD/actions/UndoableAction.js b/js/iD/actions/UndoableAction.js index 8418b99b1..b83176a8b 100644 --- a/js/iD/actions/UndoableAction.js +++ b/js/iD/actions/UndoableAction.js @@ -15,21 +15,31 @@ declare("iD.actions.UndoableAction", null, { name: "", constructor:function() { + // summary: An UndoableAction is a user-induced change to the map data (held within the Connection) + // which can be undone. The UndoableAction doesn't actually change the data: it provides + // the logic around the change, but the change itself is generally made by the model. + // Therefore the UndoableAction constructor needs to be passed a reference + // to a method that actually makes the change - for example, MoveNodeAction will be + // passed a reference to Node._setLatLonImmediate. }, doAction:function() { + // summary: Do the action. return FAIL; }, undoAction:function() { + // summary: Undo the action. return FAIL; }, mergePrevious:function() { + // summary: If two successive actions can be merged (in particular, two successive node moves), do that. return false; }, setName:function(_name) { + // summary: Set the name of an action. For UI and debugging purposes. this.name=_name; }, @@ -46,33 +56,42 @@ declare("iD.actions.UndoableEntityAction", [iD.actions.UndoableAction], { entity: null, constructor:function(_entity) { + // summary: An UndoableEntityAction is a user-induced change to the map data + // (held within the Connection) which can be undone, with a reference + // to a particular entity (to which the change was made). this.entity=_entity; }, markDirty:function() { - if (!this.initialised) { this.init(); } - if (!this.wasDirty) { this.entity._markDirty(); } -// if (!this.connectionWasDirty ) { this.entity.connection.markDirty(); } + // summary: Mark a change to the entity ('dirtying' it). + if (!this.initialised) this.init(); + if (!this.wasDirty) this.entity._markDirty(); + if (!this.connectionWasDirty) this.entity.connection.markDirty(); }, markClean:function() { - if (!this.initialised) { this.init(); } - if (!this.wasDirty) { this.entity._markClean(); } -// if (!connectionWasDirty) { this.entity.connection.markClean(); } + // summary: If the entity was clean before, revert the dirty flag to that state. + if (!this.initialised) this.init(); + if (!this.wasDirty) this.entity._markClean(); + if (!this.connectionWasDirty) this.entity.connection.markClean(); }, - // Record whether or not the entity and connection were clean before this action started init:function() { - this.wasDirty = this.entity.isDirty; -// this.connectionWasDirty = this.entity.connection.isDirty; + // summary: Record whether or not the entity and connection were clean before this action started + this.wasDirty = this.entity.isDirty(); + this.connectionWasDirty = this.entity.connection.isDirty(); this.initialised = true; }, + toString:function() { + return this.name + " " + this.entity.entityType + " " + this.entity.id; + }, + }); // ---------------------------------------------------------------------- -// UndoableEntityAction class +// CompositeUndoableAction class declare("iD.actions.CompositeUndoableAction", [iD.actions.UndoableAction], { @@ -80,18 +99,24 @@ declare("iD.actions.CompositeUndoableAction", [iD.actions.UndoableAction], { actionsDone: false, constructor:function() { + // summary: A CompositeUndoableAction is a group of user-induced changes to the map data + // (held within the Connection) which are executed, and therefore undone, + // in a batch. For example, creating a new node and a new way containing it. this.actions=[]; }, push:function(action) { + // summary: Add a single action to the list of actions comprising this CompositeUndoableAction. this.actions.push(action); }, clearActions:function() { + // summary: Clear the list of actions. this.actions=[]; }, doAction:function() { + // summary: Execute all the actions one-by-one as a transaction (i.e. roll them all back if one fails). if (this.actionsDone) { return this.FAIL; } var somethingDone=false; for (var i=0; i=0; i--) { this.actions[i].undoAction(); }