Coalesce markers that have identical markups

This prevents some drift in the mobiledoc that comes with a lot of editing. Prior to this, when contiguous markers were changed so that their markups were identically, they continued to be stored as separate markers (and rendered as contiguous textNodes). The resulted in a messy rendered mobiledoc, with many vacuously similar markers. Keeping markers coordinated should also reduce memory pressure in large documents.
bustle · Sep 17, 2015 · a83b176 · a83b176
1 parent 9c02250
commit a83b176
Show file tree

Hide file tree

Showing 7 changed files with 171 additions and 99 deletions.
diff --git a/README.md b/README.md
@@ -92,7 +92,7 @@ editor.didUpdatePost(postEditor => {
 
 The available lifecycle hooks are:
 
-* `editor.didUpdatePost(postEditor => {})` - An opprotunity to use the
+* `editor.didUpdatePost(postEditor => {})` - An opportunity to use the
   `postEditor` and possibly change the post before rendering begins.
 * `editor.willRender()` - After all post mutation has finished, but before
    the DOM is updated.
@@ -107,24 +107,22 @@ the creation of completely custom interfaces for buttons, hot-keys, and
 other interactions.
 
 To change the post in code, use the `editor.run` API. For example, the
-following usage would mark currently selected text as bold:
+following usage would mark currently selected text as "strong":
 
 ```js
-const strongMarkup = editor.builder.createMarkup('strong');
-const range = editor.cursor.offsets;
-editor.run((postEditor) => {
-  postEditor.applyMarkupToRange(range, strongMarkup);
+editor.run(postEditor => {
+  postEditor.toggleMarkup('strong');
 });
 ```
 
 It is important that you make changes to posts, sections, and markers through
 the `run` and `postEditor` API. This API allows Content-Kit to conserve
 and better understand changes being made to the post.
 
-For more details on the API of `postEditor`, see the [API documentation](https://github.com/mixonic/content-kit-editor/blob/master/src/js/editor/post.js).
+For more details on the API of `postEditor`, see the [API documentation](https://github.com/bustlelabs/content-kit-editor/blob/master/src/js/editor/post.js).
 
 For more details on the API for the builder, required to create new sections
-and markers, see the [builder API](https://github.com/mixonic/content-kit-editor/blob/master/src/js/models/post-node-builder.js).
+and markers, see the [builder API](https://github.com/bustlelabs/content-kit-editor/blob/master/src/js/models/post-node-builder.js).
 
 ### Contributing
 

diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js
@@ -224,10 +224,10 @@ class Editor {
   handleNewline(event) {
     if (!this.cursor.hasCursor()) { return; }
 
-    const range = this.cursor.offsets;
     event.preventDefault();
 
-    const cursorSection = this.run((postEditor) => {
+    const range = this.cursor.offsets;
+    const cursorSection = this.run(postEditor => {
       if (!range.isCollapsed) {
         postEditor.deleteRange(range);
         if (range.head.section.isBlank) {
@@ -262,9 +262,8 @@ class Editor {
 
   cancelSelection() {
     if (this._hasSelection) {
-      // FIXME perhaps restore cursor position to end of the selection?
-      this.cursor.clearSelection();
-      this.reportNoSelection();
+      const range = this.cursor.offsets;
+      this.moveToPosition(range.tail);
     }
   }
 
@@ -339,6 +338,8 @@ class Editor {
     this.cursor.moveToSection(headSection, headSectionOffset);
   }
 
+  // FIXME this should be able to be removed now -- if any sections are detached,
+  // it's due to a bug in the code.
   _removeDetachedSections() {
     forEach(
       filter(this.post.sections, s => !s.renderNode.isAttached()),

diff --git a/src/js/editor/post.js b/src/js/editor/post.js
@@ -3,7 +3,9 @@ import {
 } from '../models/markup-section';
 import { POST_TYPE, MARKUP_SECTION_TYPE, LIST_ITEM_TYPE } from '../models/types';
 import Position from '../utils/cursor/position';
-import { any, filter, compact } from '../utils/array-utils';
+import {
+  isArrayEqual, any, forEach, filter, compact
+} from '../utils/array-utils';
 import { DIRECTION } from '../utils/key';
 
 function isMarkupSection(section) {
@@ -26,10 +28,13 @@ class PostEditor {
   constructor(editor) {
     this.editor = editor;
     this.builder = this.editor.builder;
-    this._completionWorkQueue = [];
-    this._afterRenderQueue = [];
-    this._didRerender = false;
-    this._didUpdate = false;
+    this._queues = {
+      beforeCompletion: [],
+      completion: [],
+      afterCompletion: []
+    };
+    this._didScheduleRerender = false;
+    this._didScheduleUpdate = false;
     this._didComplete = false;
   }
 
@@ -92,14 +97,10 @@ class PostEditor {
       });
       removedSections.forEach(section => this.removeSection(section) );
     }
-
-    this._coalesceMarkers(headSection);
   }
 
   cutSection(section, headSectionOffset, tailSectionOffset) {
-    if (section.markers.isEmpty) {
-      return;
-    }
+    if (section.isBlank) { return; }
 
     let adjustedHead = 0,
         marker = section.markers.head,
@@ -147,9 +148,32 @@ class PostEditor {
   }
 
   _coalesceMarkers(section) {
-    filter(section.markers, m => m.isEmpty).forEach(marker => {
-      this.removeMarker(marker);
-    });
+    this._removeEmptyMarkers(section);
+    this._joinSimilarMarkers(section);
+  }
+
+  _removeEmptyMarkers(section) {
+    forEach(
+      filter(section.markers, m => m.isEmpty),
+      m => this.removeMarker(m)
+    );
+  }
+
+  // joins markers that have identical markups
+  _joinSimilarMarkers(section) {
+    let marker = section.markers.head;
+    let nextMarker;
+    while (marker && marker.next) {
+      nextMarker = marker.next;
+
+      if (isArrayEqual(marker.markups, nextMarker.markups)) {
+        nextMarker.value = marker.value + nextMarker.value;
+        this._markDirty(nextMarker);
+        this.removeMarker(marker);
+      }
+
+      marker = nextMarker;
+    }
   }
 
   removeMarker(marker) {
@@ -167,6 +191,9 @@ class PostEditor {
       this.scheduleRerender();
       this.scheduleDidUpdate();
     }
+    if (isMarkerable(postNode)) {
+      this._queues.beforeCompletion.push(() => this._coalesceMarkers(postNode));
+    }
   }
 
   _markDirty(postNode) {
@@ -176,6 +203,12 @@ class PostEditor {
       this.scheduleRerender();
       this.scheduleDidUpdate();
     }
+    if (postNode.section) {
+      this._markDirty(postNode.section);
+    }
+    if (isMarkerable(postNode)) {
+      this._queues.beforeCompletion.push(() => this._coalesceMarkers(postNode));
+    }
   }
 
   /**
@@ -305,7 +338,6 @@ class PostEditor {
     } else {
       marker.deleteValueAtOffset(offset);
       this._markDirty(marker);
-      this._coalesceMarkers(marker.section);
     }
 
     return nextPosition;
@@ -344,7 +376,6 @@ class PostEditor {
     marker.deleteValueAtOffset(offsetToDeleteAt);
     nextPosition.offset -= 1;
     this._markDirty(marker);
-    this._coalesceMarkers(marker.section);
 
     return nextPosition;
   }
@@ -511,19 +542,15 @@ class PostEditor {
    * value as `splitMarkers`.
    *
    * @method applyMarkupToRange
-   * @param {Range} markerRange
+   * @param {Range} range
    * @param {Markup} markup A markup post abstract node
-   * @return {Array} of markers that are inside the split
    * @public
    */
-  applyMarkupToRange(markerRange, markup) {
-    const markers = this.splitMarkers(markerRange);
-    markers.forEach(marker => {
+  applyMarkupToRange(range, markup) {
+    this.splitMarkers(range).forEach(marker => {
       marker.addMarkup(markup);
       this._markDirty(marker.section);
     });
-
-    return markers;
   }
 
   /**
@@ -547,7 +574,6 @@ class PostEditor {
    * @method removeMarkupFromRange
    * @param {Range} range Object with offsets
    * @param {Markup} markup A markup post abstract node
-   * @return {Array} of markers that are inside the split
    * @private
    */
   removeMarkupFromRange(range, markupOrMarkupCallback) {
@@ -556,8 +582,6 @@ class PostEditor {
       marker.removeMarkup(markupOrMarkupCallback);
       this._markDirty(marker.section);
     });
-
-    return markers;
   }
 
   /**
@@ -709,7 +733,7 @@ class PostEditor {
     if (this._didComplete) {
       throw new Error('Work can only be scheduled before a post edit has completed');
     }
-    this._completionWorkQueue.push(callback);
+    this._queues.completion.push(callback);
   }
 
   /**
@@ -719,12 +743,10 @@ class PostEditor {
    * @public
    */
   scheduleRerender() {
-    this.schedule(() => {
-      if (!this._didRerender) {
-        this._didRerender = true;
-        this.editor.rerender();
-      }
-    });
+    if (!this._didScheduleRerender) {
+      this.schedule(() => this.editor.rerender());
+      this._didScheduleRerender = true;
+    }
   }
 
   /**
@@ -734,16 +756,14 @@ class PostEditor {
    * @public
    */
   scheduleDidUpdate() {
-    this.schedule(() => {
-      if (!this._didUpdate) {
-        this._didUpdate = true;
-        this.editor.didUpdate();
-      }
-    });
+    if (!this._didScheduleUpdate) {
+      this.schedule(() => this.editor.didUpdate());
+      this._didScheduleUpdate = true;
+    }
   }
 
   scheduleAfterRender(callback) {
-    this._afterRenderQueue.push(callback);
+    this._queues.afterCompletion.push(callback);
   }
 
   /**
@@ -757,9 +777,14 @@ class PostEditor {
     if (this._didComplete) {
       throw new Error('Post editing can only be completed once');
     }
+
+    this._runCallbacks([this._queues.beforeCompletion]);
     this._didComplete = true;
-    this._completionWorkQueue.forEach(cb => cb());
-    this._afterRenderQueue.forEach(cb => cb());
+    this._runCallbacks([this._queues.completion, this._queues.afterCompletion]);
+  }
+
+  _runCallbacks(queues) {
+    queues.forEach(queue => queue.forEach(cb => cb()));
   }
 }
 

diff --git a/src/js/utils/array-utils.js b/src/js/utils/array-utils.js
@@ -10,9 +10,11 @@ function detect(enumerable, callback) {
   }
 }
 
-function any(array, callback) {
-  for (let i=0; i<array.length; i++) {
-    if (callback(array[i])) {
+function any(enumerable, callback) {
+  if (enumerable.any) { return enumerable.any(callback); }
+
+  for (let i=0; i<enumerable.length; i++) {
+    if (callback(enumerable[i])) {
       return true;
     }
   }
@@ -92,6 +94,17 @@ function objectToSortedKVArray(obj) {
   return result;
 }
 
+// check shallow equality of two non-nested arrays
+function isArrayEqual(arr1, arr2) {
+  let l1 = arr1.length, l2 = arr2.length;
+  if (l1 !== l2) { return false; }
+
+  for (let i=0; i < l1; i++) {
+    if (arr1[i] !== arr2[i]) { return false; }
+  }
+  return true;
+}
+
 export {
   detect,
   forEach,
@@ -101,5 +114,6 @@ export {
   compact,
   reduce,
   objectToSortedKVArray,
-  kvArrayToObject
+  kvArrayToObject,
+  isArrayEqual
 };
diff --git a/src/js/utils/linked-list.js b/src/js/utils/linked-list.js
@@ -139,6 +139,9 @@ export default class LinkedList {
       item = item.next;
     }
   }
+  any(callback) {
+    return !!this.detect(callback);
+  }
   objectAt(targetIndex) {
     let index = -1;
     return this.detect(() => {

diff --git a/tests/acceptance/editor-selections-test.js b/tests/acceptance/editor-selections-test.js
@@ -63,8 +63,8 @@ test('selecting an entire section and deleting removes it', (assert) => {
   Helpers.dom.selectText('second section', editorElement);
   Helpers.dom.triggerDelete(editor);
 
-  assert.hasElement('p:contains(first section)');
-  assert.hasNoElement('p:contains(second section)', 'deletes contents of second section');
+  assert.hasElement('#editor p:contains(first section)');
+  assert.hasNoElement('#editor p:contains(second section)', 'deletes contents of second section');
   assert.equal($('#editor p').length, 2, 'still has 2 sections');
 
   Helpers.dom.insertText(editor, 'X');
@@ -81,16 +81,12 @@ test('selecting text in a section and deleting deletes it', (assert) => {
   Helpers.dom.selectText('cond sec', editorElement);
   Helpers.dom.triggerDelete(editor);
 
-  assert.hasElement('p:contains(first section)', 'first section unchanged');
-  assert.hasNoElement('p:contains(second section)', 'second section is no longer there');
-  assert.hasElement('p:contains(setion)', 'second section has correct text');
+  assert.hasElement('#editor p:contains(first section)', 'first section unchanged');
+  assert.hasNoElement('#editor p:contains(second section)', 'second section is no longer there');
+  assert.hasElement('#editor p:contains(setion)', 'second section has correct text');
 
-  let textNode = $('p:contains(setion)')[0].childNodes[0];
-  assert.equal(textNode.textContent, 'se', 'precond - has correct text node');
-  let charOffset = 2; // after the 'e' in 'se'
-
-  assert.deepEqual(Helpers.dom.getCursorPosition(),
-                   {node: textNode, offset: charOffset});
+  Helpers.dom.insertText(editor, 'Z');
+  assert.hasElement('#editor p:contains(seZtion)', 'text inserted correctly');
 });
 
 test('selecting text across sections and deleting joins sections', (assert) => {