diff --git a/src/js/editor/post.js b/src/js/editor/post.js index b6b73c613..35e87cfc1 100644 --- a/src/js/editor/post.js +++ b/src/js/editor/post.js @@ -1,7 +1,6 @@ import { DEFAULT_TAG_NAME as DEFAULT_MARKUP_SECTION_TAG_NAME } from '../models/markup-section'; -import { isMarkerable } from '../models/_section'; import { POST_TYPE, MARKUP_SECTION_TYPE, LIST_ITEM_TYPE, LIST_SECTION_TYPE } from '../models/types'; import Position from '../utils/cursor/position'; import { isArrayEqual, forEach, filter, compact } from '../utils/array-utils'; @@ -11,14 +10,14 @@ import mixin from '../utils/mixin'; import assert from '../utils/assert'; function isJoinable(section1, section2) { - return isMarkerable(section1) && - isMarkerable(section2) && + return section1.isMarkerable && + section2.isMarkerable && section1.type === section2.type && section1.tagName === section2.tagName; } function endPosition(section) { - if (isMarkerable(section)) { + if (section.isMarkerable) { return new Position(section, section.length); } else if (section.type === LIST_SECTION_TYPE) { return endPosition(section.items.tail); @@ -261,7 +260,7 @@ class PostEditor { if (postNode.section) { this._markDirty(postNode.section); } - if (isMarkerable(postNode)) { + if (postNode.isMarkerable) { this.addCallback( CALLBACK_QUEUES.BEFORE_COMPLETE, () => this._coalesceMarkers(postNode)); } @@ -304,7 +303,7 @@ class PostEditor { const {section } = position; let nextPosition = position.clone(); - if (!isMarkerable(section)) { + if (!section.isMarkerable) { throw new Error('Cannot join non-markerable section to previous section'); } else if (isListItem(section)) { nextPosition = this._convertListItemToMarkupSection(section); @@ -378,7 +377,7 @@ class PostEditor { const { section } = position; let nextPosition = position.clone(); - if (!isMarkerable(section)) { + if (!section.isMarkerable) { throw new Error('Cannot join non-markerable section to next section'); } else { const next = section.immediatelyNextMarkerableSection(); diff --git a/src/js/models/_section.js b/src/js/models/_section.js index 7485c9243..14ec5872a 100644 --- a/src/js/models/_section.js +++ b/src/js/models/_section.js @@ -2,10 +2,6 @@ import { LIST_ITEM_TYPE } from './types'; import { normalizeTagName } from '../utils/dom-utils'; import LinkedItem from '../utils/linked-item'; -export function isMarkerable(section) { - return !!section.markers; -} - function isChild(section) { return section.type === LIST_ITEM_TYPE; } @@ -59,7 +55,7 @@ export default class Section extends LinkedItem { immediatelyNextMarkerableSection() { let next = this.nextLeafSection(); - while (next && !isMarkerable(next)) { + while (next && !next.isMarkerable) { next = next.nextLeafSection(); } return next; diff --git a/src/js/models/post.js b/src/js/models/post.js index 40eae8fad..68816199f 100644 --- a/src/js/models/post.js +++ b/src/js/models/post.js @@ -2,7 +2,6 @@ import { POST_TYPE } from './types'; import LinkedList from 'content-kit-editor/utils/linked-list'; import { forEach, compact } from 'content-kit-editor/utils/array-utils'; import Set from 'content-kit-editor/utils/set'; -import { isMarkerable } from 'content-kit-editor/models/_section'; import MobiledocRenderer from 'content-kit-editor/renderers/mobiledoc'; export default class Post { @@ -135,7 +134,7 @@ export default class Post { walkMarkerableSections(range, callback) { this.walkLeafSections(range, section => { - if (isMarkerable(section)) { + if (section.isMarkerable) { callback(section); } }); @@ -173,7 +172,7 @@ export default class Post { _nextMarkerableSection(section) { let nextSection = this._nextLeafSection(section); - while (nextSection && !isMarkerable(nextSection)) { + while (nextSection && !nextSection.isMarkerable) { nextSection = this._nextLeafSection(nextSection); } @@ -216,7 +215,7 @@ export default class Post { this.walkPostSections(range, section => { let newSection; - if (isMarkerable(section)) { + if (section.isMarkerable) { newSection = builder.createMarkupSection(section.tagName); let currentRange = range.trimTo(section); forEach( diff --git a/src/js/utils/cursor.js b/src/js/utils/cursor.js index 12dee3546..a430a1fe1 100644 --- a/src/js/utils/cursor.js +++ b/src/js/utils/cursor.js @@ -131,13 +131,14 @@ const Cursor = class Cursor { } /** - * @private * @param {textNode} node * @param {integer} offset - * @param {textNode} endNode (default: node) - * @param {integer} endOffset (default: offset) + * @param {textNode} endNode + * @param {integer} endOffset + * @param {integer} direction forward or backward, default forward + * @private */ - _moveToNode(node, offset, endNode, endOffset, direction) { + _moveToNode(node, offset, endNode, endOffset, direction=DIRECTION.FORWARD) { this.clearSelection(); if (direction === DIRECTION.BACKWARD) { diff --git a/src/js/utils/cursor/position.js b/src/js/utils/cursor/position.js index e4beb2daf..d1abcc63d 100644 --- a/src/js/utils/cursor/position.js +++ b/src/js/utils/cursor/position.js @@ -1,21 +1,12 @@ import { isTextNode, findOffsetInElement } from 'content-kit-editor/utils/dom-utils'; -import { - MARKUP_SECTION_TYPE, LIST_ITEM_TYPE, CARD_TYPE -} from 'content-kit-editor/models/types'; import { DIRECTION } from 'content-kit-editor/utils/key'; import assert from 'content-kit-editor/utils/assert'; function isSection(postNode) { if (!(postNode && postNode.type)) { return false; } - return postNode.type === MARKUP_SECTION_TYPE || - postNode.type === LIST_ITEM_TYPE || - postNode.type === CARD_TYPE; -} - -function isCardSection(section) { - return section.type === CARD_TYPE; + return postNode.isMarkerable || postNode.isCardSection; } function findParentSectionFromNode(renderTree, node) { @@ -28,7 +19,7 @@ function findParentSectionFromNode(renderTree, node) { } function findOffsetInSection(section, node, offset) { - if (!isCardSection(section)) { + if (!section.isCardSection) { return findOffsetInElement(section.renderNode.element, node, offset); } @@ -46,7 +37,7 @@ const Position = class Position { constructor(section, offset=0) { this.section = section; this.offset = offset; - this._inCard = isCardSection(section); + this._inCard = section.isCardSection; } static emptyPosition() { @@ -143,32 +134,37 @@ const Position = class Position { } static fromElementNode(renderTree, elementNode, offset) { + let section, offsetInSection; + // The browser may change the reported selection to equal the editor's root // element if the user clicks an element that is immediately removed, // which can happen when clicking to remove a card. if (elementNode === renderTree.rootElement) { - return Position.emptyPosition(); - } - - let section = findParentSectionFromNode(renderTree, elementNode); - if (!section) { throw new Error('Could not find parent section from element node'); } - - let offsetInSection; - - if (isCardSection(section)) { - // Selections in cards are usually made on a text node containing a ‌ - // on one side or the other of the card but some scenarios will result in - // selecting the card's wrapper div. If the offset is 2 we've selected - // the final zwnj and should consider the cursor at the end of the card (offset 1). Otherwise, - // the cursor is at the start of the card - offsetInSection = offset < 2 ? 0 : 1; + let post = renderTree.rootNode.postNode; + if (offset === 0) { + section = post.sections.head; + offsetInSection = 0; + } else { + section = post.sections.tail; + offsetInSection = section.length; + } } else { - offsetInSection = 0; + section = findParentSectionFromNode(renderTree, elementNode); + assert('Could not find parent section from element node', !!section); + + if (section.isCardSection) { + // Selections in cards are usually made on a text node containing a ‌ + // on one side or the other of the card but some scenarios (Firefox) will result in + // selecting the card's wrapper div. If the offset is 2 we've selected + // the final zwnj and should consider the cursor at the end of the card (offset 1). Otherwise, + // the cursor is at the start of the card + offsetInSection = offset < 2 ? 0 : 1; + } else { + // The offset is 0 if the cursor is on an element node (e.g., a
tag in + // a blank markup section) + offsetInSection = 0; + } } - - // FIXME We assume that offsetInSection will always be 0 because we assume - // that only empty br tags (offsetInSection=0) will be those that cause - // us to call `fromElementNode`. This may not be a reliable assumption. return new Position(section, offsetInSection); } diff --git a/src/js/utils/dom-utils.js b/src/js/utils/dom-utils.js index 06461863c..1f6e0cf5a 100644 --- a/src/js/utils/dom-utils.js +++ b/src/js/utils/dom-utils.js @@ -36,11 +36,15 @@ function clearChildNodes(element) { } /** - * @return {Boolean} true when the child node is contained by (and not - * the same as) the parent node + * @return {Boolean} true when the child node is contained or the same as + * (e.g., inclusive containment) the parent node * see https://github.com/webmodules/node-contains/blob/master/index.js + * Mimics the behavior of `Node.contains`, which is broken in IE 10 */ function containsNode(parentNode, childNode) { + if (parentNode === childNode) { + return true; + } const position = parentNode.compareDocumentPosition(childNode); return !!(position & Node.DOCUMENT_POSITION_CONTAINED_BY); } diff --git a/tests/acceptance/basic-editor-test.js b/tests/acceptance/basic-editor-test.js index 11df7bae5..2a8687c95 100644 --- a/tests/acceptance/basic-editor-test.js +++ b/tests/acceptance/basic-editor-test.js @@ -151,3 +151,25 @@ test('typing when on the start of a card is blocked', (assert) => { Helpers.dom.insertText(editor, 'Y'); assert.hasNoElement('#editor div:contains(Y)'); }); + +// see https://github.com/bustlelabs/content-kit-editor/issues/215 +test('select-all and type text works ok', (assert) => { + const mobiledoc = Helpers.mobiledoc.build(({post, markupSection, marker}) => { + return post([ + markupSection('p', [marker('abc')]) + ]); + }); + editor = new Editor({mobiledoc, cards}); + editor.render(editorElement); + + Helpers.dom.moveCursorTo(editorElement.firstChild, 0); + document.execCommand('selectAll'); + + assert.selectedText('abc', 'precond - abc is selected'); + assert.hasElement('#editor p:contains(abc)', 'precond - renders p'); + + Helpers.dom.insertText(editor, 'X'); + + assert.hasNoElement('#editor p:contains(abc)', 'replaces existing text'); + assert.hasElement('#editor p:contains(X)', 'inserts text'); +}); diff --git a/tests/acceptance/cursor-position-test.js b/tests/acceptance/cursor-position-test.js index da2c591f1..d517fdff3 100644 --- a/tests/acceptance/cursor-position-test.js +++ b/tests/acceptance/cursor-position-test.js @@ -199,8 +199,28 @@ test('cursor focused on card wrapper with 0 offset', (assert) => { assert.equal(offsets.tail.offset, 0, 'Cursor tail is positioned at offset 0'); }); - +// see https://github.com/bustlelabs/content-kit-editor/issues/215 +test('selecting the entire editor element reports a selection range of the entire post', (assert) => { + let mobiledoc = Helpers.mobiledoc.build(({post, markupSection, marker}) => { + return post([ + markupSection('p', [marker('abc')]), + markupSection('p', [marker('1234')]) + ]); + }); + editor = new Editor({mobiledoc}); + editor.render(editorElement); + + Helpers.dom.moveCursorTo(editorElement, 0, + editorElement, editorElement.childNodes.length); + let { offsets } = editor.cursor; + assert.ok(offsets.head.section === editor.post.sections.head, + 'head section correct'); + assert.equal(offsets.head.offset, 0, 'head offset 0'); + assert.ok(offsets.tail.section === editor.post.sections.tail, + 'tail section correct'); + assert.equal(offsets.tail.offset, 4, 'tail offset equals section length'); +}); //inside card wrapper div and before starting zwnj reports its position correctly'); diff --git a/tests/unit/utils/cursor-range-test.js b/tests/unit/utils/cursor-range-test.js index 59c1c857e..950d96ecf 100644 --- a/tests/unit/utils/cursor-range-test.js +++ b/tests/unit/utils/cursor-range-test.js @@ -1,18 +1,13 @@ import Helpers from '../../test-helpers'; -import Position from 'content-kit-editor/utils/cursor/position'; import Range from 'content-kit-editor/utils/cursor/range'; const {module, test} = Helpers; -function makeRange(s1, o1, s2, o2) { - return new Range(new Position(s1, o1), new Position(s2, o2)); -} - module('Unit: Utils: Range'); test('#trimTo(section) when range covers only one section', (assert) => { const section = Helpers.postAbstract.build(({markupSection}) => markupSection()); - const range = makeRange(section, 0, section, 5); + const range = Range.create(section, 0, section, 5); const newRange = range.trimTo(section); assert.ok(newRange.head.section === section, 'head section is correct'); @@ -28,7 +23,7 @@ test('#trimTo head section', (assert) => { const section2 = Helpers.postAbstract.build( ({markupSection, marker}) => markupSection('p', [marker(text)])); - const range = makeRange(section1, 0, section2, 5); + const range = Range.create(section1, 0, section2, 5); const newRange = range.trimTo(section1); assert.ok(newRange.head.section === section1, 'head section'); @@ -44,7 +39,7 @@ test('#trimTo tail section', (assert) => { const section2 = Helpers.postAbstract.build( ({markupSection, marker}) => markupSection('p', [marker(text)])); - const range = makeRange(section1, 0, section2, 5); + const range = Range.create(section1, 0, section2, 5); const newRange = range.trimTo(section2); assert.ok(newRange.head.section === section2, 'head section'); @@ -62,12 +57,11 @@ test('#trimTo middle section', (assert) => { const section3 = Helpers.postAbstract.build( ({markupSection, marker}) => markupSection('p', [marker(text)])); - const range = makeRange(section1, 0, section3, 5); + const range = Range.create(section1, 0, section3, 5); const newRange = range.trimTo(section2); assert.ok(newRange.head.section === section2, 'head section'); assert.ok(newRange.tail.section === section2, 'tail section'); assert.equal(newRange.head.offset, 0, 'head offset'); assert.equal(newRange.tail.offset, section2.text.length, 'tail offset'); - });