From a2ee9eabfe17ca82f1a7f07f604a2671677179f0 Mon Sep 17 00:00:00 2001 From: Ryan McCarvill Date: Wed, 19 Oct 2016 23:11:17 +1300 Subject: [PATCH] Groups Undo Refs #502 Basically there are three types of UNDO events, content insert, content delete, and everything else. content insert and content delete events group together overwriting the last element in the undo queue unless another event occurs which breaks it or a timeout occurs. So, if I write text then as long as I don't delete text or insert an atom or card, and as long as the timeout doesn't occur, those events are grouped and undo in one go. A timeout is reset whenever the `run` method is called on the editor, so it doesn't occur on the first occurence of an event in a run but rather the last. --- src/js/editor/edit-history.js | 9 +- src/js/editor/editor.js | 67 +++++++++-- tests/acceptance/editor-undo-redo-test.js | 137 ++++++++++++++++++++-- 3 files changed, 195 insertions(+), 18 deletions(-) diff --git a/src/js/editor/edit-history.js b/src/js/editor/edit-history.js index 89dcc5f45..b28b97401 100644 --- a/src/js/editor/edit-history.js +++ b/src/js/editor/edit-history.js @@ -62,10 +62,15 @@ export default class EditHistory { } } - storeSnapshot() { + storeSnapshot(overwriteLast) { // store pending snapshot if (this._pendingSnapshot) { - this._undoStack.push(this._pendingSnapshot); + // if overwriteLast === true then this state replaces the last state on the stack + if(overwriteLast) { + this._undoStack[ this._undoStack.length - 1] = this._pendingSnapshot; + } else { + this._undoStack.push(this._pendingSnapshot); + } this._redoStack.clear(); } diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js index 8c053b26c..7abec4572 100644 --- a/src/js/editor/editor.js +++ b/src/js/editor/editor.js @@ -44,6 +44,7 @@ const defaults = { spellcheck: true, autofocus: true, undoDepth: 5, + undoBlockTimeout: 5000, // ms for an undo event cards: [], atoms: [], cardOptions: {}, @@ -70,6 +71,16 @@ const CALLBACK_QUEUES = { INPUT_MODE_DID_CHANGE: 'inputModeDidChange' }; +// There are only two events that we're concerned about for Undo, that is inserting text and deleting content. +// These are the only two states that go on a "run" and create a combined undo, everything else has it's own +// deadicated undo. +const EDITOR_RUN_LAST_ACTION_STATE = { + INSERT_TEXT: 1, + DELETE: 2 +}; + + + /** * The Editor is a core component of mobiledoc-kit. After instantiating * an editor, use {@link Editor#render} to display the editor on the web page. @@ -145,6 +156,9 @@ class Editor { DEFAULT_TEXT_INPUT_HANDLERS.forEach(handler => this.onTextInput(handler)); this.hasRendered = false; + + this._undoRunLastState = null; + this._undoRunTimeout = null; } /** @@ -290,10 +304,10 @@ class Editor { * @public */ deleteAtPosition(position, direction, {unit}) { - this.run(postEditor => { + this._run(postEditor => { let nextPosition = postEditor.deleteAtPosition(position, direction, {unit}); postEditor.setRange(nextPosition); - }); + }, EDITOR_RUN_LAST_ACTION_STATE.DELETE); } /** @@ -303,10 +317,10 @@ class Editor { * @public */ deleteRange(range) { - this.run(postEditor => { + this._run(postEditor => { let nextPosition = postEditor.deleteRange(range); postEditor.setRange(nextPosition); - }); + }, EDITOR_RUN_LAST_ACTION_STATE.DELETE); } /** @@ -677,6 +691,45 @@ class Editor { * @public */ run(callback) { + return this._run(callback); + } + /** + * This does the actual work of the public {run} above, actionType is used to group undos. + * Currently there are two action types: + * 1) `EDITOR_RUN_LAST_ACTION_STATE.insert_text` which is used to group any text insertion, and + * 2) `EDITOR_RUN_LAST_ACTION_STATE.delete` which contains any delete event. + * + * Any other event that happens such as insertion of cards, atoms, or paste events creates their own undo + * entries and are not grouped. + * + * @param {Function} callback Called with an instance of + * {@link PostEditor} as its argument. + * @param {Number|undefined} actionType provides the type of the current + * action, but only if it's a "runnable" + * type. + * @return {Mixed} The return value of `callback`. + * @private + */ + _run(callback, actionType) { + + // Decides whether or not this current action is in a run of previous + // actions which dictates whether or not it overwrites the last item on the + // undo queue. + let isInARun = actionType && this._undoRunLastState === actionType; + this._undoRunLastState = actionType; + + if(this._undoRunTimeout) { + clearTimeout(this._undoRunTimeout); + this._undoRunTimeout = null; + } + + if(isInARun) { + this._undoRunTimeout = setTimeout(() => { + this._undoRunTimeout = null; + this._undoRunLastState = null; + }, + this.undoBlockTimeout); + } const postEditor = new PostEditor(this); postEditor.begin(); this._editHistory.snapshot(); @@ -688,7 +741,7 @@ class Editor { if (postEditor._shouldCancelSnapshot) { this._editHistory._pendingSnapshot = null; } - this._editHistory.storeSnapshot(); + this._editHistory.storeSnapshot(isInARun); return result; } @@ -934,13 +987,13 @@ class Editor { } let { activeMarkups, range, range: { head: position } } = this; - this.run(postEditor => { + this._run(postEditor => { if (!range.isCollapsed) { position = postEditor.deleteRange(range); } postEditor.insertTextWithMarkup(position, text, activeMarkups); - }); + }, EDITOR_RUN_LAST_ACTION_STATE.INSERT_TEXT); } /** diff --git a/tests/acceptance/editor-undo-redo-test.js b/tests/acceptance/editor-undo-redo-test.js index a79c40eec..51fe1f884 100644 --- a/tests/acceptance/editor-undo-redo-test.js +++ b/tests/acceptance/editor-undo-redo-test.js @@ -64,12 +64,11 @@ test('undo/redo the insertion of a character', (assert) => { // when typing characters test('undo/redo the insertion of multiple characters', (assert) => { let done = assert.async(); - let beforeUndo, afterUndo1, afterUndo2; + let beforeUndo, afterUndo; editor = Helpers.mobiledoc.renderIntoAndFocusTail(editorElement, ({post, markupSection, marker}) => { beforeUndo = post([markupSection('p', [marker('abcDE')])]); - afterUndo1 = post([markupSection('p', [marker('abcD')])]); - afterUndo2 = post([markupSection('p', [marker('abc')])]); - return afterUndo2; + afterUndo = post([markupSection('p', [marker('abc')])]); + return afterUndo; }); let textNode = Helpers.dom.findTextNode(editorElement, 'abc'); @@ -83,6 +82,89 @@ test('undo/redo the insertion of multiple characters', (assert) => { Helpers.wait(() => { assert.postIsSimilar(editor.post, beforeUndo); // precond + undo(editor); + assert.postIsSimilar(editor.post, afterUndo); + + redo(editor); + assert.postIsSimilar(editor.post, beforeUndo); + done(); + }); + }); +}); + + +// Test to ensure that undo events group after a timeout +test('make sure undo/redo events group when adding text', (assert) => { + let done = assert.async(); + let beforeUndo, afterUndo1, afterUndo2; + editor = Helpers.mobiledoc.renderIntoAndFocusTail(editorElement, ({post, markupSection, marker}) => { + beforeUndo = post([markupSection('p', [marker('123456789')])]); + afterUndo1 = post([markupSection('p', [marker('123456')])]); + afterUndo2 = post([markupSection('p', [marker('123')])]); + return afterUndo2; + }); + + editor.undoBlockTimeout = 100; //set the block timeout to 100ms to test + let textNode = Helpers.dom.findTextNode(editorElement, '123'); + Helpers.dom.moveCursorTo(editor, textNode, '123'.length); + + Helpers.dom.insertText(editor, '4'); + + Helpers.wait(() => { + Helpers.dom.insertText(editor, '5'); + Helpers.wait(() => { + Helpers.dom.insertText(editor, '6'); + window.setTimeout(() => { + Helpers.dom.insertText(editor, '7'); + Helpers.wait(() => { + Helpers.dom.insertText(editor, '8'); + Helpers.wait(() => { + Helpers.dom.insertText(editor, '9'); + assert.postIsSimilar(editor.post, beforeUndo); + + undo(editor); + assert.postIsSimilar(editor.post, afterUndo1); + + undo(editor); + assert.postIsSimilar(editor.post, afterUndo2); + + redo(editor); + assert.postIsSimilar(editor.post, afterUndo1); + done(); + }); + }); + },101); + }); + }); +}); + + +test('make sure undo/redo events group when deleting text', (assert) => { + let done = assert.async(); + let beforeUndo, afterUndo1, afterUndo2; + editor = Helpers.mobiledoc.renderIntoAndFocusTail(editorElement, ({post, markupSection, marker}) => { + beforeUndo = post([markupSection('p', [marker('123')])]); + afterUndo1 = post([markupSection('p', [marker('123456')])]); + afterUndo2 = post([markupSection('p', [marker('123456789')])]); + return afterUndo2; + }); + + editor.undoBlockTimeout = 100; //set the block timeout to 100ms to test + let textNode = Helpers.dom.findTextNode(editorElement, '123456789'); + Helpers.dom.moveCursorTo(editor, textNode, '123456789'.length); + + Helpers.dom.triggerDelete(editor); + Helpers.dom.triggerDelete(editor); + Helpers.dom.triggerDelete(editor); + + window.setTimeout(() => { + + Helpers.dom.triggerDelete(editor); + Helpers.dom.triggerDelete(editor); + Helpers.dom.triggerDelete(editor); + + assert.postIsSimilar(editor.post, beforeUndo); + undo(editor); assert.postIsSimilar(editor.post, afterUndo1); @@ -91,12 +173,49 @@ test('undo/redo the insertion of multiple characters', (assert) => { redo(editor); assert.postIsSimilar(editor.post, afterUndo1); - - redo(editor); - assert.postIsSimilar(editor.post, beforeUndo); done(); - }); + },101); +}); + + +test('adding and deleting characters break the undo group/run', (assert) => { + let beforeUndo, afterUndo1, afterUndo2; + let done = assert.async(); + editor = Helpers.mobiledoc.renderIntoAndFocusTail(editorElement, ({post, markupSection, marker}) => { + beforeUndo = post([markupSection('p', [marker('abcXY')])]); + afterUndo1 = post([markupSection('p', [marker('abc')])]); + afterUndo2 = post([markupSection('p', [marker('abcDE')])]); + return afterUndo2; }); + + let textNode = Helpers.dom.findTextNode(editorElement, 'abcDE'); + Helpers.dom.moveCursorTo(editor, textNode, 'abcDE'.length); + + Helpers.dom.triggerDelete(editor); + Helpers.dom.triggerDelete(editor); + + Helpers.dom.insertText(editor, 'X'); + + Helpers.wait(() => { + Helpers.dom.insertText(editor, 'Y'); + + Helpers.wait(() => { + assert.postIsSimilar(editor.post, beforeUndo); // precond + + undo(editor); + assert.postIsSimilar(editor.post, afterUndo1); + + undo(editor); + assert.postIsSimilar(editor.post, afterUndo2); + + redo(editor); + assert.postIsSimilar(editor.post, afterUndo1); + + redo(editor); + assert.postIsSimilar(editor.post, beforeUndo); + done(); + }); + }); }); test('undo the deletion of a character', (assert) => { @@ -206,7 +325,7 @@ test('undo stack length can be configured (depth 1)', (assert) => { let beforeUndo, afterUndo; editor = Helpers.mobiledoc.renderIntoAndFocusTail(editorElement, ({post, markupSection, marker}) => { beforeUndo = post([markupSection('p', [marker('abcDE')])]); - afterUndo = post([markupSection('p', [marker('abcD')])]); + afterUndo = post([markupSection('p', [marker('abc')])]); return post([markupSection('p', [marker('abc')])]); }, editorOptions);