remove post#sectionsContainedBy(range)

Author: bantic

src/js/editor/post.js

@@ -110,7 +110,7 @@ class PostEditor {
     if (headSection === tailSection) {
       nextPosition = this.cutSection(headSection, headSectionOffset, tailSectionOffset);
     } else {
-      let removedSections = post.sectionsContainedBy(range);
+      let removedSections = [];
       let appendSection = headSection;
 
       post.walkLeafSections(range, section => {
@@ -143,9 +143,7 @@ class PostEditor {
             }
             break;
           default:
-            if (removedSections.indexOf(section) === -1) {
-              removedSections.push(section);
-            }
+            removedSections.push(section);
           }
       });
       if (headSection !== appendSection) {

src/js/models/post.js

@@ -185,40 +185,6 @@ class Post {
     });
   }
 
-  /**
-   * @param {Range} range
-   * @return {Section[]} All top-level sections (direct children of `post`) wholly
-   *         contained by {range}. Sections that are partially contained by the range
-   *         are not returned.
-   * @private
-   */
-  sectionsContainedBy(range) {
-    const {head, tail} = range;
-    let containedSections = [];
-
-    const findParent = (child, conditionFn) => {
-      while (child) {
-        if (conditionFn(child)) { return child; }
-        child = child.parent;
-      }
-    };
-
-    const headTopLevelSection = findParent(head.section, s => s.parent === s.post);
-    const tailTopLevelSection = findParent(tail.section, s => s.parent === s.post);
-
-    if (headTopLevelSection === tailTopLevelSection) {
-      return containedSections;
-    }
-
-    let currentSection = headTopLevelSection.next;
-    while (currentSection && currentSection !== tailTopLevelSection) {
-      containedSections.push(currentSection);
-      currentSection = currentSection.next;
-    }
-
-    return containedSections;
-  }
-
   _nextMarkerableSection(section) {
     let nextSection = this._nextLeafSection(section);
 

tests/helpers/post-abstract.js

@@ -82,7 +82,9 @@ function parseTextIntoMarkers(text, builder) {
       }
     });
   } else if (text.indexOf('*') === -1) {
-    markers.push(builder.marker(text));
+    if (text.length) {
+      markers.push(builder.marker(text));
+    }
   } else {
     let markup = builder.markup('b');
 

tests/unit/editor/post-delete-at-position-test.js

@@ -131,7 +131,8 @@ test('across section boundary (backward)', (assert) => {
     [['abc','|def'], 'abc|def', 'markup sections'],
     [['*abc*','|def'], '*abc*|def', 'markup sections with markup'],
     [['[abc]','|def'], ['[abc]|','def'], 'prev section is card'],
-    [['abc','|[def]'], ['abc|','[def]'], 'cur section is card']
+    [['abc','|[def]'], ['abc|','[def]'], 'cur section is card'],
+    [['', '|abc'], ['|abc'], 'prev section is blank']
   ];
 
   examples.forEach(([before, after, msg]) => {
@@ -152,7 +153,8 @@ test('across section boundary (forward)', (assert) => {
     [['abc|','def'], 'abc|def', 'markup sections'],
     [['*abc*|','def'], '*abc*|def', 'markup sections with markup'],
     [['[abc]|','def'], ['[abc]|','def'], 'cur section is card'],
-    [['abc|','[def]'], ['abc|','[def]'], 'next section is card']
+    [['abc|','[def]'], ['abc|','[def]'], 'next section is card'],
+    [['abc|', ''], ['abc|'], 'next section is blank']
   ];
 
   examples.forEach(([before, after, msg]) => {
@@ -172,6 +174,7 @@ test('across list item boundary (backward)', (assert) => {
   let examples = [
     [['* abc','* |def'], ['* abc', '|def'], 'start of list item'],
     [['* abc','|def'], ['* abc|def'], 'into list item'],
+    [['', '* |abc'], ['', '|abc'], 'prev blank section'],
   ];
 
   examples.forEach(([before, after, msg]) => {

tests/unit/editor/post-delete-range-test.js

@@ -115,24 +115,23 @@ test('#deleteRange within a section with markup (single section)', (assert) => {
   });
 });
 
-test('#deleteRange entire section (single section)', (assert) => {
-  let text = '<abc>';
-  let { post, range } = Helpers.postAbstract.buildFromText(text);
-  let position = run(post, postEditor => postEditor.deleteRange(range));
-
-  assert.ok(post.sections.length === 1 && post.sections.head.isBlank, 'post has single blank section after deleteRange');
-  assert.ok(position.section === post.sections.head, 'position#section is correct');
-  assert.equal(position.offset, 0, 'position#offset is correct');
-});
+test('#deleteRange entire post', (assert) => {
+  let examples = [
+    [['<abc>'], 'single section'],
+    [['<[some-card]>'], 'single card'],
+    [['<abc','def','ghi>'], 'multiple sections'],
+    [['<>'], 'single blank section'],
+    [['<','','>'], 'multiple blank sections']
+  ];
 
-test('#deleteRange entire section (multiple sections)', (assert) => {
-  let text = ['<abc','def','ghi>'];
-  let { post, range } = Helpers.postAbstract.buildFromText(text);
-  let position = run(post, postEditor => postEditor.deleteRange(range));
+  examples.forEach(([text, msg]) => {
+    let { post, range } = Helpers.postAbstract.buildFromText(text);
+    let position = run(post, postEditor => postEditor.deleteRange(range));
 
-  assert.ok(post.sections.length === 1 && post.sections.head.isBlank, 'post has single blank section after deleteRange');
-  assert.ok(position.section === post.sections.head, 'position#section is correct');
-  assert.equal(position.offset, 0, 'position#offset is correct');
+    assert.ok(post.sections.length === 1 && post.sections.head.isBlank, `post has single blank section after deleteRange (${msg})`);
+    assert.ok(position.section === post.sections.head, `position#section is correct (${msg})`);
+    assert.equal(position.offset, 0, `position#offset is correct (${msg})`);
+  });
 });
 
 test('#deleteRange across markup section boundaries', (assert) => {
@@ -204,7 +203,12 @@ test('#deleteRange across markup/non-markup section boundaries', (assert) => {
     [['[some-card]<','>abc'], ['[some-card]|', 'abc'], 'card->markup'], 
     [['abc<','>[some-card]'], ['abc|', '[some-card]'], 'markup->card'], 
 
-    [['abc<', '[some-card]', '>def'], ['abc|def'], 'containing card, boundaries in outer sections']
+    [['abc<', '[some-card]', '>def'], ['abc|def'], 'containing card, boundaries in outer sections'],
+
+    [['abc', '<[some-card]>', 'def'], ['abc', '|', 'def'], 'containing card, boundaries in card section'],
+
+    // Ideally this would delete the blank section altogether and position cursor at the start of the card
+    [['<', '>[some-card]'], ['|', '[some-card]'], 'blank section into card']
   ];
 
   examples.forEach(([before, after, msg]) => {
@@ -227,25 +231,6 @@ test('#deleteRange across markup/non-markup section boundaries', (assert) => {
   }); 
 });
 
-test('#deleteRange surrounding card section', (assert) => {
-  let { post, range } = Helpers.postAbstract.buildFromText(['abc', '<[some-card]>', 'def']);
-  let expectedPost = Helpers.postAbstract.build(({post, markupSection, marker}) => {
-    return post([
-      markupSection('p', [marker('abc')]),
-      markupSection('p'),
-      markupSection('p', [marker('def')])
-    ]);
-  });
-
-  let position = run(post, postEditor => postEditor.deleteRange(range));
-  renderedRange = new Range(position);
-
-  let expectedRange = Range.create(post.sections.objectAt(1), 0);
-
-  assert.postIsSimilar(post, expectedPost);
-  assert.rangeIsEqual(renderedRange, expectedRange);
-});
-
 test('#deleteRange across list items', (assert) => {
   let examples = [
     [['* item 1<', '* >item 2'], ['* item 1|item 2'], 'at boundary'],

tests/unit/models/post-test.js

@@ -357,123 +357,6 @@ test('#isBlank is true when there are no sections', (assert) => {
   assert.ok(!_post.isBlank);
 });
 
-// see https://github.com/bustlelabs/mobiledoc-kit/issues/134
-test('#sectionsContainedBy when range covers two list items', (assert) => {
-  const post = Helpers.postAbstract.build(
-    ({post, markupSection, marker, listSection, listItem}) => {
-    return post([
-      listSection('ul', [listItem([marker('abc')]), listItem()]),
-      markupSection('p', [marker('123')])
-    ]);
-  });
-  const li1 = post.sections.head.items.head,
-        li2 = post.sections.head.items.tail;
-  const section = post.sections.tail;
-  assert.equal(li1.text, 'abc', 'precond - li1 text');
-  assert.equal(li2.text, '', 'precond - li2 text');
-  assert.equal(section.text, '123', 'precond - section text');
-
-  const range = Range.create(li1, 0, li2, li2.length);
-  const containedSections = post.sectionsContainedBy(range);
-  assert.equal(containedSections.length, 0, 'no sections are contained');
-});
-
-test('#sectionsContainedBy when range contains no sections', (assert) => {
-  const post = Helpers.postAbstract.build(
-    ({post, markupSection, marker}) => {
-    return post([
-      markupSection('p', [marker('abc')]),
-      markupSection('p', [marker('123')])
-    ]);
-  });
-  const s1 = post.sections.head,
-        s2 = post.sections.tail;
-  assert.equal(s1.text, 'abc', 'precond - s1 text');
-  assert.equal(s2.text, '123', 'precond - s2 text');
-
-  const range = Range.create(s1, 0, s2, s2.length);
-  const containedSections = post.sectionsContainedBy(range);
-  assert.equal(containedSections.length, 0, 'no sections are contained');
-});
-
-test('#sectionsContainedBy when range contains sections', (assert) => {
-  const post = Helpers.postAbstract.build(
-    ({post, markupSection, marker}) => {
-    return post([
-      markupSection('p', [marker('abc')]),
-      markupSection('p', [marker('inner')]),
-      markupSection('p', [marker('123')])
-    ]);
-  });
-  const s1 = post.sections.head,
-        sInner = post.sections.objectAt(1),
-        s2 = post.sections.tail;
-  assert.equal(s1.text, 'abc', 'precond - s1 text');
-  assert.equal(sInner.text, 'inner', 'precond - sInner text');
-  assert.equal(s2.text, '123', 'precond - s2 text');
-
-  const range = Range.create(s1, 0, s2, s2.length);
-  const containedSections = post.sectionsContainedBy(range);
-  assert.equal(containedSections.length, 1, '1 sections are contained');
-  assert.ok(containedSections[0] === sInner, 'inner section is contained');
-});
-
-test('#sectionsContainedBy when range contains non-markerable sections', (assert) => {
-  const post = Helpers.postAbstract.build(
-    ({post, markupSection, marker, cardSection, listSection, listItem}) => {
-    return post([
-      markupSection('p', [marker('abc')]),
-      cardSection('test-card'),
-      listSection('ul', [listItem([marker('li')])]),
-      markupSection('p', [marker('123')])
-    ]);
-  });
-  const s1 = post.sections.head,
-        card = post.sections.objectAt(1),
-        list = post.sections.objectAt(2),
-        s2 = post.sections.tail;
-
-  assert.equal(s1.text, 'abc', 'precond - s1 text');
-  assert.equal(s2.text, '123', 'precond - s2 text');
-  const range = Range.create(s1, 0, s2, s2.length);
-  const containedSections = post.sectionsContainedBy(range);
-  assert.equal(containedSections.length, 2, '2 sections are contained');
-  assert.ok(containedSections.indexOf(card) !== -1, 'contains card');
-  assert.ok(containedSections.indexOf(list) !== -1, 'contains list');
-});
-
-test('#sectionsContainedBy when range starts/ends in list item', (assert) => {
-  const post = Helpers.postAbstract.build(
-    ({post, markupSection, marker, cardSection, listSection, listItem}) => {
-    return post([
-      listSection('ul', [
-        listItem([marker('ul1 li1')]),
-        listItem([marker('ul1 li2')])
-      ]),
-      markupSection('p', [marker('abc')]),
-      cardSection('test-card'),
-      listSection('ul', [
-        listItem([marker('ul2 li1')]),
-        listItem([marker('ul2 li2')])
-      ])
-    ]);
-  });
-  const li1 = post.sections.head.items.head,
-        li2 = post.sections.tail.items.tail,
-        s1  = post.sections.objectAt(1),
-        card = post.sections.objectAt(2);
-
-  assert.equal(li1.text, 'ul1 li1', 'precond - li1 text');
-  assert.equal(li2.text, 'ul2 li2', 'precond - li2 text');
-  assert.equal(s1.text, 'abc', 'precond - s1 text');
-
-  const range = Range.create(li1, li1.length, li2, li2.length);
-  const containedSections = post.sectionsContainedBy(range);
-  assert.equal(containedSections.length, 2, '2 sections are contained');
-  assert.ok(containedSections.indexOf(card) !== -1, 'contains card');
-  assert.ok(containedSections.indexOf(s1) !== -1, 'contains section');
-});
-
 test('#trimTo creates a post from the given range', (assert) => {
   let post = Helpers.postAbstract.build(
     ({post, markupSection, marker}) => {