diff --git a/src/js/editor/edit-history.js b/src/js/editor/edit-history.js index 89dcc5f45..3d24cf9ec 100644 --- a/src/js/editor/edit-history.js +++ b/src/js/editor/edit-history.js @@ -12,9 +12,11 @@ function findLeafSectionAtIndex(post, index) { } export class Snapshot { - constructor(editor) { + constructor(takenAt, editor, editAction=null) { this.mobiledoc = editor.serialize(); this.editor = editor; + this.editAction = editAction; + this.takenAt = takenAt; this.snapshotRange(); } @@ -44,15 +46,24 @@ export class Snapshot { return head.toRange(tail); } } + + groupsWith(groupingTimeout, editAction, takenAt) { + return ( + editAction !== null && + this.editAction === editAction && + this.takenAt + groupingTimeout > takenAt + ); + } } export default class EditHistory { - constructor(editor, queueLength) { + constructor(editor, queueLength, groupingTimeout) { this.editor = editor; this._undoStack = new FixedQueue(queueLength); this._redoStack = new FixedQueue(queueLength); this._pendingSnapshot = null; + this._groupingTimeout = groupingTimeout; } snapshot() { @@ -62,15 +73,19 @@ export default class EditHistory { } } - storeSnapshot() { + storeSnapshot(editAction=null) { + let now = Date.now(); // store pending snapshot - if (this._pendingSnapshot) { - this._undoStack.push(this._pendingSnapshot); + let pendingSnapshot = this._pendingSnapshot; + if (pendingSnapshot) { + if (!pendingSnapshot.groupsWith(this._groupingTimeout, editAction, now)) { + this._undoStack.push(pendingSnapshot); + } this._redoStack.clear(); } // take new pending snapshot to store next time `storeSnapshot` is called - this._pendingSnapshot = new Snapshot(this.editor); + this._pendingSnapshot = new Snapshot(now, this.editor, editAction); } stepBackward(postEditor) { @@ -79,7 +94,7 @@ export default class EditHistory { let snapshot = this._undoStack.pop(); if (snapshot) { - this._redoStack.push(new Snapshot(this.editor)); + this._redoStack.push(new Snapshot(Date.now(), this.editor)); this._restoreFromSnapshot(snapshot, postEditor); } } @@ -87,7 +102,7 @@ export default class EditHistory { stepForward(postEditor) { let snapshot = this._redoStack.pop(); if (snapshot) { - this._undoStack.push(new Snapshot(this.editor)); + this._undoStack.push(new Snapshot(Date.now(), this.editor)); this._restoreFromSnapshot(snapshot, postEditor); } postEditor.cancelSnapshot(); diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js index e96291f46..4f43277f1 100644 --- a/src/js/editor/editor.js +++ b/src/js/editor/editor.js @@ -43,6 +43,7 @@ const defaults = { spellcheck: true, autofocus: true, undoDepth: 5, + undoBlockTimeout: 5000, // ms for an undo event cards: [], atoms: [], cardOptions: {}, @@ -135,7 +136,7 @@ class Editor { this.post = this.loadPost(); this._renderTree = new RenderTree(this.post); - this._editHistory = new EditHistory(this, this.undoDepth); + this._editHistory = new EditHistory(this, this.undoDepth, this.undoBlockTimeout); this._eventManager = new EventManager(this); this._mutationHandler = new MutationHandler(this); this._editState = new EditState(this); @@ -687,7 +688,7 @@ class Editor { if (postEditor._shouldCancelSnapshot) { this._editHistory._pendingSnapshot = null; } - this._editHistory.storeSnapshot(); + this._editHistory.storeSnapshot(postEditor.editActionTaken); return result; } diff --git a/src/js/editor/post.js b/src/js/editor/post.js index 3d63ab978..0cbabce7f 100644 --- a/src/js/editor/post.js +++ b/src/js/editor/post.js @@ -21,6 +21,15 @@ const CALLBACK_QUEUES = { AFTER_COMPLETE: 'afterComplete' }; +// 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 EDIT_ACTIONS = { + INSERT_TEXT: 1, + DELETE: 2 +}; + + /** * The PostEditor is used to modify a post. It should not be instantiated directly. * Instead, a new instance of a PostEditor is created by the editor and passed @@ -44,6 +53,7 @@ class PostEditor { this._callbacks = new LifecycleCallbacks(values(CALLBACK_QUEUES)); this._didComplete = false; + this.editActionTaken = null; this._renderRange = () => this.editor.selectRange(this._range); this._postDidChange = () => this.editor._postDidChange(); @@ -112,6 +122,8 @@ class PostEditor { deleteRange(range) { assert("Must pass MobiledocKit Range to `deleteRange`", range instanceof Range); + this.editActionTaken = EDIT_ACTIONS.DELETE; + let { head, head: {section: headSection}, tail, tail: {section: tailSection} @@ -578,6 +590,8 @@ class PostEditor { assert('Cannot insert markers at non-markerable position', section.isMarkerable); + this.editActionTaken = EDIT_ACTIONS.INSERT_TEXT; + let edit = section.splitMarkerAtOffset(offset); edit.removed.forEach(marker => this._scheduleForRemoval(marker)); diff --git a/tests/acceptance/editor-undo-redo-test.js b/tests/acceptance/editor-undo-redo-test.js index a79c40eec..f30289e41 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'); @@ -81,7 +80,97 @@ test('undo/redo the insertion of multiple characters', (assert) => { Helpers.dom.insertText(editor, 'E'); Helpers.wait(() => { - assert.postIsSimilar(editor.post, beforeUndo); // precond + assert.postIsSimilar( + editor.post, beforeUndo, + 'precond - post was updated with new characters' + ); + + undo(editor); + assert.postIsSimilar( + editor.post, afterUndo, + 'ensure undo grouped to include both characters' + ); + + redo(editor); + assert.postIsSimilar( + editor.post, beforeUndo, + 'ensure redo grouped to include both characters' + ); + 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; + }, {undoBlockTimeout: 100}); + + 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(); + }); + }); + },120); + }); + }); +}); + + +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; + }, {undoBlockTimeout: 100}); + + 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 +180,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(); - }); + },120); +}); + + +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 +332,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); @@ -349,11 +475,18 @@ test('take and undo a snapshot when removing an atom', (assert) => { let beforeUndo, afterUndo; editor = Helpers.mobiledoc.renderIntoAndFocusTail(editorElement, ({post, markupSection, marker, atom}) => { - beforeUndo = post([markupSection('p', [marker(text)])]); - afterUndo = post([ - markupSection('p', [marker(text), atom('my-atom', 'content', {})]), - ]); - return afterUndo; + beforeUndo = post([ + markupSection('p', [ + marker(text) + ]) + ]); + afterUndo = post([ + markupSection('p', [ + marker(text), + atom('my-atom', 'content', {}) + ]), + ]); + return afterUndo; }, { atoms: [myAtom] });