Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: CSS container queries break Popover positioning #3356

Open
2 tasks done
bmv437 opened this issue Mar 18, 2025 · 5 comments · May be fixed by #3379
Open
2 tasks done

[Bug]: CSS container queries break Popover positioning #3356

bmv437 opened this issue Mar 18, 2025 · 5 comments · May be fixed by #3379
Labels
bug Something isn't working

Comments

@bmv437
Copy link

bmv437 commented Mar 18, 2025

Browser

Chrome

Package version

latest from live

React version

v18.3.1

Description

When a Popover component is rendered as a descendant of an element that uses CSS container queries with the container-type property set to size or inline-size, the position of the popover is no longer correct.

Here's a video of the behavior I'm seeing:

Screen.Recording.2025-03-18.at.11.40.40.AM.mov

I've included a reproduction as well.

Source code

No response

Reproduction

https://codesandbox.io/p/sandbox/container-broken-popover-nl3prj

Code of Conduct

@bmv437 bmv437 added the bug Something isn't working label Mar 18, 2025
@bmv437
Copy link
Author

bmv437 commented Mar 18, 2025

When searching around the internals, I did find this reference to the containerType computed property for the isContainingBlock check:

export function isContainingBlock(element: HTMLElement): boolean {
const computedStyle = getComputedStyle(element);
return (
(!!computedStyle.transform && computedStyle.transform !== 'none') ||
(!!computedStyle.perspective && computedStyle.perspective !== 'none') ||
(!!computedStyle.containerType && computedStyle.containerType !== 'normal') ||
computedStyle.contain?.split(' ').some(s => ['layout', 'paint', 'strict', 'content'].includes(s))
);
}

Which is used here to determine the popover's boundary :

const { containingBlock, boundary } = findUpUntilMultiple({
startElement: popover,
tests: {
containingBlock: isContainingBlock,
boundary: (element: HTMLElement) => isContainingBlock(element) || isBoundary(element),
},
});

Looking at the git blame, the change to treat containerType != 'normal' as a "containing block" was added in this PR:
#2283. That PR does note that at the time there was no real need for this change, just an attempt to get ahead of needing to support container queries, and that there's no tests for this functionality.

If I manually edit my local copy of cloudscape in my node_modules to comment out this line, I'm seeing the popover position work properly:

(!!computedStyle.containerType && computedStyle.containerType !== 'normal') ||

@YueyingLu
Copy link
Member

Hey @bmv437
thanks for reaching out and your investigation. Our team will work on it and keep you updated.

@avinashbot
Copy link
Member

avinashbot commented Mar 21, 2025

Thanks for opening the bug report - I can reproduce this in Chrome and Firefox (but not Safari). The original PR was necessary because a query container is supposed to create a new containing block context and affect fixed positioning for child components. But it looks like this behavior was silently changed at some point (w3c/csswg-drafts#10544) after the PR was released, and updated only in Chrome and Firefox. So it looks like we'll have to do some browser/CSS support checks before applying this logic.

@bmv437
Copy link
Author

bmv437 commented Mar 21, 2025

Thanks for looking into it. Very cool find around the behavior changing in Chrome/Firefox but the MDN docs not reflecting that.

I did some additional research into similar popover implementations, and found this:
https://floating-ui.com/

Internally, they have a very similar isContainingBlock implementation with some additional checks:
https://github.com/floating-ui/floating-ui/blob/b9ad108d9b0257fe1d7d25a3b187fd5189062b9a/packages/utils/src/dom.ts#L86-L113

And they also have overflow element detection:
https://github.com/floating-ui/floating-ui/blob/master/packages/utils/src/dom.ts#L64-L70

Which seems similar to the isBoundary function use by the popover hook, which could be enhanced with their logic:

function isBoundary(element: HTMLElement) {
const computedStyle = getComputedStyle(element);
return !!computedStyle.clipPath && computedStyle.clipPath !== 'none';
}

@pan-kot
Copy link
Member

pan-kot commented Mar 26, 2025

Hey @bmv437,

Thanks for your research! Our team is still working on the mitigation. Meanwhile, you might be able to use the workaround by setting the contain: layout for your container. This forces creation of the new containing block and works consistently across Chrome, Firefox, and Safari.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants