fix(desktop): thread virtualItems into memoized timeline list

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 <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
2026-06-17 23:34:25 -04:00
co-authored by Will Pfleger
parent dff0511dca
commit 58d785ce34
4 changed files with 189 additions and 28 deletions
@@ -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}
/>
</div>
) : null}
@@ -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<ChatVirtualizer["getVirtualItems"]>;
/** 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<HTMLDivElement>}
style={{ height: `${virtualizer.getTotalSize() + topPad}px` }}
>
{virtualizer.getVirtualItems().map((virtualRow) => (
{virtualItems.map((virtualRow) => (
<div
data-index={virtualRow.index}
key={virtualRow.key}
+93 -21
View File
@@ -466,6 +466,9 @@ type MockFilter = {
"#h"?: string[];
authors?: string[];
kinds?: number[];
limit?: number;
until?: number;
since?: number;
};
type MockSocket = {
@@ -1261,6 +1264,31 @@ const mockChannels: MockChannel[] = [
createMockMember(MOCK_IDENTITY_PUBKEY, "member", 1300),
],
}),
createMockChannel({
id: "fa11bac0-0000-4000-8000-000000000013",
name: "load-older",
channel_type: "stream",
visibility: "open",
description: "Channel with more messages than the initial history limit",
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: 1400,
updated_minutes_ago: 1,
members: [
createMockMember(ALICE_PUBKEY, "owner", 1400),
createMockMember(MOCK_IDENTITY_PUBKEY, "member", 1300),
],
}),
createMockChannel({
id: "b5e2f8a1-3c44-5912-9e67-4a8d1f2b3c4e",
name: "design",
@@ -2240,31 +2268,75 @@ function getMockMessageStore(channelId: string): RelayEvent[] {
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),
},
]
: [];
: channelId === "fa11bac0-0000-4000-8000-000000000013"
? // 260 backdated messages — more than the 200 initial-history limit,
// so the first load caps at 200 and the rest page in via load-older,
// producing a genuine prepend. Spaced 60s apart, oldest first, so
// `until`-windowed paging walks strictly backward without overlap
// beyond the inclusive boundary message.
Array.from({ length: 260 }, (_, index) => ({
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;
}
@@ -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,
);
});