Merge pull request #107 from bustlelabs/select-across-sections-102

Add post#walkMarkerableSections and make post.markersFor markerable-aware

src/js/editor/post.js

@@ -30,28 +30,20 @@ class PostEditor {
   }
 
   /**
-   * Remove a range of markers from the post.
+   * Remove a range from the post
    *
    * Usage:
    *
-   *     let marker = editor.post.sections.head.markers.head;
+   *     const range = editor.cursor.offsets;
    *     editor.run((postEditor) => {
-   *       postEditor.deleteRange({
-   *         headSection: section,
-   *         headSectionOffset: 2,
-   *         tailSection: section,
-   *         tailSectionOffset: 4,
-   *       });
+   *       postEditor.deleteRange(range);
    *     });
    *
-   * `deleteRange` accepts the value of `this.cursor.offsets` for deletion.
-   *
    * @method deleteRange
-   * @param {Object} markerRange Object with offsets, {headSection, headSectionOffset, tailSection, tailSectionOffset}
-   * @return {Object} {currentSection, currentOffset} for cursor
+   * @param {Range} range Cursor Range object with head and tail Positions
    * @public
    */
-  deleteRange(markerRange) {
+  deleteRange(range) {
     // types of selection deletion:
     //   * a selection starts at the beginning of a section
     //     -- cursor should end up at the beginning of that section
@@ -66,17 +58,17 @@ class PostEditor {
     //     -- mark the end section for removal
     //     -- cursor goes at end of marker before the selection start
 
-    // markerRange should be akin to this.cursor.offset
     const {
-      headSection, headSectionOffset, tailSection, tailSectionOffset
-    } = markerRange;
+      head: {section: headSection, offset: headSectionOffset},
+      tail: {section: tailSection, offset: tailSectionOffset}
+    } = range;
     const { post } = this.editor;
 
     if (headSection === tailSection) {
       this.cutSection(headSection, headSectionOffset, tailSectionOffset);
     } else {
       let removedSections = [];
-      post.sections.walk(headSection, tailSection, section => {
+      post.walkMarkerableSections(range, section => {
         switch (section) {
           case headSection:
             this.cutSection(section, headSectionOffset, section.text.length);
@@ -656,8 +648,8 @@ class PostEditor {
    *
    * Usage:
    *
-   *     let markerRange = editor.cursor.offsets;
-   *     let sectionWithCursor = markerRange.headMarker.section;
+   *     const range = editor.cursor.offsets;
+   *     const sectionWithCursor = range.head.section;
    *     editor.run((postEditor) => {
    *       postEditor.removeSection(sectionWithCursor);
    *     });
@@ -668,7 +660,13 @@ class PostEditor {
    */
   removeSection(section) {
     section.renderNode.scheduleForRemoval();
-    section.parent.sections.remove(section);
+
+    const parent = section.parent;
+    parent.sections.remove(section);
+
+    if (parent.isBlank) {
+      this.removeSection(parent);
+    }
 
     this.scheduleRerender();
     this.scheduleDidUpdate();

src/js/models/list-section.js

@@ -18,6 +18,10 @@ export default class ListSection {
     items.forEach(i => this.items.append(i));
   }
 
+  get isBlank() {
+    return this.items.isEmpty;
+  }
+
   // returns [prevListSection, newMarkupSection, nextListSection]
   // prevListSection and nextListSection may be undefined
   splitAtListItem(listItem) {

src/js/models/post.js

@@ -1,5 +1,6 @@
 export const POST_TYPE = 'post';
-import LinkedList from "content-kit-editor/utils/linked-list";
+import LinkedList from 'content-kit-editor/utils/linked-list';
+import { compact } from 'content-kit-editor/utils/array-utils';
 
 export default class Post {
   constructor() {
@@ -37,13 +38,8 @@ export default class Post {
 
     let currentSection = firstSection;
     let removedSections = [],
-        changedSections = [];
-    if (firstSection) {
-      changedSections.push(firstSection);
-    }
-    if (lastSection) {
-      changedSections.push(lastSection);
-    }
+        changedSections = compact([firstSection, lastSection]);
+
     if (markers.length !== 0) {
       markers.forEach(marker => {
         if (marker.section !== currentSection) { // this marker is in a section we haven't seen yet
@@ -80,10 +76,41 @@ export default class Post {
       } else if (currentMarker.next) {
         currentMarker = currentMarker.next;
       } else {
-        let nextSection = currentMarker.section.next;
+        let nextSection = this._nextMarkerableSection(currentMarker.section);
         // FIXME: This will fail across cards
         currentMarker = nextSection && nextSection.markers.head;
       }
     }
   }
+
+  walkMarkerableSections(range, callback) {
+    const {head, tail} = range;
+
+    let currentSection = head.section;
+    while (currentSection) {
+      callback(currentSection);
+
+      if (currentSection === tail.section) {
+        break;
+      } else {
+        currentSection = this._nextMarkerableSection(currentSection);
+      }
+    }
+  }
+
+  // return the next section that has markers afer this one
+  _nextMarkerableSection(section) {
+    if (section.next) {
+      let next = section.next;
+      if (next.markers) {
+        return next;
+      } else if (next.items) {
+        next = next.items.head;
+        return next;
+      }
+    } else if (section.parent && section.parent.next) {
+      // FIXME the parent isn't guaranteed to be markerable
+      return section.parent.next;
+    }
+  }
 }

src/js/utils/cursor.js

@@ -6,7 +6,9 @@ import {
 import Position from './cursor/position';
 import Range from './cursor/range';
 
-export default class Cursor {
+export {Position, Range};
+
+const Cursor = class Cursor {
   constructor(editor) {
     this.editor = editor;
     this.renderTree = editor._renderTree;
@@ -142,4 +144,6 @@ export default class Cursor {
     if (selection.rangeCount === 0) { return null; }
     return selection.getRangeAt(0);
   }
-}
+};
+
+export default Cursor;

tests/acceptance/editor-selections-test.js

@@ -318,3 +318,101 @@ test('selecting all text across sections and hitting enter deletes and moves cur
     done();
   });
 });
+
+test('selecting text across markup and list sections', (assert) => {
+  const done = assert.async();
+  const build = Helpers.mobiledoc.build;
+  const mobiledoc = build(({post, markupSection, listSection, listItem, marker}) =>
+    post([
+      markupSection('p', [marker('abc')]),
+      listSection('ul', [
+        listItem([marker('123')]),
+        listItem([marker('456')])
+      ])
+    ])
+  );
+  editor = new Editor({mobiledoc});
+  editor.render(editorElement);
+
+  Helpers.dom.selectText('bc', editorElement, '12', editorElement);
+  Helpers.dom.triggerEvent(document, 'mouseup');
+
+  setTimeout(() => {
+    Helpers.dom.triggerDelete(editor);
+
+    assert.hasElement('#editor p:contains(a3)',
+                      'combines partially-selected list item onto markup section');
+
+    assert.hasNoElement('#editor p:contains(bc)', 'deletes selected text "bc"');
+    assert.hasNoElement('#editor p:contains(12)', 'deletes selected text "12"');
+
+    assert.hasElement('#editor li:contains(6)', 'leaves remaining text in list item');
+    done();
+  });
+});
+
+test('selecting text that covers a list section', (assert) => {
+  const done = assert.async();
+  const build = Helpers.mobiledoc.build;
+  const mobiledoc = build(({post, markupSection, listSection, listItem, marker}) =>
+    post([
+      markupSection('p', [marker('abc')]),
+      listSection('ul', [
+        listItem([marker('123')]),
+        listItem([marker('456')])
+      ]),
+      markupSection('p', [marker('def')])
+    ])
+  );
+  editor = new Editor({mobiledoc});
+  editor.render(editorElement);
+
+  Helpers.dom.selectText('bc', editorElement, 'de', editorElement);
+  Helpers.dom.triggerEvent(document, 'mouseup');
+
+  setTimeout(() => {
+    Helpers.dom.triggerDelete(editor);
+
+    assert.hasElement('#editor p:contains(af)',
+                      'combines sides of selection');
+
+    assert.hasNoElement('#editor li:contains(123)', 'deletes li 1');
+    assert.hasNoElement('#editor li:contains(456)', 'deletes li 2');
+    assert.hasNoElement('#editor ul', 'removes ul');
+
+    done();
+  });
+});
+
+test('selecting text that starts in a list item and ends in a markup section', (assert) => {
+  const done = assert.async();
+  const build = Helpers.mobiledoc.build;
+  const mobiledoc = build(({post, markupSection, listSection, listItem, marker}) =>
+    post([
+      listSection('ul', [
+        listItem([marker('123')]),
+        listItem([marker('456')])
+      ]),
+      markupSection('p', [marker('def')])
+    ])
+  );
+  editor = new Editor({mobiledoc});
+  editor.render(editorElement);
+
+  Helpers.dom.selectText('23', editorElement, 'de', editorElement);
+  Helpers.dom.triggerEvent(document, 'mouseup');
+
+  setTimeout(() => {
+    Helpers.dom.triggerDelete(editor);
+
+    assert.hasElement('#editor li:contains(1f)',
+                      'combines sides of selection');
+
+    assert.hasNoElement('#editor li:contains(123)', 'deletes li 1');
+    assert.hasNoElement('#editor li:contains(456)', 'deletes li 2');
+    assert.hasNoElement('#editor p:contains(def)', 'deletes p content');
+    assert.hasNoElement('#editor p', 'removes p entirely');
+
+    done();
+  });
+});

tests/unit/editor/post-test.js

@@ -5,6 +5,7 @@ import { Editor } from 'content-kit-editor';
 import Helpers from '../../test-helpers';
 import { DIRECTION } from 'content-kit-editor/utils/key';
 import PostNodeBuilder from 'content-kit-editor/models/post-node-builder';
+import {Position, Range} from 'content-kit-editor/utils/cursor';
 
 const { FORWARD } = DIRECTION;
 
@@ -14,6 +15,13 @@ let editor, editorElement;
 
 let builder, postEditor, mockEditor;
 
+function makeRange(headSection, headOffset, tailSection, tailOffset) {
+  return new Range(
+    new Position(headSection, headOffset),
+    new Position(tailSection, tailOffset)
+  );
+}
+
 function getSection(sectionIndex) {
   return editor.post.sections.objectAt(sectionIndex);
 }
@@ -211,12 +219,9 @@ test('#deleteRange when within the same marker', (assert) => {
 
   renderBuiltAbstract(post);
 
-  postEditor.deleteRange({
-    headSection: section,
-    headSectionOffset: 3,
-    tailSection: section,
-    tailSectionOffset: 4
-  });
+  const range = makeRange(section, 3, section, 4);
+
+  postEditor.deleteRange(range);
 
   postEditor.complete();
 
@@ -237,13 +242,8 @@ test('#deleteRange when same section, different markers, same markups', (assert)
 
   renderBuiltAbstract(post);
 
-  postEditor.deleteRange({
-    headSection: section,
-    headSectionOffset: 3,
-    tailSection: section,
-    tailSectionOffset: 4
-  });
-
+  const range = makeRange(section, 3, section, 4);
+  postEditor.deleteRange(range);
   postEditor.complete();
 
   assert.equal(post.sections.head.text, 'abcdef');
@@ -264,13 +264,8 @@ test('#deleteRange when same section, different markers, different markups', (as
 
   renderBuiltAbstract(post);
 
-  postEditor.deleteRange({
-    headSection: section,
-    headSectionOffset: 3,
-    tailSection: section,
-    tailSectionOffset: 4
-  });
-
+  const range = makeRange(section, 3, section, 4);
+  postEditor.deleteRange(range);
   postEditor.complete();
 
   assert.equal(post.sections.head.text, 'abcdef');
@@ -291,13 +286,8 @@ test('#deleteRange across contiguous sections', (assert) => {
 
   renderBuiltAbstract(post);
 
-  postEditor.deleteRange({
-    headSection: s1,
-    headSectionOffset: 3,
-    tailSection: s2,
-    tailSectionOffset: 1
-  });
-
+  const range = makeRange(s1, 3, s2, 1);
+  postEditor.deleteRange(range);
   postEditor.complete();
 
   assert.equal(post.sections.head.text, 'abcdef');
@@ -315,13 +305,8 @@ test('#deleteRange across entire sections', (assert) => {
 
   renderBuiltAbstract(post);
 
-  postEditor.deleteRange({
-    headSection: s1,
-    headSectionOffset: 3,
-    tailSection: s3,
-    tailSectionOffset: 0
-  });
-
+  const range = makeRange(s1, 3, s3, 0);
+  postEditor.deleteRange(range);
   postEditor.complete();
 
   assert.equal(post.sections.head.text, 'abcdef');
@@ -338,12 +323,8 @@ test('#deleteRange across all content', (assert) => {
 
   renderBuiltAbstract(post);
 
-  postEditor.deleteRange({
-    headSection: s1,
-    headSectionOffset: 0,
-    tailSection: s2,
-    tailSectionOffset: 3
-  });
+  const range = makeRange(s1, 0, s2, 3);
+  postEditor.deleteRange(range);
 
   postEditor.complete();