react-spectrum
react-spectrum copied to clipboard
adobe
fix(#3804): usePopover: non-modal popover allows page and popover to scroll when target has position: fixed
Closes #3804
✅ Pull Request Checklist:
- [x] Included link to corresponding React Spectrum GitHub Issue #3804.
- [x] Added/updated unit tests and storybook for this change (for new code or code which already has tests).
- [x] Filled out test instructions.
- [ ] Updated documentation (if it already exists for this component).
- [x] Looked at the Accessibility Practices for this feature - Aria Practices
📝 Test Instructions:
-
To test behavior before any fix, open the new Storybook example for a ComboBox with fixed positioning: https://reactspectrum.blob.core.windows.net/reactspectrum/82d74597a0176141733a4c2366392f39ebe7811d/storybook/index.html?path=/story/combobox--fixed-positioning&providerSwitcher-express=false&providerSwitcher-toastPosition=bottom
-
Expand the ComboBox and scroll the page using the mouse wheel.
-
The listbox will scroll with the underlying content.
-
To test the behavior with a simple fix that does not disable
usePreventScrollwithinusePopoverwhenisNonModal={true}, open the same Storybook example with the fix applied: https://reactspectrum.blob.core.windows.net/reactspectrum/e2856408a3291ed37397f721e22393b587b52d39/storybook/index.html?path=/story/combobox--fixed-positioning&providerSwitcher-express=false&providerSwitcher-toastPosition=bottom -
Expand the ComboBox and scroll the page using the mouse wheel.
-
The page will not scroll when the Iistbox is expanded.
🧢 Your Project:
Adobe/Accessibility
This build just adds the Storybook example that demonstrates the behavior before any fix has been applied.
Build successful! 🎉
This build includes a simple fix that does not disable usePreventScroll within usePopover when isNonModal={true}.
Build successful! 🎉
Build successful! 🎉
## API Changes
unknown top level export { type: 'identifier', name: 'Column' } unknown top level export { type: 'identifier', name: 'Column' }
@LFDanLu I agree that we will probably need to account for targets that have or inherit position: fixed by making the overlay also have position: fixed and by taking that into account when positioning the overlay.
It's odd to me that I can scroll the list of options on the iPad in the Storybook example, yet the behavior is different in the docs.
Hm, definitely weird that the storybook example works on iPad but it doesn't in the docs (maybe something to do with the storybook not being scrollable?). Maybe indicative of a bug in the usePreventScroll code since it should allow scrolling in nested scroll regions now that I'm looking at the code again
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