Merge pull request #115 from bustlelabs/ignore-event-listeners-in-car…

…ds-114

Detect when cursor is in card and ignore editor event listeners when so

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);

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);
   }

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;

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() {

src/js/utils/cursor/position.js

@@ -1,59 +1,49 @@
-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;
 }
 
 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
    */

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);
-  }
 }

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();
+  });
+});

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) => {
   /*