Always setRange in toggleMarkup and toggleSection

This helps prevent a situation where the editor element has a selection but it is not the active element. Typing while another element (like a button) is focused (active) but the editor element has the selection is problematic: The browser refocuses on the editor element and starts inserting text, but it handles the input before it has focused, which means the key* handlers do not fire. Also renamed Position#emptyPosition and Range#emptyRange -> blankPosition, blankRange. Adds `isBlank` property to Position and Range. Defensively avoid attempting to render a cursor when the range is blank. Fixes #285
bustle · Jan 11, 2016 · 3edfda7 · 3edfda7
1 parent 33eb352
commit 3edfda7
Show file tree

Hide file tree

Showing 5 changed files with 175 additions and 7 deletions.
diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js
@@ -306,7 +306,9 @@ class Editor {
 
   // @private
   renderRange() {
-    this.cursor.selectRange(this.range);
+    if (!this.range.isBlank) {
+      this.cursor.selectRange(this.range);
+    }
     this._reportSelectionState();
 
     // ensure that the range is "cleaned"/un-cached after

diff --git a/src/js/utils/cursor.js b/src/js/utils/cursor.js
@@ -59,7 +59,7 @@ const Cursor = class Cursor {
    * @return {Range} Cursor#Range object
    */
   get offsets() {
-    if (!this.hasCursor()) { return Range.emptyRange(); }
+    if (!this.hasCursor()) { return Range.blankRange(); }
 
     const { selection, renderTree } = this;
 

diff --git a/src/js/utils/cursor/position.js b/src/js/utils/cursor/position.js
@@ -39,14 +39,14 @@ const Position = class Position {
     this.offset = offset;
   }
 
-  static emptyPosition() {
+  static blankPosition() {
     return {
       section: null,
       offset: 0,
       marker: null,
       offsetInTextNode: 0,
-      _isEmpty: true,
-      isEqual(other) { return other._isEmpty; },
+      isBlank: true,
+      isEqual(other) { return other.isBlank; },
       markerPosition: {}
     };
   }
@@ -63,6 +63,10 @@ const Position = class Position {
     return this.markerPosition.offset;
   }
 
+  get isBlank() {
+    return false;
+  }
+
   isEqual(position) {
     return this.section === position.section &&
            this.offset  === position.offset;

diff --git a/src/js/utils/cursor/range.js b/src/js/utils/cursor/range.js
@@ -19,8 +19,8 @@ export default class Range {
     return new Range(section.headPosition(), section.tailPosition());
   }
 
-  static emptyRange() {
-    return new Range(Position.emptyPosition(), Position.emptyPosition());
+  static blankRange() {
+    return new Range(Position.blankPosition(), Position.blankPosition());
   }
 
   /**
@@ -59,6 +59,10 @@ export default class Range {
            this.tail.isEqual(other.tail);
   }
 
+  get isBlank() {
+    return this.head.isBlank && this.tail.isBlank;
+  }
+
   // "legacy" APIs
   get headSection() {
     return this.head.section;

diff --git a/tests/unit/editor/post-test.js b/tests/unit/editor/post-test.js
@@ -7,6 +7,7 @@ import { DIRECTION } from 'mobiledoc-kit/utils/key';
 import PostNodeBuilder from 'mobiledoc-kit/models/post-node-builder';
 import Range from 'mobiledoc-kit/utils/cursor/range';
 import Position from 'mobiledoc-kit/utils/cursor/position';
+import { clearSelection } from 'mobiledoc-kit/utils/selection-utils';
 
 const { FORWARD } = DIRECTION;
 
@@ -1006,6 +1007,53 @@ test('#toggleSection skips over non-markerable sections', (assert) => {
   assert.positionIsEqual(renderedRange.head, post.sections.head.headPosition());
 });
 
+test('#toggleSection when cursor is in non-markerable section changes nothing', (assert) => {
+  let post = Helpers.postAbstract.build(
+    ({post, markupSection, marker, cardSection}) => {
+    return post([
+      cardSection('my-card')
+    ]);
+  });
+
+  const mockEditor = renderBuiltAbstract(post);
+  const range = new Range(post.sections.head.headPosition());
+
+  postEditor = new PostEditor(mockEditor);
+  postEditor.toggleSection('blockquote', range);
+  postEditor.complete();
+
+  assert.ok(post.sections.head.isCardSection, 'card section not changed');
+  assert.positionIsEqual(renderedRange.head, post.sections.head.headPosition());
+});
+
+test('#toggleSection when editor has no cursor does nothing', (assert) => {
+  editor = buildEditorWithMobiledoc(
+    ({post, markupSection, marker, cardSection}) => {
+    return post([
+      cardSection('my-card')
+    ]);
+  });
+  let expected = Helpers.postAbstract.build(
+    ({post, markupSection, marker, cardSection}) => {
+    return post([
+      cardSection('my-card')
+    ]);
+  });
+
+  editorElement.blur();
+  clearSelection();
+
+  postEditor = new PostEditor(editor);
+  postEditor.toggleSection('blockquote');
+  postEditor.complete();
+
+  assert.postIsSimilar(editor.post, expected);
+  assert.ok(document.activeElement !== editorElement,
+            'editor element is not active');
+  assert.ok(renderedRange.isBlank, 'rendered range is blank');
+  assert.equal(window.getSelection().rangeCount, 0, 'nothing selected');
+});
+
 test('#toggleSection toggle single p -> list item', (assert) => {
   let post = Helpers.postAbstract.build(
     ({post, markupSection, marker, markup}) => {
@@ -1416,6 +1464,116 @@ test('#toggleSection joins contiguous list items', (assert) => {
                    ['abc', '123', 'def']);
 });
 
+test('#toggleMarkup when cursor is in non-markerable does nothing', (assert) => {
+  editor = buildEditorWithMobiledoc(
+    ({post, markupSection, marker, cardSection}) => {
+    return post([
+      cardSection('my-card')
+    ]);
+  });
+
+  const range = new Range(editor.post.sections.head.headPosition());
+  postEditor = new PostEditor(editor);
+  postEditor.toggleMarkup('b', range);
+  postEditor.complete();
+
+  assert.ok(editor.post.sections.head.isCardSection);
+  assert.positionIsEqual(renderedRange.head,
+                         editor.post.sections.head.headPosition());
+});
+
+test('#toggleMarkup when range has the markup removes it', (assert) => {
+  editor = buildEditorWithMobiledoc(
+    ({post, markupSection, marker, markup}) => {
+    return post([markupSection('p', [marker('abc', [markup('b')])])]);
+  });
+  let expected = Helpers.postAbstract.build(
+    ({post, markupSection, marker}) => {
+    return post([markupSection('p', [marker('abc')])]);
+  });
+
+  const range = Range.fromSection(editor.post.sections.head);
+  postEditor = new PostEditor(editor);
+  postEditor.toggleMarkup('b', range);
+  postEditor.complete();
+
+  assert.positionIsEqual(renderedRange.head, editor.post.sections.head.headPosition());
+  assert.positionIsEqual(renderedRange.tail, editor.post.sections.head.tailPosition());
+  assert.postIsSimilar(editor.post, expected);
+});
+
+test('#toggleMarkup when only some of the range has it removes it', (assert) => {
+  editor = buildEditorWithMobiledoc(
+    ({post, markupSection, marker, markup}) => {
+    return post([markupSection('p', [
+      marker('a'),
+      marker('b', [markup('b')]),
+      marker('c')
+    ])]);
+  });
+  let expected = Helpers.postAbstract.build(
+    ({post, markupSection, marker}) => {
+    return post([markupSection('p', [marker('abc')])]);
+  });
+
+  const range = Range.fromSection(editor.post.sections.head);
+  postEditor = new PostEditor(editor);
+  postEditor.toggleMarkup('b', range);
+  postEditor.complete();
+
+  assert.positionIsEqual(renderedRange.head,
+                         editor.post.sections.head.headPosition());
+  assert.positionIsEqual(renderedRange.tail,
+                         editor.post.sections.head.tailPosition());
+  assert.postIsSimilar(editor.post, expected);
+});
+
+test('#toggleMarkup when range does not have the markup adds it', (assert) => {
+  editor = buildEditorWithMobiledoc(
+    ({post, markupSection, marker}) => {
+    return post([markupSection('p', [marker('abc')])]);
+  });
+  let expected = Helpers.postAbstract.build(
+    ({post, markupSection, marker, markup}) => {
+    return post([markupSection('p', [marker('abc', [markup('b')])])]);
+  });
+
+  const range = Range.fromSection(editor.post.sections.head);
+  postEditor = new PostEditor(editor);
+  postEditor.toggleMarkup('b', range);
+  postEditor.complete();
+
+  assert.positionIsEqual(renderedRange.head,
+                         editor.post.sections.head.headPosition());
+  assert.positionIsEqual(renderedRange.tail,
+                         editor.post.sections.head.tailPosition());
+  assert.postIsSimilar(editor.post, expected);
+});
+
+test('#toggleMarkup when the editor has no cursor', (assert) => {
+  editor = buildEditorWithMobiledoc(
+    ({post, markupSection, marker}) => {
+    return post([markupSection('p', [marker('abc')])]);
+  });
+  let expected = Helpers.postAbstract.build(
+    ({post, markupSection, marker}) => {
+    return post([markupSection('p', [marker('abc')])]);
+  });
+
+  editorElement.blur();
+  clearSelection();
+  postEditor = new PostEditor(editor);
+  postEditor.toggleMarkup('b');
+  postEditor.complete();
+
+  assert.postIsSimilar(editor.post, expected);
+  assert.equal(window.getSelection().rangeCount, 0,
+               'nothing is selected');
+  assert.ok(document.activeElement !== editorElement,
+            'active element is not editor element');
+  assert.ok(renderedRange.isBlank, 'rendered range is blank');
+});
+
 test('#insertMarkers inserts the markers in middle, merging markups', (assert) => {
   let toInsert, expected;
   Helpers.postAbstract.build(({post, markupSection, marker, markup}) => {