From 58d785ce340fd0e3b6042ca8e7805315f73abe38 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 17 Jun 2026 23:34:25 -0400 Subject: [PATCH] fix(desktop): thread virtualItems into memoized timeline list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wheeling to the top of a channel left the rendered window frozen on the newest ~11 rows — users could not scroll back through history. MessageTimeline re-renders on every scroll via useVirtualizer's onChange, but TimelineMessageList is React.memo and read getVirtualItems() internally through the virtualizer's stable mutable ref. Shallow compare cannot see the range walk through that ref, so the memo skipped re-render while the virtual range advanced underneath. Thread virtualItems as a fresh-array prop from the hook-owner into the memoized renderer so memo sees a new reference each render and re-renders in lockstep — the documented react-virtual pattern. The e2e fixture now honors since/until/limit to model a true older-page gap, and a new anchor test asserts the tracked row's getBoundingClientRect().top holds within 8px across a scroll-back. Co-authored-by: Will Pfleger Signed-off-by: Will Pfleger --- .../features/messages/ui/MessageTimeline.tsx | 14 ++- .../messages/ui/TimelineMessageList.tsx | 12 +- desktop/src/testing/e2eBridge.ts | 114 ++++++++++++++---- .../tests/e2e/timeline-scroll-anchor.spec.ts | 77 ++++++++++++ 4 files changed, 189 insertions(+), 28 deletions(-) diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 63c4e9ac4..f6a420b44 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -246,12 +246,13 @@ export const MessageTimeline = React.memo(function MessageTimeline({ // `position:absolute`, so a per-row `position:sticky` cannot work; instead we // derive the day group that owns the topmost rendered row and paint ONE // header in a sibling layer pinned below the channel chrome, mirroring the - // legacy `sticky` DayDivider. Reading `getVirtualItems()[0]` each render keeps - // it live — the virtualizer re-renders this component on every scroll/measure. - const activeDay = selectActiveDayHeading( - items, - virtualizer.getVirtualItems()[0]?.index, - ); + // legacy `sticky` DayDivider. Reading `getVirtualItems()` each render keeps + // it live — the virtualizer re-renders this component on every scroll/measure + // — and the same array is handed to the row renderer so it re-renders in + // lockstep (its memo cannot see range changes through the stable virtualizer + // ref otherwise). + const virtualItems = virtualizer.getVirtualItems(); + const activeDay = selectActiveDayHeading(items, virtualItems[0]?.index); // Deep-link to `targetMessageId` once it resolves against the rendered // snapshot. `resolveDeepLinkTarget` reads the same `deferredMessages` the @@ -623,6 +624,7 @@ export const MessageTimeline = React.memo(function MessageTimeline({ spacerRef={spacerRef} unfollowThreadById={unfollowThreadById} virtualizer={virtualizer} + virtualItems={virtualItems} /> ) : null} diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index af8acffa3..43296afa8 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -77,6 +77,15 @@ type TimelineMessageListProps = { * also owns measurement; this component is a pure renderer of its rows. */ virtualizer: ChatVirtualizer; + /** + * The virtual rows to render, read from `virtualizer.getVirtualItems()` by + * the hook owner (the timeline). Passed as a prop — not read off the + * virtualizer here — so this memoized renderer re-renders when the rendered + * range changes on scroll: the virtualizer is a stable mutable ref that a + * shallow memo compare cannot see through, so reading the range internally + * would freeze the rows on a scroll that changes no other prop. + */ + virtualItems: ReturnType; /** The flat virtual-item list the virtualizer's `count` mirrors. */ items: TimelineVirtualItem[]; /** @@ -116,6 +125,7 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ threadUnreadCounts, unfollowThreadById, virtualizer, + virtualItems, items, topPad, spacerRef, @@ -318,7 +328,7 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ ref={spacerRef as React.Ref} style={{ height: `${virtualizer.getTotalSize() + topPad}px` }} > - {virtualizer.getVirtualItems().map((virtualRow) => ( + {virtualItems.map((virtualRow) => (
({ + id: `mock-load-older-${index}`, + pubkey: index % 2 === 0 ? ALICE_PUBKEY : MOCK_IDENTITY_PUBKEY, + created_at: Math.floor(Date.now() / 1000) - (260 - index) * 60, + kind: 9, + tags: [["h", channelId]], + content: `Backfilled message #${index}`, + sig: "mocksig".repeat(20).slice(0, 128), + })) + : channelId === "94a444a4-c0a3-5966-ab05-530c6ddc2301" + ? [ + // Charlie is a `bot` member of #agents (see channel seed), so this + // message renders with role="bot" — the surface whose avatar opens + // a managed-agent profile panel / hover popover with active-turn + // badges. #agents has no message-row index assertions, so seeding + // here is safe for existing specs. + { + id: "mock-agents-charlie", + pubkey: CHARLIE_PUBKEY, + created_at: Math.floor(Date.now() / 1000) - 90, + kind: 9, + tags: [["h", channelId]], + content: "Indexing the channel catalog now.", + sig: "mocksig".repeat(20).slice(0, 128), + }, + ] + : []; mockMessages.set(channelId, seeded); return seeded; } -function emitMockHistory(socket: MockSocket, subId: string, channelId: string) { - const events = getMockMessageStore(channelId); +function emitMockHistory( + socket: MockSocket, + subId: string, + channelId: string, + filter?: MockFilter, +) { + let events = getMockMessageStore(channelId); + + // Mirror the real relay's window semantics so load-older pagination returns a + // real, non-overlapping older page (a genuine prepend) instead of the whole + // store. `until` is inclusive; the relay returns the newest `limit` events at + // or before it, so we select from the tail after sorting newest-first. + if (filter?.until !== undefined) { + const until = filter.until; + events = events.filter((event) => event.created_at <= until); + } + // `since` is inclusive and bounds the window from below. The live channel + // subscription sends `since: now` (relayClientSession `subscribeToChannelLive`) + // precisely so the relay returns zero historical backlog — without honoring it + // the mock would dump the whole store on channel open, making load-older a + // re-emit of already-present rows instead of a true older-page gap-fill. + if (filter?.since !== undefined) { + const since = filter.since; + events = events.filter((event) => event.created_at >= since); + } + if (filter?.limit !== undefined) { + events = [...events] + .sort((a, b) => b.created_at - a.created_at) + .slice(0, filter.limit); + } + for (const event of events) { sendWsText(socket.handler, ["EVENT", subId, event]); } @@ -5847,7 +5919,7 @@ function sendToMockSocket(args: { return; } - emitMockHistory(socket, subId, channelId); + emitMockHistory(socket, subId, channelId, filter); return; } diff --git a/desktop/tests/e2e/timeline-scroll-anchor.spec.ts b/desktop/tests/e2e/timeline-scroll-anchor.spec.ts index 25df23832..02b13a5a3 100644 --- a/desktop/tests/e2e/timeline-scroll-anchor.spec.ts +++ b/desktop/tests/e2e/timeline-scroll-anchor.spec.ts @@ -65,3 +65,80 @@ test("short channel bottom-aligns its messages against the viewport floor", asyn // Padded from the top: the first row is pushed down, not floating at the top. expect(geometry.firstTop - geometry.timelineTop).toBeGreaterThan(96); }); + +// The channel-jump bug: loading older messages prepended rows above the +// viewport while an end-follow re-pin loop fought the user's scroll, freezing +// the rendered window on the newest messages — the user could not scroll back +// through history at all, and any anchored row was yanked off-screen. #load-older +// seeds 260 messages, more than the 200 initial-history limit, so scrolling up +// pages in a real older batch (a genuine prepend below index 60). The contract +// is geometric: a row the user is reading must hold its on-screen position as +// the window scrolls and the older page lands. We assert its +// getBoundingClientRect().top, not scrollTop — the bug moved the row even when +// scrollTop looked plausible. +test("loading older messages holds the anchored row's screen position", async ({ + page, +}) => { + await page.goto("/"); + await page.getByTestId("channel-load-older").click(); + await expect(page.getByTestId("chat-title")).toHaveText("load-older"); + + const timeline = page.getByTestId("message-timeline"); + const rows = page.getByTestId("message-row"); + await expect(rows.first()).toBeVisible(); + + // Anchor on index 100: it is inside the newest-200 initial load (oldest is + // index 60), so it is loaded from channel-open, and it is far enough from the + // bottom that reaching it requires real scrollback through the window. Under + // the freeze the rendered window never left the newest ~11 rows, so index 100 + // never mounted — the anchor is absent and the test fails at the first probe. + const ANCHOR = "mock-load-older-100"; + + // Screen position (top relative to the scroll container) of the anchor row, + // or null when it is not mounted. getBoundingClientRect is the user-visible + // geometry the bug disturbed; scrollTop is not. + const anchorTop = () => + timeline.evaluate((element, id) => { + const row = element.querySelector(`[data-message-id="${id}"]`); + if (!row) { + return null; + } + return ( + row.getBoundingClientRect().top - element.getBoundingClientRect().top + ); + }, ANCHOR); + + // A real wheel (not a synthetic scrollTop assignment) is required: it drives + // both the virtualizer and the top-sentinel IntersectionObserver that arms + // load-older. Scroll up in bounded steps until the anchor row settles into a + // stable on-screen position near the viewport top. Each step also pages in + // the older batch as the sentinel enters its 200px margin, so by the time the + // anchor is parked the prepend has already landed beneath it. + await timeline.hover(); + let before: number | null = null; + for (let i = 0; i < 120; i++) { + const top = await anchorTop(); + // Park once the anchor is mounted and sitting in the upper region of the + // viewport — the position a reader would hold while paging older history. + if (top !== null && top >= 0 && top <= 200) { + before = top; + break; + } + await page.mouse.wheel(0, -120); + await page.waitForTimeout(40); + } + // Under the freeze the window stays pinned to the newest rows, so index 100 + // never mounts and `before` stays null. Reaching a real on-screen position is + // itself proof the window tracked the scrollback. + expect(before).not.toBeNull(); + + // Let any pending prepend settle, then confirm the anchor held its place. The + // freeze regression snapped the viewport, moving the row hundreds of pixels; + // a correct end-anchor reconcile keeps it within a hair of where it was. + await page.waitForTimeout(200); + const after = await anchorTop(); + expect(after).not.toBeNull(); + expect(Math.abs((after as number) - (before as number))).toBeLessThanOrEqual( + 8, + ); +});