Skip to content

Commit

Permalink
Ensure that nodes are inserted in the correct position when nested wi…
Browse files Browse the repository at this point in the history
…thin widgets (#297)

* Failing unit test for incorrect node insertion with nested widgets

* Ensure the domNode is set up the tree on all immediate parent WNodeWrappers

* simplify test

* Improve failing unit test scenario

* Only copy over domNode on widget update if there is a parent node
  • Loading branch information
agubler authored Mar 19, 2019
1 parent 39698f9 commit 65f6ba8
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 5 deletions.
22 changes: 17 additions & 5 deletions src/widget-core/vdom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1084,7 +1084,9 @@ export function renderer(renderer: () => WNode | VNode): Renderer {
}
const instanceData = widgetInstanceMap.get(instance)!;
next.instance = instance;
next.domNode = domNode;
if (domNode && domNode.parentNode) {
next.domNode = domNode;
}
next.hasAnimations = hasAnimations;
instanceData.rendering = true;
instance!.__setProperties__(next.node.properties, next.node.bind);
Expand Down Expand Up @@ -1121,6 +1123,19 @@ export function renderer(renderer: () => WNode | VNode): Renderer {
};
}

function setDomNodeOnParentWrapper(next: VNodeWrapper) {
let parentWNodeWrapper = findParentWNodeWrapper(next);
while (parentWNodeWrapper && !parentWNodeWrapper.domNode) {
parentWNodeWrapper.domNode = next.domNode;
const nextParent = _parentWrapperMap.get(parentWNodeWrapper);
if (nextParent && isWNodeWrapper(nextParent)) {
parentWNodeWrapper = nextParent;
continue;
}
parentWNodeWrapper = undefined;
}
}

function _createDom({ next }: CreateDomInstruction): ProcessResult {
let mergeNodes: Node[] = [];
const parentDomNode = findParentDomNode(next)!;
Expand Down Expand Up @@ -1158,10 +1173,7 @@ export function renderer(renderer: () => WNode | VNode): Renderer {
next.childrenWrappers = renderedToWrapper(next.node.children, next, null);
}
}
const parentWNodeWrapper = findParentWNodeWrapper(next);
if (parentWNodeWrapper && !parentWNodeWrapper.domNode) {
parentWNodeWrapper.domNode = next.domNode;
}
setDomNodeOnParentWrapper(next);
const dom: ApplicationInstruction = {
next: next!,
parentDomNode: parentDomNode,
Expand Down
72 changes: 72 additions & 0 deletions tests/widget-core/unit/vdom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { WidgetBase, widgetInstanceMap } from '../../../src/widget-core/WidgetBa
import Registry from '../../../src/widget-core/Registry';
import { I18nMixin } from '../../../src/widget-core/mixins/I18n';
import registry from '../../../src/widget-core/decorators/registry';
import { alwaysRender } from '../../../src/widget-core/decorators/alwaysRender';

const resolvers = createResolvers();

Expand Down Expand Up @@ -943,6 +944,77 @@ jsdomDescribe('vdom', () => {
assert.lengthOf(fooDiv.childNodes, 1);
});

it('Should insert children in the correct position when returned from a nested tree of virtual widgets', () => {
class Test extends WidgetBase {
render() {
return v('div', this.children);
}
}

@alwaysRender()
class Renderer extends WidgetBase<any> {
render() {
return this.properties.renderer();
}
}

let showA: any;
let showB: any;
let showAll: any;
class App extends WidgetBase {
private _showA = true;
private _showB = true;

constructor() {
super();
showAll = () => {
this._showA = true;
this._showB = true;
this.invalidate();
};
showA = () => {
this._showA = true;
this._showB = false;
this.invalidate();
};
showB = () => {
this._showA = false;
this._showB = true;
this.invalidate();
};
}

protected render() {
return v('div', [
v('div', [
w(Renderer, {
renderer: () => {
return this._showA && w(Test, {}, ['a']);
}
}),
w(Renderer, {
renderer: () => {
return this._showB && w(Test, {}, ['b']);
}
})
])
]);
}
}

const r = renderer(() => w(App, {}));
const div = document.createElement('div');
r.mount({ domNode: div, sync: true });
const root: any = div.childNodes[0] as Element;
assert.strictEqual(root.innerHTML, '<div><div>a</div><div>b</div></div>');
showA();
assert.strictEqual(root.innerHTML, '<div><div>a</div></div>');
showB();
assert.strictEqual(root.innerHTML, '<div><div>b</div></div>');
showAll();
assert.strictEqual(root.innerHTML, '<div><div>a</div><div>b</div></div>');
});

it('Should insert nodes at correct position the previous widget returned null', () => {
class Foo extends WidgetBase {
render() {
Expand Down

0 comments on commit 65f6ba8

Please sign in to comment.