From 70b0eb2c8b9b7bff72f17cc307c4cf88ead6102a Mon Sep 17 00:00:00 2001 From: Phil Shao Date: Thu, 28 May 2026 18:58:21 -0400 Subject: [PATCH 1/2] fix: defer sentinel check until masonry container has a measured width MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When hasMore=true, the scroll sentinel effect called check() immediately on mount. At that point masonryWidth=0 (the ResizeObserver hasn't fired yet) so the sentinel sits at top≈0, check() returns true, and onLoadMore fires before any cards are visible. This triggers an immediate second-page fetch that produces the pagination flash. Fix: move the masonryRef/columnWidth/useContainerPosition declarations above the sentinel effect so masonryWidth is in scope, then add masonryWidth to the effect deps and return early when it is 0. The sentinel first runs once the masonry container has a real width, at which point the sentinel is at its true position and check() correctly reflects whether the user needs more content. The positioner useMemo and all other masonry logic are unchanged. Co-Authored-By: Claude Sonnet 4.6 --- web/src/components/PieceList.tsx | 31 ++++++++++++------- .../components/__tests__/PieceList.test.tsx | 28 +++++++++++++++++ 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/web/src/components/PieceList.tsx b/web/src/components/PieceList.tsx index f9f55167..8e0c058d 100644 --- a/web/src/components/PieceList.tsx +++ b/web/src/components/PieceList.tsx @@ -442,8 +442,26 @@ const PieceList = (props: PieceListProps) => { }, [onLoadMore]); const sentinelRef = useRef(null); + // Declare masonry geometry here so masonryWidth is in scope for the sentinel + // effect below. The positioner useMemo further down still consumes these same + // values — nothing else in the component changes. + const windowHeight = useWindowHeight(); + const masonryRef = useRef(null); + const columnWidth = isMobile + ? MASONRY_COLUMN_WIDTH_MOBILE + : MASONRY_COLUMN_WIDTH_DESKTOP; + const { width: masonryWidth, offset: masonryOffset } = useContainerPosition( + masonryRef, + [isMobile], + ); + useEffect(() => { - if (!hasMore) return; + // When masonryWidth is 0 the ResizeObserver hasn't fired yet: the masonry + // grid hasn't rendered so the sentinel sits at top≈0. Calling check() at + // that point fires onLoadMore before any cards are visible, triggering an + // immediate second-page fetch and the resulting flash. Defer until the + // container has a real width so the sentinel is at its true position. + if (!hasMore || masonryWidth === 0) return; function check() { const sentinel = sentinelRef.current; if (!sentinel) return; @@ -453,7 +471,7 @@ const PieceList = (props: PieceListProps) => { window.addEventListener("scroll", check, { passive: true }); check(); return () => window.removeEventListener("scroll", check); - }, [hasMore]); + }, [hasMore, masonryWidth]); const availableTags = useMemo(() => { const deduped = new Map(); @@ -505,15 +523,6 @@ const PieceList = (props: PieceListProps) => { }, [activeFilters, activeTagIds, activeTags]); const hasActiveFilters = activeFilters.length > 0 || activeTagIds.length > 0; - const windowHeight = useWindowHeight(); - const masonryRef = useRef(null); - const columnWidth = isMobile - ? MASONRY_COLUMN_WIDTH_MOBILE - : MASONRY_COLUMN_WIDTH_DESKTOP; - const { width: masonryWidth, offset: masonryOffset } = useContainerPosition( - masonryRef, - [isMobile], - ); const positioner = useMemo(() => { const [computedColumnWidth, computedColumnCount] = getMasonryColumns( masonryWidth, diff --git a/web/src/components/__tests__/PieceList.test.tsx b/web/src/components/__tests__/PieceList.test.tsx index eb85ffa3..dd19e333 100644 --- a/web/src/components/__tests__/PieceList.test.tsx +++ b/web/src/components/__tests__/PieceList.test.tsx @@ -1166,6 +1166,34 @@ describe("PieceList", () => { }); }); + describe("scroll sentinel", () => { + it("does not call onLoadMore before the masonry container has a measured width", () => { + // Regression for #734 Bug 1: when hasMore=true, check() fires immediately on + // mount. At that moment masonryWidth=0 (ResizeObserver not yet fired), so the + // sentinel sits at top≈0 and onLoadMore fires before any cards are visible. + // The fix adds masonryWidth to the effect deps and returns early when it is 0. + mockContainerPosition.width = 0; + const onLoadMore = vi.fn(); + const router = createMemoryRouter( + [ + { + path: "/", + element: ( + + ), + }, + ], + { initialEntries: ["/"] }, + ); + render(); + expect(onLoadMore).not.toHaveBeenCalled(); + }); + }); + describe("loading states", () => { it("dims the existing list during replace-style refreshes", () => { const router = createMemoryRouter( From 1032165172dfd80dd9e286bae2f8d6376eaa9a6a Mon Sep 17 00:00:00 2001 From: Phil Shao Date: Thu, 28 May 2026 19:19:23 -0400 Subject: [PATCH 2/2] test: add regression test for sentinel premature pagination (#734) Verifies that onLoadMore is not called while masonryWidth=0 (the ResizeObserver hasn't fired yet). Uses a real setTimeout to give React's MessageChannel-based scheduler time to fire mount effects before asserting. Fails on the pre-fix code, passes with the guard. Co-Authored-By: Claude Sonnet 4.6 --- .../components/__tests__/PieceList.test.tsx | 64 +++++++++++-------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/web/src/components/__tests__/PieceList.test.tsx b/web/src/components/__tests__/PieceList.test.tsx index dd19e333..8f30bf28 100644 --- a/web/src/components/__tests__/PieceList.test.tsx +++ b/web/src/components/__tests__/PieceList.test.tsx @@ -1166,34 +1166,6 @@ describe("PieceList", () => { }); }); - describe("scroll sentinel", () => { - it("does not call onLoadMore before the masonry container has a measured width", () => { - // Regression for #734 Bug 1: when hasMore=true, check() fires immediately on - // mount. At that moment masonryWidth=0 (ResizeObserver not yet fired), so the - // sentinel sits at top≈0 and onLoadMore fires before any cards are visible. - // The fix adds masonryWidth to the effect deps and returns early when it is 0. - mockContainerPosition.width = 0; - const onLoadMore = vi.fn(); - const router = createMemoryRouter( - [ - { - path: "/", - element: ( - - ), - }, - ], - { initialEntries: ["/"] }, - ); - render(); - expect(onLoadMore).not.toHaveBeenCalled(); - }); - }); - describe("loading states", () => { it("dims the existing list during replace-style refreshes", () => { const router = createMemoryRouter( @@ -1237,4 +1209,40 @@ describe("PieceList", () => { ).toContain("background-color: transparent"); }); }); + + describe("scroll sentinel", () => { + it("does not call onLoadMore while the masonry container width is unmeasured", async () => { + // Regression for #734: with the pre-fix code, the sentinel effect fires + // check() immediately on mount regardless of masonryWidth. At that moment + // masonryWidth=0 (ResizeObserver hasn't fired), the sentinel element sits + // at top≈0 in the unmeasured document, and check() calls onLoadMore before + // any cards are visible — fetching page 2 prematurely and causing the flash. + // + // The fix adds masonryWidth to the effect's deps and guards with + // `if (masonryWidth === 0) return`, so the effect is a no-op until the + // container has been measured. + // + // We let the event loop run (setTimeout) so React's MessageChannel-based + // scheduler has time to fire the mount effect before we assert. + mockContainerPosition.width = 0; + const onLoadMore = vi.fn(); + const firstPage = Array.from({ length: 16 }, (_, i) => + makePiece({ id: `piece-${i}` }), + ); + const router = createMemoryRouter( + [ + { + path: "/", + element: ( + + ), + }, + ], + { initialEntries: ["/"] }, + ); + render(); + await new Promise((resolve) => setTimeout(resolve, 50)); + expect(onLoadMore).not.toHaveBeenCalled(); + }); + }); });