From 8f64e8941f21f5f9215249ba82b64c8f66cce36d Mon Sep 17 00:00:00 2001 From: Ian Storm Taylor Date: Sat, 18 Jun 2016 19:18:47 -0700 Subject: [PATCH] got undo working properly for different transforms! --- lib/components/content.js | 2 +- lib/models/document.js | 49 +++++++++++++++----------------- lib/models/state.js | 16 ++++------- lib/models/transform.js | 59 +++++++++++++++++++++++++++++---------- 4 files changed, 73 insertions(+), 53 deletions(-) diff --git a/lib/components/content.js b/lib/components/content.js index ce509d7e9..104d3cfd5 100644 --- a/lib/components/content.js +++ b/lib/components/content.js @@ -111,7 +111,7 @@ class Content extends React.Component { } state = transform - .insert(data) + .insertText(data) .apply() this.onChange(state) diff --git a/lib/models/document.js b/lib/models/document.js index e02acba18..54dd30959 100644 --- a/lib/models/document.js +++ b/lib/models/document.js @@ -224,14 +224,14 @@ class Document extends Record(DEFAULT_PROPERTIES) { } /** - * Insert `data` at a `range`. + * Insert `text` at a `range`. * * @param {Selection} range - * @param {String or Node or OrderedMap} data + * @param {String} text * @return {Document} document */ - insertAtRange(range, data) { + insertTextAtRange(range, text) { let document = this // When still expanded, remove the current range first. @@ -240,33 +240,30 @@ class Document extends Record(DEFAULT_PROPERTIES) { range = range.moveToStart() } - // When the data is a string of characters... - if (typeof data == 'string') { - let { startNode, startOffset } = document - let { characters } = startNode + let { startKey, startOffset } = range + let startNode = document.getNode(startKey) + let { characters } = startNode - // Create a list of the new characters, with the right marks. - const marks = characters.has(startOffset) - ? characters.get(startOffset).marks - : null + // Create a list of the new characters, with the right marks. + const marks = characters.has(startOffset) + ? characters.get(startOffset).marks + : null - const newCharacters = data.split('').reduce((list, char) => { - const obj = { text: char } - if (marks) obj.marks = marks - return list.push(Character.create(obj)) - }, Character.createList()) + const newCharacters = text.split('').reduce((list, char) => { + const obj = { text } + if (marks) obj.marks = marks + return list.push(Character.create(obj)) + }, Character.createList()) - // Splice in the new characters. - const resumeOffset = startOffset + data.length - 1 - characters = characters.slice(0, startOffset) - .concat(newCharacters) - .concat(characters.slice(resumeOffset, Infinity)) + // Splice in the new characters. + const resumeOffset = startOffset + text.length - 1 + characters = characters.slice(0, startOffset) + .concat(newCharacters) + .concat(characters.slice(resumeOffset, Infinity)) - // Update the existing text node. - startNode = startNode.merge({ characters }) - document = document.updateNode(startNode) - return document - } + // Update the existing text node. + startNode = startNode.merge({ characters }) + document = document.updateNode(startNode) // Normalize the document. return document.normalize() diff --git a/lib/models/state.js b/lib/models/state.js index 58e3ef6a1..655076a80 100644 --- a/lib/models/state.js +++ b/lib/models/state.js @@ -146,23 +146,17 @@ class State extends Record(DEFAULT_PROPERTIES) { /** * Insert a `text` string at the current cursor position. * - * @param {String or Node or OrderedMap} data + * @param {String} text * @return {State} state */ - insert(data) { + insertText(text) { let state = this let { document, selection } = state - let after - // Determine what the selection should be after inserting. - if (typeof data == 'string') { - after = selection.moveForward(data.length) - } - - // Insert the data and update the selection. - document = document.insertAtRange(selection, data) - selection = after + // Insert the text and update the selection. + document = document.insertTextAtRange(selection, text) + selection = selection.moveForward(text.length) state = state.merge({ document, selection }) return state } diff --git a/lib/models/transform.js b/lib/models/transform.js index b505c6359..adabbc1a7 100644 --- a/lib/models/transform.js +++ b/lib/models/transform.js @@ -1,4 +1,6 @@ +import uniq from 'lodash/uniq' +import xor from 'lodash/xor' import { List, Record } from 'immutable' /** @@ -7,7 +9,8 @@ import { List, Record } from 'immutable' const Snapshot = Record({ document: null, - selection: null + selection: null, + steps: new List() }) /** @@ -39,8 +42,8 @@ const TRANSFORM_TYPES = [ 'deleteBackwardAtRange', 'deleteForward', 'deleteForwardAtRange', - 'insert', - 'insertAtRange', + 'insertText', + 'insertTextAtRange', 'split', 'splitAtRange' ] @@ -58,9 +61,9 @@ class Transform extends Record(DEFAULT_PROPERTIES) { */ snapshot() { - let { state } = this + let { state, steps } = this let { document, selection } = state - return new Snapshot({ document, selection }) + return new Snapshot({ document, selection, steps }) } /** @@ -70,17 +73,44 @@ class Transform extends Record(DEFAULT_PROPERTIES) { */ apply() { - let transform = this + const transform = this let { state, steps } = transform let { history } = state let { undos, redos } = history - // Save the current state into the history before transforming. - let snapshot = transform.snapshot() - undos = undos.unshift(snapshot) - redos = redos.clear() - history = history.merge({ undos, redos }) - state = state.merge({ history }) + // Determine whether we need to create a new snapshot. + let shouldSnapshot = false + const previous = undos.peek() + + // If there isn't a previous state, snapshot. + if (!previous) shouldSnapshot = true + + // If there is a previous state but the steps are different, snapshot. + if (!shouldSnapshot && previous) { + const types = steps.map(step => step.type) + const prevTypes = previous.steps.map(step => step.type) + const diff = xor(types.toArray(), prevTypes.toArray()) + if (diff.length) shouldSnapshot = true + } + + // If the current steps aren't one of the "combinale" types, snapshot. + if (!shouldSnapshot) { + const allCombinable = ( + steps.every(step => step.type == 'insertText') || + steps.every(step => step.type == 'deleteForward') || + steps.every(step => step.type == 'deleteBackward') + ) + if (!allCombinable) shouldSnapshot = true + } + + // If we should, save a snapshot into the history before transforming. + if (shouldSnapshot) { + const snapshot = transform.snapshot() + undos = undos.unshift(snapshot) + redos = redos.clear() + history = history.merge({ undos, redos }) + state = state.merge({ history }) + } // Apply each of the steps in the transform, arriving at a new state. state = steps.reduce((state, step) => { @@ -91,7 +121,6 @@ class Transform extends Record(DEFAULT_PROPERTIES) { return state } - /** * Undo to the previous state in the history. * @@ -99,7 +128,7 @@ class Transform extends Record(DEFAULT_PROPERTIES) { */ undo() { - let transform = this + const transform = this let { state } = transform let { history } = state let { undos, redos } = history @@ -129,7 +158,7 @@ class Transform extends Record(DEFAULT_PROPERTIES) { */ redo() { - let transform = this + const transform = this let { state } = transform let { history } = state let { undos, redos } = history