Merge pull request #53 from bustlelabs/newline-at-section-start

Handle newline at start or end of section

src/js/editor/editor.js

@@ -442,33 +442,19 @@ class Editor {
     const markerRenderNode = leftRenderNode;
     const marker = markerRenderNode.postNode;
     const section = marker.section;
-    const newMarkers = marker.split(leftOffset);
 
-    // FIXME rightMarker is not guaranteed to be there
-    let [leftMarker, rightMarker] = newMarkers;
+    let [beforeSection, afterSection] = section.splitAtMarker(marker, leftOffset);
 
-    section.markers.insertAfter(leftMarker, marker);
-    markerRenderNode.scheduleForRemoval();
+    section.renderNode.scheduleForRemoval();
 
-    const newSection = this.builder.createMarkupSection('p');
-    newSection.markers.append(rightMarker);
-
-    let nodeForMove = markerRenderNode.next;
-    while (nodeForMove) {
-      nodeForMove.scheduleForRemoval();
-      let movedMarker = nodeForMove.postNode.clone();
-      newSection.markers.append(movedMarker);
-
-      nodeForMove = nodeForMove.next;
-    }
-
-    const post = this.post;
-    post.sections.insertAfter(newSection, section);
+    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(newSection);
+    this.cursor.moveToSection(afterSection);
   }
 
   hasSelection() {

src/js/models/marker.js

@@ -26,6 +26,10 @@ const Marker = class Marker extends LinkedItem {
     return new this.constructor(this.value, clonedMarkups);
   }
 
+  empty() {
+    return this.length === 0;
+  }
+
   get length() {
     return this.value.length;
   }
@@ -88,21 +92,11 @@ const Marker = class Marker extends LinkedItem {
   split(offset=0, endOffset=this.length) {
     let markers = [];
 
-    if (offset !== 0) {
-      markers.push(
-        new Marker(this.value.substring(0, offset))
-      );
-    }
-
-    markers.push(
-      new Marker(this.value.substring(offset, endOffset))
-    );
-
-    if (endOffset < this.length) {
-      markers.push(
-        new Marker(this.value.substring(endOffset))
-      );
-    }
+    markers = [
+      this.builder.createMarker(this.value.substring(0, offset)),
+      this.builder.createMarker(this.value.substring(offset, endOffset)),
+      this.builder.createMarker(this.value.substring(endOffset))
+    ];
 
     this.markups.forEach(mu => markers.forEach(m => m.addMarkup(mu)));
     return markers;

src/js/models/markup-section.js

@@ -2,6 +2,11 @@ import {
   normalizeTagName
 } from '../utils/dom-utils';
 
+import {
+  forEach,
+  filter
+} from '../utils/array-utils';
+
 export const DEFAULT_TAG_NAME = normalizeTagName('p');
 export const VALID_MARKUP_SECTION_TAGNAMES = [
   'p', 'h3', 'h2', 'h1', 'blockquote', 'ul', 'ol'
@@ -53,16 +58,57 @@ export default class Section extends LinkedItem {
   }
 
   /**
-   * Splits the marker at the offset (until the endOffset, if given)
-   * into 1, 2, or 3 markers and replaces the existing marker
-   * with the new ones
+   * Splits the marker at the offset, filters empty markers from the result,
+   * and replaces this marker with the new non-empty ones
+   * @param {Marker} marker the marker to split
+   * @return {Array} the new markers that replaced `marker`
    */
   splitMarker(marker, offset, endOffset=marker.length) {
-    const newMarkers = marker.split(offset, endOffset);
+    const newMarkers = filter(marker.split(offset, endOffset), m => !m.empty());
     this.markers.splice(marker, 1, newMarkers);
     return newMarkers;
   }
 
+  splitAtMarker(marker, offset=0) {
+    let [beforeSection, afterSection] = [
+      this.builder.createMarkupSection(this.tagName, []),
+      this.builder.createMarkupSection(this.tagName, [])
+    ];
+
+    let currentSection = beforeSection;
+    forEach(this.markers, m => {
+      if (m === marker) {
+        const [beforeMarker, ...afterMarkers] = marker.split(offset);
+        beforeSection.markers.append(beforeMarker);
+        forEach(afterMarkers, _m => afterSection.markers.append(_m));
+        currentSection = afterSection;
+      } else {
+        currentSection.markers.append(m.clone());
+      }
+    });
+
+    beforeSection.coalesceMarkers();
+    afterSection.coalesceMarkers();
+
+    return [beforeSection, afterSection];
+  }
+
+  /**
+   * Remove extranous empty markers, adding one at the end if there
+   * are no longer any markers
+   *
+   * Mutates this section's markers
+   */
+  coalesceMarkers() {
+    forEach(
+      filter(this.markers, m => m.empty()),
+      m => this.markers.remove(m)
+    );
+    if (this.markers.empty()) {
+      this.markers.append(this.builder.createBlankMarker());
+    }
+  }
+
   /**
    * @return {Array} 2 new sections
    */

src/js/models/post-node-builder.js

@@ -23,6 +23,7 @@ export default class PostNodeBuilder {
     if (isGenerated) {
       section.isGenerated = true;
     }
+    section.builder = this;
     return section;
   }
 
@@ -45,7 +46,9 @@ export default class PostNodeBuilder {
   }
 
   createBlankMarker() {
-    return new Marker('');
+    const marker = new Marker('');
+    marker.builder = this;
+    return marker;
   }
 
   createMarkup(tagName, attributes) {

src/js/renderers/editor-dom.js

@@ -8,7 +8,7 @@ import { IMAGE_SECTION_TYPE } from "../models/image";
 import { CARD_TYPE } from "../models/card";
 import { clearChildNodes } from '../utils/dom-utils';
 
-export const UNPRINTABLE_CHARACTER = "\u200C";
+export const UNPRINTABLE_CHARACTER = "\u2006";
 
 function createElementFromMarkup(doc, markup) {
   var element = doc.createElement(markup.tagName);
@@ -55,7 +55,6 @@ function getNextMarkerElement(renderNode) {
 }
 
 function renderMarker(marker, element, previousRenderNode) {
-  const openTypes = marker.openedMarkups;
   let text = marker.value;
   if (isEmptyText(text)) {
     // This is necessary to allow the cursor to move into this area
@@ -66,6 +65,7 @@ function renderMarker(marker, element, previousRenderNode) {
   let currentElement = textNode;
   let markup;
 
+  const openTypes = marker.openedMarkups;
   for (let j=openTypes.length-1;j>=0;j--) {
     markup = openTypes[j];
     let openedElement = createElementFromMarkup(document, markup);
@@ -116,13 +116,11 @@ class Visitor {
 
       if (renderNode.prev) {
         let previousElement = renderNode.prev.element;
-        let nextElement = previousElement.nextSibling;
-        if (nextElement) {
-          nextElement.parentNode.insertBefore(element, nextElement);
-        }
-      }
-      if (!element.parentNode) {
-        renderNode.parent.element.appendChild(element);
+        let parentNode = previousElement.parentNode;
+        parentNode.insertBefore(element, previousElement.nextSibling);
+      } else {
+        let parentElement = renderNode.parent.element;
+        parentElement.insertBefore(element, parentElement.firstChild);
       }
     } else {
       renderNode.parent.element.replaceChild(element, originalElement);

src/js/utils/array-utils.js

@@ -49,9 +49,18 @@ function difference(enumerable, otherEnumerable) {
   return diff;
 }
 
+function filter(enumerable, conditionFn) {
+  const filtered = [];
+  forEach(enumerable, i => {
+    if (conditionFn(i)) { filtered.push(i); }
+  });
+  return filtered;
+}
+
 export {
   detect,
   forEach,
   any,
-  difference
+  difference,
+  filter
 };

src/js/utils/linked-list.js

@@ -9,6 +9,9 @@ export default class LinkedList {
       this.freeItem = freeItem;
     }
   }
+  empty() {
+    return this.length === 0;
+  }
   prepend(item) {
     this.insertBefore(item, this.head);
   }

tests/acceptance/editor-sections-test.js

@@ -101,18 +101,69 @@ module('Acceptance: Editor sections', {
   }
 });
 
-Helpers.skipInPhantom('typing inserts section', (assert) => {
+test('typing enter inserts new section', (assert) => {
   editor = new Editor(editorElement, {mobiledoc: mobileDocWith1Section});
   assert.equal($('#editor p').length, 1, 'has 1 paragraph to start');
 
   Helpers.dom.moveCursorTo(editorElement.childNodes[0].childNodes[0], 5);
-  Helpers.dom.triggerKeyEvent(document, 'keydown', Helpers.dom.KEY_CODES.ENTER);
+  Helpers.dom.triggerEnter(editor);
 
   assert.equal($('#editor p').length, 2, 'has 2 paragraphs after typing return');
   assert.hasElement(`#editor p:contains(only)`, 'has correct first pargraph text');
   assert.hasElement('#editor p:contains(section)', 'has correct second paragraph text');
 });
 
+test('hitting enter in first section splits it correctly', (assert) => {
+  editor = new Editor(editorElement, {mobiledoc: mobileDocWith2Sections});
+  assert.equal($('#editor p').length, 2, 'precond - has 2 paragraphs');
+
+  Helpers.dom.moveCursorTo(editorElement.childNodes[0].childNodes[0], 3);
+  Helpers.dom.triggerEnter(editor);
+
+  assert.equal($('#editor p').length, 3, 'has 3 paragraphs after typing return');
+
+  assert.equal($('#editor p:eq(0)').text(), 'fir', 'first para has correct text');
+  assert.equal($('#editor p:eq(1)').text(), 'st section', 'second para has correct text');
+  assert.equal($('#editor p:eq(2)').text(), 'second section', 'third para still has correct text');
+
+  assert.deepEqual(Helpers.dom.getCursorPosition(),
+                   {node: editorElement.childNodes[1].childNodes[0],
+                    offset: 0});
+});
+
+test('hitting enter at start of a section creates empty section where cursor was', (assert) => {
+  editor = new Editor(editorElement, {mobiledoc: mobileDocWith1Section});
+  assert.equal($('#editor p').length, 1, 'has 1 paragraph to start');
+
+  Helpers.dom.moveCursorTo(editorElement.childNodes[0].childNodes[0], 0);
+  Helpers.dom.triggerEnter(editor);
+
+  assert.equal($('#editor p').length, 2, 'has 2 paragraphs after typing return');
+
+  let firstP = $('#editor p:eq(0)');
+  assert.equal(firstP.text(), UNPRINTABLE_CHARACTER, 'first para has unprintable char');
+  assert.hasElement('#editor p:eq(1):contains(only section)', 'has correct second paragraph text');
+
+  assert.deepEqual(Helpers.dom.getCursorPosition(),
+                   {node: editorElement.childNodes[1].childNodes[0],
+                    offset: 0});
+});
+
+test('hitting enter at end of a section creates new empty section', (assert) => {
+  editor = new Editor(editorElement, {mobiledoc: mobileDocWith1Section});
+  assert.equal($('#editor p').length, 1, 'has 1 section to start');
+
+  Helpers.dom.moveCursorTo(editorElement.childNodes[0].childNodes[0], 'only section'.length);
+  Helpers.dom.triggerEnter(editor);
+
+  assert.equal($('#editor p').length, 2, 'has 2 sections after typing return');
+  assert.hasElement('#editor p:eq(0):contains(only section)', 'has same first section text');
+
+  assert.deepEqual(Helpers.dom.getCursorPosition(),
+                   {node: editorElement.childNodes[1].childNodes[0],
+                    offset: 0});
+});
+
 test('deleting across 0 sections merges them', (assert) => {
   editor = new Editor(editorElement, {mobiledoc: mobileDocWith2Sections});
   assert.equal($('#editor p').length, 2, 'precond - has 2 sections to start');

tests/acceptance/editor-selections-test.js

@@ -176,6 +176,35 @@ test('select text and apply markup multiple times', (assert) => {
   });
 });
 
-// test selecting text across markers deletes intermediary markers
+test('selecting text across markers deletes intermediary markers', (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[2];
+    Helpers.dom.selectText('i', textNode1,
+                           'tio', textNode2);
+    Helpers.dom.triggerEvent(document, 'mouseup');
+
+    setTimeout(() => {
+      Helpers.dom.triggerDelete(editor);
+
+      assert.hasElement('p:contains(fn)', 'has remaining first section');
+      assert.deepEqual(Helpers.dom.getCursorPosition(),
+                       {node: editorElement.childNodes[0].childNodes[0],
+                         offset: 1});
+
+      done();
+    });
+  });
+});
+
 // test selecting text that includes entire sections deletes the sections
+// test selecting text across two types of sections and deleting
 // test selecting text and hitting enter or keydown

tests/helpers/dom.js

@@ -146,6 +146,16 @@ function triggerDelete(editor) {
   }
 }
 
+function triggerEnter(editor) {
+  if (isPhantom()) {
+    // simulate event when testing with phantom
+    let event = { preventDefault() {} };
+    editor.handleNewline(event);
+  } else {
+    triggerKeyEvent(document, 'keydown', KEY_CODES.ENTER);
+  }
+}
+
 const DOMHelper = {
   moveCursorTo,
   selectText,
@@ -156,7 +166,8 @@ const DOMHelper = {
   KEY_CODES,
   getCursorPosition,
   getSelectedText,
-  triggerDelete
+  triggerDelete,
+  triggerEnter
 };
 
 export { triggerEvent };