Skip to content

[popups] Rework modal prop #1554

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

Closed
atomiks opened this issue Mar 13, 2025 · 0 comments · Fixed by #1571
Closed

[popups] Rework modal prop #1554

atomiks opened this issue Mar 13, 2025 · 0 comments · Fixed by #1571
Labels
component: dialog This is the name of the generic UI component, not the React module! component: popover This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature new feature New feature or request

Comments

@atomiks
Copy link
Contributor

atomiks commented Mar 13, 2025

Feature request

Summary

Discussion starting at #1459 (comment) (@vladmoroz)

The word modal is quite overloaded and confusing, since there are actually three different user inputs that can be "modal", which can have varying combinations of being on or off:

  • modal focus — you can't focus anything behind (i.e. except what's in) the popup while it's open
  • modal scroll — you can't scroll anything behind (i.e. except what's in) the popup while it's open
  • modal pointer — you can't click anything behind (i.e. except what's in) the popup while it's open

Typically when people say something is "modal", they mean all three are modal simultaneously, such as in the case with most dialogs that are positioned in the center of the screen, have a dimming backdrop, and aren't anchored to another element.

But it can be desirable to have a popover be "non-modal" (with respect to pointers) but still trap focus, i.e. be modal to keyboard focus.

Instead of the modal prop, I proposed the less-overloaded word trap instead, typed as boolean | TrapOptions:

// fully modal popover
<Popover.Popup trap />

// "non-modal" popover with trapped (modal) focus
<Popover.Popup trap={{ focus: true, scroll: false, pointer: false }} />

Although "lock" is typically used for scroll modality, and "pointer" doesn't really have a word describing its modality, I think trap correctly describes what's happening to all three user inputs:

  • Trapping their focus means their focus is trapped inside the popup — they can't tab to anything except what's in the popup
  • Trapping their pointer means their pointer is trapped inside the popup — they can't click anything except what's in the popup
  • Trapping their scroll means their scroll is trapped inside the popup — they can't scroll anything except what's in the popup

Combinations

One of the problems with this config object is that some combinations lead to an incoherent user experience.

# trap value Description
1 focus: true | scroll: true | pointer: true ✅ All three are trapped — no focus, scroll, or pointer interactions allowed behind.
2 focus: true | scroll: true | pointer: false ❌ Focus and scroll are trapped, but pointer interactions are allowed behind.
3 focus: true | scroll: false | pointer: true ⚠️ Focus and pointer are trapped, but scrolling is allowed behind. (Seen on GitHub's popups, less restrictive and just avoids sloppy pointer outside clicks. However, if the pointer is trapped, inner HTML scroll containers can't be scrolled, only document scroll is locked, so this can be confusing)
4 focus: true | scroll: false | pointer: false ✅ Only focus is trapped; scrolling and pointer interactions are allowed behind. (Vlad's popover)
5 focus: false | scroll: true | pointer: true ✅ Scroll and pointer are trapped, but focus is allowed behind. (used in Menu & Select)
6 focus: false | scroll: true | pointer: false ❌ Only scroll is trapped; focus and pointer interactions are allowed behind.
7 focus: false | scroll: false | pointer: true ⚠️ Only pointer is trapped; focus and scrolling are allowed behind. (Seen on GitHub's popups, variant of the other one)
8 focus: false | scroll: false | pointer: false ✅ No trapping — focus, scrolling, and pointer interactions are all allowed behind.
@atomiks atomiks added component: dialog This is the name of the generic UI component, not the React module! component: popover This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature new feature New feature or request labels Mar 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: dialog This is the name of the generic UI component, not the React module! component: popover This is the name of the generic UI component, not the React module! enhancement This is not a bug, nor a new feature new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant