Add forward/backward inclusivity rules for markups fix #402 fix #392

Changes `Post#markupsInRange` to check the markers to the left and right
of the position when range is collapsed. The markups in those markers
provide `isForwardInclusive` and `isBackwardInclusive` methods that are
used to determine which markups should be considered "active" at that
range.

Also updates some test suites that weren't using Helpers.test and
Helpers.module.

fix #392
fix #402

src/js/editor/editor.js

@@ -475,8 +475,11 @@ class Editor {
   /**
    * @param {Markup|String} markup A markup instance, or a string (e.g. "b")
    * @return {boolean}
+   * @deprecated after v0.10.1
    */
   hasActiveMarkup(markup) {
+    deprecate(`editor#hasActiveMarkup is deprecated. Use editor#activeMarkups instead`);
+
     let matchesFn;
     if (typeof markup === 'string') {
       markup = markup.toLowerCase();

src/js/models/markup.js

@@ -37,6 +37,14 @@ class Markup {
            VALID_MARKUP_TAGNAMES.indexOf(this.tagName) !== -1);
   }
 
+  isForwardInclusive() {
+    return this.tagName === normalizeTagName("a") ? false : true;
+  }
+
+  isBackwardInclusive() {
+    return false;
+  }
+
   hasTag(tagName) {
     return this.tagName === normalizeTagName(tagName);
   }

src/js/models/post.js

@@ -121,9 +121,23 @@ class Post {
     const markups = new Set();
 
     if (range.isCollapsed) {
-      let marker = range.head.marker;
-      if (marker) {
-        marker.markups.forEach(m => markups.add(m));
+      let pos = range.head;
+      if (pos.isMarkerable) {
+        let [back, forward] = [pos.markerIn(-1), pos.markerIn(1)];
+        if (back && forward && back === forward) {
+          back.markups.forEach(m => markups.add(m));
+        } else {
+          (back && back.markups || []).forEach(m => {
+            if (m.isForwardInclusive()) {
+              markups.add(m);
+            }
+          });
+          (forward && forward.markups || []).forEach(m => {
+            if (m.isBackwardInclusive()) {
+              markups.add(m);
+            }
+          });
+        }
       }
     } else {
       this.walkMarkerableSections(range, (section) => {

src/js/utils/cursor/position.js

@@ -6,6 +6,9 @@ import {
 } from 'mobiledoc-kit/models/marker';
 import { containsNode } from 'mobiledoc-kit/utils/dom-utils';
 import { findOffsetInNode } from 'mobiledoc-kit/utils/selection-utils';
+import { DIRECTION } from 'mobiledoc-kit/utils/key';
+
+const { FORWARD, BACKWARD } = DIRECTION;
 
 function findParentSectionFromNode(renderTree, node) {
   let renderNode =  renderTree.findRenderNodeFromElement(
@@ -65,7 +68,7 @@ const Position = class Position {
     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));
+           (typeof offset === 'number'));
 
     /** @property {Section} section */
     this.section = section;
@@ -123,10 +126,39 @@ const Position = class Position {
     return this.section && this.section.isMarkerable;
   }
 
+  /**
+   * Returns the marker at this position, in the backward direction
+   * (i.e., the marker to the left of the cursor if the cursor is on a marker boundary and text is left-to-right)
+   * @return {Marker|undefined}
+   */
   get marker() {
     return this.isMarkerable && this.markerPosition.marker;
   }
 
+  /**
+   * Returns the marker in `direction` from this position.
+   * If the position is in the middle of a marker, the direction is irrelevant.
+   * Otherwise, if the position is at a boundary between two markers, returns the
+   * marker to the left if `direction` === BACKWARD and the marker to the right
+   * if `direction` === FORWARD (assuming left-to-right text direction).
+   * @param {Direction}
+   * @return {Marker|undefined}
+   */
+  markerIn(direction) {
+    if (!this.isMarkerable) { return; }
+
+    let { marker, offsetInMarker } = this;
+    if (!marker) { return; }
+
+    if (offsetInMarker > 0 && offsetInMarker < marker.length) {
+      return marker;
+    } else if (offsetInMarker === 0) {
+      return direction === BACKWARD ? marker : marker.prev;
+    } else if (offsetInMarker === marker.length) {
+      return direction === FORWARD ? marker.next : marker;
+    }
+  }
+
   get offsetInMarker() {
     return this.markerPosition.offset;
   }

tests/unit/editor/editor-test.js

@@ -361,25 +361,24 @@ test('#activeMarkups returns the markups at cursor when range is collapsed', (as
 test('#hasActiveMarkup returns true for complex markups', (assert) => {
   editor = Helpers.mobiledoc.renderInto(editorElement, ({post, markupSection, marker, markup}) => {
     return post([markupSection('p', [
-      marker('abc'),
+      marker('abc '),
       marker('def', [markup('a', {href: 'http://bustle.com'})]),
-      marker('ghi')
+      marker(' ghi')
     ])]);
   });
 
   let head = editor.post.sections.head;
-  editor.selectRange(Range.create(head, 'abc'.length));
-  assert.equal(editor.activeMarkups.length, 0, 'no active markups at left of bold text');
+  editor.selectRange(Range.create(head, 'abc '.length));
+  assert.equal(editor.activeMarkups.length, 0, 'no active markups at left of linked text');
 
-  editor.selectRange(Range.create(head, 'abcd'.length));
+  editor.selectRange(Range.create(head, 'abc d'.length));
   assert.equal(editor.activeMarkups.length, 1, 'active markups in linked text');
   assert.ok(editor.hasActiveMarkup('a'), 'has A active markup');
 
-  editor.selectRange(Range.create(head, 'abcdef'.length));
-  assert.equal(editor.activeMarkups.length, 1, 'active markups at end of linked text');
-  assert.ok(editor.hasActiveMarkup('a'), 'has A active markup');
+  editor.selectRange(Range.create(head, 'abc def'.length));
+  assert.equal(editor.activeMarkups.length, 0, 'active markups at end of linked text');
 
-  editor.selectRange(Range.create(head, 'abcdefg'.length));
+  editor.selectRange(Range.create(head, 'abc def '.length));
   assert.equal(editor.activeMarkups.length, 0, 'no active markups after end of linked text');
 });
 

tests/unit/models/atom-test.js

@@ -1,4 +1,5 @@
-const {module, test} = QUnit;
+import Helpers from 'mobiledoc-kit/test-helpers';
+const {module, test} = Helpers;
 
 import PostNodeBuilder from 'mobiledoc-kit/models/post-node-builder';
 
@@ -22,4 +23,4 @@ test('can create an atom with value and payload', (assert) => {
   assert.ok(atom.value === value, 'has value');
   assert.ok(atom.payload === payload, 'has payload');
   assert.ok(atom.length === 1, 'has length of 1');
-});
+});

tests/unit/models/card-test.js

@@ -1,4 +1,5 @@
-const {module, test} = QUnit;
+import Helpers from 'mobiledoc-kit/test-helpers';
+const {module, test} = Helpers;
 
 import PostNodeBuilder from 'mobiledoc-kit/models/post-node-builder';
 

tests/unit/models/marker-test.js

@@ -1,4 +1,5 @@
-const {module, test} = QUnit;
+import Helpers from 'mobiledoc-kit/test-helpers';
+const {module, test} = Helpers;
 
 import PostNodeBuilder from 'mobiledoc-kit/models/post-node-builder';
 

tests/unit/models/post-test.js

@@ -220,6 +220,33 @@ test('#markupsInRange returns all markups when range is not collapsed', (assert)
   assert.inArray(a2, found, 'finds a2');
 });
 
+test('#markupsInRange obeys left- and right-inclusive rules for "A" markups', (assert) => {
+  let a;
+  let post = Helpers.postAbstract.build(({post, markupSection, marker, markup}) => {
+    a = markup('a', {href: 'example.com'});
+    return post([markupSection('p', [
+      marker('123', [a]),
+      marker(' abc '),
+      marker('def', [a]),
+      marker( ' ghi '),
+      marker( 'jkl', [a])
+    ])]);
+  });
+
+  let section = post.sections.head;
+  let start = Range.create(section, 0);
+  let left = Range.create(section, '123 abc '.length);
+  let inside = Range.create(section, '123 abc d'.length);
+  let right = Range.create(section, '123 abc def'.length);
+  let end = Range.create(section, '123 abc def ghi jkl'.length);
+
+  assert.deepEqual(post.markupsInRange(start), [], 'no markups at start');
+  assert.deepEqual(post.markupsInRange(left), [], 'no markups at left');
+  assert.deepEqual(post.markupsInRange(right), [], 'no markups at right');
+  assert.deepEqual(post.markupsInRange(inside), [a], '"A" markup inside range');
+  assert.deepEqual(post.markupsInRange(end), [], 'no markups at end');
+});
+
 test('#markersContainedByRange when range is single marker', (assert) => {
   let found;
   const post = Helpers.postAbstract.build(({post, marker, markupSection}) => {

tests/unit/parsers/mobiledoc-test.js

@@ -1,9 +1,9 @@
 import mobiledocParsers from 'mobiledoc-kit/parsers/mobiledoc';
 import { MOBILEDOC_VERSION } from 'mobiledoc-kit/renderers/mobiledoc/0-2';
 import PostNodeBuilder from 'mobiledoc-kit/models/post-node-builder';
-import Helpers from '../../test-helpers';
+import Helpers from 'mobiledoc-kit/tests/helpers';
 
-const { module, test } = window.QUnit;
+const { module, test } = Helpers;
 
 let builder, post;
 

tests/unit/parsers/mobiledoc/0-2-test.js

@@ -3,7 +3,8 @@ import { MOBILEDOC_VERSION } from 'mobiledoc-kit/renderers/mobiledoc/0-2';
 import PostNodeBuilder from 'mobiledoc-kit/models/post-node-builder';
 
 const DATA_URL = "data:image/gif;base64,R0lGODlhAQABAIAAAP///wAAACwAAAAAAQABAAACAkQBADs=";
-const { module, test } = window.QUnit;
+import Helpers from 'mobiledoc-kit/tests/helpers';
+const { module, test } = Helpers;
 
 let parser, builder, post;
 

tests/unit/parsers/mobiledoc/0-3-test.js

@@ -3,7 +3,8 @@ import { MOBILEDOC_VERSION } from 'mobiledoc-kit/renderers/mobiledoc/0-3';
 import PostNodeBuilder from 'mobiledoc-kit/models/post-node-builder';
 
 const DATA_URL = "data:image/gif;base64,R0lGODlhAQABAIAAAP///wAAACwAAAAAAQABAAACAkQBADs=";
-const { module, test } = window.QUnit;
+import Helpers from 'mobiledoc-kit/tests/helpers';
+const { module, test } = Helpers;
 
 let parser, builder, post;
 

tests/unit/utils/linked-list-test.js

@@ -1,4 +1,5 @@
-const {module, test} = QUnit;
+import Helpers from '../../test-helpers';
+const {module, test} = Helpers;
 
 import LinkedList from 'mobiledoc-kit/utils/linked-list';
 import LinkedItem from 'mobiledoc-kit/utils/linked-item';