usePopover: non-modal popover allows page and popover to scroll when target has position: fixed

[majornista]

🐛 Bug Report

A non-modal popover allows page and popover to scroll even when the target is fixed.

The new documentation search box feature in development includes a SearchAutocomplete combo box in a page header that has position: fixed

1. Open https://reactspectrum.blob.core.windows.net/reactspectrum/d5aed9e2838b40d54ba7e7ba7a7a77f8fec212c6/docs/react-spectrum/index.html
2. Enter some characters to search into the SearchAutocomplete, which should open a popover displaying the results.
3. With the popover open, use the mouse wheel to scroll the page.

🤔 Expected Behavior

The popover should not scroll with the underlying page, but should remain positioned relative to the SearchAutocomplete.

😯 Current Behavior

The popover scrolls with the page on top of the SearchAutocomplete contained in the page header with position: fixed.

💁 Possible Solution

isNonModal should not disable usePreventScroll at:

https://github.com/adobe/react-spectrum/blob/b35d5c02fe900badccd0cf1a8f23bb593419f238/packages/%40react-aria/overlays/src/usePopover.ts#L99-L107

🔦 Context

💻 Code Sample

🌍 Your Environment

Software	Version(s)
react-spectrum	@react-aria/overlays@3.12.0
Browser	All
Operating System	macOS

🧢 Your Company/Team

Adobe/Accessibility

🕷 Tracking Issue (optional)

3383

[LFDanLu]

We discussed this some more today, there might be a couple of problems that I hadn't considered previously:

1. Preventing scroll for isNonModal popovers (i.e. ComboBox) may end up preventing the user from clicking into the ComboBox input to move the text cursor while the popover is open. This might be a non-issue since I think the underlay is rendered independently of the usePreventScroll options changes in usePopover, but I haven't taken a closer look yet to confirm.
2. This might break the iOS experience for ComboBoxes/SearchAutocompletes since usePreventScroll does some touch event stuff to prevent scrolling.

[majornista]

I'll investigate today.

[majornista]

@LFDanLu See https://github.com/adobe/react-spectrum/pull/3809 to test Storybook examples before and after the simple fix:

1. 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

2. Expand the ComboBox and scroll the page using the mouse wheel.

3. The listbox will scroll with the underlying content.

4. To test the behavior with a simple fix that does not disable usePreventScroll within usePopover when isNonModal={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

5. Expand the ComboBox and scroll the page using the mouse wheel.

6. The page will not scroll when the Iistbox is expanded.

[majornista]

We discussed this some more today, there might be a couple of problems that I hadn't considered previously:

1. Preventing scroll for isNonModal popovers (i.e. ComboBox) may end up preventing the user from clicking into the ComboBox input to move the text cursor while the popover is open. This might be a non-issue since I think the underlay is rendered independently of the usePreventScroll options changes in usePopover, but I haven't taken a closer look yet to confirm.
2. This might break the iOS experience for ComboBoxes/SearchAutocompletes since usePreventScroll does some touch event stuff to prevent scrolling.

I'm not noticing any difference in behavior, besides preventing scroll of the page, between the ComboBox the prevents scroll and that which does not. I can still position the cursor within the input using either the keyboard or the mouse, and the iOS experience seems unchanged.

[EladBezalel]

Hey, any updates regarding this?