Skip to content

Commit

Permalink
Grouped undo and redo statements. refs #502
Browse files Browse the repository at this point in the history 
We created three types of events can be stored in the edit history. Markup
insertions, deletions, and all other actions. When a markup insertion or
deletions happens within Xms of a previous edit of the same type, those
edits may be "grouped" for undo/redo. Practically, it means the pending
snapshot on history is stomped with the more recent snapshot.
  • Loading branch information
@disordinary @mixonic
disordinary authored and mixonic committed Nov 16, 2016
1 parent a705767 commit ce4d8d4
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 25 deletions.
31 changes: 23 additions & 8 deletions src/js/editor/edit-history.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -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() {
Expand All @@ -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) {
Expand All @@ -79,15 +94,15 @@ 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);
}
}

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();
Expand Down
5 changes: 3 additions & 2 deletions src/js/editor/editor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const defaults = {
spellcheck: true,
autofocus: true,
undoDepth: 5,
undoBlockTimeout: 5000, // ms for an undo event
cards: [],
atoms: [],
cardOptions: {},
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -687,7 +688,7 @@ class Editor {
if (postEditor._shouldCancelSnapshot) {
this._editHistory._pendingSnapshot = null;
}
this._editHistory.storeSnapshot();
this._editHistory.storeSnapshot(postEditor.editActionTaken);

return result;
}
Expand Down
14 changes: 14 additions & 0 deletions src/js/editor/post.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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));

Expand Down
163 changes: 148 additions & 15 deletions tests/acceptance/editor-undo-redo-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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);
Expand All @@ -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) => {
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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]
});
Expand Down

0 comments on commit ce4d8d4

Please sign in to comment.