Skip to content

Commit

Permalink
Always setRange in toggleMarkup and toggleSection
Browse files Browse the repository at this point in the history
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.

When cursor contains non-markerable, toggleMarkup is no-op

Fixes #285
Fixes #287
  • Loading branch information
bantic committed Jan 11, 2016
1 parent 2cf1ff8 commit 5ae07ee
Show file tree
Hide file tree
Showing 7 changed files with 209 additions and 15 deletions.
6 changes: 5 additions & 1 deletion src/js/editor/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,11 @@ class Editor {

// @private
renderRange() {
this.cursor.selectRange(this.range);
if (this.range.isBlank) {
this.cursor.clearSelection();
} else {
this.cursor.selectRange(this.range);
}
this._reportSelectionState();

// ensure that the range is "cleaned"/un-cached after
Expand Down
14 changes: 7 additions & 7 deletions src/js/editor/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -849,13 +849,10 @@ class PostEditor {
* @param {Markup|String} markupOrString Either a markup object created using
* the builder (useful when adding a markup with attributes, like an 'a' markup),
* or, if a string, the tag name of the markup (e.g. 'strong', 'em') to toggle.
* @param {Range} range in which to toggle, defaults to current editor range
* @public
*/
toggleMarkup(markupOrMarkupString) {
const range = this.editor.cursor.offsets;
if (range.isCollapsed) {
return;
}
toggleMarkup(markupOrMarkupString, range=this.editor.range) {
const markup = typeof markupOrMarkupString === 'string' ?
this.builder.createMarkup(markupOrMarkupString) :
markupOrMarkupString;
Expand All @@ -869,7 +866,8 @@ class PostEditor {
} else {
this.addMarkupToRange(range, markup);
}
this.scheduleAfterRender(() => this.editor.selectRange(range));

this.setRange(range);
}

/**
Expand All @@ -887,6 +885,7 @@ class PostEditor {
toggleSection(sectionTagName, range=this.editor.range) {
sectionTagName = normalizeTagName(sectionTagName);
let { post } = this.editor;
let nextRange = range;

let everySectionHasTagName = true;
post.walkMarkerableSections(range, section => {
Expand All @@ -903,8 +902,9 @@ class PostEditor {
});

if (firstChanged) {
this.setRange(new Range(firstChanged.headPosition()));
nextRange = new Range(firstChanged.headPosition());
}
this.setRange(nextRange);
}

_isSameSectionType(section, sectionTagName) {
Expand Down
8 changes: 8 additions & 0 deletions src/js/models/_section.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ export default class Section extends LinkedItem {
unimplementedMethod('join', this);
}

/**
* Markerable sections should override this method
*/
splitMarkerAtOffset() {
let blankEdit = { added: [], removed: [] };
return blankEdit;
}

nextLeafSection() {
const next = this.next;
if (next) {
Expand Down
2 changes: 1 addition & 1 deletion src/js/utils/cursor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
7 changes: 4 additions & 3 deletions src/js/utils/cursor/position.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,17 @@ const Position = class Position {
constructor(section, offset=0) {
this.section = section;
this.offset = offset;
this.isBlank = false;
}

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: {}
};
}
Expand Down
8 changes: 6 additions & 2 deletions src/js/utils/cursor/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}

/**
Expand Down Expand Up @@ -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;
Expand Down
179 changes: 178 additions & 1 deletion tests/unit/editor/post-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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}) => {
Expand Down Expand Up @@ -1387,7 +1435,8 @@ test('#toggleSection toggle when cursor on card section is no-op', (assert) => {
assert.equal(post.sections.length, 1, '1 section');
assert.ok(post.sections.head.isCardSection, 'still card section');

assert.ok(!renderedRange, 'cursor position not changed');
assert.positionIsEqual(renderedRange.head, range.head, 'range head is set to same');
assert.positionIsEqual(renderedRange.tail, range.tail, 'range tail is set to same');
});

test('#toggleSection joins contiguous list items', (assert) => {
Expand Down Expand Up @@ -1415,6 +1464,134 @@ 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 cursor surrounds non-markerable does nothing', (assert) => {
editor = buildEditorWithMobiledoc(
({post, markupSection, marker, cardSection}) => {
return post([
cardSection('my-card')
]);
});

const range = Range.fromSection(editor.post.sections.head);
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}) => {
Expand Down

0 comments on commit 5ae07ee

Please sign in to comment.