react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

support scrolling outside of overlay

Open deebov opened this issue 4 years ago • 9 comments

🙋 Feature Request

Support for scrolling outside of overlay.

🤔 Expected Behavior

Overlay should not close if you scroll outside the overlay. And should remain it's position relative to the trigger.

😯 Current Behavior

Overlay closes if you scroll outside it.

💁 Possible Solution

🔦 Context

We want to create a Popover component that does not close on outside scroll and remains its relative position to the trigger.

deebov avatar Apr 30 '21 14:04 deebov

I think this will be challenging if not impossible to achieve without jank. Scroll events fire asynchronously, which means we won't be able to match the frame rate of the animation with our repositioning logic. That's why we currently hide the overlay when you scroll outside. Another option we are considering is to prevent scrolling entirely, just like we do for modals. That way it won't be hidden, and the user must click outside first.

devongovett avatar Apr 30 '21 17:04 devongovett

Are there any suggested workarounds? This behaviour is largely unexpected by our users. Tbh for us a slightly janky repositioning is still preferable to no option of disabling the current hide behaviour

davidfurlong avatar May 04 '21 10:05 davidfurlong

I think this will be challenging if not impossible to achieve without jank. Scroll events fire asynchronously, which means we won't be able to match the frame rate of the animation with our repositioning logic. That's why we currently hide the overlay when you scroll outside. Another option we are considering is to prevent scrolling entirely, just like we do for modals. That way it won't be hidden, and the user must click outside first.

have you seen the popover implementation from Radix? it seems they don't update popover's position every scroll trigger. Maybe react-aria could also implement or at least consider this approach https://radix-ui.com/primitives/docs/components/popover

deebov avatar May 05 '21 14:05 deebov

So I think this has more to do with a lucky happenstance, the popover is positioned in a place relative to the scrolling element, so it never needs updating. If that trigger were inside of a second scrollable section or if the trigger was in a fixed position element, I doubt it would be able to not update every time. I'd need to make a codesandbox or something to verify. I suspect this because they block scrolling while the Menu is open.

snowystinger avatar May 05 '21 17:05 snowystinger

For who wants to achieve this: https://codesandbox.io/s/react-aria-popover-open-on-scroll-5q8qu?file=/src/App.js

Warning: this is a "hacky" way, as we are passing down a different "state.close" function than the "useOverlayTrigger" expects.

marceloadsj avatar Jul 04 '21 10:07 marceloadsj

Btw, @snowystinger is right about how radix works with popover: https://codesandbox.io/s/radix-popover-example-8tycq

When rendering it inside a fixed wrapper, the popover does not keep it's position when we scroll the page. This would need a "updatePosition" call, like we are doing (hacky) on the example above.

marceloadsj avatar Jul 04 '21 10:07 marceloadsj

So, in summary:

Considering that the popover remains open on scroll to improve the UX


1) rendering it with portal at the end of the body:

wrapper/trigger position: ❌ Fixed: the position should be updated so it became a bit janky ❌ Non-Fixed: the position should be updated so it became a bit janky

✅ non-related content is hidden with aria-hidden

For the "❌ Fixed": maybe this could be turned into ✅ with the popover being "fixed" instead of "absolute"?


2) rendering it without portal, so, closer to the wrapper/trigger:

wrapper/trigger position: ❌ Fixed: the position should be updated so it became a bit janky ✅ Non-Fixed: the position is always right

❌ non-related content is not hidden with aria-hidden

marceloadsj avatar Jul 04 '21 11:07 marceloadsj

Confirming that the comment for "❌ Fixed" of 1st approach works: https://codesandbox.io/s/react-aria-popover-open-on-scroll-fixed-fkug6?file=/src/App.js

marceloadsj avatar Jul 04 '21 12:07 marceloadsj

Would I run into issues if I tried to use radix popover with the datepicker for example ?

bricejar avatar Sep 20 '22 13:09 bricejar