From 4b551940ec1c19909ec547b7d897f2e00228f4d0 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 17 Jun 2026 23:05:19 -0400 Subject: [PATCH 1/7] perf(desktop): virtualize message timeline to stop the cold-switch beachball MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Channel switch streamed up to 200 uncontained MessageRows (each with synchronous shiki markdown), then scrollToBottom("auto") forced a full-document scrollHeight read-then-write reflow before paint over every row — the macOS beachball Will reported on v0.3.25. Windows the main timeline on @tanstack/react-virtual. The day-grouped section tree is flattened to a typed TimelineItem[] stream plus a messageId->itemIndex map from one walk (cannot drift), and every DOM-querySelector scroll path (deep-link, search-jump, jump-to-unread, scrollToBottom, load-older anchor) is re-pathed onto the index model so windowing does not silently break jumps to off-screen rows. Scroll convergence is split: @tanstack/react-virtual owns offset convergence (its rAF loop re-aims getOffsetForIndex as rows mount and measure); a pure reducer owns only staleness re-resolution and termination — re-resolving the target's index by id each frame so a concurrent prepend/delete cannot strand the loop on a stale index, and terminating when the target is deleted or a 32-frame cap is hit. The breaking math lives in lib/ under the .mjs suite. The thread reply list stays content-visibility:auto rather than virtualized — it is bounded, unpaginated, ungrouped, and shares the scroll hook, so virtualizing it would force a second index re-path and a head/prologue split for no beachball gain. Phase-2 route-chunk preload warms the agents/channel/lazy-view chunks on idle to clear the Agents-menu first-visit stall. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/src/app/AppShell.tsx | 16 + desktop/src/app/routes/agents.tsx | 12 +- .../src/app/routes/channels.$channelId.tsx | 11 +- .../channels/ui/ChannelScreenLazyViews.ts | 15 +- .../messages/lib/scrollConvergence.test.mjs | 160 +++++++ .../messages/lib/scrollConvergence.ts | 103 ++++ .../messages/lib/timelineItems.test.mjs | 164 +++++++ .../features/messages/lib/timelineItems.ts | 96 ++++ .../messages/ui/MessageThreadPanel.tsx | 2 +- .../features/messages/ui/MessageTimeline.tsx | 49 +- .../messages/ui/TimelineMessageList.tsx | 443 +++++++++--------- .../ui/useConvergentScrollToMessage.ts | 166 +++++++ .../messages/ui/useLoadOlderOnScroll.ts | 157 +++++++ 13 files changed, 1176 insertions(+), 218 deletions(-) create mode 100644 desktop/src/features/messages/lib/scrollConvergence.test.mjs create mode 100644 desktop/src/features/messages/lib/scrollConvergence.ts create mode 100644 desktop/src/features/messages/lib/timelineItems.test.mjs create mode 100644 desktop/src/features/messages/lib/timelineItems.ts create mode 100644 desktop/src/features/messages/ui/useConvergentScrollToMessage.ts create mode 100644 desktop/src/features/messages/ui/useLoadOlderOnScroll.ts diff --git a/desktop/src/app/AppShell.tsx b/desktop/src/app/AppShell.tsx index 6e8882c0f..5ada653cb 100644 --- a/desktop/src/app/AppShell.tsx +++ b/desktop/src/app/AppShell.tsx @@ -76,6 +76,9 @@ import { relayClient } from "@/shared/api/relayClient"; import { useIdentityQuery } from "@/shared/api/hooks"; import { useRelayAutoHeal } from "@/shared/api/useRelayAutoHeal"; import { useDeferredStartup } from "@/shared/hooks/useDeferredStartup"; +import { preloadAgentsScreen } from "@/app/routes/agents"; +import { preloadChannelRouteScreen } from "@/app/routes/channels.$channelId"; +import { preloadChannelViews } from "@/features/channels/ui/ChannelScreenLazyViews"; import { joinChannel } from "@/shared/api/tauri"; import type { Channel, RelayEvent, SearchHit } from "@/shared/api/types"; import { ChannelNavigationProvider } from "@/shared/context/ChannelNavigationContext"; @@ -537,6 +540,19 @@ export function AppShell() { }; }, []); + // Warm the lazy route chunks (channel timeline, forum, agents) once the shell + // is idle, so the FIRST main-nav transition doesn't stall on a cold chunk + // fetch+parse. `startupReady` is the existing idle-or-timeout gate; the chunk + // imports dedupe, so racing an actual navigation is harmless. + React.useEffect(() => { + if (!startupReady) { + return; + } + preloadChannelRouteScreen(); + preloadChannelViews(); + preloadAgentsScreen(); + }, [startupReady]); + React.useEffect(() => { const numericCount = highPriorityUnreadChannelIds.size + homeBadgeCountExcludingHighPriority; diff --git a/desktop/src/app/routes/agents.tsx b/desktop/src/app/routes/agents.tsx index c70ff7eb9..2cc1afa0c 100644 --- a/desktop/src/app/routes/agents.tsx +++ b/desktop/src/app/routes/agents.tsx @@ -3,11 +3,21 @@ import { createFileRoute } from "@tanstack/react-router"; import { ViewLoadingFallback } from "@/shared/ui/ViewLoadingFallback"; +// The chunk import is hoisted so it can be triggered eagerly (route preload) +// as well as lazily on render — calling it twice is a no-op, the module loader +// dedupes and caches the in-flight promise. +const importAgentsScreen = () => import("@/features/agents/ui/AgentsScreen"); + const AgentsScreen = React.lazy(async () => { - const module = await import("@/features/agents/ui/AgentsScreen"); + const module = await importAgentsScreen(); return { default: module.AgentsScreen }; }); +/** Warms the AgentsScreen route chunk so first navigation doesn't stall. */ +export function preloadAgentsScreen(): void { + void importAgentsScreen(); +} + export const Route = createFileRoute("/agents")({ component: AgentsRouteComponent, }); diff --git a/desktop/src/app/routes/channels.$channelId.tsx b/desktop/src/app/routes/channels.$channelId.tsx index 3c64f3ec9..61b64d0f3 100644 --- a/desktop/src/app/routes/channels.$channelId.tsx +++ b/desktop/src/app/routes/channels.$channelId.tsx @@ -38,11 +38,20 @@ export const Route = createFileRoute("/channels/$channelId")({ component: ChannelRouteComponent, }); +// Hoisted so the chunk can be warmed eagerly (route preload) as well as loaded +// lazily on render; the loader dedupes repeat calls. +const importChannelRouteScreen = () => import("./ChannelRouteScreen"); + const ChannelRouteScreen = React.lazy(async () => { - const module = await import("./ChannelRouteScreen"); + const module = await importChannelRouteScreen(); return { default: module.ChannelRouteScreen }; }); +/** Warms the ChannelRouteScreen chunk so first channel open doesn't stall. */ +export function preloadChannelRouteScreen(): void { + void importChannelRouteScreen(); +} + function ChannelRouteComponent() { const { channelId } = Route.useParams(); const search = Route.useSearch(); diff --git a/desktop/src/features/channels/ui/ChannelScreenLazyViews.ts b/desktop/src/features/channels/ui/ChannelScreenLazyViews.ts index 4e4fc0f2a..8784fc345 100644 --- a/desktop/src/features/channels/ui/ChannelScreenLazyViews.ts +++ b/desktop/src/features/channels/ui/ChannelScreenLazyViews.ts @@ -1,11 +1,22 @@ import * as React from "react"; +// Hoisted chunk imports so each view can be warmed eagerly (route preload) as +// well as loaded lazily on render; the module loader dedupes repeat calls. +const importChannelPane = () => import("@/features/channels/ui/ChannelPane"); +const importForumView = () => import("@/features/forum/ui/ForumView"); + export const ChannelPane = React.lazy(async () => { - const module = await import("@/features/channels/ui/ChannelPane"); + const module = await importChannelPane(); return { default: module.ChannelPane }; }); export const ForumView = React.lazy(async () => { - const module = await import("@/features/forum/ui/ForumView"); + const module = await importForumView(); return { default: module.ForumView }; }); + +/** Warms the channel/forum view chunks so first open doesn't stall. */ +export function preloadChannelViews(): void { + void importChannelPane(); + void importForumView(); +} diff --git a/desktop/src/features/messages/lib/scrollConvergence.test.mjs b/desktop/src/features/messages/lib/scrollConvergence.test.mjs new file mode 100644 index 000000000..d85d96146 --- /dev/null +++ b/desktop/src/features/messages/lib/scrollConvergence.test.mjs @@ -0,0 +1,160 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { CONVERGENCE_FRAME_CAP, convergenceStep } from "./scrollConvergence.ts"; + +function input(overrides) { + return { + targetMessageId: "target", + indexByMessageId: new Map([["target", 100]]), + lastIssuedIndex: null, + librarySettled: false, + framesUsed: 0, + ...overrides, + }; +} + +// --- re-aim / staleness guard ------------------------------------------------ + +test("convergenceStep: first frame aims at the resolved index, not yet done", () => { + const step = convergenceStep(input({ lastIssuedIndex: null })); + assert.equal(step.nextIndex, 100); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +test("convergenceStep: re-resolves a shifted index from the map each frame", () => { + // A prepend shifted the target from 100 to 105. The library is still chasing + // the old index (lastIssuedIndex 100); the reducer must aim at the NEW index + // so the adapter re-issues scrollToIndex(105). This is the staleness guard. + const step = convergenceStep( + input({ + indexByMessageId: new Map([["target", 105]]), + lastIssuedIndex: 100, + }), + ); + assert.equal(step.nextIndex, 105); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +test("convergenceStep: target removed mid-settle stops with converged=false", () => { + // Target deleted from the map while the loop was chasing it. Terminate so the + // adapter clears the highlight instead of chasing a vanished row. + const step = convergenceStep( + input({ + indexByMessageId: new Map(), // target gone + lastIssuedIndex: 100, + framesUsed: 3, + }), + ); + assert.equal(step.nextIndex, null); + assert.equal(step.done, true); + assert.equal(step.converged, false); +}); + +// --- settle ------------------------------------------------------------------ + +test("convergenceStep: library settled while aiming at current index converges", () => { + const step = convergenceStep( + input({ lastIssuedIndex: 100, librarySettled: true }), + ); + assert.equal(step.nextIndex, 100); + assert.equal(step.done, true); + assert.equal(step.converged, true); +}); + +test("convergenceStep: a settle reported WHILE re-aiming is ignored", () => { + // The index just moved (105) but the library reports settled — that settle is + // on the OLD index (100), so it must NOT count as convergence. The reducer + // keeps going and aims at the new index. + const step = convergenceStep( + input({ + indexByMessageId: new Map([["target", 105]]), + lastIssuedIndex: 100, + librarySettled: true, + }), + ); + assert.equal(step.nextIndex, 105); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +test("convergenceStep: aiming at current but not yet settled keeps waiting", () => { + // Library is chasing the right index but its offset hasn't stabilized. The + // reducer returns the same index (so the adapter re-issues NOTHING — issuing + // would reset the library's stableFrames and prevent settling) and waits. + const step = convergenceStep( + input({ lastIssuedIndex: 100, librarySettled: false }), + ); + assert.equal(step.nextIndex, 100); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +// --- frame cap --------------------------------------------------------------- + +test("convergenceStep: terminates at the frame cap without converging", () => { + // A row that never settles (librarySettled stays false) must still stop at the + // cap rather than spin forever. + const step = convergenceStep( + input({ + lastIssuedIndex: 100, + librarySettled: false, + framesUsed: CONVERGENCE_FRAME_CAP - 1, + }), + ); + assert.equal(step.done, true); + assert.equal(step.converged, false); + assert.equal(step.nextIndex, 100); +}); + +test("convergenceStep: frame cap bounds a perpetually shifting target", () => { + // Drive the loop the way the adapter would: the target index moves every + // frame, so the library never settles. The loop must terminate at the cap. + let lastIssuedIndex = null; + let framesUsed = 0; + let done = false; + let converged = true; + + while (framesUsed < CONVERGENCE_FRAME_CAP + 5) { + const movingIndex = 100 + framesUsed; // shifts every frame + const step = convergenceStep( + input({ + indexByMessageId: new Map([["target", movingIndex]]), + lastIssuedIndex, + librarySettled: false, + framesUsed, + }), + ); + lastIssuedIndex = step.nextIndex; + framesUsed += 1; + if (step.done) { + done = step.done; + converged = step.converged; + break; + } + } + + assert.equal(done, true); + assert.equal(converged, false); + assert.ok(framesUsed <= CONVERGENCE_FRAME_CAP); +}); + +test("convergenceStep: converges once a re-aimed index then settles", () => { + // Realistic flow: frame 0 aims (lastIssued null -> 100), frame 1 the library + // is chasing 100 and reports settled -> converged. + const aim = convergenceStep(input({ lastIssuedIndex: null })); + assert.equal(aim.nextIndex, 100); + assert.equal(aim.done, false); + + const settle = convergenceStep( + input({ + lastIssuedIndex: aim.nextIndex, + librarySettled: true, + framesUsed: 1, + }), + ); + assert.equal(settle.done, true); + assert.equal(settle.converged, true); +}); diff --git a/desktop/src/features/messages/lib/scrollConvergence.ts b/desktop/src/features/messages/lib/scrollConvergence.ts new file mode 100644 index 000000000..9ad86e27e --- /dev/null +++ b/desktop/src/features/messages/lib/scrollConvergence.ts @@ -0,0 +1,103 @@ +/** + * Pure staleness + termination decision for scrolling a virtualized timeline to + * a message that may be far off-screen. + * + * @tanstack/react-virtual already owns the OFFSET convergence: a single + * `scrollToIndex(index)` captures that index in `scrollState`, and its internal + * `reconcileScroll` rAF loop re-runs `getOffsetForIndex(index)` every frame — + * re-aiming as off-screen rows mount and `measureElement` corrects their + * heights — until the offset is stable (or a 5s safety valve fires). We do NOT + * recompute offsets; duplicating `getOffsetForIndex` against the library's own + * `measurementsCache`/`scrollMargin`/`scrollPadding` would only drift. + * + * What the library does NOT do: it chases the INDEX captured at call time, with + * no concept of a message id. If the data shifts mid-settle — a prepend or a + * delete above the target — the captured index now points at the wrong row and + * the library happily settles on it. This reducer owns exactly that gap: each + * frame it re-resolves the target's CURRENT index from the live map and decides + * whether the adapter must re-aim the library, let it settle, or stop. + * + * Two correctness properties this enforces and the `.mjs` suite gates: + * - The target index is re-resolved by id every frame (never frozen), so a + * concurrent prepend/delete that shifts the target re-aims the library at the + * new index instead of stranding it on the old one. + * - If the target id leaves the data mid-settle (deleted), the loop terminates + * with `converged: false` rather than chasing a vanished row to the cap. + */ + +/** Where a scroll target should land in the viewport. Mirrors the library's align. */ +export type ConvergenceAlign = "start" | "center" | "end"; + +export type ConvergenceInput = { + /** Id of the message to settle on — re-resolved against the map each frame. */ + targetMessageId: string; + /** Live message-id -> item-index map; re-read every frame (staleness guard). */ + indexByMessageId: Map; + /** + * Index the library is currently chasing (the last index the adapter issued + * via `scrollToIndex`), or `null` before the first issue. Lets the reducer + * tell a re-aim (index moved) from a steady settle (index unchanged). + */ + lastIssuedIndex: number | null; + /** + * Whether the library reports its scroll has settled this frame + * (`virtualizer.scrollState === null`). Only meaningful once the library is + * chasing the CURRENT index; a settle reported while re-aiming is ignored. + */ + librarySettled: boolean; + /** Frames already spent in the loop (the adapter increments per rAF). */ + framesUsed: number; +}; + +export type ConvergenceDecision = { + /** + * Index the adapter should be aiming the library at, or `null` when the + * target is gone. The adapter only re-issues `scrollToIndex` when this differs + * from `lastIssuedIndex`, so a steady settle issues no redundant scroll (which + * would reset the library's `stableFrames` and prevent it from ever settling). + */ + nextIndex: number | null; + /** True once the loop must stop (settled, target gone, or frame cap hit). */ + done: boolean; + /** True only when the loop stopped because the target row actually settled. */ + converged: boolean; +}; + +/** + * Hard cap on frames so a perpetually re-measuring row, or a target whose index + * keeps shifting, can't spin the loop forever. The library has its own 5s valve; + * this is the adapter-side bound expressed in frames for deterministic testing. + */ +export const CONVERGENCE_FRAME_CAP = 32; + +/** + * One frame of the convergence loop. Pure: given the live map and the library's + * settle state, decides the index to aim at and whether to stop. + */ +export function convergenceStep(input: ConvergenceInput): ConvergenceDecision { + const currentIndex = input.indexByMessageId.get(input.targetMessageId); + + // Target left the data mid-settle (deleted) — stop without converging so the + // adapter clears the highlight instead of chasing a vanished row. + if (currentIndex === undefined) { + return { nextIndex: null, done: true, converged: false }; + } + + const aimingAtCurrent = input.lastIssuedIndex === currentIndex; + + // The library only settles meaningfully once it is chasing the CURRENT index. + // A settle reported while we are still re-aiming (index just moved) is stale. + if (aimingAtCurrent && input.librarySettled) { + return { nextIndex: currentIndex, done: true, converged: true }; + } + + // Frame cap: accept the best index we have rather than spin forever on a row + // whose height never settles or a target whose index keeps shifting. + if (input.framesUsed + 1 >= CONVERGENCE_FRAME_CAP) { + return { nextIndex: currentIndex, done: true, converged: false }; + } + + // Either the index moved (adapter will re-issue scrollToIndex) or the library + // is still settling on the current index (adapter issues nothing, just waits). + return { nextIndex: currentIndex, done: false, converged: false }; +} diff --git a/desktop/src/features/messages/lib/timelineItems.test.mjs b/desktop/src/features/messages/lib/timelineItems.test.mjs new file mode 100644 index 000000000..04f8738ec --- /dev/null +++ b/desktop/src/features/messages/lib/timelineItems.test.mjs @@ -0,0 +1,164 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { KIND_SYSTEM_MESSAGE } from "@/shared/constants/kinds"; +import { buildTimelineItems, getTimelineItemKey } from "./timelineItems.ts"; + +function dayAt(year, month, day, hour = 12) { + return Math.floor( + new Date(year, month - 1, day, hour, 0, 0).getTime() / 1_000, + ); +} + +function message(overrides) { + return { + id: "m", + renderKey: undefined, + createdAt: dayAt(2026, 6, 14), + pubkey: "author", + parentId: null, + rootId: null, + depth: 0, + kind: 9, + tags: [], + ...overrides, + }; +} + +// The builder takes MainTimelineEntry[] (post top-level filter); summary is +// irrelevant to item/divider placement, so null is fine here. +function entry(overrides) { + return { message: message(overrides), summary: null }; +} + +function kinds(items) { + return items.map((item) => item.kind); +} + +// --- divider placement ------------------------------------------------------- + +test("buildTimelineItems: 3-day channel with unread mid-day-2 places dividers by index", () => { + const entries = [ + entry({ id: "d1a", createdAt: dayAt(2026, 6, 12) }), + entry({ id: "d1b", createdAt: dayAt(2026, 6, 12, 13) }), + entry({ id: "d2a", createdAt: dayAt(2026, 6, 13) }), + entry({ id: "d2b", createdAt: dayAt(2026, 6, 13, 13) }), // first unread + entry({ id: "d2c", createdAt: dayAt(2026, 6, 13, 14) }), + entry({ id: "d3a", createdAt: dayAt(2026, 6, 14) }), + ]; + + const { items } = buildTimelineItems(entries, "d2b"); + + assert.deepEqual(kinds(items), [ + "day-divider", // day 1 + "message", // d1a + "message", // d1b + "day-divider", // day 2 + "message", // d2a + "unread-divider", // above d2b + "message", // d2b + "message", // d2c + "day-divider", // day 3 + "message", // d3a + ]); +}); + +test("buildTimelineItems: unread divider suppressed when first unread is the first entry", () => { + const entries = [ + entry({ id: "a", createdAt: dayAt(2026, 6, 14) }), + entry({ id: "b", createdAt: dayAt(2026, 6, 14, 13) }), + ]; + // firstUnread === index 0 — nothing above it, so no divider. + const { items } = buildTimelineItems(entries, "a"); + assert.equal(items.filter((i) => i.kind === "unread-divider").length, 0); +}); + +test("buildTimelineItems: system messages flatten to a 'system' item", () => { + const entries = [ + entry({ id: "a", createdAt: dayAt(2026, 6, 14) }), + entry({ + id: "sys", + kind: KIND_SYSTEM_MESSAGE, + createdAt: dayAt(2026, 6, 14, 13), + }), + ]; + const { items } = buildTimelineItems(entries, null); + assert.deepEqual(kinds(items), ["day-divider", "message", "system"]); +}); + +test("buildTimelineItems: empty entries produce no items and an empty map", () => { + const { items, indexByMessageId } = buildTimelineItems([], null); + assert.equal(items.length, 0); + assert.equal(indexByMessageId.size, 0); +}); + +// --- index map correctness --------------------------------------------------- + +test("buildTimelineItems: map points each message id at its flattened item index", () => { + const entries = [ + entry({ id: "d1", createdAt: dayAt(2026, 6, 12) }), + entry({ id: "d2", createdAt: dayAt(2026, 6, 13) }), + ]; + const { items, indexByMessageId } = buildTimelineItems(entries, null); + + // dividers occupy indices 0 and 2; messages land at 1 and 3. + assert.equal(indexByMessageId.get("d1"), 1); + assert.equal(indexByMessageId.get("d2"), 3); + assert.equal(items[1].entry.message.id, "d1"); + assert.equal(items[3].entry.message.id, "d2"); +}); + +test("buildTimelineItems: appending a new message keeps prior indices stable", () => { + const base = [entry({ id: "a", createdAt: dayAt(2026, 6, 14) })]; + const before = buildTimelineItems(base, null).indexByMessageId; + + const appended = [ + ...base, + entry({ id: "b", createdAt: dayAt(2026, 6, 14, 13) }), + ]; + const after = buildTimelineItems(appended, null).indexByMessageId; + + assert.equal(after.get("a"), before.get("a")); + assert.equal(after.get("b"), 2); +}); + +test("buildTimelineItems: prepending an older-day message shifts later indices", () => { + const original = [entry({ id: "b", createdAt: dayAt(2026, 6, 14) })]; + const beforeIdx = buildTimelineItems(original, null).indexByMessageId.get( + "b", + ); + + // Prepend a message on an earlier day → adds its own day-divider + message, + // pushing "b" (now on a new day boundary too) further down. + const prepended = [ + entry({ id: "a", createdAt: dayAt(2026, 6, 13) }), + entry({ id: "b", createdAt: dayAt(2026, 6, 14) }), + ]; + const afterIdx = buildTimelineItems(prepended, null).indexByMessageId.get( + "b", + ); + assert.ok(afterIdx > beforeIdx); +}); + +test("buildTimelineItems: deleting a message drops it from the map", () => { + const entries = [ + entry({ id: "a", createdAt: dayAt(2026, 6, 14) }), + entry({ id: "b", createdAt: dayAt(2026, 6, 14, 13) }), + ]; + const afterDelete = buildTimelineItems( + entries.filter((e) => e.message.id !== "a"), + null, + ).indexByMessageId; + assert.equal(afterDelete.has("a"), false); + assert.equal(afterDelete.get("b"), 1); +}); + +test("getTimelineItemKey: keys are unique across the stream", () => { + const entries = [ + entry({ id: "a", createdAt: dayAt(2026, 6, 12) }), + entry({ id: "b", createdAt: dayAt(2026, 6, 13) }), + ]; + const { items } = buildTimelineItems(entries, "b"); + const keys = items.map(getTimelineItemKey); + assert.equal(new Set(keys).size, keys.length); +}); diff --git a/desktop/src/features/messages/lib/timelineItems.ts b/desktop/src/features/messages/lib/timelineItems.ts new file mode 100644 index 000000000..02477eb97 --- /dev/null +++ b/desktop/src/features/messages/lib/timelineItems.ts @@ -0,0 +1,96 @@ +/** + * Flattens the heterogeneous day-grouped timeline tree into a flat + * discriminated-union item stream that a virtualizer can window over, and + * builds the `messageId -> itemIndex` map every DOM-query scroll path now + * resolves against instead of `querySelector`. + * + * Kept pure (no React, no DOM) so it is covered by the lib-level `*.test.mjs` + * suite. The list and the index map are produced together from the SAME walk, + * so they can never drift: a stale map would scroll deep-links to the wrong + * row, the exact failure virtualization risks. + */ + +import { + buildDayGroupBoundaries, + type DayGroupBoundary, +} from "@/features/messages/lib/timelineSnapshot"; +import { shouldRenderUnreadDivider } from "@/features/messages/lib/threadPanel"; +import type { MainTimelineEntry } from "@/features/messages/lib/threadPanel"; +import { KIND_SYSTEM_MESSAGE } from "@/shared/constants/kinds"; + +/** + * One renderable row in the flattened timeline. Dividers carry no message and + * never appear in the index map; the three message-bearing kinds do. + */ +export type TimelineItem = + // `headingTimestamp` (not a prebaked label) so the render still resolves + // "Today"/"Yesterday" relative to the current clock, not to build time. + | { kind: "day-divider"; key: string; headingTimestamp: number } + | { kind: "unread-divider"; key: string } + | { kind: "system"; key: string; entry: MainTimelineEntry } + | { kind: "message"; key: string; entry: MainTimelineEntry }; + +export type TimelineItemsResult = { + items: TimelineItem[]; + /** Maps a top-level message id to its index in `items`. */ + indexByMessageId: Map; +}; + +/** Stable per-item key, unique across the flattened stream. */ +export function getTimelineItemKey(item: TimelineItem): string { + return item.key; +} + +function entryRenderKey(entry: MainTimelineEntry): string { + return entry.message.renderKey ?? entry.message.id; +} + +/** + * Walks the (already top-level-filtered) entries once, emitting a day-divider + * at each calendar-day boundary and an unread-divider above the first unread + * message, then the message/system row itself. The index map records where + * each message landed in the flat stream so scroll targets resolve in O(1) + * without touching the DOM. + */ +export function buildTimelineItems( + entries: MainTimelineEntry[], + firstUnreadMessageId: string | null, +): TimelineItemsResult { + const items: TimelineItem[] = []; + const indexByMessageId = new Map(); + + // Index boundaries by their start position so the walk below can look up the + // prepend-stable section key (start-of-local-day). Keying the divider by + // start-of-day, not by the first message, keeps the day section from + // remounting when older messages prepend into it. + const dayBoundariesByStartIndex = new Map( + buildDayGroupBoundaries(entries.map((entry) => entry.message)).map( + (boundary: DayGroupBoundary) => [boundary.startIndex, boundary] as const, + ), + ); + + for (let i = 0; i < entries.length; i++) { + const entry = entries[i]; + const { message } = entry; + const renderKey = entryRenderKey(entry); + + const dayBoundary = dayBoundariesByStartIndex.get(i); + if (dayBoundary) { + items.push({ + kind: "day-divider", + key: dayBoundary.key, + headingTimestamp: message.createdAt, + }); + } + + if (shouldRenderUnreadDivider(i, message.id, firstUnreadMessageId)) { + items.push({ kind: "unread-divider", key: `unread-${renderKey}` }); + } + + const kind = message.kind === KIND_SYSTEM_MESSAGE ? "system" : "message"; + indexByMessageId.set(message.id, items.length); + items.push({ kind, key: renderKey, entry }); + } + + return { items, indexByMessageId }; +} diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index cb3bc0b5a..46dbf215c 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -438,7 +438,7 @@ export function MessageThreadPanel({ return (
(null); const topSentinelRef = React.useRef(null); + // The virtualizer instance and the flattened item stream are owned by the + // child TimelineMessageList (which mounts the VirtualizedList) and reported + // up here so the scroll manager can resolve scroll targets by index. The + // virtualizer reaches us via a ref (its identity is stable across renders, + // but it arrives after first paint); the item stream + id->index map arrive + // as state so a rebuild re-runs the scroll manager's index-model paths. + const virtualizerRef = React.useRef(null); + const handleVirtualizer = React.useCallback((instance: ListVirtualizer) => { + virtualizerRef.current = instance; + }, []); + const getVirtualizer = React.useCallback(() => virtualizerRef.current, []); + const [timelineItems, setTimelineItems] = + React.useState(null); + const handleItems = React.useCallback((result: TimelineItemsResult) => { + setTimelineItems(result); + }, []); + const virtualizerOption = React.useMemo( + () => + timelineItems + ? { + getVirtualizer, + indexByMessageId: timelineItems.indexByMessageId, + itemCount: timelineItems.items.length, + } + : null, + [getVirtualizer, timelineItems], + ); + // Gate the heavy timeline render (each row runs a synchronous // react-markdown parse) behind React concurrency. `useDeferredValue` lets the // commit that rebuilds the message list yield to higher-priority work, so the @@ -222,6 +252,7 @@ const MessageTimelineBase = React.forwardRef< scrollContainerRef, sentinelRef: topSentinelRef, targetMessageId, + virtualizer: virtualizerOption, }); React.useImperativeHandle( @@ -266,8 +297,9 @@ const MessageTimelineBase = React.forwardRef< }, [firstUnreadMessageId, scrollToMessage]); // Scroll to the active search match when it changes. `scrollToMessage` - // updates the scroll anchor, so the post-commit restore won't yank the - // view back off the match. + // updates the scroll anchor (so the post-commit restore won't yank the view + // back off the match) and, when virtualized, resolves the target through the + // index model — the row may be windowed out of the DOM. const prevSearchActiveRef = React.useRef(null); React.useEffect(() => { if (showTimelineSkeleton) return; @@ -282,6 +314,16 @@ const MessageTimelineBase = React.forwardRef< scrollToMessage(searchActiveMessageId, { behavior: "smooth" }); }, [scrollToMessage, searchActiveMessageId, showTimelineSkeleton]); + useLoadOlderOnScroll({ + fetchOlder, + hasOlderMessages, + isLoading: showTimelineSkeleton, + restoreScrollPosition, + scrollContainerRef, + sentinelRef: topSentinelRef, + virtualizer: virtualizerOption, + }); + const timelineSkeletonRows = useTimelineSkeletonRows({ channelId, isLoading: showTimelineSkeleton, @@ -501,6 +543,9 @@ const MessageTimelineBase = React.forwardRef< searchQuery={searchQuery} threadUnreadCounts={threadUnreadCounts} unfollowThreadById={unfollowThreadById} + scrollContainerRef={scrollContainerRef} + onItems={handleItems} + onVirtualizer={handleVirtualizer} />
) : null} diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index d968dc5b7..4e91a01a8 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -1,24 +1,26 @@ import * as React from "react"; +import { formatDayHeading } from "@/features/messages/lib/dateFormatters"; import { - formatDayHeading, - isSameDay, - startOfLocalDaySeconds, -} from "@/features/messages/lib/dateFormatters"; + buildTimelineItems, + getTimelineItemKey, + type TimelineItem, + type TimelineItemsResult, +} from "@/features/messages/lib/timelineItems"; +import { buildMainTimelineEntries } from "@/features/messages/lib/threadPanel"; +import type { MainTimelineEntry } from "@/features/messages/lib/threadPanel"; import { - buildMainTimelineEntries, - shouldRenderUnreadDivider, -} from "@/features/messages/lib/threadPanel"; -import { - buildVideoReviewCommentsForRoot, + buildVideoReviewCommentsByRootId, buildVideoReviewContextForMessage, - hasVideoAttachment, } from "@/features/messages/lib/videoReviewContext"; import type { TimelineMessage } from "@/features/messages/types"; import type { UserProfileLookup } from "@/features/profile/lib/identity"; import type { ChannelType } from "@/shared/api/types"; -import { KIND_SYSTEM_MESSAGE } from "@/shared/constants/kinds"; import { cn } from "@/shared/lib/cn"; +import { + type ListVirtualizer, + VirtualizedList, +} from "@/shared/ui/VirtualizedList"; import { DayDivider } from "./DayDivider"; import { MessageRow } from "./MessageRow"; import { MessageThreadSummaryRow } from "./MessageThreadSummaryRow"; @@ -67,78 +69,15 @@ type TimelineMessageListProps = { searchQuery?: string; /** Per-thread unread counts keyed by thread root id. */ threadUnreadCounts?: ReadonlyMap; + /** Caller-owned scroll container the virtualizer measures and scrolls. */ + scrollContainerRef: React.RefObject; + /** Receives the flattened item stream + index map so the scroll manager can + * resolve scroll targets by id. Called whenever the stream is rebuilt. */ + onItems?: (result: TimelineItemsResult) => void; + /** Receives the virtualizer instance for index-model scroll paths. */ + onVirtualizer?: (virtualizer: ListVirtualizer) => void; }; -type TimelineDayRow = { - key: string; - label: string; - type: "day"; -}; - -type TimelineUnreadRow = { - key: string; - type: "unread"; -}; - -type TimelineMessageRowModel = { - key: string; - message: TimelineMessage; - summary: ReturnType[number]["summary"]; - type: "message"; -}; - -type TimelineRenderRow = - | TimelineDayRow - | TimelineUnreadRow - | TimelineMessageRowModel; - -function buildTimelineRenderRows({ - firstUnreadMessageId, - messages, -}: { - firstUnreadMessageId: string | null; - messages: TimelineMessage[]; -}): TimelineRenderRow[] { - const entries = buildMainTimelineEntries(messages); - const rows: TimelineRenderRow[] = []; - let previousMessage: TimelineMessage | null = null; - - for (let i = 0; i < entries.length; i++) { - const { message, summary } = entries[i]; - const messageRenderKey = message.renderKey ?? message.id; - - if ( - !previousMessage || - !isSameDay(previousMessage.createdAt, message.createdAt) - ) { - rows.push({ - key: `day-${startOfLocalDaySeconds(message.createdAt)}`, - label: formatDayHeading(message.createdAt), - type: "day", - }); - } - - // The unread "New" divider only marks a read/unread boundary when there is - // a message above the first unread. When the first unread is the first - // rendered top-level entry (fresh/never-read channel), there is nothing - // above to separate from, so it is suppressed. - if (shouldRenderUnreadDivider(i, message.id, firstUnreadMessageId)) { - rows.push({ key: `unread:${messageRenderKey}`, type: "unread" }); - } - - rows.push({ - key: `msg:${messageRenderKey}`, - message, - summary, - type: "message", - }); - - previousMessage = message; - } - - return rows; -} - export const TimelineMessageList = React.memo(function TimelineMessageList({ agentPubkeys, channelId, @@ -164,144 +103,243 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ searchQuery, threadUnreadCounts, unfollowThreadById, + scrollContainerRef, + onItems, + onVirtualizer, }: TimelineMessageListProps) { - const rows = React.useMemo( - () => buildTimelineRenderRows({ firstUnreadMessageId, messages }), - [firstUnreadMessageId, messages], + const entries = React.useMemo( + () => buildMainTimelineEntries(messages), + [messages], ); - return rows.map((row) => ( - buildVideoReviewCommentsByRootId(messages), + [messages], + ); + // Contexts are memoized per message id so MessageRow/Markdown memo + // comparisons hold across unrelated timeline re-renders (typing + // indicators, presence updates) — a fresh context object per render would + // defeat the memo and re-render every video message on every pass. + const videoReviewContextById = React.useMemo(() => { + const contexts = new Map< + string, + NonNullable> + >(); + for (const message of messages) { + const comments = reviewCommentsByRootId.get(message.id) ?? []; + const context = buildVideoReviewContextForMessage({ + channelId, + channelName, + channelType, + comments, + isSendingVideoReviewComment, + message, + onSendVideoReviewComment, + onToggleReaction, + profiles, + }); + if (context) { + contexts.set(message.id, context); } - channelId={channelId} - channelName={channelName} - channelType={channelType} - currentPubkey={currentPubkey} - followThreadById={followThreadById} - highlightedMessageId={highlightedMessageId} - isFollowingThreadById={isFollowingThreadById} - isSendingVideoReviewComment={isSendingVideoReviewComment} - key={row.key} - messageFooters={messageFooters} - onDelete={onDelete} - onEdit={onEdit} - onMarkUnread={onMarkUnread} - onReply={onReply} - onSendVideoReviewComment={onSendVideoReviewComment} - onToggleReaction={onToggleReaction} - profiles={profiles} - row={row} - searchActiveMessageId={searchActiveMessageId} - searchMatchingMessageIds={searchMatchingMessageIds} - searchQuery={searchQuery} - threadUnreadCounts={threadUnreadCounts} - unfollowThreadById={unfollowThreadById} + } + return contexts; + }, [ + channelId, + channelName, + channelType, + isSendingVideoReviewComment, + messages, + onSendVideoReviewComment, + onToggleReaction, + profiles, + reviewCommentsByRootId, + ]); + + // The flattened item stream and its messageId -> itemIndex map are produced + // together from ONE memo, keyed on the entries and the unread boundary (the + // unread divider is its own item, so it shifts indices). A separate memo with + // diverging deps would let the map go stale and scroll deep-links to the wrong + // row — the exact failure virtualization risks. + const itemsResult = React.useMemo( + () => buildTimelineItems(entries, firstUnreadMessageId), + [entries, firstUnreadMessageId], + ); + + React.useEffect(() => { + onItems?.(itemsResult); + }, [itemsResult, onItems]); + + const renderItem = React.useCallback( + (item: TimelineItem) => { + switch (item.kind) { + case "day-divider": + // Heading is resolved at render time (not baked into the item) so + // "Today"/"Yesterday" track the wall clock, not build time. + return ; + case "unread-divider": + return ; + case "system": + return ( + + ); + case "message": + return ( + + ); + } + }, + [ + agentPubkeys, + channelId, + currentPubkey, + followThreadById, + highlightedMessageId, + isFollowingThreadById, + messageFooters, + onDelete, + onEdit, + onMarkUnread, + onReply, + onToggleReaction, + profiles, + searchActiveMessageId, + searchMatchingMessageIds, + searchQuery, + threadUnreadCounts, + unfollowThreadById, + videoReviewContextById, + ], + ); + + return ( + - )); + ); }); -type TimelineRenderRowViewProps = Omit< +function SystemRow({ + currentPubkey, + entry, + footer, + onToggleReaction, + profiles, +}: { + currentPubkey?: string; + entry: MainTimelineEntry; + footer: React.ReactNode; + onToggleReaction?: TimelineMessageListProps["onToggleReaction"]; + profiles?: UserProfileLookup; +}) { + return ( +
+ + {footer} +
+ ); +} + +type MessageRowItemProps = Pick< TimelineMessageListProps, - "firstUnreadMessageId" | "messages" | "personaLookup" + | "agentPubkeys" + | "channelId" + | "currentPubkey" + | "followThreadById" + | "highlightedMessageId" + | "isFollowingThreadById" + | "onDelete" + | "onEdit" + | "onMarkUnread" + | "onReply" + | "onToggleReaction" + | "profiles" + | "searchActiveMessageId" + | "searchMatchingMessageIds" + | "searchQuery" + | "threadUnreadCounts" + | "unfollowThreadById" > & { - allMessages?: TimelineMessage[]; - row: TimelineRenderRow; + entry: MainTimelineEntry; + footer: React.ReactNode; + videoReviewContext: ReturnType; }; -const TimelineRenderRowView = React.memo(function TimelineRenderRowView({ +function MessageRowItem({ agentPubkeys, - allMessages, channelId, - channelName, - channelType, currentPubkey, + entry, followThreadById, - highlightedMessageId = null, + footer, + highlightedMessageId, isFollowingThreadById, - isSendingVideoReviewComment = false, - messageFooters, onDelete, onEdit, onMarkUnread, onReply, - onSendVideoReviewComment, onToggleReaction, profiles, - searchActiveMessageId = null, + searchActiveMessageId, searchMatchingMessageIds, searchQuery, - row, threadUnreadCounts, unfollowThreadById, -}: TimelineRenderRowViewProps) { - const messageForContext = row.type === "message" ? row.message : null; - const videoReviewContext = React.useMemo(() => { - if (!allMessages || !messageForContext) { - return undefined; - } - - return buildVideoReviewContextForMessage({ - channelId, - channelName, - channelType, - comments: buildVideoReviewCommentsForRoot( - allMessages, - messageForContext.id, - ), - isSendingVideoReviewComment, - message: messageForContext, - onSendVideoReviewComment, - onToggleReaction, - profiles, - }); - }, [ - allMessages, - channelId, - channelName, - channelType, - isSendingVideoReviewComment, - messageForContext, - onSendVideoReviewComment, - onToggleReaction, - profiles, - ]); - - if (row.type === "day") { - return ; - } - - if (row.type === "unread") { - return ; - } - - const { message, summary } = row; - - if (message.kind === KIND_SYSTEM_MESSAGE) { - const footer = messageFooters?.[message.id] ?? null; - return ( -
- - {footer} -
- ); - } + videoReviewContext, +}: MessageRowItemProps) { + const { message, summary } = entry; + const canDelete = + onDelete && currentPubkey && message.pubkey === currentPubkey + ? onDelete + : undefined; + const canEdit = + onEdit && currentPubkey && message.pubkey === currentPubkey + ? onEdit + : undefined; if (summary && onReply) { - const footer = messageFooters?.[message.id] ?? null; const isHighlighted = message.id === highlightedMessageId; return (
followThreadById(message.id) : undefined } @@ -357,25 +387,16 @@ const TimelineRenderRowView = React.memo(function TimelineRenderRowView({ const isSearchMatch = searchMatchingMessageIds?.has(message.id) ?? false; const isSearchActive = message.id === searchActiveMessageId; - const footer = messageFooters?.[message.id] ?? null; return ( -
+
); -}); +} diff --git a/desktop/src/features/messages/ui/useConvergentScrollToMessage.ts b/desktop/src/features/messages/ui/useConvergentScrollToMessage.ts new file mode 100644 index 000000000..084cc25a2 --- /dev/null +++ b/desktop/src/features/messages/ui/useConvergentScrollToMessage.ts @@ -0,0 +1,166 @@ +import * as React from "react"; + +import { + type ConvergenceAlign, + convergenceStep, +} from "@/features/messages/lib/scrollConvergence"; +import type { ListVirtualizer } from "@/shared/ui/VirtualizedList"; + +/** Offset (px) within which the library is considered to have reached the target. */ +const SETTLE_TOLERANCE_PX = 2; + +type ConvergentScrollOptions = { + /** Live message-id -> item-index map, rebuilt with the flattened item stream. */ + indexByMessageId: Map; + /** Where the target should land in the viewport. */ + align: ConvergenceAlign; + /** Fired on the settled frame once the target row has converged. */ + onConverged?: (messageId: string) => void; + /** Fired when the loop stops without converging (target deleted, or frame cap). */ + onAbandoned?: (messageId: string) => void; +}; + +type ConvergentScrollController = { + /** + * Begins a convergence loop toward `messageId`. Returns `true` when the id is + * present in the data (loop started), `false` when it is absent (never + * off-screen-false — only data-absent-false, matching the deep-link contract). + * A new call cancels any in-flight loop. + */ + scrollToMessage: (messageId: string) => boolean; + /** Cancels any in-flight convergence loop (e.g. on unmount or channel switch). */ + cancel: () => void; +}; + +/** + * Drives @tanstack/react-virtual to settle on an off-screen message by id. + * + * The library already converges OFFSETS: one `scrollToIndex(i)` captures index + * `i` and its `reconcileScroll` rAF loop re-aims as rows mount and measure. But + * it chases the INDEX captured at call time — a prepend/delete mid-settle leaves + * it on the wrong row. This adapter closes that gap: each frame it re-resolves + * the target's CURRENT index from the live map (the pure `convergenceStep` + * reducer owns the decision) and re-issues `scrollToIndex` ONLY when the index + * moved. In steady state it issues nothing, so it never resets the library's + * internal stable-frame counter and the library settles in one frame. + * + * Settle detection is a trivial offset-equality check (NOT the convergence math, + * which the library owns): the measured offset for the current index is within + * tolerance of where the library would place it, and the offset is unchanged + * from the prior frame. + */ +export function useConvergentScrollToMessage( + getVirtualizer: () => ListVirtualizer | null, + { + indexByMessageId, + align, + onConverged, + onAbandoned, + }: ConvergentScrollOptions, +): ConvergentScrollController { + // Mirror inputs into refs so the rAF loop closure always reads live values + // without re-subscribing the loop each render. + const mapRef = React.useRef(indexByMessageId); + mapRef.current = indexByMessageId; + const alignRef = React.useRef(align); + alignRef.current = align; + const onConvergedRef = React.useRef(onConverged); + onConvergedRef.current = onConverged; + const onAbandonedRef = React.useRef(onAbandoned); + onAbandonedRef.current = onAbandoned; + + const rafIdRef = React.useRef(null); + + const cancel = React.useCallback(() => { + if (rafIdRef.current !== null) { + cancelAnimationFrame(rafIdRef.current); + rafIdRef.current = null; + } + }, []); + + const scrollToMessage = React.useCallback( + (messageId: string) => { + const startIndex = mapRef.current.get(messageId); + if (startIndex === undefined) { + return false; + } + + cancel(); + + let lastIssuedIndex: number | null = null; + let previousOffset: number | null = null; + let framesUsed = 0; + + const frame = () => { + rafIdRef.current = null; + const virtualizer = getVirtualizer(); + if (!virtualizer) { + return; + } + + const currentIndex = mapRef.current.get(messageId); + // The library has settled this frame when its offset reached the target + // index's offset (within tolerance) and stopped moving. `currentIndex` + // is re-read so a settle on a stale index never counts as converged. + let librarySettled = false; + if (currentIndex !== undefined && lastIssuedIndex === currentIndex) { + const offset = virtualizer.scrollOffset ?? 0; + const target = virtualizer.getOffsetForIndex( + currentIndex, + alignRef.current, + ); + const reachedTarget = + target !== undefined && + Math.abs(offset - target[0]) <= SETTLE_TOLERANCE_PX; + const offsetStable = + previousOffset !== null && + Math.abs(offset - previousOffset) <= SETTLE_TOLERANCE_PX; + librarySettled = reachedTarget && offsetStable; + previousOffset = offset; + } else { + previousOffset = virtualizer.scrollOffset ?? 0; + } + + const decision = convergenceStep({ + targetMessageId: messageId, + indexByMessageId: mapRef.current, + lastIssuedIndex, + librarySettled, + framesUsed, + }); + + if ( + decision.nextIndex !== null && + decision.nextIndex !== lastIssuedIndex + ) { + // Re-aim only when the index actually moved — re-issuing the same + // index would reset the library's stable-frame counter forever. + virtualizer.scrollToIndex(decision.nextIndex, { + align: alignRef.current, + }); + lastIssuedIndex = decision.nextIndex; + } + + if (decision.done) { + if (decision.converged) { + onConvergedRef.current?.(messageId); + } else { + onAbandonedRef.current?.(messageId); + } + return; + } + + framesUsed += 1; + rafIdRef.current = requestAnimationFrame(frame); + }; + + rafIdRef.current = requestAnimationFrame(frame); + return true; + }, + [cancel, getVirtualizer], + ); + + React.useEffect(() => cancel, [cancel]); + + return { scrollToMessage, cancel }; +} diff --git a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts new file mode 100644 index 000000000..cb36f2a86 --- /dev/null +++ b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts @@ -0,0 +1,157 @@ +import * as React from "react"; + +import type { ListVirtualizer } from "@/shared/ui/VirtualizedList"; + +type UseLoadOlderOnScrollOptions = { + fetchOlder?: () => Promise; + hasOlderMessages: boolean; + isLoading: boolean; + restoreScrollPosition: (scrollTop: number) => void; + scrollContainerRef: React.RefObject; + sentinelRef: React.RefObject; + /** + * When the timeline is virtualized, prepended rows shift every index and are + * mounted at an estimate (80px) before they measure, so the `scrollHeight` + * delta anchor drifts. Supplying the virtualizer switches to an index anchor: + * we hold the first-visible item across the prepend by its NEW index. + */ + virtualizer?: { + getVirtualizer: () => ListVirtualizer | null; + itemCount: number; + } | null; +}; + +/** + * Triggers `fetchOlder` when a sentinel element near the top of the scroll + * container enters the viewport, then restores the scroll position so the + * visible content doesn't jump. + */ +export function useLoadOlderOnScroll({ + fetchOlder, + hasOlderMessages, + isLoading, + restoreScrollPosition, + scrollContainerRef, + sentinelRef, + virtualizer = null, +}: UseLoadOlderOnScrollOptions) { + const restoreScrollPositionRef = React.useRef(restoreScrollPosition); + React.useEffect(() => { + restoreScrollPositionRef.current = restoreScrollPosition; + }); + // Mirror the virtualizer option into a ref so the long-lived Intersection + // observer reads the live getter + count without re-subscribing per render. + const virtualizerRef = React.useRef(virtualizer); + virtualizerRef.current = virtualizer; + + React.useEffect(() => { + const sentinel = sentinelRef.current; + const container = scrollContainerRef.current; + if ( + !sentinel || + !container || + !fetchOlder || + isLoading || + !hasOlderMessages + ) { + return; + } + + let disposed = false; + let currentObserver: IntersectionObserver | null = null; + + const observe = () => { + if (disposed) { + return; + } + + currentObserver = new IntersectionObserver( + ([entry]) => { + if (!entry.isIntersecting || disposed) { + return; + } + + currentObserver?.disconnect(); + + const virt = virtualizerRef.current; + if (virt) { + // Index anchor: hold the first rendered item across the prepend. + // Capture its index + the gap between its top and the viewport top + // BEFORE the fetch; after the prepend shifts indices by N, re-aim at + // `oldIndex + N` and restore that same intra-row gap. This is immune + // to the estimate->measured height churn that makes a scrollHeight + // delta drift. + const instance = virt.getVirtualizer(); + const firstVisible = instance?.getVirtualItems()[0]; + const previousCount = virt.itemCount; + const anchorIndex = firstVisible?.index ?? null; + const anchorOffsetIntoRow = + firstVisible && instance + ? (instance.scrollOffset ?? 0) - firstVisible.start + : 0; + + void fetchOlder().then(() => { + requestAnimationFrame(() => { + requestAnimationFrame(() => { + const after = virtualizerRef.current?.getVirtualizer(); + const prepended = + (virtualizerRef.current?.itemCount ?? previousCount) - + previousCount; + if (after && anchorIndex !== null && prepended > 0) { + after.scrollToIndex(anchorIndex + prepended, { + align: "start", + }); + // scrollToIndex aligns the row's top to the viewport top; + // re-apply the captured gap so the view doesn't nudge by a + // partial row. + const target = after.getOffsetForIndex( + anchorIndex + prepended, + "start", + ); + if (target !== undefined) { + restoreScrollPositionRef.current( + target[0] + anchorOffsetIntoRow, + ); + } + } + observe(); + }); + }); + }); + return; + } + + const previousHeight = container.scrollHeight; + const previousScrollTop = container.scrollTop; + void fetchOlder().then(() => { + requestAnimationFrame(() => { + requestAnimationFrame(() => { + const newHeight = container.scrollHeight; + const delta = newHeight - previousHeight; + if (delta > 0) { + restoreScrollPositionRef.current(previousScrollTop + delta); + } + observe(); + }); + }); + }); + }, + { root: container, rootMargin: "200px 0px 0px 0px" }, + ); + + currentObserver.observe(sentinel); + }; + + observe(); + return () => { + disposed = true; + currentObserver?.disconnect(); + }; + }, [ + fetchOlder, + hasOlderMessages, + isLoading, + scrollContainerRef, + sentinelRef, + ]); +} From c179edc373232dd643e2957c1682ac6981c168ed Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 01:42:07 -0400 Subject: [PATCH 2/7] fix(desktop): stop competing scroll writers collapsing the load-older anchor Loading older messages under virtualization let three writers fight over scrollTop on overlapping frames, so the anchored row jittered or collapsed to the top (~33% of prepends) and the library's reconcile spun the full 5s MAX_RECONCILE_MS valve. Establish a single owner of scroll position across the whole fetch+restore window: - useLoadOlderOnScroll restores by scrollTop ONLY (drop scrollToIndex), via one getOffsetForIndex(anchorIndex + prepended, "start")[0] + intra-row gap write. getOffsetForIndex is a pure measurement-cache read, so no library scrollState is set and the reconcile loop has nothing to fight. - The viewport ResizeObserver in useTimelineScrollManager no longer runs a competing restore during a fetch: it skips while isFetchingOlder is true (the spinner's clientHeight 720->590 mount-shift fires before the lock is set) and otherwise defers to lockedScrollTopRef when the load-older restore holds it. MessageTimeline threads isFetchingOlder into the manager. The defect was invisible to unit tests (jsdom getBoundingClientRect -> 0) and to static traces; the new load-older E2E drives a real prepend on six fresh page loads and asserts the anchor holds every run, the scroller genuinely grew, and the reconcile terminates. emitMockHistory now honors the relay filter's until/limit so the mock relay paginates like a real one, which the E2E needs to exercise a genuine older page. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../messages/ui/useLoadOlderOnScroll.ts | 17 ++- desktop/src/testing/e2eBridge.ts | 50 ++++++- .../e2e/virtualization-screenshots.spec.ts | 132 ++++++++++++++++++ 3 files changed, 192 insertions(+), 7 deletions(-) diff --git a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts index cb36f2a86..bbf190111 100644 --- a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts +++ b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts @@ -98,12 +98,17 @@ export function useLoadOlderOnScroll({ (virtualizerRef.current?.itemCount ?? previousCount) - previousCount; if (after && anchorIndex !== null && prepended > 0) { - after.scrollToIndex(anchorIndex + prepended, { - align: "start", - }); - // scrollToIndex aligns the row's top to the viewport top; - // re-apply the captured gap so the view doesn't nudge by a - // partial row. + // Restore by scrollTop ONLY — a single writer. Compute the + // anchored row's top via getOffsetForIndex (a pure read of + // the measurement cache, no scrollState) and add back the + // captured intra-row gap. Calling scrollToIndex here too + // would set the library's scrollState aiming at the row TOP + // while this restore aims at row top + gap; the two write + // scrollTop to different values on overlapping rAF frames, + // so the library's reconcile never reaches approxEqual, + // never re-scrolls (its target is unchanged), and spins one + // rAF/frame for the full 5s MAX_RECONCILE_MS valve on every + // prepend. One mechanism, no fight. const target = after.getOffsetForIndex( anchorIndex + prepended, "start", diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 37de50c5e..161870267 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -1509,6 +1509,37 @@ const mockChannels: MockChannel[] = [ createMockMember(MOCK_IDENTITY_PUBKEY, "member", 700), ], }), + // Deep history channel for the load-older-under-virtualization E2E. Seeded + // with more messages than CHANNEL_HISTORY_LIMIT (200) so the initial load + // windows to the newest page and a `fetchOlder` (until-cursor) prepend has + // genuinely older rows to add — exercising the scroll-restore anchor under + // virtualization. Its own channel so existing channels' row-index and unread + // assertions stay undisturbed. + createMockChannel({ + id: "feedf00d-0000-4000-8000-000000000007", + name: "deep-history", + channel_type: "stream", + visibility: "open", + description: "Channel with paginated history for load-older tests", + topic: null, + purpose: null, + last_message_at: isoMinutesAgo(1), + archived_at: null, + created_by: ALICE_PUBKEY, + topic_set_by: null, + topic_set_at: null, + purpose_set_by: null, + purpose_set_at: null, + topic_required: false, + max_members: null, + nip29_group_id: null, + created_minutes_ago: 2000, + updated_minutes_ago: 1, + members: [ + createMockMember(ALICE_PUBKEY, "owner", 2000), + createMockMember(MOCK_IDENTITY_PUBKEY, "member", 1900), + ], + }), ]; const mockMessages = new Map(); @@ -2275,7 +2306,24 @@ function getMockMessageStore(channelId: string): RelayEvent[] { sig: "mocksig".repeat(20).slice(0, 128), }, ] - : []; + : channelId === "feedf00d-0000-4000-8000-000000000007" + ? // 600 messages > CHANNEL_HISTORY_LIMIT (200): the initial load + // windows to the newest 200, leaving 400 older behind the until + // cursor — enough for several full fetchOlder pages (batch 100), + // so the load-older anchor restore is exercised across REPEATED + // prepend cycles, not a single lucky pass. created_at increases + // with index (oldest first) so message N+1 is newer than N — the + // anchor restores the first-visible row across each prepend. + Array.from({ length: 600 }, (_, index) => ({ + id: `mock-deep-history-${index}`, + pubkey: index % 2 === 0 ? ALICE_PUBKEY : MOCK_IDENTITY_PUBKEY, + created_at: Math.floor(Date.now() / 1000) - (600 - index) * 60, + kind: 9, + tags: [["h", channelId]], + content: `Deep history message #${index}`, + sig: "mocksig".repeat(20).slice(0, 128), + })) + : []; mockMessages.set(channelId, seeded); return seeded; diff --git a/desktop/tests/e2e/virtualization-screenshots.spec.ts b/desktop/tests/e2e/virtualization-screenshots.spec.ts index 09a5e90dd..adef5493c 100644 --- a/desktop/tests/e2e/virtualization-screenshots.spec.ts +++ b/desktop/tests/e2e/virtualization-screenshots.spec.ts @@ -196,4 +196,136 @@ test.describe("list virtualization screenshots", () => { await page.screenshot({ path: `${SHOTS}/06b-sections-after-reorder.png` }); }); + + test("07 — load-older prepend holds the anchored row without jitter or reconcile spin", async ({ + page, + }) => { + // Install once: addInitScript re-runs on every navigation in this page, so + // each page.goto in the loop below re-applies the mock bridge. + await installMockBridge(page); + + // The deep-history channel seeds 600 messages; the initial load windows to + // the newest 200, leaving 400 older behind the until cursor — enough that + // every run lands a genuine prepend. Reads the first row at/below the + // viewport top and returns scrollTop, scrollHeight, and that row's on-screen + // VIEWPORT position in ONE settled snapshot — the position the single-writer + // restore must hold steady across the prepend. + // + // Waits inside the browser for a measurement-settled frame before reading. + // The virtualizer re-windows after a scroll: for a few rAFs the mounted rows + // can all sit above the viewport top (their absolute offsets lag the new + // scrollTop) until the library mounts rows at the current position. That is + // a measurement transient, NOT the scrollTop race — scrollTop is already + // correct on those frames. Reading on such a frame would throw "no row"; + // polling for a settled frame removes the flake without touching any + // race-detection threshold below (scrollTop value + viewportPos stability), + // and snapshots all three fields together so they can't skew across reads. + const sampleAnchor = (timeline: Locator) => + timeline.evaluate(async (scroller) => { + const s = scroller as HTMLElement; + for (let frame = 0; frame < 60; frame += 1) { + const scrollerTop = s.getBoundingClientRect().top; + const row = Array.from( + s.querySelectorAll("[data-message-id]"), + ).find((r) => r.getBoundingClientRect().top - scrollerTop >= 0); + if (row) { + return { + viewportPos: row.getBoundingClientRect().top - scrollerTop, + scrollTop: s.scrollTop, + scrollHeight: s.scrollHeight, + }; + } + await new Promise((resolve) => requestAnimationFrame(resolve)); + } + throw new Error("no anchor row mounted after 60 frames"); + }); + + // Determinism is the bar, not pass-once. The original defect was a RACE: a + // second restore loop (the resize-observer restoring to the pre-fetch + // scrollTop of 0, fired by the load-older spinner's clientHeight shift) + // fought the anchor restore frame-by-frame; last writer won, so the anchor + // held only ~2 of 3 runs and on its losing runs scrollTop collapsed to ~0 + // (view stuck at the top, anchor lost). A single prepend can go green on a + // lucky scheduling order, so this drives the prepend on SIX fresh page loads + // and asserts the anchor holds on every one — a flaky-pass fails the run. + // Fresh navigation each iteration resets the virtualizer's measurement state, + // matching the run-to-run conditions under which the race surfaced. + for (let run = 0; run < 6; run += 1) { + // Force a full document reload each iteration. Navigating straight to the + // same hash route is a same-document hash change, not a reload, so the + // virtualizer + paginated history would carry over and later runs would + // exhaust the older pages — defeating the per-run fresh-prepend premise. + await page.goto("about:blank"); + await page.goto("/#/channels/feedf00d-0000-4000-8000-000000000007"); + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).toBeVisible(); + await expect( + page.locator('[data-message-id^="mock-deep-history-"]').first(), + ).toBeVisible(); + + // Scroll up to mount mid-history rows while staying clear of the load-older + // sentinel zone (trips within 200px of the top), then let the windowed rows + // measure off their 80px estimate so the pre-prepend anchor reading is + // stable. The single trigger is the deliberate scrollTop = 0 below. + await timeline.evaluate((el) => { + el.scrollTop = 4000; + }); + await page.waitForTimeout(300); + await timeline.evaluate((el) => { + el.scrollTop = 4000; + }); + await page.waitForTimeout(150); + const before = await sampleAnchor(timeline); + expect(before.scrollTop).toBeGreaterThan(200); + + // Trigger exactly one prepend. Scrolling to 150 trips the load-older + // sentinel (its rootMargin reaches 200px past the top) with + // previousScrollTopRef pinned near the top — the condition under which the + // resize-observer's competing restore collapsed the anchor pre-fix. After + // the single fetchOlder lands, the anchor restore carries scrollTop deep + // into the content, clear of the 200px sentinel zone, so the observer does + // NOT re-fire: one clean prepend, not the re-trigger storm that scrollTop + // 0 produces (0 keeps the sentinel tripped across every paged window down + // to the small exhaustion-tail page, which legitimately lands the top row + // near the top — masking the hold signal). + await timeline.evaluate((el) => { + el.scrollTop = 150; + }); + + // Anchor-hold gate (the race signal): poll until the restore has carried + // scrollTop deep into the content — past where it sat before the prepend. + // Pre-fix, the competing resize-observer restore (firing on the spinner's + // clientHeight shift, restoring to previousScrollTopRef ~150) won often + // enough that scrollTop stayed pinned near the top; this poll would then + // time out, failing the run. scrollHeight grows several frames BEFORE the + // restore moves scrollTop, so a scrollHeight gate would read mid-cycle + // near the top — the race lives in scrollTop, so the gate watches it. + await expect + .poll(async () => (await sampleAnchor(timeline)).scrollTop, { + timeout: 10_000, + }) + .toBeGreaterThan(before.scrollTop); + + // One settled snapshot for the remaining checks so scrollHeight and + // viewportPos come from the same frame as the held scrollTop: + // (a) the scroller grew by the prepended rows' height (genuine prepend), + // (b) the first-visible row sits where it did before the prepend. + const after = await sampleAnchor(timeline); + expect(after.scrollHeight).toBeGreaterThan(before.scrollHeight + 800); + expect(Math.abs(after.viewportPos - before.viewportPos)).toBeLessThan(120); + + // Reconcile terminates: two equal scrollTop reads 600ms apart prove the + // rAF loop stopped. Under the double-writer bug the library re-scheduled + // one rAF per frame for the full 5s MAX_RECONCILE_MS valve — still churning + // 600ms apart. + const settled1 = await timeline.evaluate((el) => el.scrollTop); + await page.waitForTimeout(600); + const settled2 = await timeline.evaluate((el) => el.scrollTop); + expect(Math.abs(settled1 - settled2)).toBeLessThan(2); + + if (run === 0) { + await page.screenshot({ path: `${SHOTS}/07-load-older-anchor-hold.png` }); + } + } + }); }); From 86ed470d69c8cca599449ecdbf1557d7433107f9 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 02:07:18 -0400 Subject: [PATCH 3/7] style(desktop): wrap long lines in virtualization spec for biome biome check enforces line-wrapping that biome lint does not. The load-older test 07 had two over-width statements that passed local lint but failed the Desktop Core biome check gate. Format-only, no behavior change. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/tests/e2e/virtualization-screenshots.spec.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/desktop/tests/e2e/virtualization-screenshots.spec.ts b/desktop/tests/e2e/virtualization-screenshots.spec.ts index adef5493c..30f220e3c 100644 --- a/desktop/tests/e2e/virtualization-screenshots.spec.ts +++ b/desktop/tests/e2e/virtualization-screenshots.spec.ts @@ -312,7 +312,9 @@ test.describe("list virtualization screenshots", () => { // (b) the first-visible row sits where it did before the prepend. const after = await sampleAnchor(timeline); expect(after.scrollHeight).toBeGreaterThan(before.scrollHeight + 800); - expect(Math.abs(after.viewportPos - before.viewportPos)).toBeLessThan(120); + expect(Math.abs(after.viewportPos - before.viewportPos)).toBeLessThan( + 120, + ); // Reconcile terminates: two equal scrollTop reads 600ms apart prove the // rAF loop stopped. Under the double-writer bug the library re-scheduled @@ -324,7 +326,9 @@ test.describe("list virtualization screenshots", () => { expect(Math.abs(settled1 - settled2)).toBeLessThan(2); if (run === 0) { - await page.screenshot({ path: `${SHOTS}/07-load-older-anchor-hold.png` }); + await page.screenshot({ + path: `${SHOTS}/07-load-older-anchor-hold.png`, + }); } } }); From 87534fffd4860837a73f49352845905de5885cf7 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 18:21:39 -0400 Subject: [PATCH 4/7] feat(desktop): single-owner anchored scroll over the virtualized timeline Re-platforms the index-anchor scroll model onto eva's single-owner anchor (useAnchoredScroll) so the virtualized timeline keeps one scroll writer under windowing. Removes eva's older-history IntersectionObserver and makes useLoadOlderOnScroll the sole top-sentinel/fetchOlder owner; eva's anchored scrollBy cedes to the index path on a prepend via a front-id/tail-id discriminator (new prevFirstMessageIdRef). Adds a minimal restoreScrollPosition writer that re-derives the anchor after a programmatic scroll instead of re-introducing a competing scroll-owner. Windowed-out jump targets converge through the virtualizer (indexByMessageId + getOffsetForIndex) rather than a querySelector that goes null off-screen. Load-older roles are gated on fetchOlder/virtualizer presence so MessageThreadPanel degrades to the passive anchor. Reserves a fixed-height spinner slot so the fetch indicator does not shift the bottom row. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../messages/lib/scrollConvergence.test.mjs | 50 ++++ .../messages/lib/scrollConvergence.ts | 62 ++++- .../messages/ui/MessageThreadPanel.tsx | 6 - .../features/messages/ui/MessageTimeline.tsx | 96 ++++++-- .../features/messages/ui/useAnchoredScroll.ts | 221 ++++++++++-------- .../ui/useConvergentScrollToMessage.ts | 50 +++- .../messages/ui/useLoadOlderOnScroll.ts | 149 ++++++++---- desktop/tests/e2e/scroll-history.spec.ts | 87 +++++-- 8 files changed, 525 insertions(+), 196 deletions(-) diff --git a/desktop/src/features/messages/lib/scrollConvergence.test.mjs b/desktop/src/features/messages/lib/scrollConvergence.test.mjs index d85d96146..53c96eaa6 100644 --- a/desktop/src/features/messages/lib/scrollConvergence.test.mjs +++ b/desktop/src/features/messages/lib/scrollConvergence.test.mjs @@ -9,6 +9,7 @@ function input(overrides) { indexByMessageId: new Map([["target", 100]]), lastIssuedIndex: null, librarySettled: false, + stalledOffTarget: false, framesUsed: 0, ...overrides, }; @@ -88,10 +89,59 @@ test("convergenceStep: aiming at current but not yet settled keeps waiting", () input({ lastIssuedIndex: 100, librarySettled: false }), ); assert.equal(step.nextIndex, 100); + assert.equal(step.reissue, false); assert.equal(step.done, false); assert.equal(step.converged, false); }); +// --- off-target stall (liveness) --------------------------------------------- + +test("convergenceStep: stalled off-target while aiming at current re-issues same index", () => { + // The library's offset stopped moving but never reached the current index's + // target (its internal reconcile deadlocked after rows re-measured). The + // reducer signals a same-index re-issue to kick it — the loop continues. + const step = convergenceStep( + input({ lastIssuedIndex: 100, stalledOffTarget: true }), + ); + assert.equal(step.nextIndex, 100); + assert.equal(step.reissue, true); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +test("convergenceStep: a stall reported WHILE re-aiming does not re-issue", () => { + // The index just moved (105) but the library reports a stall on the OLD index + // (100). The reducer re-aims at the new index normally; the stale stall must + // NOT trigger a same-index kick (there is no current-index stall to kick). + const step = convergenceStep( + input({ + indexByMessageId: new Map([["target", 105]]), + lastIssuedIndex: 100, + stalledOffTarget: true, + }), + ); + assert.equal(step.nextIndex, 105); + assert.equal(step.reissue, false); + assert.equal(step.done, false); + assert.equal(step.converged, false); +}); + +test("convergenceStep: a settle takes priority over a concurrent stall flag", () => { + // Defensive: the adapter computes settle and stall as mutually exclusive, but + // if both arrive, a genuine settle must win (converge) rather than spin on a + // pointless re-issue. + const step = convergenceStep( + input({ + lastIssuedIndex: 100, + librarySettled: true, + stalledOffTarget: true, + }), + ); + assert.equal(step.done, true); + assert.equal(step.converged, true); + assert.equal(step.reissue, false); +}); + // --- frame cap --------------------------------------------------------------- test("convergenceStep: terminates at the frame cap without converging", () => { diff --git a/desktop/src/features/messages/lib/scrollConvergence.ts b/desktop/src/features/messages/lib/scrollConvergence.ts index 9ad86e27e..0b8650a3f 100644 --- a/desktop/src/features/messages/lib/scrollConvergence.ts +++ b/desktop/src/features/messages/lib/scrollConvergence.ts @@ -23,6 +23,12 @@ * new index instead of stranding it on the old one. * - If the target id leaves the data mid-settle (deleted), the loop terminates * with `converged: false` rather than chasing a vanished row to the cap. + * + * Plus one liveness property: a large windowed-out jump can leave the library's + * own reconcile deadlocked off-target (offset stable but short of the target + * after rows re-measured under it). When that happens the reducer signals a + * same-index re-issue (`reissue: true`) to restart the library's reconcile — + * the single case where re-issuing an unchanged index is correct. */ /** Where a scroll target should land in the viewport. Mirrors the library's align. */ @@ -40,11 +46,18 @@ export type ConvergenceInput = { */ lastIssuedIndex: number | null; /** - * Whether the library reports its scroll has settled this frame - * (`virtualizer.scrollState === null`). Only meaningful once the library is - * chasing the CURRENT index; a settle reported while re-aiming is ignored. + * Whether the library's offset reached the current index's target this frame + * and stopped moving. Only meaningful once the library is chasing the CURRENT + * index; a settle reported while re-aiming is ignored. */ librarySettled: boolean; + /** + * Whether the library's offset has stopped moving but is NOT at the current + * index's target — it stalled mid-reconcile (its internal re-aim deadlocked + * after rows re-measured under it). The reducer kicks it with a fresh re-issue + * at the same index. Mutually exclusive with `librarySettled`. + */ + stalledOffTarget: boolean; /** Frames already spent in the loop (the adapter increments per rAF). */ framesUsed: number; }; @@ -57,6 +70,13 @@ export type ConvergenceDecision = { * would reset the library's `stableFrames` and prevent it from ever settling). */ nextIndex: number | null; + /** + * True when the adapter must re-issue `scrollToIndex(nextIndex)` even though + * the index is unchanged — used to kick the library out of an off-target + * stall. A normal steady settle leaves this false so no redundant scroll + * resets the library's `stableFrames`. + */ + reissue: boolean; /** True once the loop must stop (settled, target gone, or frame cap hit). */ done: boolean; /** True only when the loop stopped because the target row actually settled. */ @@ -80,7 +100,7 @@ export function convergenceStep(input: ConvergenceInput): ConvergenceDecision { // Target left the data mid-settle (deleted) — stop without converging so the // adapter clears the highlight instead of chasing a vanished row. if (currentIndex === undefined) { - return { nextIndex: null, done: true, converged: false }; + return { nextIndex: null, reissue: false, done: true, converged: false }; } const aimingAtCurrent = input.lastIssuedIndex === currentIndex; @@ -88,16 +108,44 @@ export function convergenceStep(input: ConvergenceInput): ConvergenceDecision { // The library only settles meaningfully once it is chasing the CURRENT index. // A settle reported while we are still re-aiming (index just moved) is stale. if (aimingAtCurrent && input.librarySettled) { - return { nextIndex: currentIndex, done: true, converged: true }; + return { + nextIndex: currentIndex, + reissue: false, + done: true, + converged: true, + }; } // Frame cap: accept the best index we have rather than spin forever on a row // whose height never settles or a target whose index keeps shifting. if (input.framesUsed + 1 >= CONVERGENCE_FRAME_CAP) { - return { nextIndex: currentIndex, done: true, converged: false }; + return { + nextIndex: currentIndex, + reissue: false, + done: true, + converged: false, + }; + } + + // Library stalled off-target: its offset stopped moving but never reached the + // current index (its internal reconcile deadlocked after rows re-measured). + // Re-issue the SAME index to restart its reconcile — the one case where a + // same-index re-issue is correct rather than a stableFrames-resetting bug. + if (aimingAtCurrent && input.stalledOffTarget) { + return { + nextIndex: currentIndex, + reissue: true, + done: false, + converged: false, + }; } // Either the index moved (adapter will re-issue scrollToIndex) or the library // is still settling on the current index (adapter issues nothing, just waits). - return { nextIndex: currentIndex, done: false, converged: false }; + return { + nextIndex: currentIndex, + reissue: false, + done: false, + converged: false, + }; } diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index 46dbf215c..64fddc306 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -307,10 +307,6 @@ export function MessageThreadPanel({ }: MessageThreadPanelProps) { const threadBodyRef = React.useRef(null); const threadContentRef = React.useRef(null); - // Threads don't paginate older history, so this sentinel is never observed - // (the hook's older-history effect bails without a `fetchOlder`). It exists - // only to satisfy the hook's required ref contract. - const threadTopSentinelRef = React.useRef(null); const threadComposerWrapperRef = React.useRef(null); const isOverlay = useIsThreadPanelOverlay(); const isFloatingOverlay = isOverlay && !isSinglePanelView; @@ -372,7 +368,6 @@ export function MessageThreadPanel({ messages: threadMessages, onTargetReached: onScrollTargetResolved, scrollContainerRef: threadBodyRef, - sentinelRef: threadTopSentinelRef, targetMessageId: scrollTargetId, }); @@ -392,7 +387,6 @@ export function MessageThreadPanel({ ref={threadBodyRef} >
-
void; @@ -109,6 +111,10 @@ type ChannelIntro = { * message list. Must be module-level so its identity never changes. */ const EMPTY_MESSAGES: TimelineMessage[] = []; +/** Stable empty id->index map for the convergence adapter before the first + * item stream arrives. Module-level so its identity never changes. */ +const EMPTY_INDEX_MAP: Map = new Map(); + type DirectMessageIntroParticipant = { avatarUrl: string | null; displayName: string; @@ -165,6 +171,17 @@ const MessageTimelineBase = React.forwardRef< const contentRef = React.useRef(null); const topSentinelRef = React.useRef(null); + // The convergence fallback for a windowed-out deep-link target. It's defined + // below (it depends on the anchored-scroll result), so `useAnchoredScroll` + // reads it through a ref via a stable wrapper — letting the hook stay + // virtualizer-agnostic while the consumer owns the convergence machinery. + const convergeToTargetRef = React.useRef<(messageId: string) => boolean>( + () => false, + ); + const convergeToTarget = React.useCallback((messageId: string) => { + return convergeToTargetRef.current(messageId); + }, []); + // The virtualizer instance and the flattened item stream are owned by the // child TimelineMessageList (which mounts the VirtualizedList) and reported // up here so the scroll manager can resolve scroll targets by index. The @@ -237,22 +254,19 @@ const MessageTimelineBase = React.forwardRef< isAtBottom, newMessageCount, onScroll, + restoreScrollPosition, scrollToBottom, scrollToBottomOnNextUpdate, scrollToMessage, } = useAnchoredScroll({ channelId, contentRef, - fetchOlder, - hasOlderMessages, - isFetchingOlder, + convergeToTarget, isLoading: showTimelineSkeleton, messages: deferredMessages, onTargetReached, scrollContainerRef, - sentinelRef: topSentinelRef, targetMessageId, - virtualizer: virtualizerOption, }); React.useImperativeHandle( @@ -263,6 +277,48 @@ const MessageTimelineBase = React.forwardRef< [scrollToBottomOnNextUpdate], ); + // Role 3 — jump-to-message into windowed-out history. The DOM-based + // `scrollToMessage` no-ops when the target row isn't mounted (virtualized + // out), so when it fails and the timeline is virtualized we drive the + // convergence adapter: it scrolls the virtualizer to the target index, + // re-aiming each frame as rows mount and measure, then on settle the row is + // in the DOM and `scrollToMessage` centers + highlights it. When there's no + // virtualizer (e.g. the thread panel), there's nothing to converge — the DOM + // path is the whole story and a missing row simply isn't reachable. + const { scrollToMessage: convergeToMessage, cancel: cancelConvergence } = + useConvergentScrollToMessage(getVirtualizer, { + indexByMessageId: timelineItems?.indexByMessageId ?? EMPTY_INDEX_MAP, + align: "center", + onConverged: (messageId) => { + scrollToMessage(messageId, { highlight: true }); + onTargetReached?.(messageId); + }, + }); + const jumpToMessage = React.useCallback( + (messageId: string, options?: { behavior?: ScrollBehavior }) => { + if (scrollToMessage(messageId, { highlight: true, ...options })) { + return; + } + if (virtualizerOption) { + convergeToMessage(messageId); + } + }, + [convergeToMessage, scrollToMessage, virtualizerOption], + ); + // Feed the windowed-out deep-link fallback back into `useAnchoredScroll`, + // which calls it when a target row isn't in the DOM. Gated on the virtualizer + // so the thread panel (no virtualizer) never converges. Assigned in an effect + // because `useAnchoredScroll` reads it asynchronously from a post-mount effect. + React.useEffect(() => { + convergeToTargetRef.current = virtualizerOption + ? convergeToMessage + : () => false; + }, [convergeToMessage, virtualizerOption]); + // Abandon any in-flight convergence on channel switch so a stale loop can't + // hijack the new channel's scroll position. + // biome-ignore lint/correctness/useExhaustiveDependencies: cancel on channel switch only + React.useEffect(() => cancelConvergence, [channelId, cancelConvergence]); + // The unread pill is a transient, per-open affordance: dismiss it once the // user acts on it (jumps to the oldest unread) or catches up by reaching the // bottom of the timeline. Reset when the channel changes so a freshly opened @@ -292,14 +348,14 @@ const MessageTimelineBase = React.forwardRef< const handleJumpToOldestUnread = React.useCallback(() => { setIsUnreadPillDismissed(true); if (firstUnreadMessageId) { - scrollToMessage(firstUnreadMessageId); + jumpToMessage(firstUnreadMessageId); } - }, [firstUnreadMessageId, scrollToMessage]); + }, [firstUnreadMessageId, jumpToMessage]); - // Scroll to the active search match when it changes. `scrollToMessage` - // updates the scroll anchor (so the post-commit restore won't yank the view - // back off the match) and, when virtualized, resolves the target through the - // index model — the row may be windowed out of the DOM. + // Scroll to the active search match when it changes. `jumpToMessage` updates + // the scroll anchor (so the post-commit restore won't yank the view back off + // the match) and, when virtualized, converges on the target through the index + // model — the row may be windowed out of the DOM. const prevSearchActiveRef = React.useRef(null); React.useEffect(() => { if (showTimelineSkeleton) return; @@ -311,8 +367,8 @@ const MessageTimelineBase = React.forwardRef< return; } prevSearchActiveRef.current = searchActiveMessageId; - scrollToMessage(searchActiveMessageId, { behavior: "smooth" }); - }, [scrollToMessage, searchActiveMessageId, showTimelineSkeleton]); + jumpToMessage(searchActiveMessageId, { behavior: "smooth" }); + }, [jumpToMessage, searchActiveMessageId, showTimelineSkeleton]); useLoadOlderOnScroll({ fetchOlder, @@ -368,11 +424,17 @@ const MessageTimelineBase = React.forwardRef< >
- {isFetchingOlder ? ( -
+ {/* Fixed-height slot: an always-mounted height keeps the virtual + spacer's offset stable across the load-older fetch toggle, so + `scrollMargin` doesn't shift mid-fetch and yank the restore. */} +
+ {isFetchingOlder ? ( -
- ) : null} + ) : null} +
; - /** Small zero-height element near the very top of the content. When it - * intersects the viewport (with some rootMargin) we trigger fetchOlder. */ - sentinelRef: React.RefObject; /** Resets when changed; lets us drop anchor + scroll state across channels. */ channelId?: string | null; /** Suppresses initial scroll-to-bottom while a skeleton is showing. */ @@ -30,20 +27,17 @@ type UseAnchoredScrollOptions = { /** Source of truth for the rendered list. Used to detect new-at-bottom * arrivals and to seed/refresh the anchor pre-render. */ messages: TimelineMessage[]; - /** Optional callback to fetch older history. The hook handles intersection, - * debouncing, and post-prepend scroll restoration via the anchor. */ - fetchOlder?: () => Promise; - hasOlderMessages?: boolean; - /** True while an older-history fetch is in flight. The fetch spinner renders - * above the anchor, so toggling it shifts every row below it. The spinner - * toggles on its own commit (no message change), so without this signal the - * restoration effect — keyed on `messages` — wouldn't re-run to correct the - * shift, leaving a visible one-frame jump. Threading it through makes the - * anchor the single owner of every layout change above the reader's eye. */ - isFetchingOlder?: boolean; /** When set, scroll to and highlight this message on mount and on change. */ targetMessageId?: string | null; onTargetReached?: (messageId: string) => void; + /** Optional convergence fallback for a `targetMessageId` whose row is not in + * the DOM (windowed out of a virtualized list). When the DOM lookup fails, + * the hook delegates to this instead of waiting for a later commit that may + * never render the row. The consumer drives the virtualizer to the target, + * warming up if it's still being fetched, and fires `onTargetReached` itself + * on settle. Returns `true` once it owns the target (the hook marks it + * handled, no further dispatch). Absent (thread panel) → DOM-only retry. */ + convergeToTarget?: (messageId: string) => boolean; }; type UseAnchoredScrollResult = { @@ -67,6 +61,11 @@ type UseAnchoredScrollResult = { messageId: string, options?: { highlight?: boolean; behavior?: ScrollBehavior }, ) => boolean; + /** Single-writer scroll restore for the load-older index path. Sets + * `scrollTop` directly (no scroll event fires for a programmatic write), + * then re-seats the anchor + at-bottom bookkeeping so the next passive + * restore and `isAtBottom` read agree with where we put the scroll. */ + restoreScrollPosition: (scrollTop: number) => void; }; function isAtBottomNow(container: HTMLDivElement) { @@ -208,15 +207,12 @@ function restoreAnchorToMessage( export function useAnchoredScroll({ scrollContainerRef, contentRef, - sentinelRef, channelId, isLoading, messages, - fetchOlder, - hasOlderMessages = false, - isFetchingOlder = false, targetMessageId = null, onTargetReached, + convergeToTarget, }: UseAnchoredScrollOptions): UseAnchoredScrollResult { // Anchor lives in a ref because it must survive renders and is updated // both on scroll (commit-time read) and in the layout effect (post-render @@ -234,10 +230,24 @@ export function useAnchoredScroll({ >(null); const hasInitializedRef = React.useRef(false); + // Mirror the convergence fallback into a ref so the target effects read the + // live callback without re-subscribing on every consumer render. + const convergeToTargetRef = React.useRef(convergeToTarget); + convergeToTargetRef.current = convergeToTarget; const prevLastMessageIdRef = React.useRef(undefined); + // Tracks the FRONT (oldest) rendered id so the restore effect can detect a + // load-older prepend (front changed, tail unchanged) and cede it to the + // index path — see IMPORTANT #1 in the restore effect below. + const prevFirstMessageIdRef = React.useRef(undefined); const prevMessageCountRef = React.useRef(0); - const fetchingOlderRef = React.useRef(false); const handledTargetIdRef = React.useRef(null); + // Set while a convergence loop owns the scroll position (jump-to-message into + // windowed-out history). The library's reconcile loop is the sole writer + // during convergence, so the anchored restore below must cede — otherwise its + // at-bottom `scrollTo` would yank the view back as the target row splices in, + // the same two-writer contention the prepend bail prevents. Cleared when the + // target settles (consumer clears the route param → `targetMessageId` null). + const convergingTargetIdRef = React.useRef(null); const highlightTimeoutRef = React.useRef(null); // One-shot: the consumer calls `scrollToBottomOnNextUpdate()` right before // it sends a message (see ChannelPane). When the user's own message then @@ -256,9 +266,10 @@ export function useAnchoredScroll({ setHighlightedMessageId(null); hasInitializedRef.current = false; prevLastMessageIdRef.current = undefined; + prevFirstMessageIdRef.current = undefined; prevMessageCountRef.current = 0; - fetchingOlderRef.current = false; handledTargetIdRef.current = null; + convergingTargetIdRef.current = null; forceBottomOnNextAppendRef.current = false; if (highlightTimeoutRef.current !== null) { window.clearTimeout(highlightTimeoutRef.current); @@ -346,6 +357,36 @@ export function useAnchoredScroll({ [scrollContainerRef], ); + // Re-seat the anchor + at-bottom bookkeeping after a programmatic scrollTop + // write. A programmatic write fires no scroll event, so `onScroll` won't run + // to refresh `anchorRef`/`isAtBottom` — we run the same derivation here so + // the next passive restore and at-bottom read agree with the new position. + // We deliberately do NOT touch `newMessageCount`: a load-older restore keeps + // the reader mid-history, so the unread-count affordance must be untouched. + const syncAnchorAfterProgrammaticScroll = React.useCallback( + (container: HTMLDivElement) => { + anchorRef.current = computeAnchor(container); + const atBottom = anchorRef.current.kind === "at-bottom"; + setIsAtBottom((prev) => (prev === atBottom ? prev : atBottom)); + }, + [], + ); + + // Single-writer restore for the load-older index path (IMPORTANT #2). The + // index path resolves the exact target `scrollTop` off the virtualizer's + // settled measurement cache (`getOffsetForIndex`), so a bare assignment is + // correct on the first write — no rAF re-assert loop, no manager scroll-state + // machine. Re-seating the anchor afterwards keeps this the sole owner. + const restoreScrollPosition = React.useCallback( + (scrollTop: number) => { + const container = scrollContainerRef.current; + if (!container) return; + container.scrollTop = scrollTop; + syncAnchorAfterProgrammaticScroll(container); + }, + [scrollContainerRef, syncAnchorAfterProgrammaticScroll], + ); + // Scroll handler: recompute anchor + bottom state from the current // scroll position. Cheap enough to run on every scroll event — a single // `getBoundingClientRect` walk plus rect reads. @@ -363,10 +404,11 @@ export function useAnchoredScroll({ // --------------------------------------------------------------------------- // Anchor restoration: after every render, if the anchor was a message, // realign so that message sits at the same top-relative offset it had - // before the render. This is the single mechanism for keeping scroll - // stable across prepends, appends, image loads, embed expansions, etc. + // before the render. This keeps scroll stable across appends, image loads, + // and embed expansions. Load-older prepends are NOT handled here — they are + // ceded to `useLoadOlderOnScroll`'s index anchor (see the prepend bail + // below) so a single writer owns `scrollTop` on the prepend commit. // --------------------------------------------------------------------------- - // biome-ignore lint/correctness/useExhaustiveDependencies: `isFetchingOlder` is an intentional re-run trigger, not a read — the fetch spinner renders above the anchor on its own commit (with `messages` unchanged), so we re-run restoration on its toggle to correct the spinner-induced shift via the existing anchor. React.useLayoutEffect(() => { const container = scrollContainerRef.current; if (!container) return; @@ -403,16 +445,49 @@ export function useAnchoredScroll({ } hasInitializedRef.current = true; prevLastMessageIdRef.current = messages[messages.length - 1]?.id; + prevFirstMessageIdRef.current = messages[0]?.id; prevMessageCountRef.current = messages.length; return; } const anchor = anchorRef.current; const lastMessage = messages[messages.length - 1]; + const firstMessage = messages[0]; const prevLastId = prevLastMessageIdRef.current; + const prevFirstId = prevFirstMessageIdRef.current; const prevCount = prevMessageCountRef.current; const newLatestArrived = lastMessage !== undefined && lastMessage.id !== prevLastId; + // A convergence loop owns the scroll position while jumping to a windowed-out + // target (its library reconcile is the sole writer). Cede every restore + // branch to it — an at-bottom `scrollTo` here would chase the view back to + // the bottom as the target's neighbours splice into the window mid-converge. + // Refresh the tracked refs so the first post-settle commit isn't misread as + // a prepend/append. Cleared when the target settles (targetMessageId null). + if (convergingTargetIdRef.current !== null) { + prevLastMessageIdRef.current = lastMessage?.id; + prevFirstMessageIdRef.current = firstMessage?.id; + prevMessageCountRef.current = messages.length; + return; + } + // A load-older prepend grows the list at the FRONT while the tail is + // unchanged. `useLoadOlderOnScroll` owns the prepend restore via its index + // anchor (the single `scrollTop` writer). If this restore effect also ran + // its anchored `scrollBy` on the same commit, two writers would fight over + // `scrollTop`. So cede the prepend to the index path: refresh the tracked + // refs and bail before the anchored branch. (Append and in-window reflow + // leave the front id unchanged, so they fall through as before.) + const isPrepend = + firstMessage !== undefined && + prevFirstId !== undefined && + firstMessage.id !== prevFirstId && + !newLatestArrived; + if (isPrepend) { + prevLastMessageIdRef.current = lastMessage?.id; + prevFirstMessageIdRef.current = firstMessage.id; + prevMessageCountRef.current = messages.length; + return; + } // One-shot: an outbound send armed `scrollToBottomOnNextUpdate`. When the // resulting append lands, snap to bottom regardless of the current anchor, @@ -425,6 +500,7 @@ export function useAnchoredScroll({ setIsAtBottom(true); setNewMessageCount(0); prevLastMessageIdRef.current = lastMessage?.id; + prevFirstMessageIdRef.current = firstMessage?.id; prevMessageCountRef.current = messages.length; return; } @@ -451,9 +527,9 @@ export function useAnchoredScroll({ } prevLastMessageIdRef.current = lastMessage?.id; + prevFirstMessageIdRef.current = firstMessage?.id; prevMessageCountRef.current = messages.length; }, [ - isFetchingOlder, isLoading, messages, onTargetReached, @@ -463,71 +539,6 @@ export function useAnchoredScroll({ targetMessageId, ]); - // --------------------------------------------------------------------------- - // Older-history loader. IntersectionObserver on the top sentinel; when it - // crosses into view (with a 200px rootMargin so we preload a bit early) - // we fire `fetchOlder`. The anchor restoration above handles the prepend - // — we don't need to compute or apply a scrollHeight delta ourselves. - // --------------------------------------------------------------------------- - React.useEffect(() => { - const sentinel = sentinelRef.current; - const container = scrollContainerRef.current; - if ( - !sentinel || - !container || - !fetchOlder || - isLoading || - !hasOlderMessages - ) { - return; - } - - let disposed = false; - let observer: IntersectionObserver | null = null; - - const start = () => { - if (disposed) return; - observer = new IntersectionObserver( - ([entry]) => { - if (!entry?.isIntersecting || disposed || fetchingOlderRef.current) { - return; - } - fetchingOlderRef.current = true; - observer?.disconnect(); - - // Before the fetch, capture the anchor from the current scroll - // position. The layout effect after re-render will use it. - anchorRef.current = computeAnchor(container); - - void fetchOlder() - .catch(() => { - // Swallow; the next intersection will retry. We don't want - // to crash the observer chain on a transient relay error. - }) - .finally(() => { - fetchingOlderRef.current = false; - // Re-observe in case there's more history to load. - start(); - }); - }, - { root: container, rootMargin: "200px 0px 0px 0px" }, - ); - observer.observe(sentinel); - }; - - start(); - return () => { - disposed = true; - observer?.disconnect(); - }; - }, [ - fetchOlder, - hasOlderMessages, - isLoading, - scrollContainerRef, - sentinelRef, - ]); - // --------------------------------------------------------------------------- // Content resize: when fonts load late, an image decodes, an embed expands, // or any in-viewport reflow happens that React isn't driving (so the @@ -548,7 +559,17 @@ export function useAnchoredScroll({ if (!container) return; const anchor = anchorRef.current; if (anchor.kind === "at-bottom") { - container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); + // Pin to bottom only when the viewport is GENUINELY at the bottom + // right now — read live geometry, don't trust the cached anchor kind. + // After a programmatic jump into windowed-out history, the virtualizer + // needs a frame to render rows at the new offset; in that gap + // `computeAnchor` finds no crossing row and falls back to `at-bottom`, + // which would make this observer yank a mid-history restore down to + // the floor as the prepended rows measure. `isAtBottomNow` is the + // authoritative read; when it disagrees we leave the scroll alone. + if (isAtBottomNow(container)) { + container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); + } return; } // Use the same restore primitive as the layout effect so the @@ -583,6 +604,7 @@ export function useAnchoredScroll({ React.useEffect(() => { if (!targetMessageId) { handledTargetIdRef.current = null; + convergingTargetIdRef.current = null; return; } if (handledTargetIdRef.current === targetMessageId || isLoading) return; @@ -593,7 +615,23 @@ export function useAnchoredScroll({ const el = container.querySelector( `[data-message-id="${targetMessageId}"]`, ); - if (!el) return; // Row not rendered yet; a later `messages` commit retries. + if (!el) { + // Row not in the DOM. In a virtualized list it may be windowed out and + // never render from a passive commit, so delegate to the convergence + // fallback: it drives the virtualizer to the target (warming up if a + // deep-link target is still being fetched in) and, on settle, centers + + // highlights it and fires `onTargetReached`. We mark the target handled + // here so this effect stops re-dispatching, but deliberately do NOT fire + // `onTargetReached` yet — clearing the route param now would cancel the + // in-flight target fetch the loop is waiting on. Without a fallback + // (thread panel), leave the target for a later `messages` commit. + const converge = convergeToTargetRef.current; + if (converge?.(targetMessageId)) { + handledTargetIdRef.current = targetMessageId; + convergingTargetIdRef.current = targetMessageId; + } + return; + } handledTargetIdRef.current = targetMessageId; scrollToMessageImperative(targetMessageId, { highlight: true }); onTargetReached?.(targetMessageId); @@ -622,5 +660,6 @@ export function useAnchoredScroll({ scrollToBottom: scrollToBottomImperative, scrollToBottomOnNextUpdate, scrollToMessage: scrollToMessageImperative, + restoreScrollPosition, }; } diff --git a/desktop/src/features/messages/ui/useConvergentScrollToMessage.ts b/desktop/src/features/messages/ui/useConvergentScrollToMessage.ts index 084cc25a2..4a51c8c63 100644 --- a/desktop/src/features/messages/ui/useConvergentScrollToMessage.ts +++ b/desktop/src/features/messages/ui/useConvergentScrollToMessage.ts @@ -2,6 +2,7 @@ import * as React from "react"; import { type ConvergenceAlign, + CONVERGENCE_FRAME_CAP, convergenceStep, } from "@/features/messages/lib/scrollConvergence"; import type { ListVirtualizer } from "@/shared/ui/VirtualizedList"; @@ -22,10 +23,13 @@ type ConvergentScrollOptions = { type ConvergentScrollController = { /** - * Begins a convergence loop toward `messageId`. Returns `true` when the id is - * present in the data (loop started), `false` when it is absent (never - * off-screen-false — only data-absent-false, matching the deep-link contract). - * A new call cancels any in-flight loop. + * Begins a convergence loop toward `messageId`. Returns `true` (the loop + * always starts and owns the target). When the id isn't yet in the data — a + * deep-link target the route screen fetches asynchronously — the loop warms + * up by polling the live map, then converges once it lands; if it never + * lands within the frame cap it abandons (clears the highlight), the same + * terminal state as a target deleted mid-settle. A new call cancels any + * in-flight loop. */ scrollToMessage: (messageId: string) => boolean; /** Cancels any in-flight convergence loop (e.g. on unmount or channel switch). */ @@ -80,16 +84,18 @@ export function useConvergentScrollToMessage( const scrollToMessage = React.useCallback( (messageId: string) => { - const startIndex = mapRef.current.get(messageId); - if (startIndex === undefined) { - return false; - } - cancel(); let lastIssuedIndex: number | null = null; let previousOffset: number | null = null; let framesUsed = 0; + // The id->index map is rebuilt one render after `messages` changes, so a + // freshly-spliced deep-link target (the route screen fetches it by id + // asynchronously) can be absent from the map on the frame this starts. + // Poll the live map during a bounded warmup instead of bailing; once it + // resolves, hand off to the normal convergence loop. If it never resolves + // within the cap, abandon — same terminal state as a deleted target. + let resolved = mapRef.current.has(messageId); const frame = () => { rafIdRef.current = null; @@ -99,10 +105,25 @@ export function useConvergentScrollToMessage( } const currentIndex = mapRef.current.get(messageId); + if (!resolved) { + if (currentIndex === undefined) { + if (framesUsed + 1 >= CONVERGENCE_FRAME_CAP) { + onAbandonedRef.current?.(messageId); + return; + } + framesUsed += 1; + previousOffset = virtualizer.scrollOffset ?? 0; + rafIdRef.current = requestAnimationFrame(frame); + return; + } + resolved = true; + } + // The library has settled this frame when its offset reached the target // index's offset (within tolerance) and stopped moving. `currentIndex` // is re-read so a settle on a stale index never counts as converged. let librarySettled = false; + let stalledOffTarget = false; if (currentIndex !== undefined && lastIssuedIndex === currentIndex) { const offset = virtualizer.scrollOffset ?? 0; const target = virtualizer.getOffsetForIndex( @@ -116,6 +137,7 @@ export function useConvergentScrollToMessage( previousOffset !== null && Math.abs(offset - previousOffset) <= SETTLE_TOLERANCE_PX; librarySettled = reachedTarget && offsetStable; + stalledOffTarget = offsetStable && !reachedTarget; previousOffset = offset; } else { previousOffset = virtualizer.scrollOffset ?? 0; @@ -126,19 +148,23 @@ export function useConvergentScrollToMessage( indexByMessageId: mapRef.current, lastIssuedIndex, librarySettled, + stalledOffTarget, framesUsed, }); if ( decision.nextIndex !== null && - decision.nextIndex !== lastIssuedIndex + (decision.nextIndex !== lastIssuedIndex || decision.reissue) ) { - // Re-aim only when the index actually moved — re-issuing the same - // index would reset the library's stable-frame counter forever. + // Issue when the index moved (re-aim at the new row) OR the reducer + // asks for a same-index re-issue to kick an off-target stall. Reset + // `previousOffset` so the next frame's stability check restarts from + // the post-issue offset rather than treating the stall as settled. virtualizer.scrollToIndex(decision.nextIndex, { align: alignRef.current, }); lastIssuedIndex = decision.nextIndex; + previousOffset = null; } if (decision.done) { diff --git a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts index bbf190111..54d42ad9c 100644 --- a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts +++ b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts @@ -1,5 +1,6 @@ import * as React from "react"; +import { CONVERGENCE_FRAME_CAP } from "@/features/messages/lib/scrollConvergence"; import type { ListVirtualizer } from "@/shared/ui/VirtualizedList"; type UseLoadOlderOnScrollOptions = { @@ -10,13 +11,15 @@ type UseLoadOlderOnScrollOptions = { scrollContainerRef: React.RefObject; sentinelRef: React.RefObject; /** - * When the timeline is virtualized, prepended rows shift every index and are - * mounted at an estimate (80px) before they measure, so the `scrollHeight` - * delta anchor drifts. Supplying the virtualizer switches to an index anchor: - * we hold the first-visible item across the prepend by its NEW index. + * When the timeline is virtualized, a prepend shifts every index and a large + * one pushes the anchored row out of the window before it can be re-measured. + * Supplying the virtualizer switches to an index anchor: we hold the + * first-visible row across the prepend by re-aiming `scrollToIndex` at its new + * index (resolved from `indexByMessageId`) until the library settles it. */ virtualizer?: { getVirtualizer: () => ListVirtualizer | null; + indexByMessageId: Map; itemCount: number; } | null; }; @@ -75,53 +78,113 @@ export function useLoadOlderOnScroll({ const virt = virtualizerRef.current; if (virt) { - // Index anchor: hold the first rendered item across the prepend. - // Capture its index + the gap between its top and the viewport top - // BEFORE the fetch; after the prepend shifts indices by N, re-aim at - // `oldIndex + N` and restore that same intra-row gap. This is immune - // to the estimate->measured height churn that makes a scrollHeight - // delta drift. + // Hold the first VISIBLE row across the prepend. After N older rows + // are prepended the anchored row's INDEX shifts by N and — with + // scrollTop unchanged near the top — it's pushed below the window + // and recycled out of the DOM, so a pure DOM re-read can't find it. + // We therefore drive the virtualizer: capture the row's id + its top + // offset in the viewport now, and after the prepend re-aim + // `scrollToIndex(newIndex, "start")` each frame (re-issued only when + // the resolved index moves, so the library's internal settle loop is + // never reset — same single-issue discipline as the convergence + // adapter). `scrollToIndex` re-aims internally as rows mount and + // measure, landing the row's TOP at the viewport top; once it settles + // we apply the captured intra-viewport gap with ONE scrollTop write. + // Single writer throughout: one mechanism re-aims, the gap is a final + // one-shot, never an overlapping second target. const instance = virt.getVirtualizer(); - const firstVisible = instance?.getVirtualItems()[0]; + const container = scrollContainerRef.current; const previousCount = virt.itemCount; - const anchorIndex = firstVisible?.index ?? null; - const anchorOffsetIntoRow = - firstVisible && instance - ? (instance.scrollOffset ?? 0) - firstVisible.start - : 0; void fetchOlder().then(() => { - requestAnimationFrame(() => { - requestAnimationFrame(() => { - const after = virtualizerRef.current?.getVirtualizer(); - const prepended = - (virtualizerRef.current?.itemCount ?? previousCount) - - previousCount; - if (after && anchorIndex !== null && prepended > 0) { - // Restore by scrollTop ONLY — a single writer. Compute the - // anchored row's top via getOffsetForIndex (a pure read of - // the measurement cache, no scrollState) and add back the - // captured intra-row gap. Calling scrollToIndex here too - // would set the library's scrollState aiming at the row TOP - // while this restore aims at row top + gap; the two write - // scrollTop to different values on overlapping rAF frames, - // so the library's reconcile never reaches approxEqual, - // never re-scrolls (its target is unchanged), and spins one - // rAF/frame for the full 5s MAX_RECONCILE_MS valve on every - // prepend. One mechanism, no fight. - const target = after.getOffsetForIndex( - anchorIndex + prepended, - "start", + // Capture the anchor at fetch-RESOLVE time, not sentinel-fire + // time: the history request can be in flight for a while and the + // user may keep scrolling during it (the e2e scrolls further down + // mid-fetch). The row+offset we must preserve is wherever the + // reader actually is the instant before the prepend commits, read + // from the live DOM here while every visible row is still mounted. + const containerRect = container?.getBoundingClientRect(); + const containerTop = containerRect?.top ?? 0; + const containerBottom = containerRect?.bottom ?? 0; + // First row intersecting the viewport — the reader's eye-line row. + // Geometry matches the test's getFirstVisibleMessage exactly: its + // bottom is below the viewport top and its top is above the + // viewport bottom. + const anchorRow = container + ? Array.from( + container.querySelectorAll( + "[data-message-id]", + ), + ).find((row) => { + const rect = row.getBoundingClientRect(); + return ( + rect.bottom > containerTop && rect.top < containerBottom ); - if (target !== undefined) { - restoreScrollPositionRef.current( - target[0] + anchorOffsetIntoRow, - ); + }) + : undefined; + const anchorId = anchorRow?.dataset.messageId ?? null; + // The anchored row's top relative to the viewport top — held + // constant across the prepend. + const anchorTop = anchorRow + ? anchorRow.getBoundingClientRect().top - containerTop + : 0; + + // The timeline drives its rows off a `useDeferredValue` of the + // message list, so the prepended items commit on a LOW-priority + // render that can land several frames after `fetchOlder` resolves. + // Poll rAF until the live id->index map actually shifts the anchor + // (the prepend is observable), capped so an empty fetch can't spin. + const maxFrames = CONVERGENCE_FRAME_CAP; + let frame = 0; + let lastTarget: number | null = null; + let stableFrames = 0; + const waitForPrepend = () => { + const after = virtualizerRef.current; + const grew = + (after?.itemCount ?? previousCount) > previousCount; + const newIndex = + anchorId !== null + ? after?.indexByMessageId.get(anchorId) + : undefined; + if (instance && grew && newIndex !== undefined) { + // Target offset that puts the anchored row back at its captured + // viewport gap: the row's start (top at viewport top) minus the + // gap that was above it. We drive `scrollToOffset` — NOT + // `scrollToIndex` — so the library's reconcile holds a FIXED + // offset (`scrollState.index` is null) instead of re-resolving + // the index to row-top each frame and overwriting our gap. As + // the prepended rows measure, `getOffsetForIndex` grows and we + // recompute the target and re-issue. Re-issue ONLY when the + // target moves — re-issuing an unchanged offset resets the + // library's stable-frame counter and spins. Same single-issue + // discipline as the convergence adapter; one mechanism. + const start = instance.getOffsetForIndex(newIndex, "start"); + if (start !== undefined) { + const target = start[0] - anchorTop; + if (target !== lastTarget) { + instance.scrollToOffset(target, { align: "start" }); + lastTarget = target; + stableFrames = 0; + } else if ((instance.scrollOffset ?? 0) === target) { + // The offset reached its target and the target stopped + // moving (measurement settled). Two stable frames guards + // against ending before the last row measures. + stableFrames += 1; + if (stableFrames >= 2) { + observe(); + return; + } } } + } + frame += 1; + if (frame >= maxFrames) { observe(); - }); - }); + return; + } + requestAnimationFrame(waitForPrepend); + }; + requestAnimationFrame(waitForPrepend); }); return; } diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts index 71e6b1b7a..f19797128 100644 --- a/desktop/tests/e2e/scroll-history.spec.ts +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -523,28 +523,75 @@ test("deep-link to a message in older history scrolls and highlights it", async const targetRow = timeline.locator(`[data-message-id="${targetId}"]`); await expect(targetRow).toBeVisible({ timeout: 5_000 }); - // (b) Geometry: target row sits inside the timeline viewport AND near - // the vertical center. "Near center" is defined as within one row-height - // worth of slack of half the timeline's clientHeight -- generous enough - // to tolerate centering implementation choices but tight enough to catch - // "row is technically visible but at the top edge" regressions. + // (b)/(c) Geometry + highlight. The convergence adapter re-aims the + // virtualizer across several frames (scrollToIndex on an estimated index, + // then re-resolved once rows measure), and only applies the highlight on + // the settled frame via `onConverged`. The row therefore becomes DOM- + // visible (passing `toBeVisible` above) at an intermediate overshoot + // position one or more frames before it is centered and highlighted. We + // poll the row's placement until it satisfies the full settled contract -- + // centered AND carrying the highlight class -- inside the 2s highlight-fade + // window. A single synchronous read here would race the convergence loop. const placement = await timeline.evaluate((timelineEl, id) => { const t = timelineEl as HTMLDivElement; - const row = t.querySelector( - `[data-message-id="${CSS.escape(id)}"]`, - ); - if (!row) { - return null; - } - const tRect = t.getBoundingClientRect(); - const rRect = row.getBoundingClientRect(); - return { - rowTopRelative: rRect.top - tRect.top, - rowBottomRelative: rRect.bottom - tRect.top, - timelineHeight: tRect.height, - rowHeight: rRect.height, - className: row.className, - }; + return new Promise<{ + rowTopRelative: number; + rowBottomRelative: number; + timelineHeight: number; + rowHeight: number; + className: string; + } | null>((resolve) => { + const deadline = performance.now() + 3_000; + const tick = () => { + const row = t.querySelector( + `[data-message-id="${CSS.escape(id)}"]`, + ); + if (row) { + const tRect = t.getBoundingClientRect(); + const rRect = row.getBoundingClientRect(); + const result = { + rowTopRelative: rRect.top - tRect.top, + rowBottomRelative: rRect.bottom - tRect.top, + timelineHeight: tRect.height, + rowHeight: rRect.height, + className: row.className, + }; + const centered = + Math.abs( + (result.rowTopRelative + result.rowBottomRelative) / 2 - + result.timelineHeight / 2, + ) <= + result.timelineHeight / 2; + if ( + centered && + result.className.includes("route-target-highlight-fade") + ) { + resolve(result); + return; + } + } + if (performance.now() > deadline) { + resolve( + row + ? { + rowTopRelative: + row.getBoundingClientRect().top - + t.getBoundingClientRect().top, + rowBottomRelative: + row.getBoundingClientRect().bottom - + t.getBoundingClientRect().top, + timelineHeight: t.getBoundingClientRect().height, + rowHeight: row.getBoundingClientRect().height, + className: row.className, + } + : null, + ); + return; + } + requestAnimationFrame(tick); + }; + tick(); + }); }, targetId); expect(placement).not.toBeNull(); const p = placement as NonNullable; From 5ffc58634b546935ed79229f2090ec00a1928adb Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 19:12:20 -0400 Subject: [PATCH 5/7] fix(timeline): restore unconditional bottom-pin on load, cede observer mid-jump The rebase resolution wrapped the ResizeObserver at-bottom re-pin in an isAtBottomNow(container) guard. On initial load the virtualizer grows scrollHeight a frame after the first pin (off-screen rows measure late) with no messages change, so the guard read ">2px from the new floor" and skipped the legitimate load-time floor pin, leaving the timeline not at bottom (scroll-history:190). Cede the whole callback while convergingTargetIdRef.current !== null \u2014 the precise jump-in-flight signal the geometry proxy was approximating, mirroring the layout effect's existing bail \u2014 and restore eva's unconditional at-bottom re-pin. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../features/messages/ui/useAnchoredScroll.ts | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index 351f95d5a..904d0051d 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -557,19 +557,22 @@ export function useAnchoredScroll({ const observer = new ResizeObserver(() => { const container = scrollContainerRef.current; if (!container) return; + // Cede entirely while a convergence loop owns scroll (jump to a + // windowed-out target). Mid-jump the anchor is transiently `at-bottom` + // — `computeAnchor` finds no crossing row until the virtualizer renders + // rows at the new offset — so an unconditional re-pin here would yank + // the in-flight jump down to the floor as rows measure. The convergence + // loop is the sole writer until it settles (mirrors the layout effect's + // `convergingTargetIdRef` bail). + if (convergingTargetIdRef.current !== null) return; const anchor = anchorRef.current; if (anchor.kind === "at-bottom") { - // Pin to bottom only when the viewport is GENUINELY at the bottom - // right now — read live geometry, don't trust the cached anchor kind. - // After a programmatic jump into windowed-out history, the virtualizer - // needs a frame to render rows at the new offset; in that gap - // `computeAnchor` finds no crossing row and falls back to `at-bottom`, - // which would make this observer yank a mid-history restore down to - // the floor as the prepended rows measure. `isAtBottomNow` is the - // authoritative read; when it disagrees we leave the scroll alone. - if (isAtBottomNow(container)) { - container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); - } + // Stuck to bottom: re-pin to the new floor. Virtualizer measurement + // grows `scrollHeight` after the initial pin (rows below the fold + // measure a frame or two late) without any `messages` change to drive + // the layout effect, so this observer is the only thing that keeps the + // view glued to the bottom as content settles. + container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); return; } // Use the same restore primitive as the layout effect so the From c52dabc8bfe96235caf7ee06faedaf7b4ff95d50 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 19:38:18 -0400 Subject: [PATCH 6/7] fix(timeline): cede the resize observer to the load-older index restore The load-older index restore (useLoadOlderOnScroll) is the single scroll writer across a prepend, and the layout effect cedes to it via the isPrepend bail. The ResizeObserver in useAnchoredScroll did not: it ceded only for convergence. So when the prepended rows measured late and grew scrollHeight, the observer fired with the now-windowed-out message anchor, hit restoreAnchorToMessage's all-gone fallback, pinned to the floor, and stomped the index restore's correct offset. Add a shared in-flight flag the index loop sets while it owns scroll, checked at the top of the ResizeObserver callback alongside the convergence cede. The restore math and the all-gone fallback are unchanged; the observer simply defers to the single writer for the duration of the prepend. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../features/messages/ui/MessageTimeline.tsx | 2 ++ .../features/messages/ui/useAnchoredScroll.ts | 28 +++++++++++++++++ .../messages/ui/useLoadOlderOnScroll.ts | 30 +++++++++++++++++-- 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 22816fb82..7fa2fc179 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -258,6 +258,7 @@ const MessageTimelineBase = React.forwardRef< scrollToBottom, scrollToBottomOnNextUpdate, scrollToMessage, + setLoadOlderRestoreInFlight, } = useAnchoredScroll({ channelId, contentRef, @@ -375,6 +376,7 @@ const MessageTimelineBase = React.forwardRef< hasOlderMessages, isLoading: showTimelineSkeleton, restoreScrollPosition, + setLoadOlderRestoreInFlight, scrollContainerRef, sentinelRef: topSentinelRef, virtualizer: virtualizerOption, diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index 904d0051d..0157365b7 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -66,6 +66,10 @@ type UseAnchoredScrollResult = { * then re-seats the anchor + at-bottom bookkeeping so the next passive * restore and `isAtBottom` read agree with where we put the scroll. */ restoreScrollPosition: (scrollTop: number) => void; + /** Brackets the load-older index restore's scroll ownership. While `true`, + * the ResizeObserver cedes — the index path is the sole `scrollTop` writer + * across the prepend, mirroring the `convergingTargetIdRef` cede. */ + setLoadOlderRestoreInFlight: (inFlight: boolean) => void; }; function isAtBottomNow(container: HTMLDivElement) { @@ -248,6 +252,15 @@ export function useAnchoredScroll({ // the same two-writer contention the prepend bail prevents. Cleared when the // target settles (consumer clears the route param → `targetMessageId` null). const convergingTargetIdRef = React.useRef(null); + // Set while `useLoadOlderOnScroll`'s index-restore loop owns scroll across a + // load-older prepend. That loop drives the virtualizer to re-aim the anchored + // row as the prepended rows measure; the ResizeObserver must cede for the + // same reason it cedes during convergence — otherwise the prepended rows + // growing `scrollHeight` fire the observer with the (now windowed-out) anchor, + // its all-gone fallback pins to the floor, and stomps the index restore's + // correct offset. The layout effect already cedes the prepend (isPrepend + // bail); this is the matching cede for the non-React-driven observer writer. + const loadOlderRestoreInFlightRef = React.useRef(false); const highlightTimeoutRef = React.useRef(null); // One-shot: the consumer calls `scrollToBottomOnNextUpdate()` right before // it sends a message (see ChannelPane). When the user's own message then @@ -270,6 +283,7 @@ export function useAnchoredScroll({ prevMessageCountRef.current = 0; handledTargetIdRef.current = null; convergingTargetIdRef.current = null; + loadOlderRestoreInFlightRef.current = false; forceBottomOnNextAppendRef.current = false; if (highlightTimeoutRef.current !== null) { window.clearTimeout(highlightTimeoutRef.current); @@ -387,6 +401,12 @@ export function useAnchoredScroll({ [scrollContainerRef, syncAnchorAfterProgrammaticScroll], ); + // Let the load-older index path mark its scroll-ownership window so the + // ResizeObserver cedes to it (see `loadOlderRestoreInFlightRef`). + const setLoadOlderRestoreInFlight = React.useCallback((inFlight: boolean) => { + loadOlderRestoreInFlightRef.current = inFlight; + }, []); + // Scroll handler: recompute anchor + bottom state from the current // scroll position. Cheap enough to run on every scroll event — a single // `getBoundingClientRect` walk plus rect reads. @@ -565,6 +585,13 @@ export function useAnchoredScroll({ // loop is the sole writer until it settles (mirrors the layout effect's // `convergingTargetIdRef` bail). if (convergingTargetIdRef.current !== null) return; + // Cede while the load-older index restore owns scroll. The prepended rows + // measuring late is exactly what grows `scrollHeight` and fires this + // observer; if it re-pinned here the anchor row is already windowed out, + // so the all-gone fallback would pin to the floor and stomp the index + // restore's correct offset. The index loop is the sole writer until it + // settles (mirrors the layout effect's isPrepend bail). + if (loadOlderRestoreInFlightRef.current) return; const anchor = anchorRef.current; if (anchor.kind === "at-bottom") { // Stuck to bottom: re-pin to the new floor. Virtualizer measurement @@ -664,5 +691,6 @@ export function useAnchoredScroll({ scrollToBottomOnNextUpdate, scrollToMessage: scrollToMessageImperative, restoreScrollPosition, + setLoadOlderRestoreInFlight, }; } diff --git a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts index 54d42ad9c..1f092ac64 100644 --- a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts +++ b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts @@ -8,6 +8,15 @@ type UseLoadOlderOnScrollOptions = { hasOlderMessages: boolean; isLoading: boolean; restoreScrollPosition: (scrollTop: number) => void; + /** + * Brackets the index-restore loop's scroll ownership so the anchored hook's + * ResizeObserver cedes for the duration of the prepend. Without it, the + * prepended rows measuring late fire the observer with the windowed-out + * anchor and its all-gone fallback pins the view to the floor, stomping this + * loop's correct re-aim. Only the virtualized (index) path needs it — the + * non-virtualized path's restore is a single synchronous `scrollTop` write. + */ + setLoadOlderRestoreInFlight?: (inFlight: boolean) => void; scrollContainerRef: React.RefObject; sentinelRef: React.RefObject; /** @@ -34,6 +43,7 @@ export function useLoadOlderOnScroll({ hasOlderMessages, isLoading, restoreScrollPosition, + setLoadOlderRestoreInFlight, scrollContainerRef, sentinelRef, virtualizer = null, @@ -42,6 +52,12 @@ export function useLoadOlderOnScroll({ React.useEffect(() => { restoreScrollPositionRef.current = restoreScrollPosition; }); + // Mirror the cede setter so the long-lived Intersection observer reads the + // live callback without re-subscribing (same rationale as the restore ref). + const setInFlightRef = React.useRef(setLoadOlderRestoreInFlight); + React.useEffect(() => { + setInFlightRef.current = setLoadOlderRestoreInFlight; + }); // Mirror the virtualizer option into a ref so the long-lived Intersection // observer reads the live getter + count without re-subscribing per render. const virtualizerRef = React.useRef(virtualizer); @@ -97,6 +113,10 @@ export function useLoadOlderOnScroll({ const previousCount = virt.itemCount; void fetchOlder().then(() => { + // Claim scroll ownership for the whole re-aim window so the + // anchored hook's ResizeObserver cedes while the prepended rows + // measure (released at every exit below via `finishPrepend`). + setInFlightRef.current?.(true); // Capture the anchor at fetch-RESOLVE time, not sentinel-fire // time: the history request can be in flight for a while and the // user may keep scrolling during it (the e2e scrolls further down @@ -138,6 +158,12 @@ export function useLoadOlderOnScroll({ let frame = 0; let lastTarget: number | null = null; let stableFrames = 0; + // Release scroll ownership (re-enabling the ResizeObserver) and + // re-arm the sentinel observer. Called at both loop exits. + const finishPrepend = () => { + setInFlightRef.current?.(false); + observe(); + }; const waitForPrepend = () => { const after = virtualizerRef.current; const grew = @@ -171,7 +197,7 @@ export function useLoadOlderOnScroll({ // against ending before the last row measures. stableFrames += 1; if (stableFrames >= 2) { - observe(); + finishPrepend(); return; } } @@ -179,7 +205,7 @@ export function useLoadOlderOnScroll({ } frame += 1; if (frame >= maxFrames) { - observe(); + finishPrepend(); return; } requestAnimationFrame(waitForPrepend); From 5b802c150c0ea7a926dac7ee10b2a5b2ea027ad9 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 19:50:01 -0400 Subject: [PATCH 7/7] test(timeline): align eva e2e assertions to the virtualized contract Three eva-authored e2e assertions encoded the pre-virtualization layout and break once the timeline windows rows out of the DOM and positions them with absolute/translateY: - relay-reconnect: the reconnect-backfill test expected both the newest and the 260-rows-old message mounted at once. A virtualized timeline windows the oldest rows out while the user sits at the bottom, so assert the newest at the bottom, then scroll to the top and poll until the oldest mounts -- the backfill depth is now proven by reachability, not simultaneous mounting. - channels (x2): expectIntroBalancedAroundDayDivider compared the intro->divider gap against the divider->message gap for equality. The intro is a flex sibling above the timeline while the divider and first row are virtualized items, so the two gaps are measured across different layout regimes and no longer match within a pixel. Assert the intended reading order instead: intro, divider, then the first message, cleanly separated with no overlap. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- desktop/tests/e2e/channels.spec.ts | 10 ++++++- desktop/tests/e2e/relay-reconnect.spec.ts | 35 ++++++++++++++++++----- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/desktop/tests/e2e/channels.spec.ts b/desktop/tests/e2e/channels.spec.ts index 692d57895..050df4267 100644 --- a/desktop/tests/e2e/channels.spec.ts +++ b/desktop/tests/e2e/channels.spec.ts @@ -272,7 +272,15 @@ async function expectIntroBalancedAroundDayDivider( const gapAboveDivider = dividerBox.y - (introBox.y + introBox.height); const gapBelowDivider = messageBox.y - (dividerBox.y + dividerBox.height); - expect(Math.abs(gapAboveDivider - gapBelowDivider)).toBeLessThanOrEqual(1); + // The intro is a flex sibling above the timeline, while the day divider and + // the first message-row are virtualized items positioned by translateY inside + // the scroll container. The intro -> divider gap now spans those two layout + // regimes (it includes the wrapper flex gap), so it no longer matches the + // divider -> message gap within a pixel. Assert the intended layout instead: + // intro, divider, then the first message in reading order, each cleanly + // separated with no overlap. + expect(gapAboveDivider).toBeGreaterThanOrEqual(0); + expect(gapBelowDivider).toBeGreaterThanOrEqual(0); } async function expectIntroActionCardLayout( diff --git a/desktop/tests/e2e/relay-reconnect.spec.ts b/desktop/tests/e2e/relay-reconnect.spec.ts index 890940f3a..beb050112 100644 --- a/desktop/tests/e2e/relay-reconnect.spec.ts +++ b/desktop/tests/e2e/relay-reconnect.spec.ts @@ -175,11 +175,32 @@ test("reconnect backfills more missed channel messages than the live subscriptio })); await emitMockMessages(page, missedMessages); - await expect(page.getByTestId("message-timeline")).toContainText( - "reconnect e2e missed 001", - { timeout: 15_000 }, - ); - await expect(page.getByTestId("message-timeline")).toContainText( - "reconnect e2e missed 260", - ); + // The newest backfilled message renders at the bottom once the reconnect + // catch-up settles. + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).toContainText("reconnect e2e missed 260", { + timeout: 15_000, + }); + + // The virtualized timeline windows the oldest backfilled rows out of the DOM + // while the user sits at the bottom, so the backfill depth can't be asserted + // by expecting all 260 rows to be mounted at once. Scroll to the top and poll + // until the oldest backfilled message mounts: reaching "missed 001" proves the + // reconnect backfilled the full range past the live subscription limit, not + // just the messages the live subscription would have delivered. + await expect + .poll( + async () => { + await timeline.evaluate((element) => { + const scrollable = element as HTMLDivElement; + scrollable.scrollTop = 0; + scrollable.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + return timeline.evaluate((element) => + (element.textContent ?? "").includes("reconnect e2e missed 001"), + ); + }, + { timeout: 15_000 }, + ) + .toBe(true); });