Merge pull request #305 from bustlelabs/refactor-cursor-positioning

Refactor cursor positioning

src/js/models/_section.js

@@ -14,6 +14,8 @@ export default class Section extends LinkedItem {
     this.type = type;
     this.isMarkerable = false;
     this.isNested = false;
+    this.isSection = true;
+    this.isLeafSection = true;
   }
 
   set tagName(val) {

src/js/models/list-section.js

@@ -16,6 +16,7 @@ export default class ListSection extends Section {
     super(LIST_SECTION_TYPE);
     this.tagName = tagName;
     this.isListSection = true;
+    this.isLeafSection = false;
 
     this.items = new LinkedList({
       adoptItem: i => {

src/js/models/post.js

@@ -4,6 +4,7 @@ import { forEach, compact } from 'mobiledoc-kit/utils/array-utils';
 import Set from 'mobiledoc-kit/utils/set';
 import mobiledocRenderers from 'mobiledoc-kit/renderers/mobiledoc';
 import Range from 'mobiledoc-kit/utils/cursor/range';
+import Position from 'mobiledoc-kit/utils/cursor/position';
 
 export default class Post {
   constructor() {
@@ -14,6 +15,22 @@ export default class Post {
     });
   }
 
+  headPosition() {
+    if (this.isBlank) {
+      return Position.blankPosition();
+    } else {
+      return this.sections.head.headPosition();
+    }
+  }
+
+  tailPosition() {
+    if (this.isBlank) {
+      return Position.blankPosition();
+    } else {
+      return this.sections.tail.tailPosition();
+    }
+  }
+
   get isBlank() {
     return this.sections.isEmpty;
   }

src/js/renderers/editor-dom.js

@@ -15,10 +15,11 @@ import { MARKUP_SECTION_ELEMENT_NAMES } from '../models/markup-section';
 import assert from '../utils/assert';
 import { TAB } from 'mobiledoc-kit/utils/characters';
 
-const CARD_ELEMENT_CLASS_NAME = '__mobiledoc-card';
+export const CARD_ELEMENT_CLASS_NAME = '__mobiledoc-card';
 export const NO_BREAK_SPACE = '\u00A0';
 export const TAB_CHARACTER = '\u2003';
 export const SPACE = ' ';
+export const ZWNJ = '\u200c';
 
 function createElementFromMarkup(doc, markup) {
   var element = doc.createElement(markup.tagName);
@@ -102,9 +103,9 @@ function renderCard() {
   let cardElement = document.createElement('div');
   cardElement.contentEditable = false;
   addClassName(cardElement, CARD_ELEMENT_CLASS_NAME);
-  wrapper.appendChild(document.createTextNode('\u200c'));
+  wrapper.appendChild(document.createTextNode(ZWNJ));
   wrapper.appendChild(cardElement);
-  wrapper.appendChild(document.createTextNode('\u200c'));
+  wrapper.appendChild(document.createTextNode(ZWNJ));
   return { wrapper, cardElement };
 }
 

src/js/utils/cursor/position.js

@@ -4,37 +4,38 @@ import {
 import { DIRECTION } from 'mobiledoc-kit/utils/key';
 import assert from 'mobiledoc-kit/utils/assert';
 
-function isSection(postNode) {
-  if (!(postNode && postNode.type)) { return false; }
-  return postNode.isMarkerable || postNode.isCardSection;
-}
-
 function findParentSectionFromNode(renderTree, node) {
   let renderNode =  renderTree.findRenderNodeFromElement(
     node,
-    (renderNode) => isSection(renderNode.postNode)
+    (renderNode) => renderNode.postNode.isSection
   );
 
   return renderNode && renderNode.postNode;
 }
 
 function findOffsetInSection(section, node, offset) {
-  if (!section.isCardSection) {
-    return findOffsetInElement(section.renderNode.element,
-                               node, offset);
-  }
-
-  // Only the card case
-  let wrapperNode = section.renderNode.element;
-  let endTextNode = wrapperNode.lastChild;
-  if (node === endTextNode) {
-    return 1;
+  if (section.isMarkerable) {
+    return findOffsetInElement(section.renderNode.element, node, offset);
+  } else {
+    assert('findOffsetInSection must be called with markerable or card section',
+           section.isCardSection);
+
+    let wrapperNode = section.renderNode.element;
+    let endTextNode = wrapperNode.lastChild;
+    if (node === endTextNode) {
+      return 1;
+    }
+    return 0;
   }
-  return 0;
 }
 
 const Position = class Position {
   constructor(section, offset=0) {
+    assert('Position must have a section that is addressable by the cursor',
+           (section && section.isLeafSection));
+    assert('Position must have numeric offset',
+           (offset !== null && offset !== undefined));
+
     this.section = section;
     this.offset = offset;
     this.isBlank = false;
@@ -155,22 +156,16 @@ const Position = class Position {
   }
 
   static fromElementNode(renderTree, elementNode, offset) {
-    let section, offsetInSection;
+    let position;
 
     // 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) {
       let post = renderTree.rootNode.postNode;
-      if (offset === 0) {
-        section = post.sections.head;
-        offsetInSection = 0;
-      } else {
-        section = post.sections.tail;
-        offsetInSection = section.length;
-      }
+      position = offset === 0 ? post.headPosition() : post.tailPosition();
     } else {
-      section = findParentSectionFromNode(renderTree, elementNode);
+      let section = findParentSectionFromNode(renderTree, elementNode);
       assert('Could not find parent section from element node', !!section);
 
       if (section.isCardSection) {
@@ -179,14 +174,15 @@ const Position = class Position {
         // 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;
+        position = offset < 2 ? section.headPosition() : section.tailPosition();
       } else {
         // The offset is 0 if the cursor is on an element node (e.g., a <br> tag in
         // a blank markup section)
-        offsetInSection = 0;
+        position = section.headPosition();
       }
     }
-    return new Position(section, offsetInSection);
+
+    return position;
   }
 
   /**

tests/helpers/assertions.js

@@ -187,10 +187,10 @@ export default function registerAssertions() {
       this.push(false,
                 `${position.section.type}:${position.section.tagName}`,
                 `${expected.section.type}:${expected.section.tagName}`,
-               `incorrect position section`);
+               `incorrect position section (${message})`);
     } else if (position.offset !== expected.offset) {
       this.push(false, position.offset, expected.offset,
-                `incorrect position offset`);
+                `incorrect position offset (${message})`);
     } else {
       this.push(true, position, expected, message);
     }

tests/helpers/mobiledoc.js

@@ -1,6 +1,8 @@
 import PostAbstractHelpers from './post-abstract';
 import mobiledocRenderers from 'mobiledoc-kit/renderers/mobiledoc';
 import MobiledocRenderer_0_2, { MOBILEDOC_VERSION } from 'mobiledoc-kit/renderers/mobiledoc/0-2';
+import Editor from 'mobiledoc-kit/editor/editor';
+import { mergeWithOptions } from 'mobiledoc-kit/utils/merge';
 
 /*
  * usage:
@@ -25,6 +27,15 @@ function build(treeFn, version) {
   }
 }
 
+function renderInto(element, treeFn, editorOptions={}) {
+  let mobiledoc = build(treeFn);
+  mergeWithOptions(editorOptions, {mobiledoc});
+  let editor = new Editor(editorOptions);
+  editor.render(element);
+  return editor;
+}
+
 export default {
-  build
+  build,
+  renderInto
 };

tests/unit/models/markup-section-test.js

@@ -253,3 +253,8 @@ test('splitMarkerAtOffset splits a marker deep in the middle', (assert) => {
   assert.deepEqual(section.markers.map(m => m.value),
                    ['a', 'bc', 'de', 'f', 'ghi']);
 });
+
+test('a section has property `isSection`', (assert) => {
+  let section = builder.createMarkupSection();
+  assert.ok(section.isSection, 'section.isSection');
+});

tests/unit/models/post-test.js

@@ -1,5 +1,6 @@
 import Helpers from '../../test-helpers';
 import Range from 'mobiledoc-kit/utils/cursor/range';
+import Position from 'mobiledoc-kit/utils/cursor/position';
 
 const {module, test} = Helpers;
 
@@ -539,3 +540,30 @@ test('#cloneRange when range contains multiple list items and more sections', (a
 
   assert.deepEqual(mobiledoc, expected);
 });
+
+test('#headPosition and #tailPosition returns head and tail', (assert) => {
+  let post = Helpers.postAbstract.build(({post, markupSection, marker}) => {
+    return post([
+      markupSection('p', [marker('abc')]),
+      markupSection('p', [marker('123')])
+    ]);
+  });
+
+  let head = post.headPosition();
+  let tail = post.tailPosition();
+
+  assert.positionIsEqual(head, post.sections.head.headPosition(), 'head pos');
+  assert.positionIsEqual(tail, post.sections.tail.tailPosition(), 'tail pos');
+});
+
+test('#headPosition and #tailPosition when post is blank return blank', (assert) => {
+  let post = Helpers.postAbstract.build(({post}) => {
+    return post();
+  });
+
+  let head = post.headPosition();
+  let tail = post.tailPosition();
+
+  assert.positionIsEqual(head, Position.blankPosition(), 'head pos');
+  assert.positionIsEqual(tail, Position.blankPosition(), 'tail pos');
+});

tests/unit/renderers/editor-dom-test.js

@@ -2,12 +2,10 @@ import PostNodeBuilder from 'mobiledoc-kit/models/post-node-builder';
 import Renderer from 'mobiledoc-kit/renderers/editor-dom';
 import RenderTree from 'mobiledoc-kit/models/render-tree';
 import Helpers from '../../test-helpers';
-import { NO_BREAK_SPACE } from 'mobiledoc-kit/renderers/editor-dom';
+import { NO_BREAK_SPACE, ZWNJ } from 'mobiledoc-kit/renderers/editor-dom';
 import { TAB } from 'mobiledoc-kit/utils/characters';
 const { module, test } = Helpers;
 
-const ZWNJ = '\u200c';
-
 import placeholderImageSrc from 'mobiledoc-kit/utils/placeholder-image-src';
 let builder;