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

Ensure dropdown is placed above other elements. #274

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Half-Shot
Copy link
Member

I believe this would ensure dropdowns work correctly when other absolute Elements are placed beneath them.

@Half-Shot Half-Shot requested a review from a team as a code owner November 11, 2024 16:01
@Half-Shot Half-Shot requested review from t3chguy and robintown and removed request for a team November 11, 2024 16:01
Copy link

Deploying compound-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: 531e390
Status: ✅  Deploy successful!
Preview URL: https://2aa71167.compound-web.pages.dev
Branch Preview URL: https://hs-z-index-dropdown.compound-web.pages.dev

View logs

Copy link
Member

@robintown robintown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been really happy that our component library has so far avoided the use of z-index: 999; hacks, and am reluctant to start using such a rule here. We've managed to make tooltips and menus display correctly with z-index: auto; by putting their "floating" bits in a React portal which gets the right layering by virtue of its location in the DOM. I would prefer to try the same here since z-indices are always going to be application-dependent, and you could still see layering glitches if the dropdown is housed inside yet another stacking context.

I do see that replacing the 999 with a custom property helps, as applications could customize the z-index if they really need to, but I'd like to return to this solution only if portals don't appear to be an option.

@Half-Shot
Copy link
Member Author

Agreed with your comments, I'll have a go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants