From 54f5819ef978247a129bcfc888643ee181674671 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=A9ry=20Debongnie?= Date: Thu, 9 Jan 2020 10:02:27 +0100 Subject: [PATCH] [ADD] hooks: add useExternalListener hook It is very useful. Also, this commit prettifies the code. closes #608 --- doc/reference/hooks.md | 12 ++++ src/hooks.ts | 24 ++++++++ src/qweb/compilation_context.ts | 26 ++++---- src/qweb/expression_parser.ts | 4 +- tests/component/component.test.ts | 89 ++++++++++++++++------------ tests/component/styles.test.ts | 6 +- tests/hooks.test.ts | 38 +++++++++++- tests/tooling/debug_script_4.test.ts | 7 +-- 8 files changed, 147 insertions(+), 59 deletions(-) diff --git a/doc/reference/hooks.md b/doc/reference/hooks.md index 34d4373b..8be666c3 100644 --- a/doc/reference/hooks.md +++ b/doc/reference/hooks.md @@ -17,6 +17,7 @@ - [`useContext`](#usecontext) - [`useRef`](#useref) - [`useSubEnv`](#usesubenv) + - [`useExternalListener`](#useexternallistener) - [`useStore`](#usestore) - [`useDispatch`](#usedispatch) - [`useGetters`](#usegetters) @@ -354,6 +355,17 @@ will be added to the parent environment. Note that it will extend, not replace the parent environment. And of course, the parent environment will not be affected. +### `useExternalListener` + +The `useExternalListener` hook helps solve a very common problem: adding and removing +a listener on some target whenever a component is mounted/unmounted. For example, +a dropdown menu (or its parent) may need to listen to a `click` event on `window` +to be closed: + +```js +useExternalListener(window, "click", this.closeMenu); +``` + ### `useStore` The `useStore` hook is the entry point for a component to connect to the store. diff --git a/src/hooks.ts b/src/hooks.ts index 99188b89..e7292a8a 100644 --- a/src/hooks.ts +++ b/src/hooks.ts @@ -131,3 +131,27 @@ export function useSubEnv(nextEnv) { const component = Component.current!; component.env = Object.assign(Object.create(component.env), nextEnv); } + +// ----------------------------------------------------------------------------- +// useExternalListener +// ----------------------------------------------------------------------------- + +/** + * When a component needs to listen to DOM Events on element(s) that are not + * part of his hierarchy, we can use the `useExternalListener` hook. + * It will correctly add and remove the event listener, whenever the + * component is mounted and unmounted. + * + * Example: + * a menu needs to listen to the click on window to be closed automatically + * + * Usage: + * in the constructor of the OWL component that needs to be notified, + * `useExternalListener(window, 'click', this._doSomething);` + * */ +export function useExternalListener(target: HTMLElement, eventName: string, handler, eventParams?) { + const boundHandler = handler.bind(Component.current); + + onMounted(() => target.addEventListener(eventName, boundHandler, eventParams)); + onWillUnmount(() => target.removeEventListener(eventName, boundHandler, eventParams)); +} diff --git a/src/qweb/compilation_context.ts b/src/qweb/compilation_context.ts index 03182f0d..c2656771 100644 --- a/src/qweb/compilation_context.ts +++ b/src/qweb/compilation_context.ts @@ -1,4 +1,4 @@ -import { compileExpr , compileExprToArray , QWebVar } from "./expression_parser"; +import { compileExpr, compileExprToArray, QWebVar } from "./expression_parser"; export const INTERP_REGEXP = /\{\{.*?\}\}/g; //------------------------------------------------------------------------------ @@ -161,16 +161,18 @@ export class CompilationContext { const argId = this.generateID(); const tokens = compileExprToArray(expr, this.variables); const done = new Set(); - return tokens.map((tok) => { - if (tok.varName) { - if (!done.has(tok.varName)) { - done.add(tok.varName); - this.addLine(`const ${tok.varName}_${argId} = ${tok.value};`); + return tokens + .map(tok => { + if (tok.varName) { + if (!done.has(tok.varName)) { + done.add(tok.varName); + this.addLine(`const ${tok.varName}_${argId} = ${tok.value};`); + } + tok.value = `${tok.varName}_${argId}`; } - tok.value = `${tok.varName}_${argId}`; - } - return tok.value; - }).join(""); + return tok.value; + }) + .join(""); } /** @@ -194,7 +196,9 @@ export class CompilationContext { const protectID = this.generateID(); this.rootContext.protectedScopeNumber++; this.rootContext.shouldDefineScope = true; - const scopeExpr = codeBlock ? `Object.create(scope);` : `Object.assign(Object.create(context), scope);`; + const scopeExpr = codeBlock + ? `Object.create(scope);` + : `Object.assign(Object.create(context), scope);`; this.addLine(`let _origScope${protectID} = scope;`); this.addLine(`scope = ${scopeExpr}`); return protectID; diff --git a/src/qweb/expression_parser.ts b/src/qweb/expression_parser.ts index d920229a..ed74051f 100644 --- a/src/qweb/expression_parser.ts +++ b/src/qweb/expression_parser.ts @@ -283,5 +283,7 @@ export function compileExprToArray(expr: string, scope: { [key: string]: QWebVar } export function compileExpr(expr: string, scope: { [key: string]: QWebVar }): string { - return compileExprToArray(expr, scope).map(t => t.value).join(""); + return compileExprToArray(expr, scope) + .map(t => t.value) + .join(""); } diff --git a/tests/component/component.test.ts b/tests/component/component.test.ts index b9b05914..1853c2b7 100644 --- a/tests/component/component.test.ts +++ b/tests/component/component.test.ts @@ -2683,7 +2683,7 @@ describe("other directives with t-component", () => {

