[BUG] scroll up glitch when setting initialTopMostItemIndex in Some Mobile Browsers and Webviews

[coopermaruyama]

Describe the bug
I have implemented basic scroll restoration in a virtuoso list by setting initialTopMostItemIndex which seems to work well EXCEPT in Mobile Chrome and iOS Safari Webviews.

The behavior is such that scrolling up and down is smooth, unless initialTopMostItemIndex is set. The behavior is such that, let's say you set the top most item index to 50 -- scrolling down works fine, but scrolling up will only let you scroll about an item at a time before the scroll gets interrupted by an unknown bug.

Reproduction
I have created a reproduction in codesandbox as well as a snack app which reproduces the behavior.

Here is the sandbox: https://codesandbox.io/s/recursing-ace-j2zvxt?file=/src/App.tsx

I added a button that hides the list so that you can simulate un-mounting the list and mounting it by toggling visibility.

The scroll position at time of unmount is stored in parent component's state and passed down to restore the scroll.

This issue is reproducible on mobile chrome with just the sandbox.

For the webview reproduction I also created a expo snack with a reproduction here: https://snack.expo.dev/@coincircle/webview-example

I am comfortable with making a PR myself and doing whatever testing/debugging is needed on my end to solve the problem, but I figured I'd bring this up as maybe you might have an idea off the top of your head regarding as to what can be the cause here. It's quite a strange issue given that webviews are just embedded safari windows, yet this issue is not reproducible in the safari browser, only in a webview (and mobile chrome).

To Reproduce
Steps to reproduce the behavior (iOS Webview):

1. Go to Reproduction
2. I recommend installign the Expo Go app from the app store and scanning the QR code to load the reproduction on your iOS device.
3. scroll down to some index in the list
4. click "Toggle List Visibility" to hide the list
5. Click again to show it and be restored toy our original position
6. Scroll up
7. Observe the glitchiness of the scroll

Steps to reproduce the behavior (iOS Chrome Browser):

1. Go to Sandbox
2. scroll down to some index in the list
3. click "Toggle List Visibility" to hide the list
4. Click again to show it and be restored toy our original position
5. Scroll up
6. Observe the glitchiness of the scroll

Expected behavior
It should work as it does in chrome desktop etc

[coopermaruyama]

Followup: I have found that you don't need to even set initialTopMostItemIndex at all to reproduce this. Simply calling listRef.current.scrollToIndex({ index: 50 }) and then trying to scroll up will reproduce the issue as well.

[petyosi]

I will see if this is fixable, webviews on iOS are weird. In #716 another user mentioned that it works as expected in 2.9.2 - if you have the capacity, let me know if there's any change in behavior in that version.

[coopermaruyama]

Thanks for the reply -- I've tested it and indeed, in 2.9.2 does not have this issue

[coopermaruyama]

@petyosi This might help -- I have installed the releases incrementally to identify at what release this started occurring.

The issue started occurring in 2.13.3 and does not occur in 2.13.2, meaning it is somewhere in this diff: v2.13.2...v2.13.3

[coopermaruyama]

I might be wrong about the above. it might actually be v2.13.2 that introduces it. To dig deeper I've cloned the repo and am running the initial-topmost-item examples via npm run browse-examples and in this case I am finding it start occurring in this change: v2.13.1...v2.13.2

So either I was having caching issues or tags don't match what's published on npm, I'm not sure.

I'm going to try individually reverting changed files in this diff and see if I can narrow down more.

[coopermaruyama]

Okay, it does seem to be related to v2.13.2 -- specifically I have narrowed it down to this diff to src/upwardScrollFixSystem.ts

diff --git a/src/upwardScrollFixSystem.ts b/src/upwardScrollFixSystem.ts
index 35dc588c..0b8424ef 100644
--- a/src/upwardScrollFixSystem.ts
+++ b/src/upwardScrollFixSystem.ts
@@ -5,6 +5,12 @@ import { sizeSystem } from './sizeSystem'
 import { UP, stateFlagsSystem } from './stateFlagsSystem'
 import { ListItem } from './interfaces'
 import { loggerSystem, LogLevel } from './loggerSystem'
