react-spectrum
react-spectrum copied to clipboard
adobe
Prevent scrolling while popovers are open
Closes #2926
TODO:
- A disabled scrollbar gets added when there isn't a scrollbar on the page
✅ Pull Request Checklist:
- [ ] Included link to corresponding React Spectrum GitHub Issue.
- [ ] Added/updated unit tests and storybook for this change (for new code or code which already has tests).
- [ ] Filled out test instructions.
- [ ] Updated documentation (if it already exists for this component).
- [ ] Looked at the Accessibility Practices for this feature - Aria Practices
📝 Test Instructions:
Check stories/docs for Picker, MenuTrigger, and DialogTrigger. When a popover is open, scrolling behind the popover should be disabled.
🧢 Your Project:
Build successful! 🎉
I was expecting to see the underlay here, any idea why I'm not? https://reactspectrum.blob.core.windows.net/reactspectrum/350bbc19ac4cac2f9dc5400fec50b1be76caca41/storybook/index.html?path=/story/dialogtrigger--type-popover&providerSwitcher-toastPosition=bottom
I only enabled in MenuTrigger and ComboBox for now to test. I can go ahead and enable them in all all RSP popovers though.
Ah, that's fine, i just didn't read your description closely enough. Though I guess it begs the question, shouldn't this just be default for all? or is there a case we're supporting that doesn't have them? and anyone implementing any overlaid components will need to know they need to add this prop to their work?
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
LGTM, tested in Chrome, FF, Safari, Chrome Android. Just wanna make sure but the issue w/ the scrollbars showing up when these popovers are opened is a followup item/out of scope for this PR right?
Not blocking, but IMO the scrollbar hiding can be weird so I'll try to fix in this PR
The scrollbar hiding comes from here, which is also the way to prevent scrolling:
https://github.com/adobe/react-spectrum/blob/c24dc2e0124fac3467b3adc48b1067f2447cd2b3/packages/%40react-aria/overlays/src/usePreventScroll.ts#L57-L64
Not sure if there's an alternative.
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉
Build successful! 🎉