diff --git a/addons/web/static/src/core/position_hook.js b/addons/web/static/src/core/position_hook.js index 341a6b4bc76..7c0c26ebd20 100644 --- a/addons/web/static/src/core/position_hook.js +++ b/addons/web/static/src/core/position_hook.js @@ -9,13 +9,15 @@ import { localization } from "@web/core/l10n/localization"; */ /** - * @typedef {{ - * popper?: string; - * container?: HTMLElement | (() => HTMLElement); - * margin?: number; - * position?: Direction | Position; - * onPositioned?: PositionEventHandler; - * }} Options + * @typedef Options + * @property {string} [popper="popper"] useRef reference to the popper element + * @property {HTMLElement} [container] container element + * @property {number} [margin=0] + * margin in pixels between the popper and the target. + * @property {Direction | Position} [position="bottom"] + * position of the popper relative to the target + * @property {PositionEventHandler} [onPositioned] + * callback called everytime the popper has just been positioned * * @typedef {keyof DirectionsData} DirectionsDataKey * @typedef {{ @@ -91,8 +93,9 @@ function getIFrame(el) { * to the requested position. * The positioning data used to determine each possible position is based on * the target, popper, and container sizes. - * Particularly, a popper must not overflow the container in any direction, - * it should actually stay at `margin` distance from the border to look good. + * Particularly, a popper must not overflow the container in any direction. + * The popper will stay at `margin` distance from its target. One could also + * use the CSS margins of the popper element to achieve the same result. * * @param {HTMLElement} target * @param {HTMLElement} popper @@ -116,6 +119,16 @@ function getBestPosition(target, popper, iframe, { container, margin, position } container = container(); } + // Account for popper actual margins + const popperStyle = getComputedStyle(popper); + const { marginTop, marginLeft, marginRight, marginBottom } = popperStyle; + const popMargins = { + top: parseFloat(marginTop), + left: parseFloat(marginLeft), + right: parseFloat(marginRight), + bottom: parseFloat(marginBottom), + }; + // Boxes const popBox = popper.getBoundingClientRect(); const targetBox = target.getBoundingClientRect(); @@ -128,10 +141,10 @@ function getBestPosition(target, popper, iframe, { container, margin, position } // Compute positioning data /** @type {DirectionsData} */ const directionsData = { - t: iframeBox.top + targetBox.top - popBox.height - margin, - b: iframeBox.top + targetBox.bottom + margin, - r: iframeBox.left + targetBox.right + margin, - l: iframeBox.left + targetBox.left - popBox.width - margin, + t: iframeBox.top + targetBox.top - popMargins.bottom - popBox.height - margin, + b: iframeBox.top + targetBox.bottom + popMargins.top + margin, + r: iframeBox.left + targetBox.right + popMargins.left + margin, + l: iframeBox.left + targetBox.left - popMargins.right - popBox.width - margin, }; /** @type {VariantsData} */ const variantsData = { @@ -153,8 +166,8 @@ function getBestPosition(target, popper, iframe, { container, margin, position } if (containerRestricted) { const [directionSize, variantSize] = vertical - ? [popBox.height + margin, popBox.width] - : [popBox.width, popBox.height + margin]; + ? [popBox.height, popBox.width] + : [popBox.width, popBox.height]; let [directionMin, directionMax] = vertical ? [contBox.top, contBox.bottom] : [contBox.left, contBox.right]; diff --git a/addons/web/static/tests/core/position_hook_tests.js b/addons/web/static/tests/core/position_hook_tests.js index 6a7e5697fd6..b5a4a14a98f 100644 --- a/addons/web/static/tests/core/position_hook_tests.js +++ b/addons/web/static/tests/core/position_hook_tests.js @@ -96,18 +96,198 @@ QUnit.test("default position is bottom-middle", async (assert) => { }); QUnit.test("can add margin", async (assert) => { - let TestComp = getTestComponent({ margin: 0 }); - let popper = await mount(TestComp, container); + // Add a sheet to set a margin on the popper + const SHEET_MARGINS = { + top: 11, + right: 12, + bottom: 13, + left: 14, + }; + const sheet = document.createElement("style"); + sheet.textContent = ` + #popper { + margin-top: ${SHEET_MARGINS.top}px; + margin-right: ${SHEET_MARGINS.right}px; + margin-bottom: ${SHEET_MARGINS.bottom}px; + margin-left: ${SHEET_MARGINS.left}px; + } + `; + document.head.appendChild(sheet); + registerCleanup(() => sheet.remove()); - const popBox1 = document.getElementById("popper").getBoundingClientRect(); - destroy(popper); + // Local helper + async function _mountTestComponentAndDestroy(popperOptions) { + const TestComp = getTestComponent(popperOptions); + const popper = await mount(TestComp, container); + const popBox = document.getElementById("popper").getBoundingClientRect(); + const targetBox = document.getElementById("target").getBoundingClientRect(); + destroy(popper); + container.removeChild(document.getElementById("target")); + return [popBox, targetBox]; + } - TestComp = getTestComponent({ margin: 20 }); - popper = await mount(TestComp, container); - const popBox2 = document.getElementById("popper").getBoundingClientRect(); - destroy(popper); + // With/without additional margin (default direction is bottom) + let [popBox, targetBox] = await _mountTestComponentAndDestroy(); + assert.strictEqual(popBox.top, targetBox.bottom + SHEET_MARGINS.top); + [popBox, targetBox] = await _mountTestComponentAndDestroy({ margin: 10 }); + assert.strictEqual(popBox.top, targetBox.bottom + SHEET_MARGINS.top + 10); - assert.strictEqual(popBox1.top + 20, popBox2.top); + // With/without additional margin, direction is top + [popBox, targetBox] = await _mountTestComponentAndDestroy({ position: "top" }); + assert.strictEqual(popBox.top, targetBox.top - popBox.height - SHEET_MARGINS.bottom); + [popBox, targetBox] = await _mountTestComponentAndDestroy({ position: "top", margin: 10 }); + assert.strictEqual(popBox.top, targetBox.top - popBox.height - SHEET_MARGINS.bottom - 10); + + // With/without additional margin, direction is left + [popBox, targetBox] = await _mountTestComponentAndDestroy({ position: "left" }); + assert.strictEqual(popBox.left, targetBox.left - popBox.width - SHEET_MARGINS.right); + [popBox, targetBox] = await _mountTestComponentAndDestroy({ position: "left", margin: 10 }); + assert.strictEqual(popBox.left, targetBox.left - popBox.width - SHEET_MARGINS.right - 10); + + // With/without additional margin, direction is right + [popBox, targetBox] = await _mountTestComponentAndDestroy({ position: "right" }); + assert.strictEqual(popBox.left, targetBox.right + SHEET_MARGINS.left); + [popBox, targetBox] = await _mountTestComponentAndDestroy({ position: "right", margin: 10 }); + assert.strictEqual(popBox.left, targetBox.right + SHEET_MARGINS.left + 10); +}); + +QUnit.test("is restricted to its container, even with margins", async (assert) => { + // Add a sheet to set a margin on the popper + const SHEET_MARGINS = { + top: 11, + right: 12, + bottom: 13, + left: 14, + }; + const sheet = document.createElement("style"); + sheet.textContent = ` + #popper { + margin-top: ${SHEET_MARGINS.top}px; + margin-right: ${SHEET_MARGINS.right}px; + margin-bottom: ${SHEET_MARGINS.bottom}px; + margin-left: ${SHEET_MARGINS.left}px; + } + `; + document.head.appendChild(sheet); + registerCleanup(() => sheet.remove()); + + // Local helper + async function _mountTestComponentAndDestroy(popperOptions) { + const TestComp = getTestComponent({ + ...popperOptions, + onPositioned: (el, { direction, variant }) => { + assert.step(`${direction}-${variant}`); + }, + }); + const popper = await mount(TestComp, container); + destroy(popper); + container.removeChild(document.getElementById("target")); + } + + const minSize = 150; // => popper is 100px, target is 50px + const margin = 10; // will serve as additional margin + + // === DIRECTION: BOTTOM === + // Container style changes: push target to top + Object.assign(container.style, { alignItems: "flex-start" }); + + // --> Without additional margin + // Leave just enough space for the popper to be contained + Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.top}px` }); + await _mountTestComponentAndDestroy({ position: "bottom" }); + assert.verifySteps(["bottom-middle"]); + // Remove 1px => popper should switch direction as it can't be contained + Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.top - 1}px` }); + await _mountTestComponentAndDestroy({ position: "bottom" }); + assert.verifySteps(["right-start"]); + + // --> With additional margin + // Leave just enough space for the popper to be contained + Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.top + margin}px` }); + await _mountTestComponentAndDestroy({ position: "bottom", margin }); + assert.verifySteps(["bottom-middle"]); + // Remove 1px => popper should switch direction as it can't be contained + Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.top + margin - 1}px` }); + await _mountTestComponentAndDestroy({ position: "bottom", margin }); + assert.verifySteps(["right-start"]); + + // === DIRECTION: TOP === + // Container style changes: push target to bottom + Object.assign(container.style, { alignItems: "flex-end" }); + + // --> Without additional margin + // Leave just enough space for the popper to be contained + Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.bottom}px` }); + await _mountTestComponentAndDestroy({ position: "top" }); + assert.verifySteps(["top-middle"]); + // Remove 1px => popper should switch direction as it can't be contained + Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.bottom - 1}px` }); + await _mountTestComponentAndDestroy({ position: "top" }); + assert.verifySteps(["right-end"]); + + // --> With additional margin + // Leave just enough space for the popper to be contained + Object.assign(container.style, { height: `${minSize + SHEET_MARGINS.bottom + margin}px` }); + await _mountTestComponentAndDestroy({ position: "top", margin }); + assert.verifySteps(["top-middle"]); + // Remove 1px => popper should switch direction as it can't be contained + Object.assign(container.style, { + height: `${minSize + SHEET_MARGINS.bottom + margin - 1}px`, + }); + await _mountTestComponentAndDestroy({ position: "top", margin }); + assert.verifySteps(["right-end"]); + + // === DIRECTION: LEFT === + // Container style changes: reset previous changes + Object.assign(container.style, { alignItems: "center", height: "450px" }); + // Container style changes: push target to right + Object.assign(container.style, { justifyContent: "flex-end" }); + + // --> Without additional margin + // Leave just enough space for the popper to be contained + Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.right}px` }); + await _mountTestComponentAndDestroy({ position: "left" }); + assert.verifySteps(["left-middle"]); + // Remove 1px => popper should switch direction as it can't be contained + Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.right - 1}px` }); + await _mountTestComponentAndDestroy({ position: "left" }); + assert.verifySteps(["bottom-end"]); + + // --> With additional margin + // Leave just enough space for the popper to be contained + Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.right + margin}px` }); + await _mountTestComponentAndDestroy({ position: "left", margin }); + assert.verifySteps(["left-middle"]); + // Remove 1px => popper should switch direction as it can't be contained + Object.assign(container.style, { + width: `${minSize + SHEET_MARGINS.right + margin - 1}px`, + }); + await _mountTestComponentAndDestroy({ position: "left", margin }); + assert.verifySteps(["bottom-end"]); + + // === DIRECTION: RIGHT === + // Container style changes: push target to left + Object.assign(container.style, { justifyContent: "flex-start" }); + + // --> Without additional margin + // Leave just enough space for the popper to be contained + Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.left}px` }); + await _mountTestComponentAndDestroy({ position: "right" }); + assert.verifySteps(["right-middle"]); + // Remove 1px => popper should switch direction as it can't be contained + Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.left - 1}px` }); + await _mountTestComponentAndDestroy({ position: "right" }); + assert.verifySteps(["top-start"]); + + // --> With additional margin + // Leave just enough space for the popper to be contained + Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.left + margin}px` }); + await _mountTestComponentAndDestroy({ position: "right", margin }); + assert.verifySteps(["right-middle"]); + // Remove 1px => popper should switch direction as it can't be contained + Object.assign(container.style, { width: `${minSize + SHEET_MARGINS.left + margin - 1}px` }); + await _mountTestComponentAndDestroy({ position: "right", margin }); + assert.verifySteps(["top-start"]); }); QUnit.test("popper is an inner element", async (assert) => {