From dbada21890ea84d1daa42e6a3beca73786021e63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Fri, 25 Jan 2019 11:12:20 +0100 Subject: [PATCH] work on mounted hook, make sure widgets are uniquely defined... --- web/static/src/ts/core/Widget.ts | 35 +++++++++++++---- web/static/src/ts/core/qweb_vdom.ts | 2 +- web/static/src/ts/widgets/Counter.ts | 3 -- web/static/src/ts/widgets/Navbar.ts | 3 -- web/static/src/ts/widgets/clock.ts | 2 - web/static/tests/widget.test.ts | 59 ++++++++++++++++++++++++++++ 6 files changed, 87 insertions(+), 17 deletions(-) diff --git a/web/static/src/ts/core/Widget.ts b/web/static/src/ts/core/Widget.ts index 27183bb8..081f701f 100644 --- a/web/static/src/ts/core/Widget.ts +++ b/web/static/src/ts/core/Widget.ts @@ -16,6 +16,7 @@ export class Widget { vnode: VNode | null = null; isStarted: boolean = false; + isMounted: boolean = false; parent: Widget | null = null; children: Widget[] = []; env: T; @@ -48,17 +49,20 @@ export class Widget { // Public //-------------------------------------------------------------------------- - async mount(target?: HTMLElement): Promise { - await this.willStart(); - this.isStarted = true; + async mount(target: HTMLElement): Promise { + await this._start(); const vnode = await this.render(); + target.appendChild(this.el!); - if (target) { - target.appendChild(this.el!); - if (document.body.contains(target)) { - this.visitSubTree(w => w.mounted()); - } + if (document.body.contains(target)) { + this.visitSubTree(w => { + if (!w.isMounted) { + w.isMounted = true; + w.mounted(); + } + }); } + // } return vnode; } @@ -93,6 +97,11 @@ export class Widget { return vnode; } + private async _start(): Promise { + await this.willStart(); + this.isStarted = true; + } + private async _render(): Promise { if (this.template) { this.env.qweb.addTemplate(this.name, this.template); @@ -103,6 +112,16 @@ export class Widget { return Promise.all(promises).then(() => vnode); } + _mount(el: HTMLElement) { + this.el = el; + if (this.parent) { + if (this.parent.isMounted) { + this.isMounted = true; + this.mounted(); + } + } + } + private visitSubTree(callback: (w: Widget) => void) { callback(this); for (let child of this.children) { diff --git a/web/static/src/ts/core/qweb_vdom.ts b/web/static/src/ts/core/qweb_vdom.ts index 7239f989..4ea5588c 100644 --- a/web/static/src/ts/core/qweb_vdom.ts +++ b/web/static/src/ts/core/qweb_vdom.ts @@ -684,7 +684,7 @@ const widgetDirective: Directive = { `let _${widgetID} = new context.widgets['${value}'](context, ${props})` ); ctx.addLine( - `let def${defID} = _${widgetID}._render().then(vnode=>{Object.assign(_${dummyID}, vnode);_${dummyID}.data.hook = {create(_,vn){_${widgetID}.el=vn.elm}}})` + `let def${defID} = _${widgetID}._start().then(() => _${widgetID}._render()).then(vnode=>{Object.assign(_${dummyID}, vnode);_${dummyID}.key="${dummyID}";_${dummyID}.data.hook = {create(_,vn){_${widgetID}._mount(vn.elm)}}})` ); ctx.addLine(`extra.promises.push(def${defID})`); diff --git a/web/static/src/ts/widgets/Counter.ts b/web/static/src/ts/widgets/Counter.ts index ffe27a13..c8c5de0b 100644 --- a/web/static/src/ts/widgets/Counter.ts +++ b/web/static/src/ts/widgets/Counter.ts @@ -21,9 +21,6 @@ export class Counter extends Widget { this.state.counter = props.initialState || 0; } - mounted() { - console.log("counter mounter", this.el); - } increment(delta: number) { this.updateState({ counter: this.state.counter + delta }); } diff --git a/web/static/src/ts/widgets/Navbar.ts b/web/static/src/ts/widgets/Navbar.ts index fd85319c..081f5b03 100644 --- a/web/static/src/ts/widgets/Navbar.ts +++ b/web/static/src/ts/widgets/Navbar.ts @@ -18,9 +18,6 @@ export class Navbar extends Widget { name = "navbar"; template = template; - mounted() { - console.log("navbar mounted", this.el); - } getUrl(menu: Menu) { const action_id = String(menu.actionID); return this.env.router.formatURL("", { action_id }); diff --git a/web/static/src/ts/widgets/clock.ts b/web/static/src/ts/widgets/clock.ts index 2b38cf43..e0f81573 100644 --- a/web/static/src/ts/widgets/clock.ts +++ b/web/static/src/ts/widgets/clock.ts @@ -15,12 +15,10 @@ export class Clock extends Widget { } mounted() { - console.log("clock mounter", this.el); setInterval(this.updateTime.bind(this), 500); } updateTime() { - debugger; this.updateState({ currentTime: new Date().toLocaleTimeString() }); diff --git a/web/static/tests/widget.test.ts b/web/static/tests/widget.test.ts index a794729f..0e274a66 100644 --- a/web/static/tests/widget.test.ts +++ b/web/static/tests/widget.test.ts @@ -130,6 +130,27 @@ describe("lifecycle hooks", () => { target.remove(); }); + test("willStart hook is called on subwidget", async () => { + expect.assertions(1); + let ok = false; + class ParentWidget extends Widget { + name = "a"; + template = `
`; + widgets = { child: ChildWidget }; + } + class ChildWidget extends Widget { + async willStart() { + ok = true; + } + } + const widget = makeWidget(ParentWidget); + const target = document.createElement("div"); + document.body.appendChild(target); + await widget.mount(target); + expect(ok).toBe(true); + target.remove(); + }); + test("mounted hook is called on subwidgets, in proper order", async () => { expect.assertions(4); let parentMounted = false; @@ -157,6 +178,44 @@ describe("lifecycle hooks", () => { expect(childMounted).toBe(true); target.remove(); }); + + test("willStart, mounted on subwidget rendered after main is mounted in some other position", async () => { + expect.assertions(3); + let hookCounter = 0; + class ParentWidget extends Widget { + name = "a"; + state = { ok: false }; + template = ` +
+ + + + +
+ +
`; // the t-else part in this template is important. This is + // necessary to have a situation that could confuse the vdom + // patching algorithm + widgets = { child: ChildWidget }; + } + class ChildWidget extends Widget { + async willStart() { + hookCounter++; + } + mounted() { + expect(hookCounter).toBe(1); + hookCounter++; + } + } + const widget = makeWidget(ParentWidget); + const target = document.createElement("div"); + document.body.appendChild(target); + await widget.mount(target); + expect(hookCounter).toBe(0); // sub widget not created yet + await widget.updateState({ ok: true }); + expect(hookCounter).toBe(2); + target.remove(); + }); }); describe("destroy method", () => {