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

The Portal part cannot be omitted safely when nesting popups #1215

Open
atomiks opened this issue Dec 23, 2024 · 0 comments · May be fixed by #1222
Open

The Portal part cannot be omitted safely when nesting popups #1215

atomiks opened this issue Dec 23, 2024 · 0 comments · May be fixed by #1222
Labels
bug 🐛 Something doesn't work component: alert dialog This is the name of the generic UI component, not the React module! component: dialog This is the name of the generic UI component, not the React module! component: menu This is the name of the generic UI component, not the React module! component: popover The React component. component: preview card The React component. component: select This is the name of the generic UI component, not the React module! component: tooltip This is the name of the generic UI component, not the React module!

Comments

@atomiks
Copy link
Contributor

atomiks commented Dec 23, 2024

Omitting the Portal part can currently lead to various bugs when dealing with nested popups. The docs always use the Portal part, but omitting it is technically allowed right now (no runtime error).

Known bugs

Some of the bugs include (non-exhaustively):

  • Nesting Popover inside a Dialog when neither has a portal prevents the esc key from only dismissing the Popover — both get dismissed simultaneously
  • Nesting Popover, Select or Menu inside Dialog when only Dialog has a portal causes focus to be lost when attempting to tab outside of the inner popup's last tabbable element
  • Nesting Menu inside Dialog when only the the root menu has a portal (with submenus not using a portal) causes the root Menu to stay stuck open when tabbing outside of one of the submenus

Some bugs are already covered in #1107, which has a PR open that fixes it already.

Solution

  1. Throw a runtime error if Portal is missing entirely (this is likely the best solution, at least for now)
  2. And/or, try to solve most of the bugs depending on the portal combination if possible, and document when omitting it won't work
@atomiks atomiks added component: select This is the name of the generic UI component, not the React module! component: menu This is the name of the generic UI component, not the React module! component: alert dialog This is the name of the generic UI component, not the React module! bug 🐛 Something doesn't work component: tooltip This is the name of the generic UI component, not the React module! component: dialog This is the name of the generic UI component, not the React module! component: popover The React component. component: preview card The React component. labels Dec 23, 2024
@atomiks atomiks linked a pull request Dec 24, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: alert dialog This is the name of the generic UI component, not the React module! component: dialog This is the name of the generic UI component, not the React module! component: menu This is the name of the generic UI component, not the React module! component: popover The React component. component: preview card The React component. component: select This is the name of the generic UI component, not the React module! component: tooltip This is the name of the generic UI component, not the React module!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant