From 1a9cc8644a2c4efedc02c522a699b32892c10923 Mon Sep 17 00:00:00 2001 From: TeodorTaushanov Date: Wed, 29 Nov 2023 13:37:09 +0200 Subject: [PATCH] fix(ui5-popover): fix "containing block" position issue (#7870) * fix(ui5-popover): fix containing block positioning issue --- packages/base/src/util/getParentElement.ts | 5 ++++ .../base/src/util/isElementContainingBlock.ts | 12 ++++++++ packages/main/src/Dialog.ts | 9 +++--- packages/main/src/Popover.ts | 26 ++++++++++++++++ packages/main/test/pages/Popover.html | 30 +++++++++++++++---- packages/main/test/pages/styles/Popover.css | 7 +++++ packages/main/test/specs/Popover.spec.js | 6 ++-- 7 files changed, 84 insertions(+), 11 deletions(-) create mode 100644 packages/base/src/util/getParentElement.ts create mode 100644 packages/base/src/util/isElementContainingBlock.ts diff --git a/packages/base/src/util/getParentElement.ts b/packages/base/src/util/getParentElement.ts new file mode 100644 index 000000000000..e2d3124d69a5 --- /dev/null +++ b/packages/base/src/util/getParentElement.ts @@ -0,0 +1,5 @@ +const getParentElement = (el: HTMLElement) => { + return (el.parentElement ? el.parentNode : (el.parentNode as ShadowRoot).host) as HTMLElement; +}; + +export default getParentElement; diff --git a/packages/base/src/util/isElementContainingBlock.ts b/packages/base/src/util/isElementContainingBlock.ts new file mode 100644 index 000000000000..cef9d9f1132b --- /dev/null +++ b/packages/base/src/util/isElementContainingBlock.ts @@ -0,0 +1,12 @@ +const isElementContainingBlock = (el: HTMLElement) => { + const computedStyle = getComputedStyle(el); + + return ["size", "inline-size"].indexOf(computedStyle.containerType) > -1 + || ["transform", "perspective"].indexOf(computedStyle.willChange) > -1 + || ["layout", "paint", "strict", "content"].indexOf(computedStyle.contain) > -1 + || computedStyle.transform !== "none" + || computedStyle.perspective !== "none" + || computedStyle.backdropFilter !== "none"; +}; + +export default isElementContainingBlock; diff --git a/packages/main/src/Dialog.ts b/packages/main/src/Dialog.ts index 353cde68fda1..ca1dff61fa02 100644 --- a/packages/main/src/Dialog.ts +++ b/packages/main/src/Dialog.ts @@ -94,11 +94,12 @@ const ICON_PER_STATE: Record = { * * import "@ui5/webcomponents/dist/Dialog"; * - * Note: We don't recommend nesting popup-like components (ui5-dialog, ui5-popover) inside ui5-dialog. - * Ideally you should create all popups on the same level inside your HTML page and just open them from one another, rather than nesting them. + * Note: We recommend placing popup-like components (ui5-dialog and ui5-popover) + * outside any other components. Preferably, the popup-like components should be placed + * in an upper level HTML element. Otherwise, in some cases the parent HTML elements can break + * the position and/or z-index management of the popup-like components. * - * Note: We don't recommend nesting popup-like components (ui5-dialog, ui5-popover) inside other components containing z-index. - * This might break z-index management. + * Note: We don't recommend nesting popup-like components (ui5-dialog, ui5-popover). * * @constructor * @author SAP SE diff --git a/packages/main/src/Popover.ts b/packages/main/src/Popover.ts index f754ba49f0ef..03998167499f 100644 --- a/packages/main/src/Popover.ts +++ b/packages/main/src/Popover.ts @@ -7,6 +7,8 @@ import { isIOS } from "@ui5/webcomponents-base/dist/Device.js"; import DOMReference from "@ui5/webcomponents-base/dist/types/DOMReference.js"; import { getClosedPopupParent } from "@ui5/webcomponents-base/dist/util/PopupUtils.js"; import clamp from "@ui5/webcomponents-base/dist/util/clamp.js"; +import isElementContainingBlock from "@ui5/webcomponents-base/dist/util/isElementContainingBlock.js"; +import getParentElement from "@ui5/webcomponents-base/dist/util/getParentElement.js"; import Popup from "./Popup.js"; import type { PopupBeforeCloseEventDetail as PopoverBeforeCloseEventDetail } from "./Popup.js"; import PopoverPlacementType from "./types/PopoverPlacementType.js"; @@ -79,6 +81,13 @@ type CalculatedPlacement = { * * import "@ui5/webcomponents/dist/Popover.js"; * + * Note: We recommend placing popup-like components (ui5-dialog and ui5-popover) + * outside any other components. Preferably, the popup-like components should be placed + * in an upper level HTML element. Otherwise, in some cases the parent HTML elements can break + * the position and/or z-index management of the popup-like components. + * + * Note: We don't recommend nesting popup-like components (ui5-dialog, ui5-popover). + * * @constructor * @author SAP SE * @alias sap.ui.webc.main.Popover @@ -464,6 +473,9 @@ class Popover extends Popup { this.arrowTranslateY = placement!.arrow.y; top = this._adjustForIOSKeyboard(top); + const containingBlockClientLocation = this._getContainingBlockClientLocation(); + left -= containingBlockClientLocation.left; + top -= containingBlockClientLocation.top; Object.assign(this.style, { top: `${top}px`, @@ -492,6 +504,20 @@ class Popover extends Popup { return top + (Number.parseInt(this.style.top || "0") - actualTop); } + _getContainingBlockClientLocation() { + let parentElement = getParentElement(this); + + while (parentElement) { + if (isElementContainingBlock(parentElement)) { + return parentElement.getBoundingClientRect(); + } + + parentElement = getParentElement(parentElement); + } + + return { left: 0, top: 0 }; + } + getPopoverSize(): PopoverSize { const rect = this.getBoundingClientRect(), width = rect.width, diff --git a/packages/main/test/pages/Popover.html b/packages/main/test/pages/Popover.html index 03c011b2ffd5..40780ad54f59 100644 --- a/packages/main/test/pages/Popover.html +++ b/packages/main/test/pages/Popover.html @@ -26,7 +26,6 @@ - Click me ! @@ -120,6 +119,24 @@

+
+
+ Containing Block Div + 
+container-type: inline-size;
+position: relative;
+z-index: 1;
+
+
+ Open Popover Inside a Containing Block + + Popover
Inside
Containing
Block + +
+ +
+
+ Open Big Popover @@ -395,7 +412,7 @@ Dialog Focus
Header text
- +
Close @@ -415,7 +432,7 @@

Horizontal Align

- +
Target opener @@ -432,7 +449,6 @@

Horizontal Align



- diff --git a/packages/main/test/pages/styles/Popover.css b/packages/main/test/pages/styles/Popover.css index 90f9cc7310c4..8e8f5905a66a 100644 --- a/packages/main/test/pages/styles/Popover.css +++ b/packages/main/test/pages/styles/Popover.css @@ -66,3 +66,10 @@ ui5-date-picker, height: 150px; width: 300px; } + +.containingBlock { + border: 1px solid black; + container-type: inline-size; + position: relative; + z-index: 1; +} diff --git a/packages/main/test/specs/Popover.spec.js b/packages/main/test/specs/Popover.spec.js index 2764c7d51bd7..1bd402356f7e 100644 --- a/packages/main/test/specs/Popover.spec.js +++ b/packages/main/test/specs/Popover.spec.js @@ -82,6 +82,8 @@ describe("Popover general interaction", () => { const btnOpenPopover = await browser.$("#btnQuickViewCardOpener"); const btnMoveFocus = await browser.$("#btnMoveFocus"); + await btnOpenPopover.scrollIntoView(); + // assert - the opener is visible assert.strictEqual(await btnOpenPopover.isDisplayedInViewport(), true, "Opener is available."); @@ -474,7 +476,7 @@ describe("Horizontal Alignment", () => { await browser.$("#horizontalAlignBtn").click(); const popover = await browser.$("#popoverHorizontalAlign"); const opener = await browser.$("#targetOpener"); - + assert.ok(await isHorizontallyCentered(popover, opener), `Popover should be centered`); }); @@ -492,7 +494,7 @@ describe("Horizontal Alignment", () => { await browser.$("#horizontalAlignBtn").click(); const popover = await browser.$("#popoverHorizontalAlign"); const opener = await browser.$("#targetOpener"); - + assert.ok(await isHorizontallyRightAligned(popover, opener), `Popover should be right aligned`); });