+import { simpleMemoize } from './utils/simpleMemoize'
+import { recalcSystem } from './recalcSystem'
+
+const isMobileSafari = simpleMemoize(() => {
+  return /iP(ad|hone|od).+Version\/[\d.]+.*Safari/i.test(navigator.userAgent)
+})
 
 type UpwardFixState = [number, ListItem<any>[], number, number]
 /**
@@ -13,10 +19,11 @@ type UpwardFixState = [number, ListItem<any>[], number, number]
 export const upwardScrollFixSystem = u.system(
   ([
     { scrollBy, scrollTop, deviation, scrollingInProgress },
-    { isScrolling, isAtBottom, atBottomState, scrollDirection, lastJumpDueToItemResize },
+    { isScrolling, isAtBottom, scrollDirection, lastJumpDueToItemResize },
     { listState },
-    { beforeUnshiftWith, shiftWithOffset, sizes, recalcInProgress },
+    { beforeUnshiftWith, shiftWithOffset, sizes },
     { log },
+    { recalcInProgress },
   ]) => {
     const deviationOffset = u.streamFromEmitter(
       u.pipe(
@@ -44,48 +51,45 @@ export const upwardScrollFixSystem = u.system(
           [0, [], 0, 0] as UpwardFixState
         ),
         u.filter(([amount]) => amount !== 0),
-        u.withLatestFrom(scrollTop, scrollDirection, scrollingInProgress, log, isAtBottom, atBottomState),
+        u.withLatestFrom(scrollTop, scrollDirection, scrollingInProgress, isAtBottom, log),
         u.filter(([, scrollTop, scrollDirection, scrollingInProgress]) => {
-          // console.log({ amount, scrollTop, scrollDirection, scrollingInProgress, isAtBottom, atBottomState })
-          return !scrollingInProgress && scrollTop !== 0 && scrollDirection === UP // && (isAtBottom ? amount > 0 : true)
+          return !scrollingInProgress && scrollTop !== 0 && scrollDirection === UP
         }),
-        u.map(([[amount], , , , log]) => {
+        u.map(([[amount], , , , , log]) => {
           log('Upward scrolling compensation', { amount }, LogLevel.DEBUG)
           return amount
         })
       )
     )
 
-    u.connect(
-      u.pipe(
-        deviationOffset,
-        u.withLatestFrom(deviation),
-        u.map(([amount, deviation]) => deviation - amount)
-      ),
-      deviation
-    )
+    function scrollByWith(offset: number) {
+      if (offset > 0) {
+        u.publish(scrollBy, { top: -offset, behavior: 'auto' })
+        u.publish(deviation, 0)
+      } else {
+        u.publish(deviation, 0)
+        u.publish(scrollBy, { top: -offset, behavior: 'auto' })
+      }
+    }
+
+    u.subscribe(u.pipe(deviationOffset, u.withLatestFrom(deviation, isScrolling)), ([offset, deviationAmount, isScrolling]) => {
+      if (isScrolling && isMobileSafari()) {
+        u.publish(deviation, deviationAmount - offset)
+      } else {
+        scrollByWith(-offset)
+      }
+    })
 
-    // when the browser stops scrolling,
-    // restore the position and reset the glitching
+    // this hack is only necessary for mobile safari which does not support scrollBy while scrolling is in progress.
+    // when the browser stops scrolling, restore the position and reset the glitching
     u.subscribe(
       u.pipe(
-        u.combineLatest(u.statefulStreamFromEmitter(isScrolling, false), deviation),
-        u.filter(([is, deviation]) => !is && deviation !== 0),
-        u.map(([_, deviation]) => deviation)
-        // u.throttleTime(1)
+        u.combineLatest(u.statefulStreamFromEmitter(isScrolling, false), deviation, recalcInProgress),
+        u.filter(([is, deviation, recalc]) => !is && !recalc && deviation !== 0),
+        u.map(([_, deviation]) => deviation),
+        u.throttleTime(1)
       ),
-      (offset) => {
-        if (offset > 0) {
-          u.publish(scrollBy, { top: -offset, behavior: 'auto' })
-          u.publish(deviation, 0)
-        } else {
-          u.publish(deviation, 0)
-          u.handleNext(scrollTop, () => {
-            u.publish(recalcInProgress, false)
-          })
-          u.publish(scrollBy, { top: -offset, behavior: 'auto' })
-        }
-      }
+      scrollByWith
     )
 
     u.connect(
@@ -98,16 +102,25 @@ export const upwardScrollFixSystem = u.system(
       scrollBy
     )
 
-    u.connect(
+    u.subscribe(
       u.pipe(
         beforeUnshiftWith,
         u.withLatestFrom(sizes),
         u.map(([offset, { lastSize }]) => offset * lastSize)
       ),
-      deviationOffset
+      (offset) => {
+        u.publish(deviation, offset)
+        requestAnimationFrame(() => {
+          u.publish(scrollBy, { top: offset })
+          requestAnimationFrame(() => {
+            u.publish(deviation, 0)
+            u.publish(recalcInProgress, false)
+          })
+        })
+      }
     )
 
     return { deviation }
   },
-  u.tup(domIOSystem, stateFlagsSystem, listStateSystem, sizeSystem, loggerSystem)
+  u.tup(domIOSystem, stateFlagsSystem, listStateSystem, sizeSystem, loggerSystem, recalcSystem)
 )

[coopermaruyama]

I've discovered the cause and have made a PR for a fix, it was surprisingly simple. The isMobileSafari() check returns false in a webview, and also returns false in iOS chrome. However iOS chrome uses Webkit so it needs isMobileSafari() to be true in order to make it not be glitchy.

I've run the tests to make sure my change does not cause any regressions.

[github-actions]

🎉 This issue has been resolved in version 3.1.4 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