Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid rerendering atoms when section content changes. #471

Merged
merged 1 commit into from
Aug 25, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 6 additions & 11 deletions src/js/models/atom-node.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,13 @@ export default class AtomNode {
}

render() {
this.teardown();

let rendered = this.atom.render({
options: this.atomOptions,
env: this.env,
value: this.model.value,
payload: this.model.payload
});
if (!this._rendered) {
let {atomOptions: options, env, model: { value, payload } } = this;
// cache initial render
this._rendered = this.atom.render({options, env, value, payload});
}

this._validateAndAppendRenderResult(rendered);
this._validateAndAppendRenderResult(this._rendered);
}

get env() {
Expand Down Expand Up @@ -54,7 +51,5 @@ export default class AtomNode {
!!rendered.nodeType
);
this.element.appendChild(rendered);
this._rendered = rendered;
}

}
11 changes: 8 additions & 3 deletions src/js/renderers/editor-dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -449,9 +449,14 @@ class Visitor {
} = renderAtom(atomModel, parentElement, renderNode.prev);
const atom = this._findAtom(atomModel.name);

const atomNode = new AtomNode(
editor, atom, atomModel, atomElement, options
);
let atomNode = renderNode.atomNode;
if (!atomNode) {
// create new AtomNode
atomNode = new AtomNode(editor, atom, atomModel, atomElement, options);
} else {
// retarget atomNode to new atom element
atomNode.element = atomElement;
}

atomNode.render();

Expand Down
7 changes: 5 additions & 2 deletions tests/helpers/post-abstract.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,15 @@ function parsePositionOffsets(text) {
return offsets;
}

const DEFAULT_ATOM_NAME = 'some-atom';

function parseTextIntoMarkers(text, builder) {
text = text.replace(cursorRegex,'');
let markers = [];

if (text.indexOf('@') !== -1) {
let atomIndex = text.indexOf('@');
let atom = builder.atom('some-atom');
let atom = builder.atom(DEFAULT_ATOM_NAME);
let pieces = [text.slice(0, atomIndex), atom, text.slice(atomIndex+1)];
pieces.forEach(piece => {
if (piece === atom) {
Expand Down Expand Up @@ -211,5 +213,6 @@ function buildFromText(texts) {

export default {
build,
buildFromText
buildFromText,
DEFAULT_ATOM_NAME
};
59 changes: 56 additions & 3 deletions tests/unit/editor/atom-lifecycle-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ let editorElement, editor;
import { MOBILEDOC_VERSION as MOBILEDOC_VERSION_0_3 } from 'mobiledoc-kit/renderers/mobiledoc/0-3';

const { module, test } = Helpers;
const { postAbstract: { DEFAULT_ATOM_NAME } } = Helpers;

module('Unit: Editor: Atom Lifecycle', {
beforeEach() {
Expand All @@ -18,10 +19,11 @@ module('Unit: Editor: Atom Lifecycle', {
}
});


function makeEl(id) {
function makeEl(id, text='@atom') {
let el = document.createElement('span');
el.id = id;
text = document.createTextNode(text);
el.appendChild(text);
return el;
}

Expand Down Expand Up @@ -231,15 +233,63 @@ test('onTeardown hook is called when editor is destroyed', (assert) => {
assert.ok(teardown, 'onTeardown hook called');
});

test('onTeardown hook is called when atom is destroyed', (assert) => {
let teardown;

let atom = {
name: DEFAULT_ATOM_NAME,
type: 'dom',
render({env}) {
env.onTeardown(() => teardown = true);
return makeEl('atom-id','atom-text');
}
};
editor = Helpers.editor.buildFromText('abc@d|ef', {autofocus: true, atoms:[atom], element: editorElement});
assert.hasElement('#editor #atom-id:contains(atom-text)', 'precond - shows atom');
assert.ok(!teardown, 'precond - no teardown yet');
Helpers.dom.triggerDelete(editor);

assert.hasElement('#editor #atom-id:contains(atom-text)', 'precond - still shows atom');
assert.ok(!teardown, 'precond - no teardown yet');
Helpers.dom.triggerDelete(editor);

assert.hasNoElement('*:contains(atom-text)', 'atom destroyed');
assert.ok(teardown, 'calls teardown');
});

// See https://github.com/bustlelabs/mobiledoc-kit/issues/421
test('render is not called again when modifying other parts of the section', (assert) => {
let renderCount = 0;
let atom = {
name: DEFAULT_ATOM_NAME,
type: 'dom',
render() {
renderCount++;
return makeEl('the-atom');
}
};
editor = Helpers.editor.buildFromText('abc|@def', {autofocus: true, atoms:[atom], element: editorElement});
assert.equal(renderCount, 1, 'renders the atom initially');
editor.insertText('123');
assert.hasElement('#editor *:contains(abc123)', 'precond - inserts text');
assert.equal(renderCount, 1, 'does not rerender the atom');
});

test('mutating the content of an atom does not trigger an update', (assert) => {
assert.expect(5);
const done = assert.async();

const atomName = 'test-atom';

let renderCount = 0;
let teardown;

const atom = {
name: atomName,
type: 'dom',
render() {
render({env}) {
renderCount++;
env.onTeardown(() => teardown = true);
return makeEl('the-atom');
}
};
Expand All @@ -254,12 +304,15 @@ test('mutating the content of an atom does not trigger an update', (assert) => {

assert.hasNoElement('#editor #the-atom', 'precond - atom not rendered');
editor.render(editorElement);
assert.equal(renderCount, 1, 'renders atom');

$("#the-atom").html("updated");

// ensure the mutations have had time to trigger
Helpers.wait(function(){
assert.ok(!updateTriggered);
assert.equal(renderCount, 1, 'does not rerender atom');
assert.ok(!teardown, 'does not teardown atom');
done();
});
});