[IMP] web: improve code of popover

Popover services:
  - move bus from file scope into the service scope
  - add "specializeForComponent" to add some logics when component is
    unmounted
  - parameters passed to the manager isn't stored in state anymore to
    prevent props to be proxified.

Popover tests:
  - remove a todo comment
  - prettify template strings
  - fix "Recompute position" test

Popover services tests:
  - extract PseudoWebClient creation to lighten tests
  - add test related to "specializeForComponent"

Doc updated

Author: mcm-odoo

addons/web/static/src/core/popover/popover.js

@@ -293,7 +293,7 @@ Popover.props = {
  * @param {HTMLElement} popoverElement The popover element
  * @param {HTMLElement} targetElement The target element, to which
  *  the popover will be visually "bound"
- * @param {integer} [margin=16] Minimal accepted margin from the border
+ * @param {number} [margin=16] Minimal accepted margin from the border
  *  of the viewport.
  * @returns {Object}
  */

addons/web/static/src/core/popover/popover_service.js

@@ -2,15 +2,14 @@
 
 import { useBus } from "../bus_hook";
 import { registry } from "../registry";
+import { useService } from "../service_hook";
 import { Popover } from "./popover";
 
 const { Component } = owl;
 const { EventBus } = owl.core;
-const { useState } = owl.hooks;
+const { onWillUnmount } = owl.hooks;
 const { xml } = owl.tags;
 
-const bus = new EventBus();
-
 export class KeyAlreadyExistsError extends Error {
     constructor(key) {
         super(`PopoverManager already contains key "${key}"`);
@@ -25,50 +24,56 @@ export class KeyNotFoundError extends Error {
 
 export class PopoverManager extends Component {
     setup() {
-        this.popovers = useState({});
-        this.nextId = 0;
+        // do not include popover params in state to keep original popover props
+        this.popovers = {};
 
+        const { bus } = useService("popover");
         useBus(bus, "ADD", this.addPopover);
         useBus(bus, "REMOVE", this.removePopover);
     }
 
     /**
-     * @param {Object}    params
-     * @param {string}    [params.key]
-     * @param {string}    [params.content]
-     * @param {any}       [params.Component]
-     * @param {Object}    [params.props]
-     * @param {Function}  [params.onClose]
-     * @param {boolean}   [params.keepOnClose=false]
+     * @param {Object}                params
+     * @param {string}                params.key
+     * @param {string}                [params.content]
+     * @param {any}                   [params.Component]
+     * @param {Object}                [params.props]
+     * @param {(key: string) => void} [params.onClose]
+     * @param {boolean}               [params.keepOnClose=false]
      */
     addPopover(params) {
-        const key = params.key || this.nextId;
-        if (this.popovers[key]) {
-            throw new KeyAlreadyExistsError(key);
+        if (params.key in this.popovers) {
+            throw new KeyAlreadyExistsError(params.key);
         }
 
-        this.popovers[key] = Object.assign({ key }, params);
-        this.nextId += 1;
+        this.popovers[params.key] = params;
+        this.render();
     }
     /**
-     * @param {string | number} key
+     * @param {string} key
      */
     removePopover(key) {
-        if (!this.popovers[key]) {
+        if (!(key in this.popovers)) {
             throw new KeyNotFoundError(key);
         }
 
         delete this.popovers[key];
+        this.render();
     }
 
     /**
-     * @param {string | number} key
+     * @param {string} key
      */
     onPopoverClosed(key) {
-        if (this.popovers[key].onClose) {
-            this.popovers[key].onClose();
+        if (!(key in this.popovers)) {
+            // It can happen that the popover was removed manually just before this call
+            return;
+        }
+        const popover = this.popovers[key];
+        if (popover.onClose) {
+            popover.onClose(key);
         }
-        if (!this.popovers[key].keepOnClose) {
+        if (!popover.keepOnClose) {
             this.removePopover(key);
         }
     }
@@ -99,21 +104,30 @@ PopoverManager.template = xml`
 registry.category("main_components").add("PopoverManager", PopoverManager);
 
 export const popoverService = {
-    start(env) {
+    start() {
+        let nextId = 0;
+        const bus = new EventBus();
         return {
+            bus,
             /**
              * Signals the manager to add a popover.
              *
-             * @param {Object}    params
-             * @param {string}    [params.key]
-             * @param {string}    [params.content]
-             * @param {any}       [params.Component]
-             * @param {Object}    [params.props]
-             * @param {Function}  [params.onClose]
-             * @param {boolean}   [params.keepOnClose=false]
+             * @param {Object}                params
+             * @param {string}                [params.key]
+             * @param {string}                [params.content]
+             * @param {any}                   [params.Component]
+             * @param {Object}                [params.props]
+             * @param {(key: string) => void} [params.onClose]
+             * @param {boolean}               [params.keepOnClose=false]
+             * @returns {string}
              */
             add(params) {
+                if (!("key" in params)) {
+                    params.key = `popover_${nextId}`;
+                    nextId += 1;
+                }
                 bus.trigger("ADD", params);
+                return params.key;
             },
             /**
              * Signals the manager to remove the popover with key = `key`.
@@ -125,6 +139,36 @@ export const popoverService = {
             },
         };
     },
+    specializeForComponent(component, service) {
+        const keys = new Set();
+        onWillUnmount(function () {
+            for (const key of keys) {
+                service.remove(key);
+            }
+            keys.clear();
+        });
+        return Object.assign(Object.create(service), {
+            add(params) {
+                const newParams = Object.create(params);
+                newParams.onClose = function (key) {
+                    if (!params.keepOnClose) {
+                        // manager will delete the popover if keepOnClose is falsy
+                        keys.delete(key);
+                    }
+                    if (params.onClose && component.__owl__.status !== 5 /* DESTROYED */) {
+                        params.onClose(key);
+                    }
+                };
+                const key = service.add(newParams);
+                keys.add(key);
+                return key;
+            },
+            remove(key) {
+                keys.delete(key);
+                service.remove(key);
+            },
+        });
+    },
 };
 
 registry.category("services").add("popover", popoverService);