theKashey / react-focus-on

🎯 Solution for WAI ARIA compatible modal dialogs or full-screen tasks, you were looking for
MIT License
333 stars 14 forks source link

Scroll lock prevents multi-touch zoom and back functionality #49

Closed ShaneHudson closed 1 year ago

ShaneHudson commented 3 years ago

Hi there, thanks for making this package it's really handled a lot of the situations I was manually catering for.

One issue I have with it is that I want scroll lock enabled, to prevent the page behind a modal scrolling, but I also want multi-touch zoom and swipe to go back to work. Is there currently any flags that will allow this while maintaining the scroll-lock for the rest of the page?

I have a modal inside a Portal if that's relevant to it.

theKashey commented 3 years ago

Generally, you are looking for the allowPinchZoom option. Please give it a try and check if it's working for you. It might not work that well, and I don't have a better solution here.

You might not need the actual logic behind remove-scroll part, but only remove-scroll-bar - historically this code was created to handle Safary, and right now the newer versions can "scroll-lock" with a simple overflow on the body. So can you try as well just skip extra logic:

ShaneHudson commented 3 years ago

Thanks @theKashey, I can confirm that disabling scrollLock and using the package manually did do the job nicely. Thanks!

equinusocio commented 2 years ago

Thanks @theKashey, I can confirm that disabling scrollLock and using the package manually did do the job nicely. Thanks!

Doesn't seems a solution...

cee-chen commented 1 year ago

Just wanted to add a +1 that @theKashey's suggestion (forcing scrollLock={false} and including <RemoveScrollBar /> manually) worked perfectly for us. In our use-case, we didn't need pinch/zoom behavior - but we had popovers/dropdown menus within our focus-trapped modals/flyouts that were portalled at the top level of the document, and those dynamic portals needed to be scrollable.

One small thought:

historically this code was created to handle Safari, and right now the newer versions can "scroll-lock" with a simple overflow on the body.

Since Safari no longer has this issue, could you consider removing this extra behavior from scrollLock by default or allowing it to be configurable by a prop, e.g. scrollLock="cssOnly" or similar? We're fine with this workaround for now, but just throwing this out there as the scroll prevention feels like it's causing more issues at this point than solving them.

theKashey commented 1 year ago

@cee-chen+++:

Time changes, so should we

stale[bot] commented 1 year ago

This issue has been marked as "stale" because there has been no activity for 2 months. If you have any new information or would like to continue the discussion, please feel free to do so. If this issue got buried among other tasks, maybe this message will reignite the conversation. Otherwise, this issue will be closed in 7 days. Thank you for your contributions so far.