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

[DOCS]: PopoverPanel modal incorrectly says default=true and makes others inert #3223

Closed
david-crespo opened this issue May 21, 2024 · 3 comments

Comments

@david-crespo
Copy link

This bit of the docs appears to be incorrect.

https://headlessui.com/react/popover

image

Based on the description of #3124 both of the following are intentional, so the fix would be to the docs rather than the code.

modal defaults to false

PopoverPanel does not call useOthersInert

// Ensure we close the popover as soon as the button becomes hidden
useOnDisappear(state.button, () => dispatch({ type: ActionTypes.ClosePopover }), visible)
// Enable scroll locking when the popover is visible, and `modal` is enabled
useScrollLock(ownerDocument, state.__demoMode ? false : modal && visible)

@RobinMalfait
Copy link
Member

Good catch!

You are right that the Popover component doesn't set the inert attribute on other elements. We've updated the docs accordingly. Thanks

@david-crespo
Copy link
Author

david-crespo commented May 25, 2024

Thanks! I don’t want to be pedantic, but it still says modal defaults to true and I don’t think it does.

Fixed now too.

@vincerubinetti
Copy link

vincerubinetti commented Feb 11, 2025

The docs currently say:

Whether to enable accessibility features like scroll locking and focus trapping.

Looking at the code, it only does scroll locking. It doesn't make anything inert. It doesn't prevent interaction outside the popover. And it definitely doesn't focus trap; you can tab out of the popover to close it.

Calling this option modal is very misleading. It doesn't do what is expected, or even what the HUI Dialog component does.


Related, the focus option docs currently say:

This will force focus inside the PopoverPanel when the popover is open. It will also close the popover if focus left this component.

Taken literally those sentences contradict each other. I believe it should be when the popover is opened. Critical distinction.

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

No branches or pull requests

3 participants