From debefffd2630acd464123edc587e7bc4d6182d34 Mon Sep 17 00:00:00 2001 From: Bruno Boi Date: Sun, 18 Jun 2023 11:38:49 +0000 Subject: [PATCH] [FIX] web: sync reposition for nested poppers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit will have nested components making use of the usePosition hook to synchronize their repositioning. **Before this commit** Each component handles its own repositioning logic. If it is mounted somehow inside another component using that same logic, conflicts occur in some circumstances. **After this commit** It is now ensured that, in case of nested positioned components, the whole chain of repositioning computations is called in the proper order: from the parent to the children. **Example** - Before ![before] - After ![after] [before]: https://github.com/odoo/odoo/assets/1159815/3c9cfae7-db68-4707-a73c-ad7e2e71d4bf [after]: https://github.com/odoo/odoo/assets/1159815/a21f56e0-d8e3-48c1-9509-8e6ee5040d4b closes odoo/odoo#125531 X-original-commit: 9f947ba3fd3a00a0c80efc40186c36e0de78de16 Signed-off-by: Michaël Mattiello (mcm) Signed-off-by: Bruno Boi (boi) --- addons/web/static/src/core/position_hook.js | 53 +++++++++++----- .../static/tests/core/position_hook_tests.js | 62 ++++++++++++++++++- 2 files changed, 99 insertions(+), 16 deletions(-) diff --git a/addons/web/static/src/core/position_hook.js b/addons/web/static/src/core/position_hook.js index e43044cc51a..0d87c7e78be 100644 --- a/addons/web/static/src/core/position_hook.js +++ b/addons/web/static/src/core/position_hook.js @@ -1,7 +1,14 @@ /** @odoo-module */ import { useThrottleForAnimation } from "./utils/timing"; -import { useEffect, useRef } from "@odoo/owl"; +import { + EventBus, + onWillDestroy, + useChildSubEnv, + useComponent, + useEffect, + useRef, +} from "@odoo/owl"; import { localization } from "@web/core/l10n/localization"; /** @@ -281,6 +288,8 @@ export function reposition(target, popper, iframe, options) { } } +const POSITION_BUS = Symbol("position-bus"); + /** * Makes sure that the `popper` element is always * placed at `position` from the `target` element. @@ -296,10 +305,9 @@ export function reposition(target, popper, iframe, options) { * @param {Options} options */ export function usePosition(target, options) { - const popperRef = useRef(options.popper || DEFAULTS.popper); + const popperRef = useRef(options?.popper || DEFAULTS.popper); const getTarget = typeof target === "function" ? target : () => target; - const throttledReposition = useThrottleForAnimation(reposition); - useEffect(() => { + const update = () => { const targetEl = getTarget(); const popperEl = popperRef.el; if (!targetEl || !popperEl) { @@ -307,20 +315,35 @@ export function usePosition(target, options) { } // Prepare - const targetDocument = targetEl.ownerDocument; const iframe = getIFrame(targetEl); const currentOptions = { ...DEFAULTS, ...options }; - - // Reposition reposition(targetEl, popperEl, iframe, currentOptions); + }; - // Attach listeners to keep the positioning up to date - const listener = () => throttledReposition(targetEl, popperEl, iframe, currentOptions); - targetDocument.addEventListener("scroll", listener, { capture: true }); - window.addEventListener("resize", listener); - return () => { - targetDocument.removeEventListener("scroll", listener, { capture: true }); - window.removeEventListener("resize", listener); - }; + const component = useComponent(); + const bus = component.env[POSITION_BUS] || new EventBus(); + bus.addEventListener("update", update); + onWillDestroy(() => bus.removeEventListener("update", update)); + + const isTopmost = !(POSITION_BUS in component.env); + if (isTopmost) { + useChildSubEnv({ [POSITION_BUS]: bus }); + } + + const throttledUpdate = useThrottleForAnimation(() => bus.trigger("update")); + useEffect(() => { + // Reposition + bus.trigger("update"); + + if (isTopmost) { + // Attach listeners to keep the positioning up to date + const targetDocument = getTarget()?.ownerDocument; + targetDocument?.addEventListener("scroll", throttledUpdate, { capture: true }); + window.addEventListener("resize", throttledUpdate); + return () => { + targetDocument?.removeEventListener("scroll", throttledUpdate, { capture: true }); + window.removeEventListener("resize", throttledUpdate); + }; + } }); } diff --git a/addons/web/static/tests/core/position_hook_tests.js b/addons/web/static/tests/core/position_hook_tests.js index 87b55519375..7a973d92d05 100644 --- a/addons/web/static/tests/core/position_hook_tests.js +++ b/addons/web/static/tests/core/position_hook_tests.js @@ -14,7 +14,7 @@ import { triggerEvent, } from "../helpers/utils"; import { localization } from "@web/core/l10n/localization"; -import { Component, xml } from "@odoo/owl"; +import { Component, useRef, xml } from "@odoo/owl"; const FLEXBOX_STYLE = { display: "flex", @@ -543,6 +543,66 @@ QUnit.test("iframe: both popper and target inside", async (assert) => { assert.strictEqual(popperBox.left, onPositionedArgs.solution.left); }); +QUnit.test("popper as child of another", async (assert) => { + class Child extends Component { + static template = /* xml */ xml` +
+
+
+
+ `; + setup() { + const ref = useRef("ref"); + usePosition(() => ref.el, { position: "left" }); + } + } + const target = document.createElement("div"); + target.id = "target"; + Object.assign(target.style, TARGET_STYLE); + container.appendChild(target); + class Parent extends Component { + static components = { Child }; + static template = /* xml */ xml`
`; + setup() { + usePosition(target); + } + } + + const sheet = document.createElement("style"); + sheet.textContent = ` + #child .target { + background-color: peachpuff; + height: 100px; + width: 10px; + } + #child .popper { + background-color: olive; + height: 100px; + width: 100px; + } + `; + document.head.appendChild(sheet); + registerCleanup(() => sheet.remove()); + + await mount(Parent, container); + const parentPopBox1 = container.querySelector("#popper").getBoundingClientRect(); + const childPopBox1 = container.querySelector("#child .popper").getBoundingClientRect(); + const spacer = document.createElement("div"); + spacer.id = "foo"; + spacer.style.height = "1px"; + spacer.style.width = "100px"; + container.prepend(spacer); + await triggerEvent(document, null, "scroll"); + + const parentPopBox2 = container.querySelector("#popper").getBoundingClientRect(); + const childPopBox2 = container.querySelector("#child .popper").getBoundingClientRect(); + + assert.strictEqual(parentPopBox1.top, parentPopBox2.top); + assert.strictEqual(childPopBox1.top, childPopBox2.top); + assert.strictEqual(parentPopBox2.left, parentPopBox1.left + spacer.offsetWidth * 0.5); + assert.strictEqual(childPopBox2.left, childPopBox1.left + spacer.offsetWidth * 0.5); +}); + function getPositionTest(position, positionToCheck) { return async (assert) => { assert.expect(2);