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

Popover is rendered above the dialog #2156

Open
GerardasB opened this issue Jul 23, 2024 · 2 comments
Open

Popover is rendered above the dialog #2156

GerardasB opened this issue Jul 23, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@GerardasB
Copy link

Describe the bug (current behavior)

Popover is displayed above the dialog when rendered as a sibling to Dialog or Modal components.

Expected Behavior

Popover should be rendered below the Dialog and it's backdrop.
To render the Popover above the Dialog it should be portalled into the Dialog (or rendered as a child of the dialog).
Currently, each Popover portal component is wrapped by the ThemeProvider to handle nested popups, while Dialog is not and would require an explicit ThemeProvider or target prop to be passed into each popover, which seems inconsistent.

Link to minimal repro

https://stackblitz.com/edit/github-xa43bc?file=src%2FApp.tsx

Steps To Reproduce

  • Click Test2 button
  • Click Dialog button
  • Observe that the popover is rendered above the dialog

Anything else?

No response

@GerardasB GerardasB added the bug Something isn't working label Jul 23, 2024
@Ben-Pusey-Bentley
Copy link
Contributor

Seems that the z-index for the Popover is being set to such a high number that it gets drawn over absolutely everything currently. Setting it from 9999 to 999 has the Popover get drawn underneath the Dialog and it's backdrop correctly. Video:

Recording.2024-07-23.155022.mp4

@mayank99 is there a reason that the z-index is being set so high? Would it make sense for us to lower it to 999?

@mayank99
Copy link
Contributor

This issue is more complicated than just adjusting the z-index. We discussed this a bit in an internal Teams thread, but I'll summarize my thoughts:

Generally, I agree that modal dialogs should appear on top of everything else. Beyond that, I'm thinking we might need to maintain a stacking order: the one that was opened last should appear on top. This also matches how the browser-native top-layer mechanism works.

As a bonus, maintaining a stacking order would also allow us to reorder the stack as needed, which should help with #2104.


Further thoughts that have been in the back of my mind:

For the stack to work, we would likely need to make our portal containers work more reliably. Every ThemeProvider on the page should use the same portal container. Currently, this kinda works, assuming that React context is behaving as intended. But we have seen repeatedly that our users are unable to make context work in their applications, because of version/module conflicts. So we need a DOM-native (React-agnostic) way of inheriting portal containers across ThemeProviders from different versions.

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

No branches or pull requests

3 participants