diff --git a/spec/wrapper-spec.js b/spec/wrapper-spec.js index 9f6860b10..9d4364417 100644 --- a/spec/wrapper-spec.js +++ b/spec/wrapper-spec.js @@ -3,14 +3,14 @@ var mockery = require('mockery'); mockery.enable(); -mockery.registerAllowables(['../src/js/converter/declarations.js', './wrapper.js', 'console', './utils.js', './domutils.js', 'console', '../node_modules/mensch', 'fs', 'path', 'mkdirp']); +mockery.registerAllowables(['../src/js/converter/declarations.js', './wrapper.js', 'console', './utils.js', './domutils.js', 'console', 'mensch', 'fs', 'path', 'mkdirp']); var main = require('../src/js/converter/main.js'); var fs = require('fs'); -mockery.registerMock('knockout', require('../node_modules/knockout')); +mockery.registerMock('knockout', require('knockout')); // mockery.registerMock('knockout.wrap', require('../node_modules/knockout.wrap/knockout.wrap.js')); -mockery.registerMock('knockoutjs-reactor', require('../node_modules/ko-reactor/dist/ko-reactor.js')); +mockery.registerMock('knockoutjs-reactor', require('ko-reactor/dist/ko-reactor.js')); // mockery.registerMock('knockout-undomanager', require('../node_modules/knockout-undomanager/knockout-undomanager.js')); var undoserializer = require("../src/js/undomanager/undoserializer.js"); @@ -75,9 +75,8 @@ describe('model wrapper and undomanager', function() { content().mainBlocks().blocks.push(titleBlock); - /* This is not supported in the current code var unwrapped = ko.utils.parseJson(jsonContent); - content(unwrapped); + content._wrap(unwrapped); expect(content().titleText()).toEqual("TITLE"); @@ -87,7 +86,23 @@ describe('model wrapper and undomanager', function() { undoRedoStack.undoCommand.execute(); + // This requires the "sync" option (async: false) of ko-reactor expect(content().titleText()).toEqual("TITLE"); + + undoRedoStack.redoCommand.execute(); + + expect(content().titleText()).toEqual("New Title 2"); + + var unwrapped = ko.utils.parseJson(jsonContent); + content._wrap(unwrapped); + + expect(content().titleText()).toEqual("TITLE"); + + // Fails on current code because the "undo" of a full content _wrap doesn't wrap the previous model + /* + undoRedoStack.undoCommand.execute(); + + expect(content().titleText()).toEqual("New Title 2"); */ }); @@ -128,12 +143,12 @@ describe('model wrapper and undomanager', function() { var debug = function(prefix, blocks) { for (var i = 0; i < blocks().length; i++) { - console.log(prefix, i, blocks()[i]().text()); + // console.log(prefix, i, blocks()[i]().text()); } }; undoserializer.setListener(function(path, child, oldVal, item) { - console.log("UL:", path, oldVal, item.status, item.index, item.moved, item.value.text); + // console.log("UL:", path, oldVal, item.status, item.index, item.moved, item.value.text); }); debug("A", blocks); @@ -141,7 +156,7 @@ describe('model wrapper and undomanager', function() { blocks.subscribe(function (changes) { var ch = ko.toJS(changes); for (var i = 0; i < ch.length; i++) { - console.log("AC", i, ch[i].status, ch[i].index, ch[i].moved, ch[i].value.text); + // console.log("AC", i, ch[i].status, ch[i].index, ch[i].moved, ch[i].value.text); } }, undefined, 'arrayChange'); @@ -156,6 +171,7 @@ describe('model wrapper and undomanager', function() { debug("B", blocks); expect(blocks()[0]().text()).toEqual("Title 3"); + expect(blocks()[1]().text()).toEqual("Title 1"); expect(blocks()[2]().text()).toEqual("Title 2"); undoRedoStack.undoCommand.execute(); @@ -163,9 +179,9 @@ describe('model wrapper and undomanager', function() { debug("C", blocks); expect(blocks()[0]().text()).toEqual("Title 1"); + expect(blocks()[1]().text()).toEqual("Title 2"); expect(blocks()[2]().text()).toEqual("Title 3"); - /* This is not supported in the current code // using "move action" (sortable with move strategy will use valueWillMute/hasMutated blocks.valueWillMutate(); @@ -177,6 +193,7 @@ describe('model wrapper and undomanager', function() { debug("D", blocks); expect(blocks()[0]().text()).toEqual("Title 3"); + expect(blocks()[1]().text()).toEqual("Title 1"); expect(blocks()[2]().text()).toEqual("Title 2"); undoRedoStack.undoCommand.execute(); @@ -184,8 +201,37 @@ describe('model wrapper and undomanager', function() { debug("E", blocks); expect(blocks()[0]().text()).toEqual("Title 1"); + expect(blocks()[1]().text()).toEqual("Title 2"); expect(blocks()[2]().text()).toEqual("Title 3"); - */ + + // using "move action" (sortable with move strategy will use valueWillMute/hasMutated + blocks.valueWillMutate(); + var underlyingBlocks = ko.utils.unwrapObservable(blocks); + var removed2 = underlyingBlocks.splice(0, 1); + underlyingBlocks.splice(2, 0, removed2[0]); + blocks.valueHasMutated(); + + debug("F", blocks); + + expect(blocks()[0]().text()).toEqual("Title 2"); + expect(blocks()[1]().text()).toEqual("Title 3"); + expect(blocks()[2]().text()).toEqual("Title 1"); + + undoRedoStack.undoCommand.execute(); + + debug("G", blocks); + + expect(blocks()[0]().text()).toEqual("Title 1"); + expect(blocks()[1]().text()).toEqual("Title 2"); + expect(blocks()[2]().text()).toEqual("Title 3"); + + undoRedoStack.redoCommand.execute(); + + debug("H", blocks); + + expect(blocks()[0]().text()).toEqual("Title 2"); + expect(blocks()[1]().text()).toEqual("Title 3"); + expect(blocks()[2]().text()).toEqual("Title 1"); }); diff --git a/src/js/converter/wrapper.js b/src/js/converter/wrapper.js index 9eb1f5eae..04ce830b7 100644 --- a/src/js/converter/wrapper.js +++ b/src/js/converter/wrapper.js @@ -136,12 +136,15 @@ var _getVariants = function(def) { var _makeComputedFunction = function(def, defs, thms, ko, contentModel, isContent, t) { if (typeof def == 'undefined') { if (typeof ko.utils.unwrapObservable(t).type === 'undefined') { - console.log("TODO ERROR Found a non-typed def ", def, t); + console.error("Found a non-typed def ", def, t); throw "Found a non-typed def " + def; } var type = ko.utils.unwrapObservable(ko.utils.unwrapObservable(t).type); def = defs[type]; - if (typeof def !== 'object') console.log("TODO ERROR Found a non-object def ", def, "for", type); + if (typeof def !== 'object') { + console.error("Found a non-object def ", def, "for", type); + throw "Found a non-object def " + def; + } } if (typeof contentModel == 'undefined' && typeof isContent != 'undefined' && isContent) { @@ -166,7 +169,8 @@ var _makeComputedFunction = function(def, defs, thms, ko, contentModel, isConten if (schemePathOrig.substr(0, selfPath.length) == selfPath) { schemePath = schemePathOrig.substr(selfPath.length); } else { - console.log("IS THIS CORRECT?", schemePathOrig, selfPath); + // Debug this scenario if it happens + console.log("Scheme path doesn't match selfPath", schemePathOrig, selfPath); schemePath = schemePathOrig; } @@ -216,8 +220,8 @@ var _makeComputedFunction = function(def, defs, thms, ko, contentModel, isConten pTarget = pTarget._defaultComputed; } if (typeof pTarget == 'undefined') { - console.log("ERROR looking for variant target", def._variant, t); - throw "ERROR looking for variant target " + def._variant; + console.error("Error looking for variant target", def._variant, t); + throw "Error looking for variant target " + def._variant; } pParent._nextVariant = _nextVariantFunction.bind(pTarget, ko, pTarget, _getVariants(def)); } diff --git a/src/js/undomanager/undomanager.js b/src/js/undomanager/undomanager.js index 73331b595..40c3a7372 100644 --- a/src/js/undomanager/undomanager.js +++ b/src/js/undomanager/undomanager.js @@ -90,9 +90,7 @@ var undoManager = function (model, options) { state = workState; var oldMode = mode; mode = MODE_MERGE; - // console.log("XDO", "before", label); action(); - // console.log("XDO", "after", label); _removeMergedAction(lastPushedStack); mode = oldMode; state = prevState; @@ -106,7 +104,6 @@ var undoManager = function (model, options) { if (typeof myStack == 'undefined') throw "Unexpected operation: stack cleaner called with undefined stack"; if (myStack().length > 0 && typeof myStack()[myStack().length - 1].mergedAction !== 'undefined') { - // console.log("Removing mergedAction from stack"); delete myStack()[myStack().length - 1].mergedAction; } }; @@ -123,16 +120,25 @@ var undoManager = function (model, options) { }; var executeUndoAction = function(child, value, item) { - // console.log("executeUndoAction", child, value, item); if (typeof value !== 'undefined') { child(value); } else if (item) { - if (item.status == 'deleted') { - child.splice(item.index, 0, item.value); - } else if (item.status == 'added') { - child.splice(item.index, 1); + if (item.status !== 'added' && item.status !== 'deleted') throw "Unsupported item.status: "+item.status; + + // When the items are "moved" we do both delete and add on the "add" action and ignore the delete action + if (typeof item.moved !== 'undefined') { + if (item.status == 'added') { + child.splice(item.index, 1); + child.splice(item.moved, 0, item.value); + } else { + // console.log("Ignoring undo move", item); + } } else { - throw "Unsupported item.status: "+item.status; + if (item.status == 'added') { + child.splice(item.index, 1); + } else { + child.splice(item.index, 0, item.value); + } } } else { throw "Unexpected condition: no item and no child.oldValues!"; @@ -146,14 +152,12 @@ var undoManager = function (model, options) { var makeUndoAction = makeUndoActionDefault; var changePusher = function(parents, child, item) { - // console.log("CP", item, child.oldValues); var oldVal = typeof child.oldValues != 'undefined' ? child.oldValues[0] : undefined; var act = makeUndoAction(executeUndoAction, parents, child, oldVal, item); if (mode == MODE_IGNORE) return; if (mode == MODE_MERGE) { - // console.log("UR", "mergemode"); if (typeof act !== 'undefined') { act.mergedAction = function(newAction) { if (typeof newAction.mergeMe !== 'undefined' && newAction.mergeMe) { @@ -167,23 +171,21 @@ var undoManager = function (model, options) { if (child.oldValues && mode == MODE_ONCE) { act.mergedAction = function(oldChild, oldItem, newAction) { if (typeof newAction.mergeableAction == 'object' && oldChild == newAction.mergeableAction.child) { - // console.log("UR", "ignore update for property in MODE_ONCE"); return this; } else return null; }.bind(act, child, item); act.mergeableAction = { child: child, item: item }; } - // console.log("UR", "item.status", item.status); + // "item" is valued when an item is added/removed/reteined in an array // sometimes KO detect "moves" and add a "moved" property with the index but // this doesn't happen for example using knockout-sortable or when moving objects // between arrays. // So this ends up handling this with "mergeableMove" and "mergedAction": - if (item && item.status == 'deleted') { + if (item && (item.status == 'deleted' || item.status == 'added')) { // TODO se sono in MODE = MERGE devo metteer una funzione di merge che accetta tutto. // altrimenti lascio questa. act.mergedAction = function(oldChild, oldItem, newAction) { - // console.log("UR", "act.mergedAction", typeof newAction.mergeableMove); // a deleted action is able to merge with a added action if they apply to the same // object. if (typeof newAction.mergeableMove == 'object' && oldItem.value == newAction.mergeableMove.item.value) { @@ -191,24 +193,23 @@ var undoManager = function (model, options) { // this way the "undo" will need to undo only once for a "move" operation. return _combinedFunction(newAction, this); } else { - // console.log("UR", "not mergeable", typeof newAction.mergeableMove); } return null; }.bind(act, child, item); - } else if (item && item.status == 'added') { + // add a mergeableMove property that will be used by the next action "mergedAction" to see if this action // can be merged. act.mergeableMove = { child: child, item: item }; } else if (item) { - console.warn("Unsupported item.status", item.status); + console.warn("Unsupported item.status", item.status, typeof item, item); } } } if (typeof act !== 'undefined') _push(act); }; - var reactorOptions = { depth: -1, oldValues: 1, mutable: true, /* tagParentsWithName: true */ tagFields: true }; + var reactorOptions = { depth: -1, oldValues: 1, mutable: true, /* tagParentsWithName: true */ tagFields: true, async: false /*, splitArrayChanges: false */ }; var context = {}; var react = typeof reactor == 'function' ? reactor : ko.watch; diff --git a/src/js/undomanager/undoserializer.js b/src/js/undomanager/undoserializer.js index 459b12dda..8851a6254 100644 --- a/src/js/undomanager/undoserializer.js +++ b/src/js/undomanager/undoserializer.js @@ -41,24 +41,28 @@ var _reference = function(model, path) { var _getPath = function(parents, child) { var path = ""; + var len = parents.length; var p; - for (var k = 0; k <= parents.length; k++) { + for (var k = 0; k <= len; k++) { p = k < parents.length ? parents[k] : child; - if (ko.isObservable(p)) path += '()'; if (typeof p._fieldName !== 'undefined') { + if (ko.isObservable(p)) path += '()'; path += "." + p._fieldName; } else if (k > 0 && typeof parents[k - 1].pop == 'function') { var parentArray = ko.isObservable(parents[k - 1]) ? ko.utils.peekObservable(parents[k - 1]) : parents[k - 1]; var pos = ko.utils.arrayIndexOf(parentArray, p); if (pos != -1) { + if (ko.isObservable(p)) path += '()'; path += "[" + pos + "]"; } else { // NOTE this happen, sometimes when TinyMCE sends updates for objects already removed. console.error("Unexpected object not found in parent array", parentArray, p, k, parents.length, ko.toJS(parentArray), ko.utils.unwrapObservable(p)); throw "Unexpected object not found in parent array"; } + } else if (len === 0 && typeof p._fieldName === 'undefined') { + // root path } else { - console.error("Unexpected parent with no _fieldName and no parent array", k, parents); + console.error("Unexpected parent with no _fieldName and no parent array", k, parents, typeof p); throw "Unexpected parent with no _fieldName and no parent array"; } }