Skip to content

Commit

Permalink
fix(ui5-popover): fix "containing block" position issue (#7870)
Browse files Browse the repository at this point in the history
* fix(ui5-popover): fix containing block positioning issue
  • Loading branch information
TeodorTaushanov authored Nov 29, 2023
1 parent 2f3742b commit 1a9cc86
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 11 deletions.
5 changes: 5 additions & 0 deletions packages/base/src/util/getParentElement.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const getParentElement = (el: HTMLElement) => {
return (el.parentElement ? el.parentNode : (el.parentNode as ShadowRoot).host) as HTMLElement;
};

export default getParentElement;
12 changes: 12 additions & 0 deletions packages/base/src/util/isElementContainingBlock.ts
Original file line number Diff line number Diff line change
@@ -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;
9 changes: 5 additions & 4 deletions packages/main/src/Dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,12 @@ const ICON_PER_STATE: Record<ValueStateWithIcon, string> = {
*
* <code>import "@ui5/webcomponents/dist/Dialog";</code>
*
* <b>Note:</b> We don't recommend nesting popup-like components (<code>ui5-dialog</code>, <code>ui5-popover</code>) inside <code>ui5-dialog</code>.
* 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.
* <b>Note: </b> We recommend placing popup-like components (<code>ui5-dialog</code> and <code>ui5-popover</code>)
* 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.
*
* <b>Note:</b> We don't recommend nesting popup-like components (<code>ui5-dialog</code>, <code>ui5-popover</code>) inside other components containing z-index.
* This might break z-index management.
* <b>Note:</b> We don't recommend nesting popup-like components (<code>ui5-dialog</code>, <code>ui5-popover</code>).
*
* @constructor
* @author SAP SE
Expand Down
26 changes: 26 additions & 0 deletions packages/main/src/Popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -79,6 +81,13 @@ type CalculatedPlacement = {
*
* <code>import "@ui5/webcomponents/dist/Popover.js";</code>
*
* <b>Note: </b> We recommend placing popup-like components (<code>ui5-dialog</code> and <code>ui5-popover</code>)
* 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.
*
* <b>Note:</b> We don't recommend nesting popup-like components (<code>ui5-dialog</code>, <code>ui5-popover</code>).
*
* @constructor
* @author SAP SE
* @alias sap.ui.webc.main.Popover
Expand Down Expand Up @@ -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`,
Expand Down Expand Up @@ -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,
Expand Down
30 changes: 25 additions & 5 deletions packages/main/test/pages/Popover.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@

<body class="popover2auto">
<iframe id="clickThisIframe" src="//localhost:8080"></iframe>

<ui5-button id="btn">Click me !</ui5-button>

<ui5-popover id="pop" class="popover6auto" placement-type="Top" accessible-name="This popover is important">
Expand Down Expand Up @@ -120,6 +119,24 @@
<br>
<br>

<div class="containingBlock">
<div>
Containing Block Div
<pre>
container-type: inline-size;
position: relative;
z-index: 1;
</pre>
</div>
<ui5-button id="openPopoverInsideContainingBlockButton">Open Popover Inside a Containing Block</ui5-button>
<ui5-popover id="popoverInsideContainingBlock" placement-type="Bottom">
Popover<br>Inside<br>Containing<br>Block
</ui5-popover>
</div>

<br>
<br>

<ui5-button id="big-popover-button">
Open Big Popover
</ui5-button>
Expand Down Expand Up @@ -395,7 +412,7 @@
<ui5-button id="popoverFocusButton">Dialog Focus</ui5-button>
<ui5-popover id="popoverFocus">
<div slot="header">Header text</div>

<div slot="footer" class="dialog-footer">
<div style="flex: 1"></div>
<ui5-button id="closeButton" design="Emphasized">Close</ui5-button>
Expand All @@ -415,7 +432,7 @@ <h2>Horizontal Align</h2>
<div>
<ui5-checkbox id="rtlCb" text="rtl"></ui5-checkbox>
</div>

<div id="horizontalAlignContainer">
<div id="targetOpener" class="popover10auto">
<span>Target opener</span>
Expand All @@ -432,7 +449,6 @@ <h2>Horizontal Align</h2>
<span id="createAndRemoveResult" style="display: none;"></span>
<br>
<br>

<script>
btn.addEventListener("click", function (event) {
pop.showAt(btn);
Expand Down Expand Up @@ -626,7 +642,7 @@ <h2>Horizontal Align</h2>

createAndRemove.addEventListener("click", function () {
var pop = document.createElement("ui5-popover");

pop.setAttribute("open", true);
pop.setAttribute("opener", "createAndRemove");

Expand All @@ -650,6 +666,10 @@ <h2>Horizontal Align</h2>
})
});

openPopoverInsideContainingBlockButton.addEventListener("click", function () {
popoverInsideContainingBlock.showAt(openPopoverInsideContainingBlockButton);
});

</script>
</body>

Expand Down
7 changes: 7 additions & 0 deletions packages/main/test/pages/styles/Popover.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
6 changes: 4 additions & 2 deletions packages/main/test/specs/Popover.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
Expand Down Expand Up @@ -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`);
});

Expand All @@ -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`);
});

Expand Down

0 comments on commit 1a9cc86

Please sign in to comment.