From d39fb399f083ce7c47ada57379b26031969d6b80 Mon Sep 17 00:00:00 2001 From: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> Date: Mon, 15 Jun 2026 22:47:48 -0700 Subject: [PATCH] fix(timeline): gate first-load bottom pin until scroll margin is measured MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Kills the first-load out-of-place flash. On `isLoading: true→false` in one commit, two sibling layout effects race: the init bottom pin (scrollToBottom → scrollToIndex align:end) and the scrollMargin re-measure (which only mounts once the list outer ref mounts). The pin can fire against a STALE `scrollMargin: 0`, landing scrollMargin px short of true bottom — rows paint at translateY(start - 0) while the list actually sits scrollMargin lower. That's the flash. The margin then lands and the settle pin re-anchors, catching up. Fix: gate the init pin on a measured margin. - useVirtualScrollMargin now returns { value, measured }. `measured` flips true only after a real measurement against a mounted list — a `> 0` test won't do, since a legitimate margin can be 0. - The init pin bails on `isLoading || !scrollMarginReady` WITHOUT marking hasInitialized, so it re-fires once the margin lands and pins against a trustworthy offset. scrollMarginReady added to the effect deps. - MessageTimeline threads scrollMargin.value into the virtualizer + list prop and scrollMargin.measured into the scroll hook. Adds a DOM regression test: 0 end-pins while the margin is unmeasured, exactly 1 once it lands. Co-authored-by: Taylor Ho Signed-off-by: Taylor Ho --- .../features/messages/ui/MessageTimeline.tsx | 9 +++- .../messages/ui/useVirtualScrollMargin.ts | 22 +++++++- .../ui/useVirtualTimelineScroll.dom.test.tsx | 54 +++++++++++++++++++ .../messages/ui/useVirtualTimelineScroll.ts | 18 ++++++- 4 files changed, 98 insertions(+), 5 deletions(-) diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index c184b54c4..935f907ef 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -247,7 +247,7 @@ export const MessageTimeline = React.memo(function MessageTimeline({ overscan, // Account for the sentinel/spinner/intro above the list inside the same // scroll container, so item offsets line up with where they actually paint. - scrollMargin, + scrollMargin: scrollMargin.value, }); const { @@ -263,6 +263,11 @@ export const MessageTimeline = React.memo(function MessageTimeline({ rows, scrollContainerRef, virtualizer, + // The init bottom pin must wait until the list's scroll margin is measured; + // pinning against the pre-mount stale `0` lands `scrollMargin` px short of + // true bottom and paints the rows out of place for a beat (the first-load + // flash) before re-anchoring. `measured` gates that first pin. + scrollMarginReady: scrollMargin.measured, targetMessageId, onTargetReached, searchActiveMessageId, @@ -540,7 +545,7 @@ export const MessageTimeline = React.memo(function MessageTimeline({ entries={entries} renderEntry={renderEntry} rows={rows} - scrollMargin={scrollMargin} + scrollMargin={scrollMargin.value} virtualizer={virtualizer} /> diff --git a/desktop/src/features/messages/ui/useVirtualScrollMargin.ts b/desktop/src/features/messages/ui/useVirtualScrollMargin.ts index 39d970af2..5250b4095 100644 --- a/desktop/src/features/messages/ui/useVirtualScrollMargin.ts +++ b/desktop/src/features/messages/ui/useVirtualScrollMargin.ts @@ -15,14 +15,29 @@ import * as React from "react"; * We re-measure whenever the above-content can change height (intro mount/ * unmount, spinner toggle) AND via a ResizeObserver on the scroll container, so * the margin stays correct as content streams in. + * + * Returns both the margin and a `measured` flag. The flag matters because a + * legitimate margin can be `0` (nothing above the list), so callers that must + * not act on a STALE pre-mount margin — e.g. the first-load bottom pin — can't + * just test `margin > 0`. `measured` flips true only after the list has mounted + * and we've taken a real measurement, so the init pin can wait for a trustworthy + * offset instead of pinning against the pre-mount `0` and flashing out of place. */ +export type VirtualScrollMargin = { + /** The list's measured offset within the scroll container (px). */ + value: number; + /** True once a real measurement has been taken (list was mounted). */ + measured: boolean; +}; + export function useVirtualScrollMargin( scrollContainerRef: React.RefObject, listOuterRef: React.RefObject, // Re-measure triggers — values whose change can shift the list's offset. deps: ReadonlyArray, -): number { +): VirtualScrollMargin { const [scrollMargin, setScrollMargin] = React.useState(0); + const [measured, setMeasured] = React.useState(false); React.useLayoutEffect(() => { const container = scrollContainerRef.current; @@ -45,6 +60,9 @@ export function useVirtualScrollMargin( c.scrollTop, ); setScrollMargin((current) => (current === next ? current : next)); + // We've taken a real measurement against a mounted list — the margin is + // now trustworthy for the init pin (even if its value is 0). + setMeasured((current) => (current ? current : true)); }; measure(); @@ -60,5 +78,5 @@ export function useVirtualScrollMargin( // deps drive intentional re-measures (intro/spinner/list visibility). }, [scrollContainerRef, listOuterRef, ...deps]); - return scrollMargin; + return { value: scrollMargin, measured }; } diff --git a/desktop/src/features/messages/ui/useVirtualTimelineScroll.dom.test.tsx b/desktop/src/features/messages/ui/useVirtualTimelineScroll.dom.test.tsx index 568facd2f..d3b596a8e 100644 --- a/desktop/src/features/messages/ui/useVirtualTimelineScroll.dom.test.tsx +++ b/desktop/src/features/messages/ui/useVirtualTimelineScroll.dom.test.tsx @@ -72,6 +72,7 @@ test("on init with no deep-link target, scrolls to the last row (sticky bottom)" rows, scrollContainerRef: makeContainerRef(true), virtualizer, + scrollMarginReady: true, }), ); @@ -96,6 +97,7 @@ test("a new latest message while pinned autoscrolls; accent uses smooth", () => rows, scrollContainerRef: makeContainerRef(true), virtualizer, + scrollMarginReady: true, }), { initialProps: { messages: initial, rows: rows1 } }, ); @@ -132,6 +134,7 @@ test("a deep-link target scrolls to that message's flat row and centers it", () rows, scrollContainerRef: makeContainerRef(false), virtualizer, + scrollMarginReady: true, targetMessageId: "b", onTargetReached, }), @@ -165,6 +168,7 @@ test("first-load settle: re-pins to bottom as total size grows while pinned", () // Pinned at bottom (default scroll metrics read as at-bottom). scrollContainerRef: makeContainerRef(true), virtualizer: harness.virtualizer, + scrollMarginReady: true, // 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), @@ -212,6 +216,7 @@ test("first-load settle: does NOT re-pin after the user scrolls away", () => { rows, scrollContainerRef: containerRef, virtualizer: harness.virtualizer, + scrollMarginReady: true, ...({ totalSizeTick } as Record), }), { initialProps: { totalSizeTick: 0 } }, @@ -239,3 +244,52 @@ test("first-load settle: does NOT re-pin after the user scrolls away", () => { "must not re-pin to bottom once the user has scrolled away", ); }); + +test("init pin waits for scrollMarginReady, then pins once the margin lands", () => { + // The first-load flash (step 5): the init bottom pin and the scrollMargin + // re-measure are sibling layout effects racing in the same `isLoading→false` + // commit. If the pin fires while the margin is still the stale pre-mount `0`, + // it lands `scrollMargin` px short of true bottom and paints rows out of + // place before re-anchoring. Gate: while `scrollMarginReady` is false, the + // init pin must NOT fire; once it flips true, it pins exactly once. + const messages = [message({ id: "a" }), message({ id: "b" })]; + const rows = buildVirtualTimelineRows(messages); + const { scrollToIndex, virtualizer } = makeVirtualizer(); + + const { rerender } = renderHook( + ({ scrollMarginReady }: { scrollMarginReady: boolean }) => + useVirtualTimelineScroll({ + channelId: "c1", + isLoading: false, + messages, + rows, + scrollContainerRef: makeContainerRef(true), + virtualizer, + scrollMarginReady, + }), + { initialProps: { scrollMarginReady: false } }, + ); + + // Margin not yet measured — the init pin must hold. + assert.equal( + scrollToIndex.mock.calls.filter((c) => c.arguments[1]?.align === "end") + .length, + 0, + "init pin must not fire before the scroll margin is measured", + ); + + // Margin lands — the init pin fires now, against a trustworthy offset. + act(() => { + rerender({ scrollMarginReady: true }); + }); + + const endPins = scrollToIndex.mock.calls.filter( + (c) => c.arguments[1]?.align === "end", + ); + assert.equal( + endPins.length, + 1, + "init pin must fire exactly once after the margin is measured", + ); + assert.equal(endPins[0]?.arguments[0], rows.length - 1); +}); diff --git a/desktop/src/features/messages/ui/useVirtualTimelineScroll.ts b/desktop/src/features/messages/ui/useVirtualTimelineScroll.ts index 5e6a3bff8..4e772d405 100644 --- a/desktop/src/features/messages/ui/useVirtualTimelineScroll.ts +++ b/desktop/src/features/messages/ui/useVirtualTimelineScroll.ts @@ -21,6 +21,13 @@ type UseVirtualTimelineScrollOptions = { rows: VirtualTimelineRow[]; scrollContainerRef: React.RefObject; virtualizer: Virtualizer; + /** + * True once the list's `scrollMargin` has been measured against a mounted + * list. The first-load bottom pin waits on this: pinning while the margin is + * still the pre-mount stale `0` lands `scrollMargin` px short of true bottom + * and paints the rows out of place for a beat before re-anchoring. + */ + scrollMarginReady: boolean; targetMessageId?: string | null; onTargetReached?: (messageId: string) => void; /** The currently active find-in-channel match, drives scroll-to-row. */ @@ -50,6 +57,7 @@ export function useVirtualTimelineScroll({ rows, scrollContainerRef, virtualizer, + scrollMarginReady, targetMessageId, onTargetReached, searchActiveMessageId, @@ -128,7 +136,14 @@ export function useVirtualTimelineScroll({ // autoscroll if pinned or accented, otherwise bump the "N new messages" pill. React.useLayoutEffect(() => { if (!hasInitializedRef.current) { - if (isLoading) { + // Wait for the first paint to settle: `isLoading` clearing means the rows + // are mounting, but the list's `scrollMargin` is measured in a SIBLING + // layout effect that races this one in the same commit. Pinning before + // the margin lands anchors against the stale pre-mount `0`, landing + // `scrollMargin` px short of true bottom — the out-of-place first-load + // flash. Hold the init pin (without marking initialized) until the margin + // is measured, so the very first pin lands against a trustworthy offset. + if (isLoading || !scrollMarginReady) { return; } if (!targetMessageId) { @@ -174,6 +189,7 @@ export function useVirtualTimelineScroll({ latestMessage, latestMessageKey, messages.length, + scrollMarginReady, scrollToBottom, targetMessageId, ]);