Refactor newline insertion to use postEditor

src/js/commands/bold.js

@@ -15,6 +15,9 @@ export default class BoldCommand extends TextFormatCommand {
   }
   exec() {
     let markerRange = this.editor.cursor.offsets;
+    if (!markerRange.leftRenderNode || !markerRange.rightRenderNode) {
+      return;
+    }
     let markers = this.editor.run((postEditor) => {
       return postEditor.applyMarkupToMarkers(markerRange, this.markup);
     });

src/js/commands/italic.js

@@ -15,6 +15,9 @@ export default class ItalicCommand extends TextFormatCommand {
   }
   exec() {
     let markerRange = this.editor.cursor.offsets;
+    if (!markerRange.leftRenderNode || !markerRange.rightRenderNode) {
+      return;
+    }
     let markers = this.editor.run((postEditor) => {
       return postEditor.applyMarkupToMarkers(markerRange, this.markup);
     });

src/js/editor/editor.js

@@ -314,53 +314,46 @@ class Editor {
   handleDeletion(event) {
     event.preventDefault();
 
+    let offsets = this.cursor.offsets;
+    let currentMarker, currentOffset;
     this.run((postEditor) => {
+      let results;
       if (this.cursor.hasSelection()) {
-        postEditor.deleteRange(this.cursor.offsets);
+        results = postEditor.deleteRange(offsets);
       } else {
-        let {
-          headMarker: marker,
-          headOffset: offset
-        } = this.cursor.offsets;
         // FIXME: perhaps this should accept this.cursor.offsets?
-        postEditor.deleteCharAt(marker, offset-1);
+        results = postEditor.deleteCharAt(offsets.headMarker, offsets.headOffset-1);
       }
+      currentMarker = results.currentMarker;
+      currentOffset = results.currentOffset;
     });
+    this.cursor.moveToMarker(currentMarker, currentOffset);
   }
 
   handleNewline(event) {
-    if (this.cursor.hasSelection()) {
-      this.handleDeletion(event);
-    }
-
-    const {
-      leftRenderNode,
-      rightRenderNode,
-      leftOffset
-    } = this.cursor.offsets;
+    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 (!leftRenderNode || !rightRenderNode) { return; }
+    if (!offsets.leftRenderNode || !offsets.rightRenderNode) {
+      return;
+    }
 
     event.preventDefault();
 
-    const markerRenderNode = leftRenderNode;
-    const marker = markerRenderNode.postNode;
-    const section = marker.section;
-
-    let [beforeSection, afterSection] = section.splitAtMarker(marker, leftOffset);
-
-    section.renderNode.scheduleForRemoval();
-
-    this.post.sections.insertAfter(beforeSection, section);
-    this.post.sections.insertAfter(afterSection, beforeSection);
-    this.post.sections.remove(section);
-
-    this.rerender();
-    this.trigger('update');
-
-    this.cursor.moveToSection(afterSection);
+    let cursorSection;
+    this.run((postEditor) => {
+      let offsetAfterDeletion;
+      if (this.cursor.hasSelection()) {
+        let result = postEditor.deleteRange(offsets);
+        offsetAfterDeletion = {
+          headMarker: result.currentMarker,
+          headOffset: result.currentOffset
+        };
+      }
+      cursorSection = postEditor.splitSection(offsetAfterDeletion || offsets)[1];
+    });
+    this.cursor.moveToSection(cursorSection);
   }
 
   hasSelection() {

src/js/editor/post.js

@@ -35,14 +35,13 @@ class PostEditor {
     changedSections.forEach(section => section.renderNode.markDirty());
     removedSections.forEach(section => section.renderNode.scheduleForRemoval());
 
-    this.didUpdate();
     this.rerender();
-    this.schedule(() => {
-      // FIXME the cursor API should accept markers, not elements. Or there
-      // should be a proxy method on editor
-      let currentTextNode = currentMarker.renderNode.element;
-      this.editor.cursor.moveToNode(currentTextNode, currentOffset);
-    });
+    this.didUpdate();
+
+    return {
+      currentMarker,
+      currentOffset
+    };
   }
 
   // need to handle these cases:
@@ -114,15 +113,13 @@ class PostEditor {
       }
     }
 
-    this.didUpdate();
     this.rerender();
-    this.schedule(() => {
-      let nextElement = nextCursorMarker.renderNode.element;
-      this.editor.cursor.moveToNode(
-        nextElement,
-        nextCursorOffset
-      );
-    });
+    this.didUpdate();
+
+    return {
+      currentMarker: nextCursorMarker,
+      currentOffset: nextCursorOffset
+    };
   }
 
   /*
@@ -138,9 +135,9 @@ class PostEditor {
     // for removal before doing that. FIXME this seems prime for
     // refactoring onto the postEditor as a split function
     headMarker.renderNode.scheduleForRemoval();
-    headMarker.renderNode.scheduleForRemoval();
-    headMarker.section.renderNode.markDirty();
+    tailMarker.renderNode.scheduleForRemoval();
     headMarker.section.renderNode.markDirty();
+    tailMarker.section.renderNode.markDirty();
 
     if (headMarker === tailMarker) {
       let markers = headSection.splitMarker(headMarker, headOffset, tailOffset);
@@ -174,12 +171,36 @@ class PostEditor {
       });
     }
 
-    this.didUpdate();
     this.rerender();
+    this.didUpdate();
 
     return selectedMarkers;
   }
 
+  /*
+   * @return {Array} of new sections
+   */
+  splitSection({headMarker, headOffset}) {
+    const { post } = this.editor;
+    const { section } = headMarker;
+
+    const [
+      beforeSection,
+      afterSection
+    ] = section.splitAtMarker(headMarker, headOffset);
+
+    this.removeSection(section);
+
+    post.sections.insertAfter(beforeSection, section);
+    post.sections.insertAfter(afterSection, beforeSection);
+    post.renderNode.markDirty();
+
+    this.rerender();
+    this.didUpdate();
+
+    return [beforeSection, afterSection];
+  }
+
   applyMarkupToMarkers(markerRange, markup) {
     const markers = this.splitMarkers(markerRange);
     markers.forEach(marker => {

src/js/models/cursor.js

@@ -129,16 +129,16 @@ export default class Cursor {
   moveToSection(section) {
     const marker = section.markers.head;
     if (!marker) { throw new Error('Cannot move cursor to section without a marker'); }
-    const markerElement = marker.renderNode.element;
+    this.moveToMarker(marker);
+  }
 
-    let r = document.createRange();
-    r.selectNode(markerElement);
-    r.collapse(true);
-    const selection = this.selection;
-    if (selection.rangeCount > 0) {
-      selection.removeAllRanges();
-    }
-    selection.addRange(r);
+  // moves cursor to marker
+  moveToMarker(headMarker, headOffset=0, tailMarker=headMarker, tailOffset=headOffset) {
+    if (!headMarker) { throw new Error('Cannot move cursor to section without a marker'); }
+    const headElement = headMarker.renderNode.element;
+    const tailElement = tailMarker.renderNode.element;
+
+    this.moveToNode(headElement, headOffset, tailElement, tailOffset);
   }
 
   selectSections(sections) {

tests/acceptance/editor-selections-test.js

@@ -205,6 +205,37 @@ test('selecting text across markers deletes intermediary markers', (assert) => {
   });
 });
 
+test('selecting text across markers preserves node after', (assert) => {
+  const done = assert.async();
+  editor = new Editor(editorElement, {mobiledoc: mobileDocWith2Sections});
+
+  Helpers.dom.selectText('rst sec', editorElement);
+  Helpers.dom.triggerEvent(document, 'mouseup');
+
+  setTimeout(() => {
+    Helpers.toolbar.clickButton(assert, 'bold');
+
+    const textNode1 = editorElement.childNodes[0].childNodes[0],
+          textNode2 = editorElement.childNodes[0].childNodes[1];
+    Helpers.dom.selectText('i', textNode1,
+                           'sec', textNode2);
+    Helpers.dom.triggerEvent(document, 'mouseup');
+
+    setTimeout(() => {
+      Helpers.dom.triggerDelete(editor);
+
+      assert.deepEqual(
+        editorElement.childNodes[0].innerHTML, 'ftion',
+        'has remaining first section'
+      );
+      assert.deepEqual(Helpers.dom.getCursorPosition(),
+                       {node: editorElement.childNodes[0].childNodes[0],
+                         offset: 1});
+      done();
+    });
+  });
+});
+
 test('selecting text across sections and hitting enter deletes and moves cursor to last selected section', (assert) => {
   const done = assert.async();
   editor = new Editor(editorElement, {mobiledoc: mobileDocWith2Sections});