From faf6a63f3eb5836fa4336c86a69bbabbf080f2da Mon Sep 17 00:00:00 2001 From: Cory Forsyth Date: Mon, 17 Aug 2015 17:47:44 -0400 Subject: [PATCH] Add forward and backward deletion to PostEditor fixes #36 --- src/js/editor/editor.js | 49 ++++----- src/js/editor/post.js | 165 +++++++++++++++++------------ src/js/models/post-node-builder.js | 5 +- src/js/utils/key.js | 16 +++ tests/helpers/dom.js | 2 +- tests/helpers/mobiledoc.js | 26 +++++ tests/test-helpers.js | 4 +- tests/unit/editor/post-test.js | 159 +++++++++++++++++++++++++++ 8 files changed, 327 insertions(+), 99 deletions(-) create mode 100644 tests/helpers/mobiledoc.js create mode 100644 tests/unit/editor/post-test.js diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js index 76a0153e6..c33550884 100644 --- a/src/js/editor/editor.js +++ b/src/js/editor/editor.js @@ -238,11 +238,13 @@ class Editor { element.setAttribute('contentEditable', true); if (this.mobiledoc) { - this.parseModelFromMobiledoc(this.mobiledoc); + this.post = new MobiledocParser(this.builder).parse(this.mobiledoc); } else { - this.parseModelFromDOM(this.element); + this.post = new DOMParser(this.builder).parse(this.element); } + this._renderTree = this.prepareRenderTree(this.post); + clearChildNodes(element); this.rerender(); @@ -276,21 +278,11 @@ class Editor { this._views.push(view); } - parseModelFromDOM(element) { - let parser = new DOMParser(this.builder); - this.post = parser.parse(element); - this._renderTree = new RenderTree(); - let node = this._renderTree.buildRenderNode(this.post); - this._renderTree.node = node; - this.trigger('update'); - } - - parseModelFromMobiledoc(mobiledoc) { - this.post = new MobiledocParser(this.builder).parse(mobiledoc); - this._renderTree = new RenderTree(); - let node = this._renderTree.buildRenderNode(this.post); - this._renderTree.node = node; - this.trigger('update'); + prepareRenderTree(post) { + let renderTree = new RenderTree(); + let node = renderTree.buildRenderNode(post); + renderTree.node = node; + return renderTree; } rerender() { @@ -308,20 +300,23 @@ class Editor { handleDeletion(event) { event.preventDefault(); - let offsets = this.cursor.offsets; - let currentMarker, currentOffset; - this.run((postEditor) => { - let results; + + const results = this.run(postEditor => { + const offsets = this.cursor.offsets; + if (this.cursor.hasSelection()) { - results = postEditor.deleteRange(offsets); + return postEditor.deleteRange(offsets); } else { - // FIXME: perhaps this should accept this.cursor.offsets? - results = postEditor.deleteCharAt(offsets.headMarker, offsets.headOffset-1); + const {headMarker, headOffset} = offsets; + const key = Key.fromEvent(event); + + const deletePosition = {marker: headMarker, offset: headOffset}, + direction = key.direction; + return postEditor.deleteFrom(deletePosition, direction); } - currentMarker = results.currentMarker; - currentOffset = results.currentOffset; }); - this.cursor.moveToMarker(currentMarker, currentOffset); + + this.cursor.moveToMarker(results.currentMarker, results.currentOffset); } handleNewline(event) { diff --git a/src/js/editor/post.js b/src/js/editor/post.js index e86a3716d..55dc4ab1c 100644 --- a/src/js/editor/post.js +++ b/src/js/editor/post.js @@ -1,4 +1,11 @@ import { MARKUP_SECTION_TYPE } from '../models/markup-section'; + +import { DIRECTION } from '../utils/key'; + +function isMarkupSection(section) { + return section.type === MARKUP_SECTION_TYPE; +} + class PostEditor { constructor(editor) { this.editor = editor; @@ -68,94 +75,114 @@ class PostEditor { } /** - * Remove a character from a marker. + * Remove a character from a {marker, offset} position, in either + * forward or backward (default) direction. * * Usage: * * let marker = editor.post.sections.head.markers.head; * // marker has text of "Howdy!" * editor.run((postEditor) => { - * postEditor.deleteCharAt(marker, 3); + * postEditor.deleteFrom({marker, offset: 3}); * }); * // marker has text of "Hody!" * - * `deleteCharAt` may remove a character from a different marker or section - * if the position of the deletion is at the 0th offset. Offset behaves like - * a cursor position, with the deletion going to the previous character. + * `deleteFrom` may remove a character from a different marker or join the + * marker's section with the previous/next section (depending on the + * deletion direction) if direction is `BACKWARD` and the offset is 0, + * or direction is `FORWARD` and the offset is equal to the length of the + * marker. * - * @method deleteCharAt - * @param {Object} marker the marker to delete the character from - * @param {Object} offset offset in the text of the marker to delete, first character is 1 - * @return {Object} {currentMarker, currentOffset} for cursor + * @method deleteFrom + * @param {Object} position object with {marker, offset} the marker and offset to delete from + * @param {Number} direction The direction to delete in (default is BACKWARD) + * @return {Object} {currentMarker, currentOffset} for positioning the cursor * @public */ - deleteCharAt(marker, offset) { - // need to handle these cases: - // when cursor is: - // * A in the middle of a marker -- just delete the character - // * B offset is 0 and there is a previous marker - // * delete last char of previous marker - // * C offset is 0 and there is no previous marker - // * join this section with previous section - const currentMarker = marker; - let nextCursorMarker = currentMarker; - let nextCursorOffset = offset; - let renderNode = marker.renderNode; - - // A: in the middle of a marker - if (offset >= 0) { - currentMarker.deleteValueAtOffset(offset); - if (currentMarker.length === 0 && currentMarker.section.markers.length > 1) { - if (marker.renderNode) { - marker.renderNode.scheduleForRemoval(); - } + deleteFrom({marker, offset}, direction=DIRECTION.BACKWARD) { + if (direction === DIRECTION.BACKWARD) { + return this._deleteBackwardFrom({marker, offset}); + } else { + return this._deleteForwardFrom({marker, offset}); + } + } - let isFirstRenderNode = renderNode === renderNode.parent.childNodes.head; - if (isFirstRenderNode) { - // move cursor to start of next node - nextCursorMarker = renderNode.next.postNode; - nextCursorOffset = 0; - } else { - // move cursor to end of prev node - nextCursorMarker = renderNode.prev.postNode; - nextCursorOffset = renderNode.prev.postNode.length; - } + /** + * delete 1 character in the FORWARD direction from the given position + * @method _deleteForwardFrom + * @private + */ + _deleteForwardFrom({marker, offset}) { + const nextCursorMarker = marker, + nextCursorOffset = offset; + + if (offset === marker.length) { + const nextMarker = marker.next; + + if (nextMarker) { + this._deleteForwardFrom({marker: nextMarker, offset: 0}); } else { - renderNode.markDirty(); + const nextSection = marker.section.next; + if (nextSection && isMarkupSection(nextSection)) { + const currentSection = marker.section; + + currentSection.join(nextSection); + + currentSection.renderNode.markDirty(); + nextSection.renderNode.scheduleForRemoval(); + } } } else { - let currentSection = currentMarker.section; - let previousMarker = currentMarker.prev; - if (previousMarker) { // (B) - let markerLength = previousMarker.length; - previousMarker.deleteValueAtOffset(markerLength - 1); - } else { // (C) - // possible previous sections: - // * none -- do nothing - // * markup section -- join to it - // * non-markup section (card) -- select it? delete it? - let previousSection = currentSection.prev; - if (previousSection) { - let isMarkupSection = previousSection.type === MARKUP_SECTION_TYPE; - - if (isMarkupSection) { - let lastPreviousMarker = previousSection.markers.tail; - previousSection.join(currentSection); - previousSection.renderNode.markDirty(); - currentSection.renderNode.scheduleForRemoval(); - - nextCursorMarker = lastPreviousMarker.next; - nextCursorOffset = 0; - /* - } else { - // card section: ?? - */ + marker.deleteValueAtOffset(offset); + marker.renderNode.markDirty(); + } + + this.scheduleRerender(); + this.scheduleDidUpdate(); + + return { + currentMarker: nextCursorMarker, + currentOffset: nextCursorOffset + }; + } + + /** + * delete 1 character in the BACKWARD direction from the given position + * @method _deleteBackwardFrom + * @private + */ + _deleteBackwardFrom({marker, offset}) { + let nextCursorMarker = marker, + nextCursorOffset = offset; + + if (offset === 0) { + const prevMarker = marker.prev; + + if (prevMarker) { + return this._deleteBackwardFrom({marker: prevMarker, offset: prevMarker.length}); + } else { + const prevSection = marker.section.prev; + + if (prevSection) { + if (isMarkupSection(prevSection)) { + nextCursorMarker = prevSection.markers.tail; + nextCursorOffset = nextCursorMarker.length; + + prevSection.join(marker.section); + prevSection.renderNode.markDirty(); + marker.section.renderNode.scheduleForRemoval(); } - } else { // no previous section -- do nothing - nextCursorMarker = currentMarker; - nextCursorOffset = 0; + // ELSE: FIXME: card section -- what should deleting into it do? } } + + } else if (offset <= marker.length) { + const offsetToDeleteAt = offset - 1; + + marker.deleteValueAtOffset(offsetToDeleteAt); + marker.renderNode.markDirty(); + + nextCursorOffset = offsetToDeleteAt; } this.scheduleRerender(); diff --git a/src/js/models/post-node-builder.js b/src/js/models/post-node-builder.js index a2738625f..4fa782d0b 100644 --- a/src/js/models/post-node-builder.js +++ b/src/js/models/post-node-builder.js @@ -11,9 +11,12 @@ export default class PostNodeBuilder { this.markupCache = {}; } - createPost() { + createPost(sections=[]) { const post = new Post(); post.builder = this; + + sections.forEach(s => post.sections.append(s)); + return post; } diff --git a/src/js/utils/key.js b/src/js/utils/key.js index 9963d8c37..996789466 100644 --- a/src/js/utils/key.js +++ b/src/js/utils/key.js @@ -1,4 +1,8 @@ import Keycodes from './keycodes'; +export const DIRECTION = { + FORWARD: 1, + BACKWARD: 2 +}; /** * An abstraction around a KeyEvent @@ -24,6 +28,18 @@ const Key = class Key { this.keyCode === Keycodes.DELETE; } + isForwardDelete() { + return this.keyCode === Keycodes.DELETE; + } + + + get direction() { + if (!this.isDelete()) { + throw new Error('Only delete keys have direction'); + } + return this.isForwardDelete() ? DIRECTION.FORWARD : DIRECTION.BACKWARD; + } + isSpace() { return this.keyCode === Keycodes.SPACE; } diff --git a/tests/helpers/dom.js b/tests/helpers/dom.js index 857eb8d7e..56e81ff03 100644 --- a/tests/helpers/dom.js +++ b/tests/helpers/dom.js @@ -145,7 +145,7 @@ function triggerDelete(editor) { let event = { preventDefault() {} }; editor.handleDeletion(event); } else { - triggerKeyEvent(document, 'keydown', KEY_CODES.DELETE); + triggerKeyEvent(document, 'keydown', KEY_CODES.BACKSPACE); } } diff --git a/tests/helpers/mobiledoc.js b/tests/helpers/mobiledoc.js new file mode 100644 index 000000000..bc9601562 --- /dev/null +++ b/tests/helpers/mobiledoc.js @@ -0,0 +1,26 @@ +import PostNodeBuilder from 'content-kit-editor/models/post-node-builder'; +import MobiledocRenderer from 'content-kit-editor/renderers/mobiledoc'; + +/* + * makeMD({post, section, marker, markup} => + post([ + section('P', [ + marker('some text', [markup('B')]) + ]) + }) + */ +function makeMD(treeFn) { + let builder = new PostNodeBuilder(); + + const post = (...args) => builder.createPost(...args); + const section = (...args) => builder.createMarkupSection(...args); + const markup = (...args) => builder.createMarkup(...args); + const marker = (...args) => builder.createMarker(...args); + + let builtPost = treeFn({post, section, markup, marker}); + return MobiledocRenderer.render(builtPost); +} + +export default { + makeMD +}; diff --git a/tests/test-helpers.js b/tests/test-helpers.js index c7ffec5cb..519f458ff 100644 --- a/tests/test-helpers.js +++ b/tests/test-helpers.js @@ -4,9 +4,11 @@ registerAssertions(); import DOMHelpers from './helpers/dom'; import ToolbarHelpers from './helpers/toolbar'; import skipInPhantom from './helpers/skip-in-phantom'; +import MobiledocHelpers from './helpers/mobiledoc'; export default { dom: DOMHelpers, toolbar: ToolbarHelpers, - skipInPhantom + skipInPhantom, + mobiledoc: MobiledocHelpers }; diff --git a/tests/unit/editor/post-test.js b/tests/unit/editor/post-test.js new file mode 100644 index 000000000..6a26d93d5 --- /dev/null +++ b/tests/unit/editor/post-test.js @@ -0,0 +1,159 @@ +import PostEditor from 'content-kit-editor/editor/post'; +import { Editor } from 'content-kit-editor'; +import Helpers from '../../test-helpers'; +import { DIRECTION } from 'content-kit-editor/utils/key'; + +const { FORWARD } = DIRECTION; + +const { module, test } = window.QUnit; + +let editor, editorElement; + +function getSection(sectionIndex) { + return editor.post.sections.objectAt(sectionIndex); +} + +function getMarker(sectionIndex, markerIndex) { + return getSection(sectionIndex).markers.objectAt(markerIndex); +} + +function postEditorWithMobiledoc(treeFn) { + const mobiledoc = Helpers.mobiledoc.makeMD(treeFn); + editor = new Editor(editorElement, {mobiledoc}); + return new PostEditor(editor); +} + +module('Unit: PostEditor', { + beforeEach() { + $('#qunit-fixture').append('
'); + editorElement = $('#editor')[0]; + }, + + afterEach() { + if (editor) { + editor.destroy(); + editor = null; + } + } +}); + +test('#deleteFrom in middle of marker deletes char before offset', (assert) => { + const postEditor = postEditorWithMobiledoc(({post, section, marker}) => + post([ + section('P', [marker('abc def')]) + ]) + ); + + const nextPosition = postEditor.deleteFrom({marker: getMarker(0,0), offset:4}); + postEditor.complete(); + + assert.equal(getMarker(0, 0).value, 'abcdef'); + assert.deepEqual(nextPosition, {currentMarker: getMarker(0, 0), currentOffset: 3}); +}); + + +test('#deleteFrom (forward) in middle of marker deletes char after offset', (assert) => { + const postEditor = postEditorWithMobiledoc(({post, section, marker}) => + post([ + section('p', [marker('abc def')]) + ]) + ); + + const nextPosition = postEditor.deleteFrom({marker: getMarker(0,0), offset:3}, FORWARD); + postEditor.complete(); + + assert.equal(getMarker(0, 0).value, 'abcdef'); + assert.deepEqual(nextPosition, {currentMarker: getMarker(0, 0), currentOffset: 3}); +}); + +test('#deleteFrom offset 0 joins section with previous if first marker', (assert) => { + const postEditor = postEditorWithMobiledoc(({post, section, marker}) => + post([ + section('P', [marker('abc')]), + section('P', [marker('def')]) + ]) + ); + + const nextPosition = postEditor.deleteFrom({marker: getMarker(1,0), offset:0}); + postEditor.complete(); + + assert.equal(editor.post.sections.length, 1, + 'sections joined'); + assert.equal(getSection(0).markers.length, 2, + 'joined section has 2 markers'); + + let newMarkers = getSection(0).markers.toArray(); + let newValues = newMarkers.map(m => m.value); + + assert.deepEqual(newValues, ['abc','def'], 'new markers have correct values'); + + assert.deepEqual(nextPosition, + {currentMarker: getMarker(0, 0), currentOffset: newValues[0].length}); +}); + +test('#deleteFrom (FORWARD) end of marker joins section with next if last marker', (assert) => { + const postEditor = postEditorWithMobiledoc(({post, section, marker}) => + post([ + section('P', [marker('abc')]), + section('P', [marker('def')]) + ]) + ); + + let marker = getMarker(0,0); + const nextPosition = postEditor.deleteFrom({marker, offset:marker.length}, + FORWARD); + postEditor.complete(); + + assert.equal(editor.post.sections.length, 1, + 'sections joined'); + assert.equal(getSection(0).markers.length, 2, + 'joined section has 2 markers'); + + let newMarkers = getSection(0).markers.toArray(); + let newValues = newMarkers.map(m => m.value); + + assert.deepEqual(newValues, ['abc','def'], 'new markers have correct values'); + + assert.deepEqual(nextPosition, + {currentMarker: getMarker(0, 0), currentOffset: newValues[0].length}); +}); + +test('#deleteFrom offset 0 deletes last character of previous marker when there is one', (assert) => { + const postEditor = postEditorWithMobiledoc(({post, section, marker}) => + post([ + section('P', [marker('abc'), marker('def')]) + ]) + ); + + const nextPosition = postEditor.deleteFrom({marker: getMarker(0, 1), offset:0}); + postEditor.complete(); + + let markers = getSection(0).markers.toArray(); + let values = markers.map(m => m.value); + + assert.deepEqual(values, ['ab', 'def'], 'markers have correct values'); + + assert.deepEqual(nextPosition, + {currentMarker: getMarker(0, 0), currentOffset: values[0].length}); +}); + +test('#deleteFrom (FORWARD) end of marker deletes first character of next marker when there is one', (assert) => { + const postEditor = postEditorWithMobiledoc(({post, section, marker}) => + post([ + section('P', [marker('abc'), marker('def')]) + ]) + ); + + let marker = getMarker(0, 0); + const nextPosition = postEditor.deleteFrom({marker, offset:marker.length}, + FORWARD); + postEditor.complete(); + + let markers = getSection(0).markers.toArray(); + let values = markers.map(m => m.value); + + assert.deepEqual(values, ['abc', 'ef'], 'markers have correct values'); + + assert.deepEqual(nextPosition, + {currentMarker: getMarker(0, 0), currentOffset: values[0].length}); +});