search

taion / scroll-behavior

Pluggable browser scroll management
MIT License
548 stars 60 forks source link

Freezing main thread on iOS Safari #130

Open coreyward opened 5 years ago

coreyward commented 5 years ago

Using Gatsby v2, this library is causing an issue with scrolling in a container on iOS Safari in a particular situation.

<div class="nav">
  <div class="item">Some content</div>
  <div class="item">Some content</div>
</div>
body {
  position: fixed;
  overflow: hidden;
 }

.nav {
  position: fixed;
  top: 0;
  left: 0;
  bottom: 0;
  right: 0;
  overflow-y: auto;
  -webkit-overflow-scroll: touch; 
}

This is a fairly basic navigation overlay with scrolling content. Assuming the child elements are long enough, this should scroll up and down while the <body> remains fixed.

It works generally, but if I'm at the top of the .nav element and try to scroll up further (swiping down, which would normally give you a peak above the document w/ the springboard animation), the page doesn't scroll at all and scrolling down (swiping up) immediately after is similarly “frozen”. After ~1s with no interaction the behavior resets and scrolling down (swiping up) works again. The same behavior occurs when at the bottom of the .nav container, only in reverse: scrolling down further (swiping up) causes the freeze.

I checked out the timeline to observe and found that this method is getting called excessively during the periods where behavior is “frozen”, but not at all otherwise:

https://github.com/taion/scroll-behavior/blob/4f6f2a57239c4a1825986304bd3d1479dbc35468/src/index.js#L146-L167

I suspect that this is somewhat expensive, and deferring the action using requestAnimationFrame is amplifying the problem, locking the main thread. I'm not sure what the intent is for this code, and as such, I'm not sure the best course of action to mitigate the issue.

taion commented 5 years ago

Why would collapsing events with rAF make the problem worse?

And you say this only happens if you have a scrollable element inside a not-otherwise-scrollable window?

coreyward commented 5 years ago

As mentioned, I'm not really familiar with scroll-behavior, but as best I can tell the above is true. Happy to provide any other data you need to dive in.

Oh, and just realized what you meant by “rAF” (requestAnimationFrame). Again, don't know what this code is doing, but performing an expensive calculation on every paint means more, not fewer, events. Is it necessary to do this? Is it possible to use a passive event listener?

coreyward commented 5 years ago

After some more testing I found that the originally described behavior does not occur if the page is not already “docked” at the top or bottom. I.e. if there's a scroll in progress it's completely possible to scroll past the top or bottom of the document with springing action per usual. It's only once the page comes to a rest and a scroll in an unexpected direction (above the top of the document or below the bottom) that the issue comes into play.

I also did some additional rough timing and it appears that the minimum duration between the last interaction and the end of the “frozen” behavior is closer to 200ms than it is the originally reported ~1s. This is still significant since it's the duration from the last interaction, so as long as a user is trying to swipe the page will be locked.

coreyward commented 5 years ago

Any thoughts on what's happening here, @taion?

taion commented 5 years ago

I don't really know, sorry. Can you explain your reasoning why requestAnimationFrame would make things worse? It seems strictly better to defer the work – and note the use of the callback collapses events, such that things run at most once per frame.

You could try to add some logging to see what's getting called.

coreyward commented 5 years ago

I explained previously, but to be clear I don't know if rAF is the primary issue, it just caught my eye. This likely has something to do with the way iOS Safari handles scrolling and scrolling past bounds, and seems specific to scrolling within overflow: [scroll | auto] containers. It may have something to do with the way you are capturing and forcibly setting scroll position, too.

I narrowed in on this specific call to _onWindowScroll being repeatedly called (not debounced or throttled) via the timeline recorder/debugger. You can observe the same behavior here on an iOS device (simulator may work as well, but desktop Safari does not behave the same):

  1. Open the nav menu (icon in the top right)
  2. Click on the search icon and search for “design”
  3. Swipe down (scroll up) while at the top of the page, then immediately try to go the other direction
  4. The page will not respond to the input from the previous step

If you record this interaction you'll see something like this:

screen shot 2018-11-27 at 10 06 27 am

The calls shown in purple occur only during the “freeze” scenario; once it alleviates, scrolling normally produces the composites shown in green in the second grouping where there are no calls to the _onWindowScroll method. A single cycle of this looks like this:

screen shot 2018-11-27 at 10 14 21 am

taion commented 5 years ago

I'm not seeing this on the Figma search on my phone.

Note also that we're not setting scroll position in the _saveWindowPosition callback. We're just saving the current position to local storage. We only update the window scroll position on something like a triggered navigation event.

Again, the point of the rAF call is exactly to defer and debounce these updates. You can test a more aggressive debouncing scheme if you'd like, but I'm not seeing the issue you're reporting.

coreyward commented 5 years ago

Hmm, just to confirm, you went into the search and tried scrolling in the results on iOS and didn't see the same behavior? I can reproduce this reliably (100% success) on multiple iOS devices (iPhone XS Max, iPhone 8+, iPad Pro 12.9"), all relatively up to date.

coreyward commented 5 years ago

I recorded a quick video to show you what I mean. It doesn't show taps, but the sequence of events is: open nav, tap into search, type “desi”, tap “Done”, try to scroll up and then repeatedly try to scroll down; pause for a moment, then scroll down and back up (past the top). Pause again and repeat the scroll up and then down repeatedly motion.

https://streamable.com/zoupu

taion commented 5 years ago

Not seeing it on my iPhone X: https://streamable.com/fb7me.

In any event, I'm happy to leave this issue open for you or others to further pursue what's happening, but I'm not going to pursue this further at the moment.

coreyward commented 5 years ago

You didn’t pause at the top before scrolling up so the event never fired.

macoughl commented 5 years ago

Hey @tiaon, I work at Figma and I'm experiencing the same issue described by @coreyward. It's happening to me on an iPhoneX and iPhone8 on Safari

slk333 commented 5 years ago

the problem is back with iOS 12.2 Beta, but maybe it will be fixed by Apple..