Add Post#sectionsContainedBy and update #walkMarkerableSections

fixes #108

demo/demo.js

@@ -442,7 +442,10 @@ var sampleMobiledocs = {
         [1, "H2", [
           [[], 0, "Input Card"]
         ]],
-        [10, "input-card"]
+        [10, "input-card"],
+        [1, "P", [
+          [[], 0, "Text after the card."]
+        ]]
       ]
     ]
   },

src/js/editor/editor.js

@@ -329,27 +329,19 @@ class Editor {
   }
 
   handleNewline(event) {
-    let offsets = this.cursor.offsets;
-
-    // if there's no left/right nodes, we are probably not in the editor,
-    // or we have selected some non-marker thing like a card
-    if (!offsets.headSection || !offsets.tailSection) {
-      return;
-    }
+    if (!this.cursor.hasCursor()) { return ;}
 
+    const range = this.cursor.offsets;
     event.preventDefault();
 
-    let cursorSection;
-    this.run((postEditor) => {
-      if (this.cursor.hasSelection()) {
-        postEditor.deleteRange(offsets);
-        if (offsets.headSection.isBlank) {
-          cursorSection = offsets.headSection;
-          return;
+    const cursorSection = this.run((postEditor) => {
+      if (!range.isCollapsed) {
+        postEditor.deleteRange(range);
+        if (range.head.section.isBlank) {
+          return range.head.section;
         }
       }
-      const headPosition = offsets.head;
-      cursorSection = postEditor.splitSection(headPosition)[1];
+      return postEditor.splitSection(range.head)[1];
     });
     this.cursor.moveToSection(cursorSection);
   }

src/js/editor/post.js

@@ -67,21 +67,23 @@ class PostEditor {
     if (headSection === tailSection) {
       this.cutSection(headSection, headSectionOffset, tailSectionOffset);
     } else {
-      let removedSections = [];
+      let removedSections = post.sectionsContainedBy(range);
       post.walkMarkerableSections(range, section => {
         switch (section) {
           case headSection:
             this.cutSection(section, headSectionOffset, section.text.length);
             break;
           case tailSection:
-            tailSection.markersFor(tailSectionOffset, section.text.length).forEach(m => {
+            section.markersFor(tailSectionOffset, section.text.length).forEach(m => {
               headSection.markers.append(m);
             });
             headSection.renderNode.markDirty(); // May have added nodes
-            removedSections.push(tailSection);
+            removedSections.push(section);
             break;
           default:
-            removedSections.push(section);
+            if (removedSections.indexOf(section) === -1) {
+              removedSections.push(section);
+            }
           }
       });
       removedSections.forEach(section => this.removeSection(section) );
@@ -659,12 +661,22 @@ class PostEditor {
    * @public
    */
   removeSection(section) {
+    const parent = section.parent;
+    const parentIsRemoved = parent.renderNode.isRemoved;
+
+    if (parentIsRemoved) {
+      // This can happen if we remove a list section and later
+      // try to remove one of the section's list items;
+      return;
+    }
+
     section.renderNode.scheduleForRemoval();
 
-    const parent = section.parent;
     parent.sections.remove(section);
 
     if (parent.isBlank) {
+      // If we removed the last child from a parent (e.g. the last li in a ul),
+      // also remove the parent
       this.removeSection(parent);
     }
 

src/js/models/post.js

@@ -98,19 +98,58 @@ export default class Post {
     }
   }
 
+  // return an array of all top-level sections (direct children of `post`)
+  // that are wholly contained by the range.
+  sectionsContainedBy(range) {
+    const {head, tail} = range;
+    let containedSections = [];
+
+    const findParent = (child, conditionFn) => {
+      while (child) {
+        if (conditionFn(child)) { return child; }
+        child = child.parent;
+      }
+    };
+
+    let headTopLevelSection = findParent(head.section, s => !!s.post);
+    let tailTopLevelSection = findParent(tail.section, s => !!s.post);
+
+    let currentSection = headTopLevelSection.next;
+    while (currentSection && currentSection !== tailTopLevelSection) {
+      containedSections.push(currentSection);
+      currentSection = currentSection.next;
+    }
+
+    return containedSections;
+  }
+
   // 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;
+    if (!section) { return null; }
+    const isMarkerable = s => !!s.markers;
+    const hasChildren  = s => !!s.items;
+    const firstChild   = s => s.items.head;
+    const isChild      = s => s.parent && !s.post;
+    const parent       = s => s.parent;
+
+    let next = section.next;
+    if (next) {
+      if (isMarkerable(next)) {
         return next;
+      } else if (hasChildren(next)) { // e.g. a ListSection
+        return firstChild(next);
+      } else {
+        // e.g. a cardSection that has no children or parent but
+        // may have a markerable after it in the AT
+        return this._nextMarkerableSection(next);
+      }
+    } else {
+      if (isChild(section)) {
+        // if there is no section after this, but this section is a child
+        // (e.g. a ListItem inside a ListSection), check for a markerable
+        // section after its parent
+        return this._nextMarkerableSection(parent(section));
       }
-    } else if (section.parent && section.parent.next) {
-      // FIXME the parent isn't guaranteed to be markerable
-      return section.parent.next;
     }
   }
 }

tests/acceptance/editor-selections-test.js

@@ -416,3 +416,44 @@ test('selecting text that starts in a list item and ends in a markup section', (
     done();
   });
 });
+
+test('selecting text that includes a card section and deleting deletes card section', (assert) => {
+  const done = assert.async();
+  const build = Helpers.mobiledoc.build;
+  const mobiledoc = build(({post, markupSection, cardSection, marker}) =>
+    post([
+      markupSection('p', [marker('abc')]),
+      cardSection('simple-card'),
+      markupSection('p', [marker('def')])
+    ])
+  );
+  const cards = [{
+    name: 'simple-card',
+    display: {
+      setup(element) {
+        const span = document.createElement('span');
+        span.setAttribute('id', 'card-el');
+        element.appendChild(span);
+      }
+    }
+  }];
+  editor = new Editor({mobiledoc, cards});
+  editor.render(editorElement);
+
+  assert.hasElement('#card-el', 'precond - card el is rendered');
+
+  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 span#card-el', 'card el is removed');
+    assert.hasNoElement('#editor p:contains(abc)', 'previous section 1 is removed');
+    assert.hasNoElement('#editor p:contains(def)', 'previous section 2 is removed');
+
+    done();
+  });
+});

tests/unit/models/post-test.js

@@ -1,4 +1,5 @@
 import Helpers from '../../test-helpers';
+import { Position, Range } from 'content-kit-editor/utils/cursor';
 
 const {module, test} = Helpers;
 
@@ -69,3 +70,34 @@ test('#markersFrom finds markers across non-homogeneous sections', (assert) => {
                     's3m1', 's3m2'         ],
                    'iterates correct markers');
 });
+
+test('#walkMarkerableSections skips non-markerable sections', (assert) => {
+  const post = Helpers.postAbstract.build(builder => {
+    const {post, markupSection, marker, cardSection} = builder;
+
+    return post([
+      markupSection('p', ['s1m1'].map(t => marker(t))),
+      markupSection('p', ['s2m1'].map(t => marker(t))),
+      cardSection('simple-card'),
+      markupSection('p', ['s3m1'].map(t => marker(t))),
+      markupSection('p', ['s4m1'].map(t => marker(t)))
+    ]);
+  });
+
+  let foundSections = [];
+
+  const s1 = post.sections.objectAt(0);
+  const s4 = post.sections.objectAt(4);
+
+  assert.equal(s1.text, 's1m1', 'precond - find s1');
+  assert.equal(s4.text, 's4m1', 'precond - find s4');
+
+  const range = new Range(new Position(s1, 0), new Position(s4, 0));
+
+  post.walkMarkerableSections(range, s => foundSections.push(s));
+
+  assert.deepEqual(foundSections.map(s => s.text),
+                   ['s1m1', 's2m1', 's3m1', 's4m1'],
+                   'iterates correct sections');
+
+});