EndLoop:

`; - state = useState({values: ['a', 'b']}); + state = useState({ values: ["a", "b"] }); } const widget = new SomeWidget(); await widget.mount(fixture); @@ -2710,23 +2710,23 @@ describe("other directives with t-component", () => {

`; - state = {flag: 'if'}; + state = { flag: "if" }; } const widget = new SomeWidget(); await widget.mount(fixture); expect(fixture.innerHTML).toBe("

2

"); - widget.state.flag = 'elif'; + widget.state.flag = "elif"; await widget.render(); expect(fixture.innerHTML).toBe("

3

"); - widget.state.flag = 'false'; + widget.state.flag = "false"; await widget.render(); expect(fixture.innerHTML).toBe("

4

"); }); test("t-set in t-if", async () => { // Weird that code block within 'if' leaks outside of it - // Python does the same + // Python does the same class SomeWidget extends Component { static template = xml`
@@ -2743,16 +2743,16 @@ describe("other directives with t-component", () => {

`; - state = {flag: 'if'}; + state = { flag: "if" }; } const widget = new SomeWidget(); await widget.mount(fixture); expect(fixture.innerHTML).toBe("

2

"); - widget.state.flag = 'elif'; + widget.state.flag = "elif"; await widget.render(); expect(fixture.innerHTML).toBe("

3

"); - widget.state.flag = 'false'; + widget.state.flag = "false"; await widget.render(); expect(fixture.innerHTML).toBe("

4

"); }); @@ -2777,7 +2777,10 @@ describe("other directives with t-component", () => { }); test("t-set can't alter from within callee", async () => { - env.qweb.addTemplate("ChildWidget", `
`); + env.qweb.addTemplate( + "ChildWidget", + `
` + ); class SomeWidget extends Component { static template = xml`
@@ -2795,7 +2798,10 @@ describe("other directives with t-component", () => { }); test("t-set can't alter in t-call body", async () => { - env.qweb.addTemplate("ChildWidget", `
`); + env.qweb.addTemplate( + "ChildWidget", + `
` + ); class SomeWidget extends Component { static template = xml`
@@ -2841,7 +2847,7 @@ describe("other directives with t-component", () => { let child; class ChildWidget extends Component { static template = xml`
`; - iter = 'child'; + iter = "child"; constructor() { super(...arguments); child = this; @@ -2861,7 +2867,7 @@ describe("other directives with t-component", () => { await widget.mount(fixture); expect(fixture.innerHTML).toBe("

source

childcalled

source

"); - expect(child.iter).toBe('child'); + expect(child.iter).toBe("child"); expect(QWeb.TEMPLATES[SomeWidget.template].fn.toString()).toMatchSnapshot(); }); @@ -2877,7 +2883,7 @@ describe("other directives with t-component", () => {

EndLoop:

`; - state = useState({values: ['a', 'b']}); + state = useState({ values: ["a", "b"] }); } const widget = new SomeWidget(); await widget.mount(fixture); @@ -2896,7 +2902,7 @@ describe("other directives with t-component", () => {

EndLoop:

`; - state = useState({values: ['a', 'b']}); + state = useState({ values: ["a", "b"] }); } const widget = new SomeWidget(); await widget.mount(fixture); @@ -2913,18 +2919,20 @@ describe("other directives with t-component", () => { `; - state = useState({values: ['a', 'b']}); - otherState = {vals: []}; + state = useState({ values: ["a", "b"] }); + otherState = { vals: [] }; } const widget = new SomeWidget(); await widget.mount(fixture); - expect(fixture.innerHTML).toBe("
0: a
1: b
"); + expect(fixture.innerHTML).toBe( + "
0: a
1: b
" + ); expect(widget.otherState.vals).toStrictEqual([]); - const buttons = fixture.querySelectorAll('button'); + const buttons = fixture.querySelectorAll("button"); buttons[0].click(); buttons[1].click(); - expect(widget.otherState.vals).toStrictEqual(['a', 'b']); + expect(widget.otherState.vals).toStrictEqual(["a", "b"]); expect(QWeb.TEMPLATES[SomeWidget.template].fn.toString()).toMatchSnapshot(); }); @@ -2940,18 +2948,20 @@ describe("other directives with t-component", () => { `; - state = useState({values: ['a', 'b']}); - otherState = {vals: []}; + state = useState({ values: ["a", "b"] }); + otherState = { vals: [] }; } const widget = new SomeWidget(); await widget.mount(fixture); - expect(fixture.innerHTML).toBe("
0: a
1: b
"); + expect(fixture.innerHTML).toBe( + "
0: a
1: b
" + ); expect(widget.otherState.vals).toStrictEqual([]); - const buttons = fixture.querySelectorAll('button'); + const buttons = fixture.querySelectorAll("button"); buttons[0].click(); buttons[1].click(); - expect(widget.otherState.vals).toStrictEqual(['a_nova_0', 'b_nova_0_1']); + expect(widget.otherState.vals).toStrictEqual(["a_nova_0", "b_nova_0_1"]); expect(QWeb.TEMPLATES[SomeWidget.template].fn.toString()).toMatchSnapshot(); }); @@ -2965,8 +2975,8 @@ describe("other directives with t-component", () => { `; - state = useState({values: ['a', 'b']}); - otherState = {vals: new Array()}; + state = useState({ values: ["a", "b"] }); + otherState = { vals: new Array() }; addVal(val: string) { this.otherState.vals.push(val); @@ -2975,13 +2985,14 @@ describe("other directives with t-component", () => { const widget = new SomeWidget(); await widget.mount(fixture); - - expect(fixture.innerHTML).toBe("
0: a
1: b
"); + expect(fixture.innerHTML).toBe( + "
0: a
1: b
" + ); expect(widget.otherState.vals).toStrictEqual([]); - const buttons = fixture.querySelectorAll('button'); + const buttons = fixture.querySelectorAll("button"); buttons[0].click(); buttons[1].click(); - expect(widget.otherState.vals).toStrictEqual(['a', 'b']); + expect(widget.otherState.vals).toStrictEqual(["a", "b"]); expect(QWeb.TEMPLATES[SomeWidget.template].fn.toString()).toMatchSnapshot(); }); @@ -2996,19 +3007,20 @@ describe("other directives with t-component", () => { `; - arr = ['a', 'b'] - otherState = {vals: new Array()}; + arr = ["a", "b"]; + otherState = { vals: new Array() }; } const widget = new SomeWidget(); await widget.mount(fixture); - - expect(fixture.innerHTML).toBe("
"); + expect(fixture.innerHTML).toBe( + "
" + ); expect(widget.otherState.vals).toStrictEqual([]); - const buttons = fixture.querySelectorAll('button'); + const buttons = fixture.querySelectorAll("button"); buttons[0].click(); buttons[1].click(); - expect(widget.otherState.vals).toStrictEqual(['0_0', '1_1']); + expect(widget.otherState.vals).toStrictEqual(["0_0", "1_1"]); expect(QWeb.TEMPLATES[SomeWidget.template].fn.toString()).toMatchSnapshot(); }); }); @@ -6069,7 +6081,7 @@ describe("t-call", () => { }); test("sub components in two t-calls", async () => { - class Child extends Component { + class Child extends Component { static template = xml``; } @@ -6083,7 +6095,7 @@ describe("t-call", () => {
`; static components = { Child }; - state = useState({val: 1}); + state = useState({ val: 1 }); } const parent = new Parent(); await parent.mount(fixture); @@ -6092,5 +6104,4 @@ describe("t-call", () => { await nextTick(); expect(fixture.innerHTML).toBe("
2
"); }); - }); diff --git a/tests/component/styles.test.ts b/tests/component/styles.test.ts index 6fc0fab3..2b3fb860 100644 --- a/tests/component/styles.test.ts +++ b/tests/component/styles.test.ts @@ -137,8 +137,9 @@ describe("styles and component", () => { color: red; } }`); - - expect(sheet).toBe(`.parent-a .child-a, .parent-a .child-b, .parent-b .child-a, .parent-b .child-b { + + expect(sheet) + .toBe(`.parent-a .child-a, .parent-a .child-b, .parent-b .child-a, .parent-b .child-b { color: red; }`); }); @@ -166,5 +167,4 @@ describe("styles and component", () => { color: red; }`); }); - }); diff --git a/tests/hooks.test.ts b/tests/hooks.test.ts index bc39bb87..1d5ee44f 100644 --- a/tests/hooks.test.ts +++ b/tests/hooks.test.ts @@ -9,7 +9,8 @@ import { onWillPatch, onWillStart, onWillUpdateProps, - useSubEnv + useSubEnv, + useExternalListener } from "../src/hooks"; import { xml } from "../src/tags"; @@ -592,4 +593,39 @@ describe("hooks", () => { expect(fixture.innerHTML).toBe("
2
"); expect(steps).toEqual(["onWillStart", "onWillUpdateProps"]); }); + + test("useExternalListener", async () => { + let n = 0; + + class MyComponent extends Component { + static template = xml``; + constructor(parent, props) { + super(parent, props); + useExternalListener(window as any, "click", this.increment); + } + increment() { + n++; + } + } + class App extends Component { + static template = xml`
`; + static components = { MyComponent }; + state = useState({ flag: false }); + } + + const app = new App(); + await app.mount(fixture); + + expect(n).toBe(0); + window.dispatchEvent(new Event("click")); + expect(n).toBe(0); + app.state.flag = true; + await nextTick(); + window.dispatchEvent(new Event("click")); + expect(n).toBe(1); + app.state.flag = false; + await nextTick(); + window.dispatchEvent(new Event("click")); + expect(n).toBe(1); + }); }); diff --git a/tests/tooling/debug_script_4.test.ts b/tests/tooling/debug_script_4.test.ts index 7b71f63c..6c080614 100644 --- a/tests/tooling/debug_script_4.test.ts +++ b/tests/tooling/debug_script_4.test.ts @@ -21,18 +21,17 @@ test("log a sub component with non stringifiable props", async () => { const log = console.log; console.log = arg => steps.push(arg); - class Child extends Component { - static template = xml``; + class Child extends Component { + static template = xml``; } - const circularObject: any = {val: 1}; + const circularObject: any = { val: 1 }; circularObject.circularObject = circularObject; class Parent extends Component { static template = xml`
`; static components = { Child }; obj = circularObject; - } const parent = new Parent(null, {});