From a8b21597221563d8c759c90277ef01066a461a2d Mon Sep 17 00:00:00 2001 From: npub1223z34hd7vtwc6qj4s7flsxkj644nlre2nthu7lrrmkumhu3xddsrx9r6w <52a228d6edf316ec6812ac3c9fc0d696ab59fc7954d77e7be31eedcddf91335b@sprout-oss.stage.blox.sqprod.co> Date: Mon, 15 Jun 2026 21:16:46 -0700 Subject: [PATCH] fix(timeline): anchor virtualizer with scrollMargin for content above the list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes two loading-state jank symptoms tho flagged on the main timeline, both the same root cause. Root cause: the scroll container holds content ABOVE the virtualized list inside the SAME scrollable element — the pagination sentinel, the load-older spinner, and the channel/DM intro banner. @tanstack/react-virtual positions items at paddingStart + scrollMargin and defaults scrollMargin to 0, so it assumed row 0 sat at scrollTop 0 when it actually painted lower by the above-content height. That offset mismatch produced: - the header/list 'sandwich' (freshly-loaded rows wedging into the seam between the intro/spinner and the list) - viewport drift while rows filled above + below (the anchor math was off by a variable above-content height) Fix: measure the list's offset within the scroll container and feed it as the virtualizer's scrollMargin (useVirtualScrollMargin — re-measures on a ResizeObserver + when the intro/spinner/list visibility changes). Rows are now positioned at virtualItem.start - scrollMargin within the spacer, which sits at that offset, so item offsets line up with where they paint regardless of what's above. The intro banner stays a sibling above the list and is only visible at the very top, as intended; native key-stable prepend retention now holds steady because the offset origin is correct. Main timeline only; thread pane untouched. Adds a DOM test locking in the start - scrollMargin positioning. Co-authored-by: Taylor Ho Signed-off-by: Taylor Ho --- .../features/messages/ui/MessageTimeline.tsx | 26 ++++++++ .../ui/VirtualizedTimelineList.dom.test.tsx | 33 ++++++++++ .../messages/ui/VirtualizedTimelineList.tsx | 10 ++- .../messages/ui/useVirtualScrollMargin.ts | 64 +++++++++++++++++++ 4 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 desktop/src/features/messages/ui/useVirtualScrollMargin.ts diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 3fc6b2d55..46967673f 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -23,6 +23,7 @@ import { } from "./timelineEntryRender"; import { useLoadOlderOnScroll } from "./useLoadOlderOnScroll"; import { useVideoReviewContextById } from "./useVideoReviewContextById"; +import { useVirtualScrollMargin } from "./useVirtualScrollMargin"; import { useVirtualTimelineScroll } from "./useVirtualTimelineScroll"; import { VirtualizedTimelineList } from "./VirtualizedTimelineList"; @@ -167,6 +168,9 @@ export const MessageTimeline = React.memo(function MessageTimeline({ const internalScrollRef = React.useRef(null); const scrollContainerRef = externalScrollRef ?? internalScrollRef; const topSentinelRef = React.useRef(null); + // Wraps the virtualized list; its offset within the scroll container is the + // virtualizer's `scrollMargin` (content above it: sentinel, spinner, intro). + const listOuterRef = React.useRef(null); // Gate the heavy timeline render (each row runs a synchronous // react-markdown parse) behind React concurrency. `useDeferredValue` lets the @@ -211,6 +215,23 @@ export const MessageTimeline = React.memo(function MessageTimeline({ ? rows.length : VIRTUAL_OVERSCAN; + // Offset of the virtualized list within the scroll container — content above + // it (sentinel, "load older" spinner, intro banner) lives in the SAME + // scrollable element, so the virtualizer must know that offset or rows paint + // at the wrong scrollTop (header/list sandwich + anchor drift on fill). + const scrollMargin = useVirtualScrollMargin( + scrollContainerRef, + listOuterRef, + [ + isLoading, + isFetchingOlder, + deferredMessages.length, + channelIntro, + directMessageIntro, + rows.length, + ], + ); + const virtualizer = useVirtualizer({ count: rows.length, getScrollElement: () => scrollContainerRef.current, @@ -224,6 +245,9 @@ export const MessageTimeline = React.memo(function MessageTimeline({ // before/after scrollHeight delta math, no double-rAF correction. getItemKey: (index) => rows[index]?.key ?? index, 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, }); const { @@ -494,11 +518,13 @@ export const MessageTimeline = React.memo(function MessageTimeline({ isRenderPending && "opacity-60 transition-opacity", )} data-render-pending={isRenderPending ? "true" : undefined} + ref={listOuterRef} > diff --git a/desktop/src/features/messages/ui/VirtualizedTimelineList.dom.test.tsx b/desktop/src/features/messages/ui/VirtualizedTimelineList.dom.test.tsx index 30e4c7a72..414c4bd80 100644 --- a/desktop/src/features/messages/ui/VirtualizedTimelineList.dom.test.tsx +++ b/desktop/src/features/messages/ui/VirtualizedTimelineList.dom.test.tsx @@ -75,6 +75,7 @@ test("renders a day divider row with its formatted label", () => { , @@ -95,6 +96,7 @@ test("dispatches message rows to their mapped entry, interleaved with dividers", , @@ -112,6 +114,7 @@ test("renders nothing for an empty row list", () => { , @@ -129,6 +132,7 @@ test("renders a message row's wrapper even if its entry is missing (no throw)", , @@ -136,3 +140,32 @@ test("renders a message row's wrapper even if its entry is missing (no throw)", assert.equal(container.querySelectorAll("[data-index]").length, 1); assert.equal(screen.queryByTestId("entry"), null); }); + +test("positions rows at start minus scrollMargin (content-above offset)", () => { + // With a scrollMargin of 128px, the first row (virtualItem.start = 0 from the + // fake virtualizer, since start includes the margin in real usage) must paint + // at translateY(start - 128). This is the fix for the header/list sandwich: + // the spacer sits at offsetTop = scrollMargin, so rows subtract it back out. + const rows: VirtualTimelineRow[] = [divider(DAY_1)]; + const fake = { + getTotalSize: () => 64, + getVirtualItems: () => [ + { index: 0, key: rows[0].key, start: 200, size: 64, end: 264, lane: 0 }, + ], + measureElement: () => {}, + } as unknown as Virtualizer; + + const { container } = render( + , + ); + const wrapper = container.querySelector('[data-index="0"]'); + assert.ok(wrapper); + // 200 - 128 = 72 + assert.match(wrapper.style.transform, /translateY\(72px\)/); +}); diff --git a/desktop/src/features/messages/ui/VirtualizedTimelineList.tsx b/desktop/src/features/messages/ui/VirtualizedTimelineList.tsx index 1e4f64936..b66a56631 100644 --- a/desktop/src/features/messages/ui/VirtualizedTimelineList.tsx +++ b/desktop/src/features/messages/ui/VirtualizedTimelineList.tsx @@ -11,6 +11,13 @@ type VirtualizedTimelineListProps = { rows: VirtualTimelineRow[]; /** Filtered main-timeline entries, indexed by `VirtualMessageRow.messageIndex`. */ entries: MainTimelineEntry[]; + /** + * The virtualizer's `scrollMargin` — the list's offset within the scroll + * container (content above it: sentinel, spinner, intro). `virtualItem.start` + * is in scroll-element coords (includes this margin), so rows are positioned + * at `start - scrollMargin` within the spacer, which sits at that offset. + */ + scrollMargin: number; /** * Renders one message entry's content. Injected (rather than imported) so the * heavy `MessageRow` subtree stays out of this component's concern and the @@ -34,6 +41,7 @@ export const VirtualizedTimelineList = React.memo( virtualizer, rows, entries, + scrollMargin, renderEntry, }: VirtualizedTimelineListProps) { const virtualItems = virtualizer.getVirtualItems(); @@ -63,7 +71,7 @@ export const VirtualizedTimelineList = React.memo( top: 0, left: 0, width: "100%", - transform: `translateY(${virtualItem.start}px)`, + transform: `translateY(${virtualItem.start - scrollMargin}px)`, }} > {row.kind === "day-divider" ? ( diff --git a/desktop/src/features/messages/ui/useVirtualScrollMargin.ts b/desktop/src/features/messages/ui/useVirtualScrollMargin.ts new file mode 100644 index 000000000..39d970af2 --- /dev/null +++ b/desktop/src/features/messages/ui/useVirtualScrollMargin.ts @@ -0,0 +1,64 @@ +import * as React from "react"; + +/** + * Measure the virtualized list's offset from the top of the scroll container's + * scrollable content, to feed `useVirtualizer({ scrollMargin })`. + * + * The main timeline's scroll container holds content ABOVE the virtualized list + * inside the SAME scrollable element: the pagination sentinel, the + * "load older" spinner, and the channel/DM intro banner. `@tanstack/react-virtual` + * positions items at `paddingStart + scrollMargin`, so without this the + * virtualizer assumes row 0 sits at scrollTop 0 — but it's actually painted + * `scrollMargin` px lower. That mismatch is what makes freshly-loaded rows + * sandwich into the header/list seam and the viewport drift while rows fill. + * + * 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. + */ +export function useVirtualScrollMargin( + scrollContainerRef: React.RefObject, + listOuterRef: React.RefObject, + // Re-measure triggers — values whose change can shift the list's offset. + deps: ReadonlyArray, +): number { + const [scrollMargin, setScrollMargin] = React.useState(0); + + React.useLayoutEffect(() => { + const container = scrollContainerRef.current; + const list = listOuterRef.current; + if (!container || !list) { + return; + } + + const measure = () => { + const c = scrollContainerRef.current; + const l = listOuterRef.current; + if (!c || !l) { + return; + } + // Offset of the list within the scroll container's scrollable content: + // distance from the container's content top to the list's top. + const next = Math.round( + l.getBoundingClientRect().top - + c.getBoundingClientRect().top + + c.scrollTop, + ); + setScrollMargin((current) => (current === next ? current : next)); + }; + + measure(); + + if (typeof ResizeObserver === "undefined") { + return; + } + // The above-content lives inside the container; observe the container so a + // height change in the sentinel/spinner/intro re-measures the margin. + const observer = new ResizeObserver(measure); + observer.observe(container); + return () => observer.disconnect(); + // deps drive intentional re-measures (intro/spinner/list visibility). + }, [scrollContainerRef, listOuterRef, ...deps]); + + return scrollMargin; +}