From 15ceb0f20dc07c1c692a4156632a62fa5d4695c3 Mon Sep 17 00:00:00 2001 From: Cory Forsyth Date: Thu, 17 Dec 2015 11:54:13 -0500 Subject: [PATCH] Clear selection in editor#destroy, make editorDomRenderer#destroy safer * Add a `hasRendered` property to Editor and EditorDomRenderer instances. * If an editor is destroyed when it has an active cursor, clear that selection. This fixes an issue with calling `editor.cursor.offsets` when the editor's dom element is selected but the editor has not yet rendered. See https://github.com/bustlelabs/ember-mobiledoc-editor/pull/39 * Update Cursor `hasCursor` and `hasSelection` methods to always return false when the editor has not yet rendered. * Fix a bug when calling EditorDomRenderer#destroy if it has not yet rendered. --- src/js/editor/editor.js | 25 ++++++++++------ src/js/renderers/editor-dom.js | 5 ++++ src/js/utils/cursor.js | 6 ++-- tests/unit/editor/editor-test.js | 40 +++++++++++++++++++++++++ tests/unit/renderers/editor-dom-test.js | 21 +++++++++++-- 5 files changed, 84 insertions(+), 13 deletions(-) diff --git a/src/js/editor/editor.js b/src/js/editor/editor.js index b90fff33f..7b0314893 100644 --- a/src/js/editor/editor.js +++ b/src/js/editor/editor.js @@ -38,6 +38,7 @@ import { } from '../utils/paste-utils'; import { DIRECTION } from 'mobiledoc-kit/utils/key'; import { TAB } from 'mobiledoc-kit/utils/characters'; +import assert from '../utils/assert'; export const EDITOR_ELEMENT_CLASS_NAME = '__mobiledoc-editor'; @@ -119,8 +120,9 @@ class Editor { } else if (this.html) { if (typeof this.html === 'string') { return new HTMLParser(this.builder).parse(this.html); - } else { // DOM - return this._parser.parse(this.html); + } else { + let dom = this.html; + return this._parser.parse(dom); } } else { return this.builder.createPost(); @@ -132,9 +134,8 @@ class Editor { // if we haven't rendered this post's renderNode before, mark it dirty if (!postRenderNode.element) { - if (!this.element) { - throw new Error('Initial call to `render` must happen before `rerender` can be called.'); - } + assert('Must call `render` before `rerender` can be called', + this.hasRendered); postRenderNode.element = this.element; postRenderNode.markDirty(); } @@ -147,10 +148,9 @@ class Editor { } render(element) { - if (this.element) { - throw new Error('Cannot render an editor twice. Use `rerender` to update the rendering of an existing editor instance'); - } - + assert('Cannot render an editor twice. Use `rerender` to update the ' + + 'rendering of an existing editor instance.', + !this.hasRendered); addClassName(element, EDITOR_ELEMENT_CLASS_NAME); element.spellcheck = this.spellcheck; @@ -437,6 +437,9 @@ class Editor { destroy() { this._isDestroyed = true; + if (this.cursor.hasCursor()) { + this.cursor.clearSelection(); + } this.removeMutationObserver(); this._mutationObserver = null; this.removeAllEventListeners(); @@ -769,6 +772,10 @@ class Editor { cardSection.setInitialMode(mode); } } + + get hasRendered() { + return !!this.element; + } } mixin(Editor, EventEmitter); diff --git a/src/js/renderers/editor-dom.js b/src/js/renderers/editor-dom.js index df7daf0a7..c104347de 100644 --- a/src/js/renderers/editor-dom.js +++ b/src/js/renderers/editor-dom.js @@ -393,9 +393,13 @@ export default class Renderer { this.editor = editor; this.visitor = new Visitor(editor, cards, unknownCardHandler, options); this.nodes = []; + this.hasRendered = false; } destroy() { + if (!this.hasRendered) { + return; + } let renderNode = this.renderTree.rootNode; let force = true; removeDestroyedChildren(renderNode, force); @@ -413,6 +417,7 @@ export default class Renderer { } render(renderTree) { + this.hasRendered = true; this.renderTree = renderTree; let renderNode = renderTree.rootNode; let method, postNode; diff --git a/src/js/utils/cursor.js b/src/js/utils/cursor.js index be42a43ad..64de23f4f 100644 --- a/src/js/utils/cursor.js +++ b/src/js/utils/cursor.js @@ -25,11 +25,13 @@ const Cursor = class Cursor { * editor's element or a selection that is contained in the editor's element */ hasCursor() { - return this._hasCollapsedSelection() || this._hasSelection(); + return this.editor.hasRendered && + (this._hasCollapsedSelection() || this._hasSelection()); } hasSelection() { - return this._hasSelection(); + return this.editor.hasRendered && + this._hasSelection(); } /** diff --git a/tests/unit/editor/editor-test.js b/tests/unit/editor/editor-test.js index e40617146..c0c530544 100644 --- a/tests/unit/editor/editor-test.js +++ b/tests/unit/editor/editor-test.js @@ -259,3 +259,43 @@ test('activeSections of a rendered blank mobiledoc is an empty array', (assert) assert.equal(0, editor.activeSections.length, 'empty activeSections'); }); + +test('editor.cursor.hasCursor() is false before rendering', (assert) => { + let mobiledoc = Helpers.mobiledoc.build(({post}) => post()); + editor = new Editor({mobiledoc}); + + assert.ok(!editor.cursor.hasCursor(), 'no cursor before rendering'); + + Helpers.dom.moveCursorTo(editorElement, 0); + + assert.ok(!editor.cursor.hasCursor(), + 'no cursor before rendering, even when selection exists'); +}); + +test('#destroy clears selection if it has one', (assert) => { + let mobiledoc = Helpers.mobiledoc.build(({post}) => post()); + editor = new Editor({mobiledoc}); + editor.render(editorElement); + + Helpers.dom.moveCursorTo(editorElement, 0); + assert.ok(editor.cursor.hasCursor(), 'precond - has cursor'); + + editor.destroy(); + + assert.equal(window.getSelection().rangeCount, 0, + 'selection is cleared'); +}); + +test('#destroy does not clear selection if it is outside the editor element', (assert) => { + let mobiledoc = Helpers.mobiledoc.build(({post}) => post()); + editor = new Editor({mobiledoc}); + editor.render(editorElement); + + Helpers.dom.moveCursorTo($('#qunit-fixture')[0], 0); + assert.ok(!editor.cursor.hasCursor(), 'precond - has no cursor'); + assert.equal(window.getSelection().rangeCount, 1, 'precond - has selection'); + + editor.destroy(); + + assert.equal(window.getSelection().rangeCount, 1, 'selection is not cleared'); +}); diff --git a/tests/unit/renderers/editor-dom-test.js b/tests/unit/renderers/editor-dom-test.js index a2a2a44b5..9407ddf5b 100644 --- a/tests/unit/renderers/editor-dom-test.js +++ b/tests/unit/renderers/editor-dom-test.js @@ -11,15 +11,22 @@ const ZWNJ = '\u200c'; import placeholderImageSrc from 'mobiledoc-kit/utils/placeholder-image-src'; let builder; +let renderer; function render(renderTree, cards=[]) { let editor = {}; - let renderer = new Renderer(editor, cards); + renderer = new Renderer(editor, cards); return renderer.render(renderTree); } module('Unit: Renderer: Editor-Dom', { beforeEach() { builder = new PostNodeBuilder(); + }, + afterEach() { + if (renderer) { + renderer.destroy(); + renderer = null; + } } }); @@ -174,7 +181,6 @@ test('renders a post with multiple markers', (assert) => { assert.equal(renderTree.rootElement.innerHTML, '

hello bold, italic, world.

'); }); - test('renders a post with image', (assert) => { let url = placeholderImageSrc; let post = builder.createPost(); @@ -598,6 +604,17 @@ test('renders a bunch of spaces with nbsp', (assert) => { assert.equal(renderTree.rootElement.innerHTML, expectedDOM.outerHTML); }); +test('#destroy is safe to call if renderer has not rendered', (assert) => { + let mockEditor = {}, cards = []; + let renderer = new Renderer(mockEditor, cards); + + assert.ok(!renderer.hasRendered, 'precond - has not rendered'); + + renderer.destroy(); + + assert.ok(true, 'ok to destroy'); +}); + /* test("It renders a renderTree with rendered dirty section", (assert) => { /*