diff --git a/src/js/models/render-node.js b/src/js/models/render-node.js index 8f8e74a81..3bd0b38e3 100644 --- a/src/js/models/render-node.js +++ b/src/js/models/render-node.js @@ -13,6 +13,9 @@ export default class RenderNode extends LinkedItem { this._childNodes = null; this._element = null; this.renderTree = renderTree; + + // RenderNodes for Markers keep track of their markupElement + this.markupElement = null; } isAttached() { assert('Cannot check if a renderNode is attached without an element.', diff --git a/src/js/renderers/editor-dom.js b/src/js/renderers/editor-dom.js index c66168eb8..58372eb04 100644 --- a/src/js/renderers/editor-dom.js +++ b/src/js/renderers/editor-dom.js @@ -22,7 +22,7 @@ export const SPACE = ' '; export const ZWNJ = '\u200c'; function createElementFromMarkup(doc, markup) { - var element = doc.createElement(markup.tagName); + let element = doc.createElement(markup.tagName); Object.keys(markup.attributes).forEach(k => { element.setAttribute(k, markup.attributes[k]); }); @@ -120,30 +120,44 @@ function getNextMarkerElement(renderNode) { return element; } -function renderMarker(marker, element, previousRenderNode) { +/** + * Render the marker + * @param {Marker} marker the marker to render + * @param {DOMNode} element the element to attach the rendered marker to + * @param {RenderNode} [previousRenderNode] The render node before this one, which + * affects the determination of where to insert this rendered marker. + * @return {element, markupElement} The element (textNode) that has the text for + * this marker, and the outermost rendered element. If the marker has no + * markups, element and markupElement will be the same textNode + */ +function renderMarker(marker, parentElement, previousRenderNode) { let text = renderHTMLText(marker); - let textNode = document.createTextNode(text); - let currentElement = textNode; + let element = document.createTextNode(text); + let markupElement = element; let markup; const openTypes = marker.openedMarkups; for (let j=openTypes.length-1;j>=0;j--) { markup = openTypes[j]; let openedElement = createElementFromMarkup(document, markup); - openedElement.appendChild(currentElement); - currentElement = openedElement; + openedElement.appendChild(markupElement); + markupElement = openedElement; } + let referenceElement; + if (previousRenderNode) { let previousSibling = previousRenderNode.element; - let previousSiblingPenultimate = penultimateParentOf(previousSibling, element); - element.insertBefore(currentElement, previousSiblingPenultimate.nextSibling); + let previousSiblingPenultimate = penultimateParentOf(previousSibling, parentElement); + referenceElement = previousSiblingPenultimate.nextSibling; } else { - element.insertBefore(currentElement, element.firstChild); + referenceElement = parentElement.firstChild; } - return textNode; + parentElement.insertBefore(markupElement, referenceElement); + + return { element, markupElement }; } function attachRenderNodeElementToDOM(renderNode, originalElement) { @@ -274,7 +288,11 @@ class Visitor { parentElement = renderNode.parent.element; } - renderNode.element = renderMarker(marker, parentElement, renderNode.prev); + let { element, markupElement } = + renderMarker(marker, parentElement, renderNode.prev); + + renderNode.element = element; + renderNode.markupElement = markupElement; } [IMAGE_SECTION_TYPE](renderNode, section) { @@ -342,19 +360,15 @@ let destroyHooks = { // FIXME before we render marker, should delete previous renderNode's element // and up until the next marker element - let element = renderNode.element; - let nextMarkerElement = getNextMarkerElement(renderNode); - while (element.parentNode && element.parentNode !== nextMarkerElement) { - element = element.parentNode; - } + let { markupElement } = renderNode; if (marker.section) { marker.section.markers.remove(marker); } - if (element.parentNode) { + if (markupElement.parentNode) { // if no parentNode, the browser already removed this element - element.parentNode.removeChild(element); + markupElement.parentNode.removeChild(markupElement); } }, diff --git a/tests/acceptance/basic-editor-test.js b/tests/acceptance/basic-editor-test.js index bb9d48508..94d470752 100644 --- a/tests/acceptance/basic-editor-test.js +++ b/tests/acceptance/basic-editor-test.js @@ -236,3 +236,34 @@ test('typing enter splits lines, sets cursor', (assert) => { done(); }, 0); }); + +// see https://github.com/bustlelabs/mobiledoc-kit/issues/306 +test('adding/removing bold text between two bold markers works', (assert) => { + editor = Helpers.mobiledoc.renderInto(editorElement, ({post, markupSection, marker, markup}) => { + return post([ + markupSection('p', [ + marker('abc', [markup('b')]), + marker('123', []), + marker('def', [markup('b')]) + ]) + ]); + }); + + // preconditions + assert.hasElement('#editor b:contains(abc)'); + assert.hasElement('#editor b:contains(def)'); + assert.hasNoElement('#editor b:contains(123)'); + + Helpers.dom.selectText('123', editorElement); + editor.run(postEditor => postEditor.toggleMarkup('b')); + + assert.hasElement('#editor b:contains(abc123def)', 'adds B to selection'); + + assert.equal(Helpers.dom.getSelectedText(), '123', '123 still selected'); + + editor.run(postEditor => postEditor.toggleMarkup('b')); + + assert.hasElement('#editor b:contains(abc)', 'removes B from middle, leaves abc'); + assert.hasElement('#editor b:contains(def)', 'removes B from middle, leaves def'); + assert.hasNoElement('#editor b:contains(123)', 'removes B from middle'); +}); diff --git a/tests/unit/renderers/editor-dom-test.js b/tests/unit/renderers/editor-dom-test.js index ec06595da..f51143de7 100644 --- a/tests/unit/renderers/editor-dom-test.js +++ b/tests/unit/renderers/editor-dom-test.js @@ -5,6 +5,7 @@ import Helpers from '../../test-helpers'; import { NO_BREAK_SPACE, ZWNJ } from 'mobiledoc-kit/renderers/editor-dom'; import { TAB } from 'mobiledoc-kit/utils/characters'; const { module, test } = Helpers; +import Range from 'mobiledoc-kit/utils/cursor/range'; import placeholderImageSrc from 'mobiledoc-kit/utils/placeholder-image-src'; let builder; @@ -16,15 +17,20 @@ function render(renderTree, cards=[]) { return renderer.render(renderTree); } +let editor, editorElement; module('Unit: Renderer: Editor-Dom', { beforeEach() { builder = new PostNodeBuilder(); + editorElement = $('#editor')[0]; }, afterEach() { if (renderer) { renderer.destroy(); renderer = null; } + if (editor) { + editor.destroy(); + } } }); @@ -669,6 +675,34 @@ test('#destroy is safe to call if renderer has not rendered', (assert) => { assert.ok(true, 'ok to destroy'); }); +test('rerender after adding markup to a marker when the marker siblings have that markup', (assert) => { + let strong, midMarker; + + editor = Helpers.mobiledoc.renderInto(editorElement, ({post, markupSection, marker, markup}) => { + strong = markup('strong'); + midMarker = marker('X'); + return post([ + markupSection('p', [ + marker('a', [strong]), + midMarker, + marker('c', [strong]) + ]) + ]); + }); + + assert.hasElement('#editor strong:contains(a)'); + assert.hasElement('#editor strong:contains(c)'); + assert.hasNoElement('#editor strong:contains(X)'); + + editor.run(postEditor => { + let range = Range.create(editor.post.sections.head, 1, + editor.post.sections.head, 2); + postEditor.toggleMarkup('strong', range); + }); + + assert.hasElement('#editor strong:contains(aXc)'); +}); + /* test("It renders a renderTree with rendered dirty section", (assert) => { /*