Merge pull request #361 from inokawa/shift-cancel

Cancel scheduled imperative scrolling when prepended
inokawa · Feb 4, 2024 · fd7ae5d · fd7ae5d
2 parents c704365 + b621926
commit fd7ae5d
Show file tree

Hide file tree

Showing 5 changed files with 85 additions and 40 deletions.
diff --git a/e2e/VList.spec.ts b/e2e/VList.spec.ts
@@ -18,8 +18,32 @@ import {
   scrollToLeft,
   getVirtualizer,
   getScrollable,
+  clearTimer,
 } from "./utils";
 
+const listenScrollCount = (
+  component: ElementHandle<SVGElement | HTMLElement>
+): Promise<number> => {
+  return component.evaluate((c) => {
+    let timer: null | ReturnType<typeof setTimeout> = null;
+    let called = 0;
+
+    return new Promise<number>((resolve) => {
+      const cb = () => {
+        called++;
+        if (timer !== null) {
+          clearTimeout(timer);
+        }
+        timer = setTimeout(() => {
+          c.removeEventListener("scroll", cb);
+          resolve(called);
+        }, 2000);
+      };
+      c.addEventListener("scroll", cb);
+    });
+  });
+};
+
 test.describe("smoke", () => {
   test("vertically scrollable", async ({ page }) => {
     await page.goto(storyUrl("basics-vlist--default"));
@@ -280,12 +304,7 @@ test.describe("check if scroll jump compensation works", () => {
     const initialItem = await getLastItem(component);
     expectInRange(initialItem.bottom, { min: 0, max: 1 });
 
-    await page.evaluate(() => {
-      // stop all timer
-      for (let i = 1; i < 65536; i++) {
-        clearTimeout(i);
-      }
-    });
+    await clearTimer(page);
 
     const button = (await page
       .getByRole("button", { name: "submit" })
@@ -381,29 +400,6 @@ test.describe("check if scrollToIndex works", () => {
     await page.goto(storyUrl("basics-vlist--scroll-to"));
   });
 
-  const listenScrollCount = (
-    component: ElementHandle<SVGElement | HTMLElement>
-  ): Promise<number> => {
-    return component.evaluate((c) => {
-      let timer: null | ReturnType<typeof setTimeout> = null;
-      let called = 0;
-
-      return new Promise<number>((resolve) => {
-        const cb = () => {
-          called++;
-          if (timer !== null) {
-            clearTimeout(timer);
-          }
-          timer = setTimeout(() => {
-            c.removeEventListener("scroll", cb);
-            resolve(called);
-          }, 2000);
-        };
-        c.addEventListener("scroll", cb);
-      });
-    });
-  };
-
   test.describe("align start", () => {
     test("mid", async ({ page }) => {
       const component = await getScrollable(page);
@@ -997,6 +993,30 @@ test.describe("check if item shift compensation works", () => {
 
     expect(i).toBeGreaterThanOrEqual(8);
   });
+
+  test("check if prepending cancels imperative scroll", async ({ page }) => {
+    await page.goto(storyUrl("advanced-chat--default"));
+    const component = await getScrollable(page);
+    await component.waitForElementState("stable");
+    // check if end is displayed
+    const initialItem = await getLastItem(component);
+    expectInRange(initialItem.bottom, { min: 0, max: 1 });
+
+    await clearTimer(page);
+
+    const scrollListener = listenScrollCount(component);
+
+    const button = (await page
+      .getByRole("button", { name: "jump to top" })
+      .elementHandle())!;
+
+    // scroll to top
+    await button.click();
+
+    // check if imperative scrolling doesn't cause infinite loop
+    const scrollCount = await scrollListener;
+    expect(scrollCount).toBeLessThanOrEqual(3);
+  });
 });
 
 test.describe("RTL", () => {

diff --git a/e2e/utils.ts b/e2e/utils.ts
@@ -29,6 +29,15 @@ export const approxymate = (v: number): number => Math.round(v / 100) * 100;
 export const clearInput = (input: ElementHandle<HTMLElement | SVGElement>) =>
   input.evaluate((element) => ((element as HTMLInputElement).value = ""));
 
+export const clearTimer = async (page: Page) => {
+  await page.evaluate(() => {
+    // stop all timer
+    for (let i = 1; i < 65536; i++) {
+      clearTimeout(i);
+    }
+  });
+};
+
 export const getFirstItem = (
   scrollable: ElementHandle<HTMLElement | SVGElement>
 ) => {

diff --git a/src/core/scroller.ts b/src/core/scroller.ts
@@ -125,10 +125,11 @@ const createScrollObserver = (
       onScrollEnd._cancel();
     },
     _fixScrollJump: () => {
-      const jump = store._flushJump();
+      const [jump, prepend] = store._flushJump();
       if (!jump) return;
       updateScrollOffset(jump, stillMomentumScrolling);
       stillMomentumScrolling = false;
+      return prepend;
     },
   };
 };
@@ -274,10 +275,7 @@ export const createScroller = (
       scrollObserver && scrollObserver._dispose();
     },
     _scrollTo(offset) {
-      if (viewportElement) {
-        // https://github.com/inokawa/virtua/issues/357
-        viewportElement[scrollToKey] = normalizeOffset(offset, isHorizontal);
-      }
+      scheduleImperativeScroll(() => offset);
     },
     _scrollBy(offset) {
       offset += store._getScrollOffset();
@@ -318,7 +316,12 @@ export const createScroller = (
       }, smooth);
     },
     _fixScrollJump: () => {
-      scrollObserver && scrollObserver._fixScrollJump();
+      if (scrollObserver) {
+        if (scrollObserver._fixScrollJump()) {
+          // https://github.com/inokawa/virtua/issues/357
+          cancelScroll && cancelScroll();
+        }
+      }
     },
   };
 };

diff --git a/src/core/store.ts b/src/core/store.ts
@@ -163,7 +163,7 @@ export type VirtualStore = {
   _getEndSpacerSize(): number;
   _getTotalSize(): number;
   _getJumpCount(): number;
-  _flushJump(): number;
+  _flushJump(): [number, boolean];
   _subscribe(target: number, cb: Subscriber): () => void;
   _update(...action: Actions): void;
 };
@@ -187,6 +187,7 @@ export const createVirtualStore = (
   let jumpCount = 0;
   let jump = 0;
   let pendingJump = 0;
+  let isJumpByPrepend = false;
   let _flushedJump = 0;
   let _scrollDirection: ScrollDirection = SCROLL_IDLE;
   let _scrollMode: ScrollMode = SCROLL_BY_NATIVE;
@@ -282,14 +283,17 @@ export const createVirtualStore = (
       return jumpCount;
     },
     _flushJump() {
+      const flushedJump = jump;
+      const flushedIsJumpByPrepend = isJumpByPrepend;
+      jump = 0;
+      isJumpByPrepend = false;
       if (viewportSize > getScrollableSize()) {
         // In this case applying jump will not cause scroll.
         // Current logic expects scroll event occurs after applying jump so discard it.
-        return (jump = 0);
+        return [0, false];
+      } else {
+        return [(_flushedJump = flushedJump), flushedIsJumpByPrepend];
       }
-      _flushedJump = jump;
-      jump = 0;
-      return _flushedJump;
     },
     _subscribe(target, cb) {
       const sub: [number, Subscriber] = [target, cb];
@@ -384,6 +388,7 @@ export const createVirtualStore = (
 
             if (isAdd) {
               _scrollMode = SCROLL_BY_PREPENDING;
+              isJumpByPrepend = true;
             }
             mutated = UPDATE_SCROLL_STATE;
           } else {

diff --git a/stories/react/advanced/Chat.stories.tsx b/stories/react/advanced/Chat.stories.tsx
@@ -167,6 +167,14 @@ export const Default: StoryObj = {
           <button type="submit" disabled={disabled}>
             submit
           </button>
+          <button
+            type="button"
+            onClick={() => {
+              ref.current?.scrollTo(0);
+            }}
+          >
+            jump to top
+          </button>
         </form>
       </div>
     );