Fix sheet losing scroll behaviour

[ryan-rushton]

Overview

The fundamental issue here is that once we set overflow-y hidden on the container, the useResizeObserver hook we use no longer detects resize events on that contents node we use in the sheet.

This fixes the issue by splitting the boundary of what gets measured by useResizeObserver and the node we are setting overflow settings on. Specifically, we measure the node one depth lower in the tree than where we are setting the overflow behaviour.

Examples

Before

Recording.2024-05-26.095208.mp4

After

Recording.2024-05-26.095258.mp4

Fixes #10346

[ryan-rushton]

I will move these to named params in an object with docs if we are happy with this approach. I didn't want to go do that refactor until we are sure of the solution, as I am still undecided how much I like it. It feels like there should be a solution which doesn't require more refs and nesting of divs but I am not sure what that is atm.

[bhollis]

Yeah I'm not sure either. The big question is whether this still solves the original problem, which is to allow for overscroll within a sheet without it scrolling the background.

[ryan-rushton]

Yeah so on my windows PC where I reproduced the issue, using the filter help, I can conform that I can scroll the entirety of the sheet and we never see the inventory screen scroll either during or when hitting the ends. When I get a chance I will test it on mac and on my iphone to ensure touch is behaving as expected.

[ryan-rushton]

Actually, what is expected behaviour with a background scrolling element? Because I see this on both prod and this branch.

Recording.2024-05-26.131031.mp4

[bhollis]

The goal is that attempting to scroll on the sheet contents should not chain to scrolling the body. This is mostly targeted at touch scrolling, but it seems like my fix doesn't even work there, now. So who knows. Maybe we should go back to body-scroll-lock libraries, if there's still a maintained one.

[ryan-rushton]

Let me have a play around to see if I can get something that works everywhere. Mobile is the most important bit I think due to how disruptive this can be to the touch experience but desktop is also a nice to have.

[ryan-rushton]

Hmmm this does in fact break mobile as i can now the background behind the filter help when there is no help results.

I did a bit of a dig around and the state of overscroll behaviour and overscroll chaining is a mess and has been for a while.

[delphiactual]

This does fix the issue I reported.

[bhollis]

If it fixes this issue without regressing the original functionality of useFixOverscrollBehavior, I'm into it.

[bhollis]

Yeah I'm not sure either. The big question is whether this still solves the original problem, which is to allow for overscroll within a sheet without it scrolling the background.

[ryan-rushton]

Ok, I will do some more testing on mac a bit later

[bhollis]

@liamdebeasi you might want to poke at this one, too. We can always go back to using a body-scroll-lock library but those had their own problems...

[ryan-rushton]

@liamdebeasi and @bhollis feel free to have at it. I have been trying to figure out how to fix this using the tools we have at the moment but I am not entirely sure it is possible. As I commented on a comment here, I was digging into the state of scroll chaining in browsers and its a nightmare. I think we might be forced to choose a least bad solution

[liamdebeasi]

@bhollis What were some of the problems with body-scroll-lock? I’m out of the office at the moment, but I’ll try to take a look at this sometime over the next week.

[bhollis]

There were two big problems - one is that it mostly works by setting overflow: hidden on the body to prevent it scrolling, and that made the page jump up and do a bunch of weird things. The other was that it had a sort of reference-counting thing going on that meant it would sometimes fail to unlock (or unlock too early). Lastly it just wasn't maintained anymore, and it wasn't clear that modern browsers needed the same fixes it applied.

[ryan-rushton]

Yeah I did a bit of digging into body-scroll-lock libraries and most of them are unmaintained these days. The most maintained one seems to be https://github.com/tuax/tua-body-scroll-lock so I would probably go with that if we go back.

[liamdebeasi]

I spent a couple days looking into this and found a potential solution. This is a slightly modified technique that I used in Ionic to better control scrolling.

In terms of what would need to change:

1. Set overflow: hidden on body when the first sheet is opened, and remove it when the last sheet is dismissed.
   This should avoid the scroll chaining on Chromium browsers similar to what the 3rd party libraries have been doing.

2. Force rubber band scrolling on iOS
   WebKit (particularly on iOS) is a bit tricky to develop a solution for since it does not respect overflow: hidden on the body. To work around this, we need to force rubber band scrolling so that swiping on a non-scrollable sheet causes the rubber band effect to kick in (similar to native applications) and thus prevents scrolling from chaining underneath to the body. Attached is a video sample:

(Note: I changed the height of the sheet so you can see that the content underneath it does not scroll)

Simulator.Screen.Recording.-.iPhone.13.-.2024-07-09.at.22.11.31.mp4

---

This approach does require some refactors of the sheet component. In addition to implementing the CSS that triggers the rubber band effect
you'd likely need to do some platform detection to make sure it only activates on iOS. Otherwise, you'll get all sheets with a minimum of 1px scrolling which may look strange. You'd also need to change some of the position styles, which could increase the scope of the work since other styles may need to change to account for it.

Additionally, the presence of the rubber band effect seems to interfere with swiping on the sheet content to close the sheet. You'd likely need to modify some of that gesture too.

---

In the end, it really depends on how much effort you want to put into building and maintaining a workaround until this issue is fixed at the browser level. This approach, while performant and dependency-free, does require a fair amount of upfront work. I will note that the rubber band effect in the sheet does seem like a nice UX improvement and makes DIM feel more like a mobile app since traditional native apps have this effect too.

Happy to discuss more!

[bhollis]

Thanks for the research! The sticking point here is really the swipe-to-dismiss behavior on sheets. In emulating native sheet components, I really want the sheet itself to drag (and eventually close) once the scrollable interior is scrolled to the top. This works perfectly in many native apps, and appears to be nearly impossible to get right using the web platform (or at least, I haven't been able to make it work consistently without other problems cropping up).

[liamdebeasi]

Makes sense. Let me do some more digging and see if there's a good way to preserve the native sheet behavior.

[liamdebeasi]

So I think one of the main issues here is that Framer's gesture util uses the Pointer Events API which can cause pointercancel to be fired when scrolling occurs.

With how it is in master, there's no rubber band scrolling on the sheet content, so the gesture works even when pulling down on content.

With my proposed change, pulling down on the sheet content causes a scroll, so pointercancel kicks in thus aborting the gesture. I'm not sure if there's a good way of allowing the gesture to go through while preventing scrolling using the Pointer Events API.

---

Ionic uses a combination of mouse events and touch events for gestures, neither of which seem to have the same cancel behavior for scrolling. One option could be to avoid pointer events and use mouse/touch events instead. https://docs-demo.ionic.io/ shows generally how the sheet would work with the gesture (Load this on an iPhone, click "Modal", then click "Open Sheet Modal". You can expand the sheet then try to swipe down to close it). What is your appetite for trying a more custom gesture implementation?

Alternatively, I can ask around with some browser vendor contacts and see if there's a recommended way of doing this with the Pointer Events API.

[bhollis]

That's interesting, and it explains why it used to work better but got worse - as Pointer Events stabilized, I believe Framer switched entirely to them (and so did I for all of our custom gesture handling). I'm open to using mouse/touch instead (with docs explaining why!), though it's sad to see that pointer events cannot be the mythical solution to all gesture handling.

[liamdebeasi]

Sounds good! I'll take a stab at getting this working as I have time.