Skip to content

Commit

Permalink
Refactor cursor positioning
Browse files Browse the repository at this point in the history
It was previously possible to create a position with a null section or
an invalid section (like a list section). A position must have a section
that the cursor can be rendered at; this PR adds an assertion to that
fact.

Before this change, if the window.getSelection() pointed at the editor's
element, Position#fromNode would naively assume that the post's first
section at offset 0 was the position, but this creates an invalid
position when the first section is a list section.

 * add Post#headPosition and #tailPosition
 * add Section#isSection property
 * Refactor Position#fromNode
 * tests for Position#fromNode for text nodes, element nodes, card-related nodes
 * Position constructor asserts it has a valid leaft section and offset
  • Loading branch information
bantic committed Jan 26, 2016
1 parent 6498862 commit af5ae91
Show file tree
Hide file tree
Showing 11 changed files with 241 additions and 39 deletions.
2 changes: 2 additions & 0 deletions src/js/models/_section.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ export default class Section extends LinkedItem {
this.type = type;
this.isMarkerable = false;
this.isNested = false;
this.isSection = true;
this.isLeafSection = true;
}

set tagName(val) {
Expand Down
1 change: 1 addition & 0 deletions src/js/models/list-section.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export default class ListSection extends Section {
super(LIST_SECTION_TYPE);
this.tagName = tagName;
this.isListSection = true;
this.isLeafSection = false;

this.items = new LinkedList({
adoptItem: i => {
Expand Down
17 changes: 17 additions & 0 deletions src/js/models/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { forEach, compact } from 'mobiledoc-kit/utils/array-utils';
import Set from 'mobiledoc-kit/utils/set';
import mobiledocRenderers from 'mobiledoc-kit/renderers/mobiledoc';
import Range from 'mobiledoc-kit/utils/cursor/range';
import Position from 'mobiledoc-kit/utils/cursor/position';

export default class Post {
constructor() {
Expand All @@ -14,6 +15,22 @@ export default class Post {
});
}

headPosition() {
if (this.isBlank) {
return Position.blankPosition();
} else {
return this.sections.head.headPosition();
}
}

tailPosition() {
if (this.isBlank) {
return Position.blankPosition();
} else {
return this.sections.tail.tailPosition();
}
}

get isBlank() {
return this.sections.isEmpty;
}
Expand Down
7 changes: 4 additions & 3 deletions src/js/renderers/editor-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ import { MARKUP_SECTION_ELEMENT_NAMES } from '../models/markup-section';
import assert from '../utils/assert';
import { TAB } from 'mobiledoc-kit/utils/characters';

const CARD_ELEMENT_CLASS_NAME = '__mobiledoc-card';
export const CARD_ELEMENT_CLASS_NAME = '__mobiledoc-card';
export const NO_BREAK_SPACE = '\u00A0';
export const TAB_CHARACTER = '\u2003';
export const SPACE = ' ';
export const ZWNJ = '\u200c';

function createElementFromMarkup(doc, markup) {
var element = doc.createElement(markup.tagName);
Expand Down Expand Up @@ -102,9 +103,9 @@ function renderCard() {
let cardElement = document.createElement('div');
cardElement.contentEditable = false;
addClassName(cardElement, CARD_ELEMENT_CLASS_NAME);
wrapper.appendChild(document.createTextNode('\u200c'));
wrapper.appendChild(document.createTextNode(ZWNJ));
wrapper.appendChild(cardElement);
wrapper.appendChild(document.createTextNode('\u200c'));
wrapper.appendChild(document.createTextNode(ZWNJ));
return { wrapper, cardElement };
}

Expand Down
54 changes: 25 additions & 29 deletions src/js/utils/cursor/position.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,37 +4,38 @@ import {
import { DIRECTION } from 'mobiledoc-kit/utils/key';
import assert from 'mobiledoc-kit/utils/assert';

function isSection(postNode) {
if (!(postNode && postNode.type)) { return false; }
return postNode.isMarkerable || postNode.isCardSection;
}

function findParentSectionFromNode(renderTree, node) {
let renderNode = renderTree.findRenderNodeFromElement(
node,
(renderNode) => isSection(renderNode.postNode)
(renderNode) => renderNode.postNode.isSection
);

return renderNode && renderNode.postNode;
}

function findOffsetInSection(section, node, offset) {
if (!section.isCardSection) {
return findOffsetInElement(section.renderNode.element,
node, offset);
}

// Only the card case
let wrapperNode = section.renderNode.element;
let endTextNode = wrapperNode.lastChild;
if (node === endTextNode) {
return 1;
if (section.isMarkerable) {
return findOffsetInElement(section.renderNode.element, node, offset);
} else {
assert('findOffsetInSection must be called with markerable or card section',
section.isCardSection);

let wrapperNode = section.renderNode.element;
let endTextNode = wrapperNode.lastChild;
if (node === endTextNode) {
return 1;
}
return 0;
}
return 0;
}

const Position = class Position {
constructor(section, offset=0) {
assert('Position must have a section that is addressable by the cursor',
(section && section.isLeafSection));
assert('Position must have numeric offset',
(offset !== null && offset !== undefined));

this.section = section;
this.offset = offset;
this.isBlank = false;
Expand Down Expand Up @@ -155,22 +156,16 @@ const Position = class Position {
}

static fromElementNode(renderTree, elementNode, offset) {
let section, offsetInSection;
let position;

// The browser may change the reported selection to equal the editor's root
// element if the user clicks an element that is immediately removed,
// which can happen when clicking to remove a card.
if (elementNode === renderTree.rootElement) {
let post = renderTree.rootNode.postNode;
if (offset === 0) {
section = post.sections.head;
offsetInSection = 0;
} else {
section = post.sections.tail;
offsetInSection = section.length;
}
position = offset === 0 ? post.headPosition() : post.tailPosition();
} else {
section = findParentSectionFromNode(renderTree, elementNode);
let section = findParentSectionFromNode(renderTree, elementNode);
assert('Could not find parent section from element node', !!section);

if (section.isCardSection) {
Expand All @@ -179,14 +174,15 @@ const Position = class Position {
// selecting the card's wrapper div. If the offset is 2 we've selected
// the final zwnj and should consider the cursor at the end of the card (offset 1). Otherwise,
// the cursor is at the start of the card
offsetInSection = offset < 2 ? 0 : 1;
position = offset < 2 ? section.headPosition() : section.tailPosition();
} else {
// The offset is 0 if the cursor is on an element node (e.g., a <br> tag in
// a blank markup section)
offsetInSection = 0;
position = section.headPosition();
}
}
return new Position(section, offsetInSection);

return position;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions tests/helpers/assertions.js
Original file line number Diff line number Diff line change
Expand Up @@ -187,10 +187,10 @@ export default function registerAssertions() {
this.push(false,
`${position.section.type}:${position.section.tagName}`,
`${expected.section.type}:${expected.section.tagName}`,
`incorrect position section`);
`incorrect position section (${message})`);
} else if (position.offset !== expected.offset) {
this.push(false, position.offset, expected.offset,
`incorrect position offset`);
`incorrect position offset (${message})`);
} else {
this.push(true, position, expected, message);
}
Expand Down
13 changes: 12 additions & 1 deletion tests/helpers/mobiledoc.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import PostAbstractHelpers from './post-abstract';
import mobiledocRenderers from 'mobiledoc-kit/renderers/mobiledoc';
import MobiledocRenderer_0_2, { MOBILEDOC_VERSION } from 'mobiledoc-kit/renderers/mobiledoc/0-2';
import Editor from 'mobiledoc-kit/editor/editor';
import { mergeWithOptions } from 'mobiledoc-kit/utils/merge';

/*
* usage:
Expand All @@ -25,6 +27,15 @@ function build(treeFn, version) {
}
}

function renderInto(element, treeFn, editorOptions={}) {
let mobiledoc = build(treeFn);
mergeWithOptions(editorOptions, {mobiledoc});
let editor = new Editor(editorOptions);
editor.render(element);
return editor;
}

export default {
build
build,
renderInto
};
5 changes: 5 additions & 0 deletions tests/unit/models/markup-section-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,3 +253,8 @@ test('splitMarkerAtOffset splits a marker deep in the middle', (assert) => {
assert.deepEqual(section.markers.map(m => m.value),
['a', 'bc', 'de', 'f', 'ghi']);
});

test('a section has property `isSection`', (assert) => {
let section = builder.createMarkupSection();
assert.ok(section.isSection, 'section.isSection');
});
28 changes: 28 additions & 0 deletions tests/unit/models/post-test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Helpers from '../../test-helpers';
import Range from 'mobiledoc-kit/utils/cursor/range';
import Position from 'mobiledoc-kit/utils/cursor/position';

const {module, test} = Helpers;

Expand Down Expand Up @@ -539,3 +540,30 @@ test('#cloneRange when range contains multiple list items and more sections', (a

assert.deepEqual(mobiledoc, expected);
});

test('#headPosition and #tailPosition returns head and tail', (assert) => {
let post = Helpers.postAbstract.build(({post, markupSection, marker}) => {
return post([
markupSection('p', [marker('abc')]),
markupSection('p', [marker('123')])
]);
});

let head = post.headPosition();
let tail = post.tailPosition();

assert.positionIsEqual(head, post.sections.head.headPosition(), 'head pos');
assert.positionIsEqual(tail, post.sections.tail.tailPosition(), 'tail pos');
});

test('#headPosition and #tailPosition when post is blank return blank', (assert) => {
let post = Helpers.postAbstract.build(({post}) => {
return post();
});

let head = post.headPosition();
let tail = post.tailPosition();

assert.positionIsEqual(head, Position.blankPosition(), 'head pos');
assert.positionIsEqual(tail, Position.blankPosition(), 'tail pos');
});
4 changes: 1 addition & 3 deletions tests/unit/renderers/editor-dom-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ import PostNodeBuilder from 'mobiledoc-kit/models/post-node-builder';
import Renderer from 'mobiledoc-kit/renderers/editor-dom';
import RenderTree from 'mobiledoc-kit/models/render-tree';
import Helpers from '../../test-helpers';
import { NO_BREAK_SPACE } from 'mobiledoc-kit/renderers/editor-dom';
import { NO_BREAK_SPACE, ZWNJ } from 'mobiledoc-kit/renderers/editor-dom';
import { TAB } from 'mobiledoc-kit/utils/characters';
const { module, test } = Helpers;

const ZWNJ = '\u200c';

import placeholderImageSrc from 'mobiledoc-kit/utils/placeholder-image-src';
let builder;

Expand Down
Loading

0 comments on commit af5ae91

Please sign in to comment.