stripe / react-connect-js

React components for Connect.js and Connect embedded components
https://stripe.com/docs/connect/get-started-connect-embedded-components
MIT License
20 stars 8 forks source link

Feature request: onOverlayChange prop #103

Open minisemi opened 1 month ago

minisemi commented 1 month ago

When using embedded components in general, some will open up an overlay window (like the one to the right in my screenshot). That overlay will have its own scrollbar. Since the page has its own scrollbar, there will now be two scrollbars on the page, as seen in the screenshot (the two grey vertical bars to the far right). This leads to a buggy interaction for the user, since sometimes scrolling will scroll the page behind the overlay and sometimes it will scroll the overlay itself. I therefore request adding a prop to all embedded components called e.g. onOverlayChange which will receive a function. The onOverlayChange should then be called each time the embedded component opens or closes an overlay, returning a true or false value depending on if it closed or opened an overlay. We developers can then through that onOverlayChange function turn on/off the scrollbar on the page behind the overlay, so the overlay is the only scrollable element on the page when it is open.

image

jorgea-stripe commented 1 month ago

Hey @minisemi , do you have a repro for this (maybe a small repository where we'd be able to see this happening)?

minisemi commented 1 month ago

@jorgea-stripe This is a feature request, i.e I assumed the embedded components didn't try to disable the scroll on the page themselves. In that case you can reproduce this behaviour on any setup.

But if you are saying the embedded components are meant to disable the scroll on the page, i.e. I should label this as a bug instead, then I can try to make small repro for this. And then I would also like to know where in the code react-connect-js is trying to do this so I can take a look, because after inspecting the html and css of the and tags of the page I can't find any traces of react-connect-js trying to change any css when opening an overlay.

jorgea-stripe commented 1 month ago

Yes, I believe this is a bug on our side. They are meant to disable scrolling on the page, however this doesn't appear to be working as expected on the "drawer" option.

Note the actual code that does this isn't on these repo's, as these are pure loaders for connect.js.

minisemi commented 1 month ago

Ah okey. Hmm it isn't working for me without the drawer option either. We disable scroll on the tag, so maybe the repo only disables it on the tag, but I couldn't find traces of it editing the css of the tag either when the overlay opens.

jorgea-stripe commented 1 month ago

Right, the code that does that isn't on this repo. The reason I am asking for a repro is that this doesn't happen generally, for example check out the payments embedded components demo in the doc site: https://docs.stripe.com/connect/supported-embedded-components/payments - you can see there we do disable scrolling. I believe this happens in some pages that don't use the body for scrolling, which is why a repro would be helpful

image
minisemi commented 1 month ago

The page on the link you provided doesn't use the body for scrolling. It uses the div with class "Context Box-root". I can't see that it disables any scrolling using CSS though, on either the html, body or the "Context Box-root"-div, because none of their CSS is changed when opening/closing the stripe popup. So perhaps it uses javascript to disable scrolling instead.

I made a quick codesandbox here to display our use case. I don't know how to get the stripeConnectInstance since it requires backend though, but you get the idea.

https://codesandbox.io/s/laughing-bush-nf9jns?file=/src/App.js