Merge pull request #123 from bustlelabs/delete-start-of-section-117

Fix joining of previous section in postEditor when prev section is list

src/js/editor/editor.js

@@ -219,23 +219,17 @@ class Editor {
   handleDeletion(event) {
     event.preventDefault();
 
-    const offsets = this.cursor.offsets;
+    const range = this.cursor.offsets;
 
     if (this.cursor.hasSelection()) {
-      this.run(postEditor => {
-        postEditor.deleteRange(offsets);
-      });
-      this.cursor.moveToSection(offsets.headSection, offsets.headSectionOffset);
+      this.run(postEditor => postEditor.deleteRange(range));
+      this.cursor.moveToPosition(range.head);
     } else {
-      let results = this.run(postEditor => {
-        const {headSection, headSectionOffset} = offsets;
-        const key = Key.fromEvent(event);
-
-        const deletePosition = {section: headSection, offset: headSectionOffset},
-              direction = key.direction;
-        return postEditor.deleteFrom(deletePosition, direction);
+      const key = Key.fromEvent(event);
+      const nextPosition = this.run(postEditor => { 
+        return postEditor.deleteFrom(range.head, key.direction);
       });
-      this.cursor.moveToSection(results.currentSection, results.currentOffset);
+      this.cursor.moveToPosition(nextPosition);
     }
   }
 
@@ -588,9 +582,9 @@ class Editor {
       this.handleNewline(event);
     } else if (key.isPrintable()) {
       if (this.cursor.hasSelection()) {
-        const offsets = this.cursor.offsets;
-        this.run(postEditor => postEditor.deleteRange(offsets));
-        this.cursor.moveToSection(offsets.headSection, offsets.headSectionOffset);
+        const range = this.cursor.offsets;
+        this.run(postEditor => postEditor.deleteRange(range));
+        this.cursor.moveToPosition(range.head);
       }
     }
 

src/js/editor/post.js

@@ -1,5 +1,7 @@
 import { MARKUP_SECTION_TYPE } from '../models/markup-section';
 import { LIST_ITEM_TYPE } from '../models/list-item';
+import { LIST_SECTION_TYPE } from '../models/list-section';
+import Position from '../utils/cursor/position';
 import {
   filter,
   compact
@@ -19,6 +21,25 @@ function isBlankAndListItem(section) {
   return isListItem(section) && section.isBlank;
 }
 
+function isMarkerable(section) {
+  return !!section.markers;
+}
+
+function isListSection(section) {
+  return section.type === LIST_SECTION_TYPE;
+}
+
+// finds the immediately preceding section that is markerable
+function findPreviousMarkerableSection(section) {
+  const prev = section.prev;
+  if (!prev) { return null; }
+  if (isMarkerable(prev)) {
+    return prev;
+  } else if (isListSection(prev)) {
+    return prev.items.tail;
+  }
+}
+
 class PostEditor {
   constructor(editor) {
     this.editor = editor;
@@ -188,59 +209,68 @@ class PostEditor {
    * marker.
    *
    * @method deleteFrom
-   * @param {Object} position object with {section, offset} the marker and offset to delete from
+   * @param {Position} position object with {section, offset} the marker and offset to delete from
    * @param {Number} direction The direction to delete in (default is BACKWARD)
-   * @return {Object} {currentSection, currentOffset} for positioning the cursor
+   * @return {Position} for positioning the cursor
    * @public
    */
-  deleteFrom({section, offset}, direction=DIRECTION.BACKWARD) {
-    if (section.markers.length) {
-      // {{marker, offset}}
-      let result = section.markerPositionAtOffset(offset);
-      if (direction === DIRECTION.BACKWARD) {
-        return this._deleteBackwardFrom(result);
-      } else {
-        return this._deleteForwardFrom(result);
-      }
+  deleteFrom(position, direction=DIRECTION.BACKWARD) {
+    if (direction === DIRECTION.BACKWARD) {
+      return this._deleteBackwardFrom(position);
     } else {
-      if (direction === DIRECTION.BACKWARD) {
-        if (isMarkupSection(section) && section.prev) {
-          let prevSection = section.prev;
-          prevSection.join(section);
-          prevSection.renderNode.markDirty();
-          this.removeSection(section);
-          this.scheduleRerender();
-          this.scheduleDidUpdate();
-          return { currentSection: prevSection, currentOffset: prevSection.text.length };
-        } else if (isListItem(section)) {
-          this.scheduleRerender();
-          this.scheduleDidUpdate();
-
-          const results = this._convertListItemToMarkupSection(section);
-          return {currentSection: results.section, currentOffset: results.offset};
-        }
-      } else if (section.prev || section.next) {
-        let nextSection = section.next || section.post.tail;
+      return this._deleteForwardFrom(position);
+    }
+  }
+
+  _joinPositionToPreviousSection(position) {
+    const {section } = position;
+    let nextPosition = position.clone();
+
+    if (!isMarkerable(section)) {
+      throw new Error('Cannot join non-markerable section to previous section');
+    } else if (isListItem(section)) {
+      nextPosition = this._convertListItemToMarkupSection(section);
+    } else {
+      const prevSection = findPreviousMarkerableSection(section);
+
+      if (prevSection) {
+        const { beforeMarker } = prevSection.join(section);
+        prevSection.renderNode.markDirty();
         this.removeSection(section);
-        return { currentSection: nextSection, currentOffset: 0 };
+
+        nextPosition.section = prevSection;
+        nextPosition.offset = beforeMarker ?
+          prevSection.offsetOfMarker(beforeMarker, beforeMarker.length) : 0;
       }
     }
+
+    this.scheduleRerender();
+    this.scheduleDidUpdate();
+
+    return nextPosition;
   }
 
   /**
    * delete 1 character in the FORWARD direction from the given position
    * @method _deleteForwardFrom
+   * @param {Position} position
    * @private
    */
-  _deleteForwardFrom({marker, offset}) {
-    const nextCursorSection = marker.section,
-          nextCursorOffset = nextCursorSection.offsetOfMarker(marker, offset);
+  _deleteForwardFrom(position) {
+    return this._deleteForwardFromMarkerPosition(position);
+  }
+
+  _deleteForwardFromMarkerPosition(markerPosition) {
+    const {marker, offset} = markerPosition;
+    const {section} = marker;
+    let nextPosition = new Position(section, section.offsetOfMarker(marker, offset));
 
     if (offset === marker.length) {
       const nextMarker = marker.next;
 
       if (nextMarker) {
-        this._deleteForwardFrom({marker: nextMarker, offset: 0});
+        const nextMarkerPosition = {marker: nextMarker, offset: 0};
+        return this._deleteForwardFromMarkerPosition(nextMarkerPosition);
       } else {
         const nextSection = marker.section.next;
         if (nextSection && isMarkupSection(nextSection)) {
@@ -261,10 +291,7 @@ class PostEditor {
     this.scheduleRerender();
     this.scheduleDidUpdate();
 
-    return {
-      currentSection: nextCursorSection,
-      currentOffset: nextCursorOffset
-    };
+    return nextPosition;
   }
 
   _convertListItemToMarkupSection(listItem) {
@@ -275,72 +302,35 @@ class PostEditor {
 
     this._replaceSection(listSection, compact(newSections));
 
-    const newCursorPosition = {
-      section: newMarkupSection,
-      offset: 0
-    };
-    return newCursorPosition;
+    return new Position(newMarkupSection, 0);
   }
 
   /**
    * delete 1 character in the BACKWARD direction from the given position
    * @method _deleteBackwardFrom
+   * @param {Position} position
    * @private
    */
-  _deleteBackwardFrom({marker, offset}) {
-    let nextCursorSection = marker.section,
-        nextCursorOffset = nextCursorSection.offsetOfMarker(marker, offset);
+  _deleteBackwardFrom(position) {
+    const { offset:sectionOffset } = position;
 
-    if (offset === 0) {
-      const prevMarker = marker.prev;
+    if (sectionOffset === 0) {
+      return this._joinPositionToPreviousSection(position);
+    }
 
-      if (prevMarker) {
-        return this._deleteBackwardFrom({marker: prevMarker, offset: prevMarker.length});
-      } else {
-        const section = marker.section;
-
-        if (isListItem(section)) {
-          const newCursorPos = this._convertListItemToMarkupSection(section);
-          nextCursorSection = newCursorPos.section;
-          nextCursorOffset = newCursorPos.offset;
-        } else {
-          const prevSection = section.prev;
-          if (prevSection && isMarkupSection(prevSection)) {
-            nextCursorSection = prevSection;
-            nextCursorOffset = prevSection.text.length;
-
-            let {
-              beforeMarker
-            } = prevSection.join(marker.section);
-            prevSection.renderNode.markDirty();
-            this.removeSection(marker.section);
-
-            nextCursorSection = prevSection;
-
-            if (beforeMarker) {
-              nextCursorOffset = prevSection.offsetOfMarker(beforeMarker, beforeMarker.length);
-            } else {
-              nextCursorOffset = 0;
-            }
-          }
-        }
-      }
+    let nextPosition = position.clone();
+    const { marker, offset:markerOffset } = position.markerPosition;
 
-    } else if (offset <= marker.length) {
-      const offsetToDeleteAt = offset - 1;
-      marker.deleteValueAtOffset(offsetToDeleteAt);
-      nextCursorOffset--;
-      marker.renderNode.markDirty();
-      this._coalesceMarkers(marker.section);
-    }
+    const offsetToDeleteAt = markerOffset - 1;
 
+    marker.deleteValueAtOffset(offsetToDeleteAt);
+    nextPosition.offset -= 1;
+    marker.renderNode.markDirty();
+    this._coalesceMarkers(marker.section);
     this.scheduleRerender();
     this.scheduleDidUpdate();
 
-    return {
-      currentSection: nextCursorSection,
-      currentOffset: nextCursorOffset
-    };
+    return nextPosition;
   }
 
   /**

src/js/models/marker.js

@@ -77,6 +77,9 @@ const Marker = class Marker extends LinkedItem {
   // delete the character at this offset,
   // update the value with the new value
   deleteValueAtOffset(offset) {
+    if (offset < 0 || offset > this.length) {
+      throw new Error(`Invalid offset "${offset}"`);
+    }
     const [ left, right ] = [
       this.value.slice(0, offset),
       this.value.slice(offset+1)

src/js/models/post.js

@@ -134,7 +134,7 @@ export default class Post {
     return containedSections;
   }
 
-  // return the next section that has markers afer this one
+  // return the next section that has markers after this one
   _nextMarkerableSection(section) {
     if (!section) { return null; }
     const isMarkerable = s => !!s.markers;
@@ -143,7 +143,7 @@ export default class Post {
     const isChild      = s => s.parent && !s.post;
     const parent       = s => s.parent;
 
-    let next = section.next;
+    const next = section.next;
     if (next) {
       if (isMarkerable(next)) {
         return next;

src/js/utils/cursor.js

@@ -63,43 +63,35 @@ const Cursor = class Cursor {
 
   // moves cursor to the start of the section
   moveToSection(section, offsetInSection=0) {
-    const {marker, offset} = section.markerPositionAtOffset(offsetInSection);
-    if (marker) {
-      this.moveToMarker(marker, offset);
-    } else {
-      this._moveToNode(section.renderNode.element, offsetInSection);
-    }
+    this.moveToPosition(new Position(section, offsetInSection));
   }
 
-  // moves cursor to marker
-  moveToMarker(headMarker, headOffset=0, tailMarker=headMarker, tailOffset=headOffset) {
-    if (!headMarker) { throw new Error('Cannot move cursor to marker without a marker'); }
-    const headElement = headMarker.renderNode.element;
-    const tailElement = tailMarker.renderNode.element;
-
-    this._moveToNode(headElement, headOffset, tailElement, tailOffset);
+  selectSections(sections) {
+    const headSection = sections[0], tailSection = sections[sections.length - 1];
+    const range = Range.create(headSection, 0, tailSection, tailSection.length);
+    this.selectRange(range);
   }
 
-  selectSections(sections) {
-    const headSection = sections[0],
-          tailSection = sections[sections.length - 1];
+  _findNodeForPosition(position) {
+    const { section } = position;
+    let node, offset;
+    if (section.isBlank) {
+      node = section.renderNode.element;
+      offset = 0;
+    } else {
+      const {marker, offsetInMarker} = position;
+      node = marker.renderNode.element;
+      offset = offsetInMarker;
+    }
 
-    const range = new Range(
-      new Position(headSection, 0),
-      new Position(tailSection, tailSection.text.length)
-    );
-    this.selectRange(range);
+    return {node, offset};
   }
 
   selectRange(range) {
-    const {
-      headMarker,
-      headMarkerOffset,
-      tailMarker,
-      tailMarkerOffset
-    } = range;
-    this.moveToMarker(
-      headMarker, headMarkerOffset, tailMarker, tailMarkerOffset);
+    const { head, tail } = range;
+    const { node:headNode, offset:headOffset } = this._findNodeForPosition(head),
+          { node:tailNode, offset:tailOffset } = this._findNodeForPosition(tail);
+    this._moveToNode(headNode, headOffset, tailNode, tailOffset);
   }
 
   get selection() {

src/js/utils/cursor/position.js

@@ -46,6 +46,10 @@ const Position = class Position {
     this._inCard = isCardSection(section);
   }
 
+  clone() {
+    return new Position(this.section, this.offset);
+  }
+
   get marker() {
     return this.markerPosition.marker;
   }

src/js/utils/key.js

@@ -1,7 +1,7 @@
 import Keycodes from './keycodes';
 export const DIRECTION = {
   FORWARD: 1,
-  BACKWARD: 2
+  BACKWARD: -1 
 };
 
 export const MODIFIERS = {