diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js index f72a45c59..79b0ff472 100644 --- a/src/js/editor/editor.js +++ b/src/js/editor/editor.js @@ -219,23 +219,17 @@ class Editor { handleDeletion(event) { event.preventDefault(); - const offsets = this.cursor.offsets; + const range = this.cursor.offsets; if (this.cursor.hasSelection()) { - this.run(postEditor => { - postEditor.deleteRange(offsets); - }); - this.cursor.moveToSection(offsets.headSection, offsets.headSectionOffset); + this.run(postEditor => postEditor.deleteRange(range)); + this.cursor.moveToPosition(range.head); } else { - let results = this.run(postEditor => { - const {headSection, headSectionOffset} = offsets; - const key = Key.fromEvent(event); - - const deletePosition = {section: headSection, offset: headSectionOffset}, - direction = key.direction; - return postEditor.deleteFrom(deletePosition, direction); + const key = Key.fromEvent(event); + const nextPosition = this.run(postEditor => { + return postEditor.deleteFrom(range.head, key.direction); }); - this.cursor.moveToSection(results.currentSection, results.currentOffset); + this.cursor.moveToPosition(nextPosition); } } @@ -588,9 +582,9 @@ class Editor { this.handleNewline(event); } else if (key.isPrintable()) { if (this.cursor.hasSelection()) { - const offsets = this.cursor.offsets; - this.run(postEditor => postEditor.deleteRange(offsets)); - this.cursor.moveToSection(offsets.headSection, offsets.headSectionOffset); + const range = this.cursor.offsets; + this.run(postEditor => postEditor.deleteRange(range)); + this.cursor.moveToPosition(range.head); } } diff --git a/src/js/editor/post.js b/src/js/editor/post.js index 09fe39f67..3380fb19b 100644 --- a/src/js/editor/post.js +++ b/src/js/editor/post.js @@ -1,5 +1,7 @@ import { MARKUP_SECTION_TYPE } from '../models/markup-section'; import { LIST_ITEM_TYPE } from '../models/list-item'; +import { LIST_SECTION_TYPE } from '../models/list-section'; +import Position from '../utils/cursor/position'; import { filter, compact @@ -19,6 +21,25 @@ function isBlankAndListItem(section) { return isListItem(section) && section.isBlank; } +function isMarkerable(section) { + return !!section.markers; +} + +function isListSection(section) { + return section.type === LIST_SECTION_TYPE; +} + +// finds the immediately preceding section that is markerable +function findPreviousMarkerableSection(section) { + const prev = section.prev; + if (!prev) { return null; } + if (isMarkerable(prev)) { + return prev; + } else if (isListSection(prev)) { + return prev.items.tail; + } +} + class PostEditor { constructor(editor) { this.editor = editor; @@ -188,59 +209,68 @@ class PostEditor { * marker. * * @method deleteFrom - * @param {Object} position object with {section, offset} the marker and offset to delete from + * @param {Position} position object with {section, offset} the marker and offset to delete from * @param {Number} direction The direction to delete in (default is BACKWARD) - * @return {Object} {currentSection, currentOffset} for positioning the cursor + * @return {Position} for positioning the cursor * @public */ - deleteFrom({section, offset}, direction=DIRECTION.BACKWARD) { - if (section.markers.length) { - // {{marker, offset}} - let result = section.markerPositionAtOffset(offset); - if (direction === DIRECTION.BACKWARD) { - return this._deleteBackwardFrom(result); - } else { - return this._deleteForwardFrom(result); - } + deleteFrom(position, direction=DIRECTION.BACKWARD) { + if (direction === DIRECTION.BACKWARD) { + return this._deleteBackwardFrom(position); } else { - if (direction === DIRECTION.BACKWARD) { - if (isMarkupSection(section) && section.prev) { - let prevSection = section.prev; - prevSection.join(section); - prevSection.renderNode.markDirty(); - this.removeSection(section); - this.scheduleRerender(); - this.scheduleDidUpdate(); - return { currentSection: prevSection, currentOffset: prevSection.text.length }; - } else if (isListItem(section)) { - this.scheduleRerender(); - this.scheduleDidUpdate(); - - const results = this._convertListItemToMarkupSection(section); - return {currentSection: results.section, currentOffset: results.offset}; - } - } else if (section.prev || section.next) { - let nextSection = section.next || section.post.tail; + return this._deleteForwardFrom(position); + } + } + + _joinPositionToPreviousSection(position) { + const {section } = position; + let nextPosition = position.clone(); + + if (!isMarkerable(section)) { + throw new Error('Cannot join non-markerable section to previous section'); + } else if (isListItem(section)) { + nextPosition = this._convertListItemToMarkupSection(section); + } else { + const prevSection = findPreviousMarkerableSection(section); + + if (prevSection) { + const { beforeMarker } = prevSection.join(section); + prevSection.renderNode.markDirty(); this.removeSection(section); - return { currentSection: nextSection, currentOffset: 0 }; + + nextPosition.section = prevSection; + nextPosition.offset = beforeMarker ? + prevSection.offsetOfMarker(beforeMarker, beforeMarker.length) : 0; } } + + this.scheduleRerender(); + this.scheduleDidUpdate(); + + return nextPosition; } /** * delete 1 character in the FORWARD direction from the given position * @method _deleteForwardFrom + * @param {Position} position * @private */ - _deleteForwardFrom({marker, offset}) { - const nextCursorSection = marker.section, - nextCursorOffset = nextCursorSection.offsetOfMarker(marker, offset); + _deleteForwardFrom(position) { + return this._deleteForwardFromMarkerPosition(position); + } + + _deleteForwardFromMarkerPosition(markerPosition) { + const {marker, offset} = markerPosition; + const {section} = marker; + let nextPosition = new Position(section, section.offsetOfMarker(marker, offset)); if (offset === marker.length) { const nextMarker = marker.next; if (nextMarker) { - this._deleteForwardFrom({marker: nextMarker, offset: 0}); + const nextMarkerPosition = {marker: nextMarker, offset: 0}; + return this._deleteForwardFromMarkerPosition(nextMarkerPosition); } else { const nextSection = marker.section.next; if (nextSection && isMarkupSection(nextSection)) { @@ -261,10 +291,7 @@ class PostEditor { this.scheduleRerender(); this.scheduleDidUpdate(); - return { - currentSection: nextCursorSection, - currentOffset: nextCursorOffset - }; + return nextPosition; } _convertListItemToMarkupSection(listItem) { @@ -275,72 +302,35 @@ class PostEditor { this._replaceSection(listSection, compact(newSections)); - const newCursorPosition = { - section: newMarkupSection, - offset: 0 - }; - return newCursorPosition; + return new Position(newMarkupSection, 0); } /** * delete 1 character in the BACKWARD direction from the given position * @method _deleteBackwardFrom + * @param {Position} position * @private */ - _deleteBackwardFrom({marker, offset}) { - let nextCursorSection = marker.section, - nextCursorOffset = nextCursorSection.offsetOfMarker(marker, offset); + _deleteBackwardFrom(position) { + const { offset:sectionOffset } = position; - if (offset === 0) { - const prevMarker = marker.prev; + if (sectionOffset === 0) { + return this._joinPositionToPreviousSection(position); + } - if (prevMarker) { - return this._deleteBackwardFrom({marker: prevMarker, offset: prevMarker.length}); - } else { - const section = marker.section; - - if (isListItem(section)) { - const newCursorPos = this._convertListItemToMarkupSection(section); - nextCursorSection = newCursorPos.section; - nextCursorOffset = newCursorPos.offset; - } else { - const prevSection = section.prev; - if (prevSection && isMarkupSection(prevSection)) { - nextCursorSection = prevSection; - nextCursorOffset = prevSection.text.length; - - let { - beforeMarker - } = prevSection.join(marker.section); - prevSection.renderNode.markDirty(); - this.removeSection(marker.section); - - nextCursorSection = prevSection; - - if (beforeMarker) { - nextCursorOffset = prevSection.offsetOfMarker(beforeMarker, beforeMarker.length); - } else { - nextCursorOffset = 0; - } - } - } - } + let nextPosition = position.clone(); + const { marker, offset:markerOffset } = position.markerPosition; - } else if (offset <= marker.length) { - const offsetToDeleteAt = offset - 1; - marker.deleteValueAtOffset(offsetToDeleteAt); - nextCursorOffset--; - marker.renderNode.markDirty(); - this._coalesceMarkers(marker.section); - } + const offsetToDeleteAt = markerOffset - 1; + marker.deleteValueAtOffset(offsetToDeleteAt); + nextPosition.offset -= 1; + marker.renderNode.markDirty(); + this._coalesceMarkers(marker.section); this.scheduleRerender(); this.scheduleDidUpdate(); - return { - currentSection: nextCursorSection, - currentOffset: nextCursorOffset - }; + return nextPosition; } /** diff --git a/src/js/models/marker.js b/src/js/models/marker.js index 9ec6c0fcf..f2892dd16 100644 --- a/src/js/models/marker.js +++ b/src/js/models/marker.js @@ -77,6 +77,9 @@ const Marker = class Marker extends LinkedItem { // delete the character at this offset, // update the value with the new value deleteValueAtOffset(offset) { + if (offset < 0 || offset > this.length) { + throw new Error(`Invalid offset "${offset}"`); + } const [ left, right ] = [ this.value.slice(0, offset), this.value.slice(offset+1) diff --git a/src/js/models/post.js b/src/js/models/post.js index fc4e92f29..3faaf7626 100644 --- a/src/js/models/post.js +++ b/src/js/models/post.js @@ -134,7 +134,7 @@ export default class Post { return containedSections; } - // return the next section that has markers afer this one + // return the next section that has markers after this one _nextMarkerableSection(section) { if (!section) { return null; } const isMarkerable = s => !!s.markers; @@ -143,7 +143,7 @@ export default class Post { const isChild = s => s.parent && !s.post; const parent = s => s.parent; - let next = section.next; + const next = section.next; if (next) { if (isMarkerable(next)) { return next; diff --git a/src/js/utils/cursor.js b/src/js/utils/cursor.js index a11ee3ace..ec6cdcc68 100644 --- a/src/js/utils/cursor.js +++ b/src/js/utils/cursor.js @@ -63,43 +63,35 @@ const Cursor = class Cursor { // moves cursor to the start of the section moveToSection(section, offsetInSection=0) { - const {marker, offset} = section.markerPositionAtOffset(offsetInSection); - if (marker) { - this.moveToMarker(marker, offset); - } else { - this._moveToNode(section.renderNode.element, offsetInSection); - } + this.moveToPosition(new Position(section, offsetInSection)); } - // moves cursor to marker - moveToMarker(headMarker, headOffset=0, tailMarker=headMarker, tailOffset=headOffset) { - if (!headMarker) { throw new Error('Cannot move cursor to marker without a marker'); } - const headElement = headMarker.renderNode.element; - const tailElement = tailMarker.renderNode.element; - - this._moveToNode(headElement, headOffset, tailElement, tailOffset); + selectSections(sections) { + const headSection = sections[0], tailSection = sections[sections.length - 1]; + const range = Range.create(headSection, 0, tailSection, tailSection.length); + this.selectRange(range); } - selectSections(sections) { - const headSection = sections[0], - tailSection = sections[sections.length - 1]; + _findNodeForPosition(position) { + const { section } = position; + let node, offset; + if (section.isBlank) { + node = section.renderNode.element; + offset = 0; + } else { + const {marker, offsetInMarker} = position; + node = marker.renderNode.element; + offset = offsetInMarker; + } - const range = new Range( - new Position(headSection, 0), - new Position(tailSection, tailSection.text.length) - ); - this.selectRange(range); + return {node, offset}; } selectRange(range) { - const { - headMarker, - headMarkerOffset, - tailMarker, - tailMarkerOffset - } = range; - this.moveToMarker( - headMarker, headMarkerOffset, tailMarker, tailMarkerOffset); + const { head, tail } = range; + const { node:headNode, offset:headOffset } = this._findNodeForPosition(head), + { node:tailNode, offset:tailOffset } = this._findNodeForPosition(tail); + this._moveToNode(headNode, headOffset, tailNode, tailOffset); } get selection() { diff --git a/src/js/utils/cursor/position.js b/src/js/utils/cursor/position.js index fb43fdc07..0498e8774 100644 --- a/src/js/utils/cursor/position.js +++ b/src/js/utils/cursor/position.js @@ -46,6 +46,10 @@ const Position = class Position { this._inCard = isCardSection(section); } + clone() { + return new Position(this.section, this.offset); + } + get marker() { return this.markerPosition.marker; } diff --git a/src/js/utils/key.js b/src/js/utils/key.js index d7071b7b8..a0d230763 100644 --- a/src/js/utils/key.js +++ b/src/js/utils/key.js @@ -1,7 +1,7 @@ import Keycodes from './keycodes'; export const DIRECTION = { FORWARD: 1, - BACKWARD: 2 + BACKWARD: -1 }; export const MODIFIERS = { diff --git a/tests/acceptance/editor-list-test.js b/tests/acceptance/editor-list-test.js index 5525c60d9..e6769d4c0 100644 --- a/tests/acceptance/editor-list-test.js +++ b/tests/acceptance/editor-list-test.js @@ -5,8 +5,9 @@ const { module, test } = Helpers; let editor, editorElement; -function createEditorWithListMobiledoc() { - const mobiledoc = Helpers.mobiledoc.build(({post, listSection, listItem, marker}) => + +function listMobileDoc() { + return Helpers.mobiledoc.build(({post, listSection, listItem, marker}) => post([ listSection('ul', [ listItem([marker('first item')]), @@ -14,11 +15,17 @@ function createEditorWithListMobiledoc() { ]) ]) ); +} +function createEditorWithMobiledoc(mobiledoc) { editor = new Editor({mobiledoc}); editor.render(editorElement); } +function createEditorWithListMobiledoc() { + createEditorWithMobiledoc(listMobileDoc()); +} + module('Acceptance: Editor: Lists', { beforeEach() { editorElement = document.createElement('div'); @@ -176,7 +183,8 @@ test('hitting enter to add list item, deleting to remove it, adding new list ite Helpers.dom.triggerEnter(editor); - assert.equal($('#editor li').length, 2, 'removes newly added li after enter on last list item'); + assert.equal($('#editor li').length, 2, + 'removes newly added li after enter on last list item'); assert.equal($('#editor p').length, 2, 'adds a second p section'); Helpers.dom.insertText(editor, 'X'); @@ -206,3 +214,44 @@ test('hitting enter at empty last list item exists list', (assert) => { Helpers.dom.insertText(editor, 'X'); assert.hasElement('#editor p:contains(X)', 'text goes in right spot'); }); + +// https://github.com/bustlelabs/content-kit-editor/issues/117 +test('deleting at start of non-empty section after list item joins it with list item', (assert) => { + const mobiledoc = Helpers.mobiledoc.build(builder => { + const {post, markupSection, marker, listSection, listItem} = builder; + return post([ + listSection('ul', [listItem([marker('abc')])]), + markupSection('p', [marker('def')]) + ]); + }); + createEditorWithMobiledoc(mobiledoc); + + const p = $('#editor p:contains(def)')[0]; + Helpers.dom.moveCursorTo(p.childNodes[0], 0); + Helpers.dom.triggerDelete(editor); + + assert.hasNoElement('#editor p'); + assert.hasElement('#editor li:contains(abcdef)'); +}); + +// https://github.com/bustlelabs/content-kit-editor/issues/117 +test('deleting at start of empty section after list item joins it with list item', (assert) => { + const mobiledoc = Helpers.mobiledoc.build(builder => { + const {post, markupSection, marker, listSection, listItem} = builder; + return post([ + listSection('ul', [listItem([marker('abc')])]), + markupSection('p') + ]); + }); + createEditorWithMobiledoc(mobiledoc); + + const node = $('#editor p br')[0]; + Helpers.dom.moveCursorTo(node, 0); + Helpers.dom.triggerDelete(editor); + + assert.hasNoElement('#editor p', 'removes p'); + + Helpers.dom.insertText(editor, 'X'); + + assert.hasElement('#editor li:contains(abcX)', 'inserts text at right spot'); +}); diff --git a/tests/acceptance/editor-sections-test.js b/tests/acceptance/editor-sections-test.js index f70edc8bb..af7b14c28 100644 --- a/tests/acceptance/editor-sections-test.js +++ b/tests/acceptance/editor-sections-test.js @@ -348,7 +348,7 @@ test('keystroke of delete at start of section joins with previous section', (ass let secondSectionTextNode = editor.element.childNodes[1].firstChild; assert.equal(secondSectionTextNode.textContent, 'second section', - 'finds section section text node'); + 'precond - section section text node'); Helpers.dom.moveCursorTo(secondSectionTextNode, 0); Helpers.dom.triggerDelete(editor); @@ -361,10 +361,10 @@ test('keystroke of delete at start of section joins with previous section', (ass 'first sectionsecond section', 'joins two sections'); - assert.deepEqual(Helpers.dom.getCursorPosition(), - {node: secondSectionTextNode, - offset: secondSectionTextNode.textContent.length}, - 'cursor moves to end of first section'); + Helpers.dom.insertText(editor, 'X'); + + assert.hasElement('#editor p:contains(first sectionXsecond section)', + 'inserts text at correct spot'); }); diff --git a/tests/unit/editor/post-test.js b/tests/unit/editor/post-test.js index 177cd18f4..b92d7d2aa 100644 --- a/tests/unit/editor/post-test.js +++ b/tests/unit/editor/post-test.js @@ -6,6 +6,7 @@ import Helpers from '../../test-helpers'; import { DIRECTION } from 'content-kit-editor/utils/key'; import PostNodeBuilder from 'content-kit-editor/models/post-node-builder'; import Range from 'content-kit-editor/utils/cursor/range'; +import Position from 'content-kit-editor/utils/cursor/position'; const { FORWARD } = DIRECTION; @@ -65,11 +66,13 @@ test('#deleteFrom in middle of marker deletes char before offset', (assert) => { ]) ); - const nextPosition = postEditor.deleteFrom({section: getSection(0), offset:4}); + const position = new Position(getSection(0), 4); + const nextPosition = postEditor.deleteFrom(position); postEditor.complete(); assert.equal(getMarker(0, 0).value, 'abcdef'); - assert.deepEqual(nextPosition, {currentSection: getSection(0), currentOffset: 3}); + assert.ok(nextPosition.section === getSection(0), 'correct position section'); + assert.equal(nextPosition.offset, 3, 'correct position offset'); }); @@ -80,11 +83,13 @@ test('#deleteFrom (forward) in middle of marker deletes char after offset', (ass ]) ); - const nextPosition = postEditor.deleteFrom({section: getSection(0), offset:3}, FORWARD); + const position = new Position(getSection(0), 3); + const nextPosition = postEditor.deleteFrom(position, FORWARD); postEditor.complete(); assert.equal(getMarker(0, 0).value, 'abcdef'); - assert.deepEqual(nextPosition, {currentSection: getSection(0), currentOffset: 3}); + assert.ok(nextPosition.section === getSection(0), 'correct position section'); + assert.equal(nextPosition.offset, 3, 'correct position offset'); }); test('#deleteFrom offset 0 joins section with previous if first marker', (assert) => { @@ -95,7 +100,8 @@ test('#deleteFrom offset 0 joins section with previous if first marker', (assert ]) ); - const nextPosition = postEditor.deleteFrom({section: getSection(1), offset:0}); + const position = new Position(getSection(1), 0); + const nextPosition = postEditor.deleteFrom(position); postEditor.complete(); assert.equal(editor.post.sections.length, 1, @@ -103,13 +109,9 @@ test('#deleteFrom offset 0 joins section with previous if first marker', (assert 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, - {currentSection: getSection(0), currentOffset: newValues[0].length}); + assert.equal(getSection(0).text, 'abcdef', 'text is joined'); + assert.ok(nextPosition.section === getSection(0), 'correct position section'); + assert.equal(nextPosition.offset, 'abc'.length, 'correct position offset'); }); test('#deleteFrom (FORWARD) end of marker joins section with next if last marker', (assert) => { @@ -121,8 +123,8 @@ test('#deleteFrom (FORWARD) end of marker joins section with next if last marker ); let section = getSection(0); - const nextPosition = postEditor.deleteFrom({section, offset: 3}, - FORWARD); + const position = new Position(section, 3); + const nextPosition = postEditor.deleteFrom(position, FORWARD); postEditor.complete(); assert.equal(editor.post.sections.length, 1, @@ -130,13 +132,9 @@ test('#deleteFrom (FORWARD) end of marker joins section with next if last marker 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, - {currentSection: getSection(0), currentOffset: newValues[0].length}); + assert.equal(getSection(0).text, 'abcdef', 'text is joined'); + assert.ok(nextPosition.section === getSection(0), 'correct position section'); + assert.equal(nextPosition.offset, 'abc'.length, 'correct position offset'); }); test('#deleteFrom offset 0 deletes last character of previous marker when there is one', (assert) => { @@ -146,16 +144,13 @@ test('#deleteFrom offset 0 deletes last character of previous marker when there ]) ); - const nextPosition = postEditor.deleteFrom({section: getSection(0), offset:3}); + const position = new Position(getSection(0), 3); + const nextPosition = postEditor.deleteFrom(position); 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, - {currentSection: getSection(0), currentOffset: values[0].length}); + assert.equal(getSection(0).text, 'abdef', 'text is deleted'); + assert.ok(nextPosition.section === getSection(0), 'correct position section'); + assert.equal(nextPosition.offset, 'ab'.length, 'correct position offset'); }); test('#deleteFrom (FORWARD) end of marker deletes first character of next marker when there is one', (assert) => { @@ -166,17 +161,13 @@ test('#deleteFrom (FORWARD) end of marker deletes first character of next marker ); let section = getSection(0); - const nextPosition = postEditor.deleteFrom({section, offset: 3}, - FORWARD); + const position = new Position(section, 3); + const nextPosition = postEditor.deleteFrom(position, 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, - {currentSection: getSection(0), currentOffset: values[0].length}); + assert.equal(getSection(0).text, 'abcef', 'text is correct'); + assert.ok(nextPosition.section === getSection(0), 'correct position section'); + assert.equal(nextPosition.offset, 'abc'.length, 'correct position offset'); }); @@ -213,9 +204,7 @@ test('#deleteRange when within the same marker', (assert) => { renderBuiltAbstract(post); const range = Range.create(section, 3, section, 4); - postEditor.deleteRange(range); - postEditor.complete(); assert.equal(post.sections.head.text, 'abcdef');