pengyuthon/odoo_source
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[FIX] web: account for popper margins from style

Browse Source 
**Before this commit**
Since [1] poppers margins computed from the style are
not taken into account.

**Additional note**
The commit [1] has been backported to 16.0 branch
right in front of this commit.

**After this commit**
This commit fixes that by taking those margins into account.

[1] b9551a50

closes odoo/odoo#124113

X-original-commit: 453f38527d0d232c886daa9dc71717f678918064
Signed-off-by: Damien Abeloos (abd) <abd@odoo.com>
Signed-off-by: Bruno Boi (boi) <boi@odoo.com>
This commit is contained in:
Bruno Boi
2023-06-07 14:43:24 +02:00
parent 2a6696ce02
commit e68c1d7308
2 changed files with 217 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
addons/web/static/src/core/position_hook.js
+28 -15
View File
@@ -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];
addons/web/static/tests/core/position_hook_tests.js
+189 -9
View File
@@ -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) => {