From e6bfdef384fa73acb66d5f94810c0b7491eb15e9 Mon Sep 17 00:00:00 2001 From: Cory Forsyth Date: Tue, 8 Sep 2015 12:32:25 -0400 Subject: [PATCH] Detect when cursor is in card and ignore editor event listeners when so fixes #114 --- src/js/editor/editor.js | 2 + src/js/models/render-tree.js | 3 + src/js/renderers/editor-dom.js | 8 +- src/js/utils/cursor.js | 18 ++-- src/js/utils/cursor/position.js | 128 ++++++++++++------------ src/js/utils/cursor/range.js | 4 - tests/acceptance/editor-cards-test.js | 33 ++++-- tests/unit/renderers/editor-dom-test.js | 34 ++++++- 8 files changed, 138 insertions(+), 92 deletions(-) diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js index e1503ece3..95d16a81d 100644 --- a/src/js/editor/editor.js +++ b/src/js/editor/editor.js @@ -561,6 +561,8 @@ class Editor { } handleEvent(eventName, ...args) { + if (this.cursor.isInCard()) { return; } + const methodName = `handle${capitalize(eventName)}`; if (!this[methodName]) { throw new Error(`No handler for ${eventName}`); } this[methodName](...args); diff --git a/src/js/models/render-tree.js b/src/js/models/render-tree.js index 5141b4db9..7ff9c4916 100644 --- a/src/js/models/render-tree.js +++ b/src/js/models/render-tree.js @@ -6,6 +6,9 @@ export default class RenderTree { this.node = node; this.elements = new ElementMap(); } + get rootElement() { + return this.node.element; + } getElementRenderNode(element) { return this.elements.get(element); } diff --git a/src/js/renderers/editor-dom.js b/src/js/renderers/editor-dom.js index be0e5ff92..47d078d79 100644 --- a/src/js/renderers/editor-dom.js +++ b/src/js/renderers/editor-dom.js @@ -159,6 +159,7 @@ class Visitor { renderNode.element = element; attachRenderNodeElementToDOM(renderNode, element, originalElement); + renderNode.renderTree.elements.set(element, renderNode); if (section.markers.length) { const visitAll = true; @@ -207,9 +208,9 @@ class Visitor { parentElement = renderNode.parent.element; } - let markerNode = renderMarker(marker, parentElement, renderNode.prev); - renderNode.renderTree.elements.set(markerNode, renderNode); - renderNode.element = markerNode; + const element = renderMarker(marker, parentElement, renderNode.prev); + renderNode.renderTree.elements.set(element, renderNode); + renderNode.element = element; } [IMAGE_SECTION_TYPE](renderNode, section) { @@ -243,6 +244,7 @@ class Visitor { attachRenderNodeElementToDOM(renderNode, element, originalElement); + renderNode.renderTree.elements.set(element, renderNode); if (card) { const cardNode = new CardNode(editor, card, section, element, options); renderNode.cardNode = cardNode; diff --git a/src/js/utils/cursor.js b/src/js/utils/cursor.js index 5b7cee9c8..e534da4a7 100644 --- a/src/js/utils/cursor.js +++ b/src/js/utils/cursor.js @@ -27,6 +27,11 @@ const Cursor = class Cursor { return this._hasCollapsedSelection() || this._hasSelection(); } + isInCard() { + const {head, tail} = this.offsets; + return head && tail && (head._inCard || tail._inCard); + } + hasSelection() { return this._hasSelection(); } @@ -37,21 +42,16 @@ const Cursor = class Cursor { get offsets() { if (!this.hasCursor()) { return {}; } - const { sections } = this.post; - const { selection } = this; + const { selection, renderTree } = this; const { headNode, headOffset, tailNode, tailOffset } = comparePosition(selection); - const headPosition = Position.fromNode( - this.renderTree, sections, headNode, headOffset - ); - const tailPosition = Position.fromNode( - this.renderTree, sections, tailNode, tailOffset - ); + const headPosition = Position.fromNode(renderTree, headNode, headOffset); + const tailPosition = Position.fromNode(renderTree, tailNode, tailOffset); - return Range.fromPositions(headPosition, tailPosition); + return new Range(headPosition, tailPosition); } get activeSections() { diff --git a/src/js/utils/cursor/position.js b/src/js/utils/cursor/position.js index 051673127..fb43fdc07 100644 --- a/src/js/utils/cursor/position.js +++ b/src/js/utils/cursor/position.js @@ -1,52 +1,41 @@ -import { detect } from 'content-kit-editor/utils/array-utils'; -import { - detectParentNode, - isTextNode, - walkTextNodes -} from 'content-kit-editor/utils/dom-utils'; +import { isTextNode, walkTextNodes } from 'content-kit-editor/utils/dom-utils'; import { MARKUP_SECTION_TYPE } from 'content-kit-editor/models/markup-section'; import { LIST_ITEM_TYPE } from 'content-kit-editor/models/list-item'; -import { MARKER_TYPE } from 'content-kit-editor/models/marker'; - -// FIXME This assumes that all sections are children of the Post, -// but that isn't a valid assumption, some sections (ListItem) are -// grand-children of the post. -function findSectionContaining(sections, childNode) { - const { result: section } = detectParentNode(childNode, node => { - return detect(sections, section => { - return section.renderNode.element === node; - }); - }); - return section; +import { CARD_TYPE } from 'content-kit-editor/models/card'; + +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 findSectionFromNode(node, renderTree) { - const renderNode = renderTree.getElementRenderNode(node); - const postNode = renderNode && renderNode.postNode; - return postNode; +function isCardSection(section) { + return section.type === CARD_TYPE; } -// cursorElement is the DOM element that the browser reports that the cursor -// is on -function findOffsetInSection(sectionElement, cursorElement, offsetInElement) { - if (!isTextNode(cursorElement)) { - // if the cursor element is not a text node, assume that the cursor is - // on the section element itself and return 0 - return 0; +function findParentSectionFromNode(renderTree, node) { + let renderNode; + while (node && node !== renderTree.rootElement) { + renderNode = renderTree.getElementRenderNode(node); + if (renderNode && isSection(renderNode.postNode)) { + return renderNode.postNode; + } + node = node.parentNode; } +} +function findOffsetInElement(elementNode, textNode, offsetInTextNode) { let offset = 0, found = false; - walkTextNodes(sectionElement, (textNode) => { + walkTextNodes(elementNode, _textNode => { if (found) { return; } - - if (textNode === cursorElement) { + if (_textNode === textNode) { found = true; - offset += offsetInElement; + offset += offsetInTextNode; } else { - offset += textNode.textContent.length; + offset += _textNode.textContent.length; } }); - return offset; } @@ -54,6 +43,7 @@ const Position = class Position { constructor(section, offset=0) { this.section = section; this.offset = offset; + this._inCard = isCardSection(section); } get marker() { @@ -69,45 +59,55 @@ const Position = class Position { this.offset === position.offset; } - static fromNode(renderTree, sections, node, offsetInNode) { - // Sections and markers are registered into the element/renderNode map - let renderNode = renderTree.getElementRenderNode(node), - section = null, - offsetInSection = null; - - if (renderNode) { - switch (renderNode.postNode.type) { - case MARKUP_SECTION_TYPE: - section = renderNode.postNode; - offsetInSection = offsetInNode; - break; - case LIST_ITEM_TYPE: - section = renderNode.postNode; - offsetInSection = offsetInNode; - break; - case MARKER_TYPE: - let marker = renderNode.postNode; - section = marker.section; - offsetInSection = section.offsetOfMarker(marker, offsetInNode); - break; - } + static fromNode(renderTree, node, offset) { + if (isTextNode(node)) { + return Position.fromTextNode(renderTree, node, offset); + } else { + return Position.fromElementNode(renderTree, node, offset); } + } + + static fromTextNode(renderTree, textNode, offsetInNode) { + const renderNode = renderTree.getElementRenderNode(textNode); + let section, offsetInSection; - if (!section) { - section = findSectionFromNode(node.parentNode, renderTree) || - findSectionContaining(sections, node); + if (renderNode) { + let marker = renderNode.postNode; + section = marker.section; - if (section) { - const sectionElement = section.renderNode.element; - offsetInSection = findOffsetInSection(sectionElement, node, offsetInNode); + if (!section) { throw new Error(`Could not find parent section for mapped text node "${textNode.textContent}"`); } + offsetInSection = section.offsetOfMarker(marker, offsetInNode); + } else { + // all text nodes should be rendered by markers except: + // * text nodes inside cards + // * text nodes created by the browser during text input + // both of these should have rendered parent sections, though + section = findParentSectionFromNode(renderTree, textNode); + if (!section) { throw new Error(`Could not find parent section for un-mapped text node "${textNode.textContent}"`); } + + if (isCardSection(section)) { + offsetInSection = 0; // we don't care about offsets in card sections } else { - throw new Error('Unable to determine section for cursor'); + offsetInSection = findOffsetInElement(section.renderNode.element, + textNode, offsetInNode); } } return new Position(section, offsetInSection); } + static fromElementNode(renderTree, elementNode) { + let section, offsetInSection = 0; + + section = findParentSectionFromNode(renderTree, elementNode); + if (!section) { throw new Error('Could not find parent section from element node'); } + + // 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); + } + /** * @private */ diff --git a/src/js/utils/cursor/range.js b/src/js/utils/cursor/range.js index 205d2b575..b78e61d6b 100644 --- a/src/js/utils/cursor/range.js +++ b/src/js/utils/cursor/range.js @@ -32,8 +32,4 @@ export default class Range { get tailMarkerOffset() { return this.tail.offsetInMarker; } - - static fromPositions(head, tail) { - return new Range(head, tail); - } } diff --git a/tests/acceptance/editor-cards-test.js b/tests/acceptance/editor-cards-test.js index 11adb2677..f1a634073 100644 --- a/tests/acceptance/editor-cards-test.js +++ b/tests/acceptance/editor-cards-test.js @@ -1,20 +1,14 @@ import { Editor } from 'content-kit-editor'; import Helpers from '../test-helpers'; -import { MOBILEDOC_VERSION } from 'content-kit-editor/renderers/mobiledoc'; -const { test, module } = QUnit; +const { test, module } = Helpers; let fixture, editor, editorElement; +const cardText = 'card text'; -const mobiledoc = { - version: MOBILEDOC_VERSION, - sections: [ - [], - [ - [10, 'simple-card'] - ] - ] -}; +const mobiledoc = Helpers.mobiledoc.build(({post, cardSection}) => { + return post([cardSection('simple-card')]); +}); const simpleCard = { name: 'simple-card', @@ -23,6 +17,7 @@ const simpleCard = { let button = document.createElement('button'); button.setAttribute('id', 'display-button'); element.appendChild(button); + element.appendChild(document.createTextNode(cardText)); button.onclick = env.edit; return {button}; }, @@ -81,3 +76,19 @@ test('changing to display state triggers update on editor', (assert) => { 'update is triggered after switching to display mode'); }); +test('editor listeners are quieted for card actions', (assert) => { + const done = assert.async(); + + const cards = [simpleCard]; + editor = new Editor({mobiledoc, cards}); + editor.render(editorElement); + + Helpers.dom.selectText(cardText, editorElement); + Helpers.dom.triggerEvent(document, 'mouseup'); + + setTimeout(() => { + // FIXME should have a better assertion here + assert.ok(true, 'made it here with no javascript errors'); + done(); + }); +}); diff --git a/tests/unit/renderers/editor-dom-test.js b/tests/unit/renderers/editor-dom-test.js index 2a4e32c7f..30bb32cc6 100644 --- a/tests/unit/renderers/editor-dom-test.js +++ b/tests/unit/renderers/editor-dom-test.js @@ -76,9 +76,15 @@ test("renders a dirty post with un-rendered sections", (assert) => { { name: 'card', section: (builder) => builder.createCardSection('new-card') + }, + { + name: 'list-section', + section: (builder) => builder.createListSection('ul', [ + builder.createListItem([builder.createMarker('item')]) + ]) } ].forEach((testInfo) => { - test(`remove nodes with ${testInfo.name} section`, (assert) => { + test(`removes nodes with ${testInfo.name} section`, (assert) => { let post = builder.createPost(); let section = testInfo.section(builder); post.sections.append(section); @@ -563,6 +569,32 @@ test('removes list sections', (assert) => { assert.equal(node.element.innerHTML, expectedHTML, 'removes list section'); }); +test('includes card sections in renderTree element map', (assert) => { + const post = Helpers.postAbstract.build(({post, cardSection}) => + post([cardSection('simple-card')]) + ); + const cards = [{ + name: 'simple-card', + display: { + setup(element) { + element.setAttribute('id', 'simple-card'); + } + } + }]; + + const node = new RenderNode(post); + const renderTree = new RenderTree(node); + node.renderTree = renderTree; + render(renderTree, cards); + + $('#qunit-fixture')[0].appendChild(node.element); + + const element = $('#simple-card')[0]; + assert.ok(!!element, 'precond - simple card is rendered'); + assert.ok(!!renderTree.getElementRenderNode(element), + 'has render node for card element'); +}); + /* test("It renders a renderTree with rendered dirty section", (assert) => { /*