fix: scrollToItem width dynamic size causes infinitely loop

wellyshen · Jul 7, 2021 · 41ab2a7 · 41ab2a7
1 parent 8a9c291
commit 41ab2a7
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 3 deletions.
diff --git a/.changeset/strange-readers-care.md b/.changeset/strange-readers-care.md
@@ -0,0 +1,5 @@
+---
+"react-cool-virtual": patch
+---
+
+fix: `scrollToItem` width dynamic size causes infinitely loop
diff --git a/src/__tests__/useVirtual.tsx b/src/__tests__/useVirtual.tsx
@@ -357,7 +357,7 @@ describe("useVirtual", () => {
       expect(outer.scrollTop).toBe(425);
     });
 
-    it("should work with dynamic size correctly", () => {
+    it("should work with dynamic size correctly (item size < outer size)", () => {
       // @ts-expect-error
       window.ResizeObserver = createResizeObserver({ size: 100 });
       const { scrollToItem, outerRef } = render({ isDynamic: true });
@@ -380,6 +380,26 @@ describe("useVirtual", () => {
       expect(outer.scrollTop).toBe(600);
       expect(cb).toHaveBeenCalledTimes(1);
     });
+
+    it("should work with dynamic size correctly (item size > outer size)", () => {
+      // @ts-expect-error
+      window.ResizeObserver = createResizeObserver({ size: rect.height + 100 });
+      const { scrollToItem, outerRef } = render({ isDynamic: true });
+      const { current: outer } = outerRef;
+
+      const cb = jest.fn();
+      scrollToItem(8, cb);
+      expect(outer.scrollTop).toBe(2600);
+      expect(cb).not.toHaveBeenCalled();
+      fireEvent.scroll(outer, { target: { scrollTop: 2600 } });
+      scrollToItem(8, cb);
+      expect(outer.scrollTop).toBe(3300);
+      expect(cb).not.toHaveBeenCalled();
+      fireEvent.scroll(outer, { target: { scrollTop: 3300 } });
+      scrollToItem(8, cb);
+      expect(outer.scrollTop).toBe(3300);
+      expect(cb).toHaveBeenCalledTimes(1);
+    });
   });
 
   describe("horizontal", () => {

diff --git a/src/useVirtual.ts b/src/useVirtual.ts
@@ -261,13 +261,17 @@ export default <
 
       if (
         align === Align.start ||
-        (align === Align.auto && scrollOffset > start)
+        (align === Align.auto &&
+          scrollOffset + outerSize > end &&
+          scrollOffset > start)
       ) {
         scrollOffset =
           totalSize - start <= outerSize ? totalSize - outerSize : start;
       } else if (
         align === Align.end ||
-        (align === Align.auto && scrollOffset + outerSize < end)
+        (align === Align.auto &&
+          scrollOffset + outerSize < end &&
+          scrollOffset < start)
       ) {
         scrollOffset = start + size <= outerSize ? 0 : start - outerSize + size;
       } else if (align === Align.center && start + size / 2 > outerSize / 2) {