From 6f6d2320ed9a8795b0c3233ab66d7802e19d8f32 Mon Sep 17 00:00:00 2001 From: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> Date: Mon, 15 Jun 2026 21:55:48 -0700 Subject: [PATCH] fix(timeline): hold first-load viewport at the bottom while the document settles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause: the init pin was one-shot. On first load the virtualizer paints with ESTIMATED row heights and the deferred snapshot streams in over several commits, so the true bottom keeps moving AFTER the single init scrollToBottom. Nothing re-pinned: the new-message branch only fires on a new latest-message key (the bottom message is already present during initial fill), so the viewport ended up anchored where the estimated bottom was and the rest filled in below as heights resolved — exactly the 'doesn't stay attached to the bottom' tho saw. Fix: a settle-pin layout effect re-anchors to the bottom whenever the virtualizer's getTotalSize() changes (estimate→measured heights, streaming rows, container resize) — but ONLY while still pinned (stickToBottomRef) and not chasing a deep-link, so a user who scrolled up is never yanked back down. Guarded on a real size change (lastPinnedTotalSizeRef) so the pin's own scroll-induced re-render can't loop; scrollToBottom records the pinned size to avoid a redundant double-fire. Distinct from the mid-scroll prepend retention (a8b21597) and layered cleanly on the scrollMargin work + existing sticky-bottom autoscroll — no regression to either. Main timeline only; thread pane untouched. Two DOM tests lock in: re-pin on size growth while pinned, and NO re-pin after the user scrolls away. Co-authored-by: Taylor Ho Signed-off-by: Taylor Ho --- .../ui/useVirtualTimelineScroll.dom.test.tsx | 110 +++++++++++++++++- .../messages/ui/useVirtualTimelineScroll.ts | 35 ++++++ 2 files changed, 139 insertions(+), 6 deletions(-) diff --git a/desktop/src/features/messages/ui/useVirtualTimelineScroll.dom.test.tsx b/desktop/src/features/messages/ui/useVirtualTimelineScroll.dom.test.tsx index 63a7a6c07..568facd2f 100644 --- a/desktop/src/features/messages/ui/useVirtualTimelineScroll.dom.test.tsx +++ b/desktop/src/features/messages/ui/useVirtualTimelineScroll.dom.test.tsx @@ -28,18 +28,23 @@ function message( } // Records scrollToIndex calls so we can assert the virtualizer is driven -// correctly. The hook never reads layout off the virtualizer directly. -function makeVirtualizer() { +// correctly. `getTotalSize` is controllable so we can simulate the document +// growing during first-load settle (estimate→measured heights, streaming rows). +function makeVirtualizer(initialTotalSize = 100) { + let totalSize = initialTotalSize; const scrollToIndex = mock.fn< (index: number, opts?: { align?: string; behavior?: string }) => void >(); return { scrollToIndex, - virtualizer: { scrollToIndex } as unknown as Virtualizer< - HTMLDivElement, - Element - >, + setTotalSize(next: number) { + totalSize = next; + }, + virtualizer: { + scrollToIndex, + getTotalSize: () => totalSize, + } as unknown as Virtualizer, }; } @@ -141,3 +146,96 @@ test("a deep-link target scrolls to that message's flat row and centers it", () assert.equal(onTargetReached.mock.calls.length, 1); assert.equal(onTargetReached.mock.calls[0].arguments[0], "b"); }); + +test("first-load settle: re-pins to bottom as total size grows while pinned", () => { + // First load: rows paint with estimated heights, then measured heights / + // streaming rows grow getTotalSize(). While pinned, each growth re-anchors to + // the bottom so the viewport holds at the newest message. + const messages = [message({ id: "a" }), message({ id: "b" })]; + const rows = buildVirtualTimelineRows(messages); + const harness = makeVirtualizer(100); + + const { rerender } = renderHook( + ({ totalSizeTick }: { totalSizeTick: number }) => + useVirtualTimelineScroll({ + channelId: "c1", + isLoading: false, + messages, + rows, + // Pinned at bottom (default scroll metrics read as at-bottom). + scrollContainerRef: makeContainerRef(true), + virtualizer: harness.virtualizer, + // totalSizeTick is unused by the hook — it just forces a re-render after + // we bump the fake's total size, mirroring react-virtual's onChange. + ...({ totalSizeTick } as Record), + }), + { initialProps: { totalSizeTick: 0 } }, + ); + + const endPinsBefore = harness.scrollToIndex.mock.calls.filter( + (c) => c.arguments[1]?.align === "end", + ).length; + + // Document grows (measured heights settle / more rows stream in). + act(() => { + harness.setTotalSize(900); + rerender({ totalSizeTick: 1 }); + }); + + const endPinsAfter = harness.scrollToIndex.mock.calls.filter( + (c) => c.arguments[1]?.align === "end", + ).length; + assert.ok( + endPinsAfter > endPinsBefore, + "expected a re-pin to bottom when total size grew while pinned", + ); + const lastEnd = harness.scrollToIndex.mock.calls + .filter((c) => c.arguments[1]?.align === "end") + .at(-1); + assert.equal(lastEnd?.arguments[0], rows.length - 1); +}); + +test("first-load settle: does NOT re-pin after the user scrolls away", () => { + const messages = [message({ id: "a" }), message({ id: "b" })]; + const rows = buildVirtualTimelineRows(messages); + const harness = makeVirtualizer(100); + // Container reads as NOT at bottom — the initial syncScrollState/scroll state + // leaves stickToBottom false, so the settle effect must not yank back down. + const containerRef = makeContainerRef(false); + + const { result, rerender } = renderHook( + ({ totalSizeTick }: { totalSizeTick: number }) => + useVirtualTimelineScroll({ + channelId: "c1", + isLoading: false, + messages, + rows, + scrollContainerRef: containerRef, + virtualizer: harness.virtualizer, + ...({ totalSizeTick } as Record), + }), + { initialProps: { totalSizeTick: 0 } }, + ); + + // User has scrolled up — reflect that through syncScrollState. + act(() => { + result.current.syncScrollState(); + }); + const endPinsBefore = harness.scrollToIndex.mock.calls.filter( + (c) => c.arguments[1]?.align === "end", + ).length; + + act(() => { + harness.setTotalSize(900); + rerender({ totalSizeTick: 1 }); + }); + + const endPinsAfter = harness.scrollToIndex.mock.calls.filter( + (c) => c.arguments[1]?.align === "end", + ).length; + assert.equal( + endPinsAfter, + endPinsBefore, + "must not re-pin to bottom once the user has scrolled away", + ); +}); diff --git a/desktop/src/features/messages/ui/useVirtualTimelineScroll.ts b/desktop/src/features/messages/ui/useVirtualTimelineScroll.ts index b9341316a..5e6a3bff8 100644 --- a/desktop/src/features/messages/ui/useVirtualTimelineScroll.ts +++ b/desktop/src/features/messages/ui/useVirtualTimelineScroll.ts @@ -60,6 +60,10 @@ export function useVirtualTimelineScroll({ const previousMessageCountRef = React.useRef(0); const handledTargetMessageIdRef = React.useRef(null); const handledSearchActiveIdRef = React.useRef(null); + // Total virtual size at the last time we pinned to bottom — lets the + // settle-pin effect below re-anchor only when the size actually changed, + // instead of looping on its own scroll-induced re-renders. + const lastPinnedTotalSizeRef = React.useRef(-1); const [isAtBottom, setIsAtBottom] = React.useState(true); const [newMessageCount, setNewMessageCount] = React.useState(0); @@ -78,6 +82,9 @@ export function useVirtualTimelineScroll({ setNewMessageCount(0); setIsAtBottom(true); virtualizer.scrollToIndex(lastRowIndex, { align: "end", behavior }); + // Mark the current size as pinned so the settle effect doesn't redundantly + // re-fire for this same commit. + lastPinnedTotalSizeRef.current = virtualizer.getTotalSize(); }, [lastRowIndex, virtualizer], ); @@ -91,6 +98,7 @@ export function useVirtualTimelineScroll({ previousMessageCountRef.current = 0; handledTargetMessageIdRef.current = null; handledSearchActiveIdRef.current = null; + lastPinnedTotalSizeRef.current = -1; setIsAtBottom(true); setNewMessageCount(0); setHighlightedMessageId(null); @@ -170,6 +178,33 @@ export function useVirtualTimelineScroll({ targetMessageId, ]); + // Keep pinned to the bottom while the document settles. On first load the + // virtualizer paints with ESTIMATED row heights and the deferred snapshot + // streams in over several commits, so the true bottom keeps moving after the + // one-shot init pin above. As `getTotalSize()` grows (estimate→measured + // heights, more rows, container resize), re-anchor to the bottom — but ONLY + // while still pinned and not chasing a deep-link, so a user who scrolled up is + // never yanked back down. Guarded on a real size change so the pin's own + // scroll-induced re-render can't loop. This is what makes first-load "land and + // hold at the bottom" instead of anchoring up top as content fills in. + const totalSize = virtualizer.getTotalSize(); + React.useLayoutEffect(() => { + if ( + isLoading || + targetMessageId || + !hasInitializedRef.current || + !stickToBottomRef.current || + lastRowIndex < 0 + ) { + return; + } + if (totalSize === lastPinnedTotalSizeRef.current) { + return; + } + lastPinnedTotalSizeRef.current = totalSize; + virtualizer.scrollToIndex(lastRowIndex, { align: "end" }); + }, [totalSize, isLoading, targetMessageId, lastRowIndex, virtualizer]); + // Deep-link jump-to-message. Drives the virtualizer to mount and center the // target row, replacing the bespoke querySelector + scrollIntoView path that // breaks under virtualization (the row may be unmounted).