From 28c550e48fe58c77eeb328f5a1e4c04d9319f717 Mon Sep 17 00:00:00 2001 From: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta Date: Fri, 12 Jun 2026 20:40:22 -0400 Subject: [PATCH 1/2] fix(desktop): preserve scroll anchor when loading older messages Co-authored-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta Signed-off-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta --- .../messages/ui/useLoadOlderOnScroll.ts | 52 ++++++--- desktop/src/testing/e2eBridge.ts | 69 +++++++++-- desktop/tests/e2e/smoke.spec.ts | 109 ++++++++++++++++++ 3 files changed, 209 insertions(+), 21 deletions(-) diff --git a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts index 73efbd3fc..2d1ee9b79 100644 --- a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts +++ b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts @@ -23,10 +23,36 @@ export function useLoadOlderOnScroll({ sentinelRef, }: UseLoadOlderOnScrollOptions) { const restoreScrollPositionRef = React.useRef(restoreScrollPosition); + const [, scheduleRestore] = React.useReducer((count: number) => count + 1, 0); + const pendingRestoreRef = React.useRef<{ + messageId: string; + top: number; + } | null>(null); React.useEffect(() => { restoreScrollPositionRef.current = restoreScrollPosition; }); + React.useLayoutEffect(() => { + const pendingRestore = pendingRestoreRef.current; + const container = scrollContainerRef.current; + if (!pendingRestore || !container) { + return; + } + + pendingRestoreRef.current = null; + const anchor = container.querySelector( + `[data-message-id="${pendingRestore.messageId}"]`, + ); + if (!anchor) { + return; + } + + const delta = anchor.getBoundingClientRect().top - pendingRestore.top; + if (delta !== 0) { + restoreScrollPositionRef.current(container.scrollTop + delta); + } + }); + React.useEffect(() => { const sentinel = sentinelRef.current; const container = scrollContainerRef.current; @@ -56,20 +82,20 @@ export function useLoadOlderOnScroll({ currentObserver?.disconnect(); - 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(); - }); + const anchor = + container.querySelector("[data-message-id]"); + const messageId = anchor?.dataset.messageId; + const top = anchor?.getBoundingClientRect().top; + void fetchOlder() + .then(() => { + if (messageId && top !== undefined) { + pendingRestoreRef.current = { messageId, top }; + scheduleRestore(); + } + }) + .finally(() => { + observe(); }); - }); }, { root: container, rootMargin: "200px 0px 0px 0px" }, ); diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 6c6ad2128..d95db35fd 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -452,6 +452,9 @@ type MockFilter = { "#h"?: string[]; authors?: string[]; kinds?: number[]; + limit?: number; + since?: number; + until?: number; }; type MockSocket = { @@ -574,6 +577,9 @@ declare global { kind?: number; mentionPubkeys?: string[]; extraTags?: string[][]; + createdAt?: number; + id?: string; + emitLive?: boolean; }) => RelayEvent; __BUZZ_E2E_EMIT_MOCK_TYPING__?: (input: { channelName: string; @@ -2163,9 +2169,29 @@ function getMockMessageStore(channelId: string): RelayEvent[] { return seeded; } -function emitMockHistory(socket: MockSocket, subId: string, channelId: string) { - const events = getMockMessageStore(channelId); - for (const event of events) { +function filterMockHistory(channelId: string, filter: MockFilter) { + return getMockMessageStore(channelId) + .filter((event) => + filter.kinds ? filter.kinds.includes(event.kind) : true, + ) + .filter((event) => + filter.since !== undefined ? event.created_at >= filter.since : true, + ) + .filter((event) => + filter.until !== undefined ? event.created_at <= filter.until : true, + ) + .sort((left, right) => right.created_at - left.created_at) + .slice(0, filter.limit ?? 50) + .sort((left, right) => left.created_at - right.created_at); +} + +function emitMockHistory( + socket: MockSocket, + subId: string, + channelId: string, + filter: MockFilter, +) { + for (const event of filterMockHistory(channelId, filter)) { sendWsText(socket.handler, ["EVENT", subId, event]); } sendWsText(socket.handler, ["EOSE", subId]); @@ -2275,6 +2301,9 @@ function emitMockChannelMessage( kind?: number, mentionPubkeys?: string[], extraTags?: string[][], + createdAt?: number, + id?: string, + emitLive = true, ) { const eventKind = kind ?? 9; if (!parentEventId) { @@ -2284,9 +2313,18 @@ function emitMockChannelMessage( pubkey ?? DEFAULT_MOCK_IDENTITY.pubkey, ); if (extraTags) tags.push(...extraTags); - const event = createMockEvent(eventKind, content, tags, pubkey); + const event = createMockEvent( + eventKind, + content, + tags, + pubkey, + createdAt, + id, + ); recordMockMessage(channelId, event); - emitMockLiveEvent(channelId, event); + if (emitLive) { + emitMockLiveEvent(channelId, event); + } return event; } @@ -2309,9 +2347,18 @@ function emitMockChannelMessage( mentionPubkeys, ); if (extraTags) tags.push(...extraTags); - const event = createMockEvent(eventKind, content, tags, authorPubkey); + const event = createMockEvent( + eventKind, + content, + tags, + authorPubkey, + createdAt, + id, + ); recordMockMessage(channelId, event); - emitMockLiveEvent(channelId, event); + if (emitLive) { + emitMockLiveEvent(channelId, event); + } return event; } @@ -5644,7 +5691,7 @@ function sendToMockSocket(args: { return; } - emitMockHistory(socket, subId, channelId); + emitMockHistory(socket, subId, channelId, filter); return; } @@ -5785,6 +5832,9 @@ export function maybeInstallE2eTauriMocks() { kind, mentionPubkeys, extraTags, + createdAt, + id, + emitLive = true, }) => { const channel = mockChannels.find( (candidate) => candidate.name === channelName, @@ -5801,6 +5851,9 @@ export function maybeInstallE2eTauriMocks() { kind, mentionPubkeys, extraTags, + createdAt, + id, + emitLive, ); }; window.__BUZZ_E2E_EMIT_MOCK_TYPING__ = ({ channelName, pubkey }) => { diff --git a/desktop/tests/e2e/smoke.spec.ts b/desktop/tests/e2e/smoke.spec.ts index 85d2ccc6b..f57d1db30 100644 --- a/desktop/tests/e2e/smoke.spec.ts +++ b/desktop/tests/e2e/smoke.spec.ts @@ -388,6 +388,115 @@ test("supports multiline drafts with Ctrl+Enter and sends with Enter", async ({ ); }); +test("keeps viewport anchored when older messages load above", async ({ + page, +}) => { + const baseTimestamp = Math.floor(Date.now() / 1000) - 10_000; + + await page.goto("/"); + await page.evaluate((base) => { + const emit = ( + window as Window & { + __BUZZ_E2E_EMIT_MOCK_MESSAGE__?: (input: { + channelName: string; + content: string; + createdAt?: number; + emitLive?: boolean; + id?: string; + }) => void; + } + ).__BUZZ_E2E_EMIT_MOCK_MESSAGE__; + + if (!emit) { + throw new Error("Mock message emitter is unavailable."); + } + + for (let index = 0; index < 260; index += 1) { + emit({ + channelName: "general", + content: `Paged history seed ${index.toString().padStart(3, "0")}`, + createdAt: base + index, + emitLive: false, + id: `paged-history-${index.toString().padStart(3, "0")}`, + }); + } + }, baseTimestamp); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + await expect(page.getByText("Paged history seed 259")).toBeVisible(); + + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).not.toContainText("Paged history seed 000"); + const flickerMaxDelta = await timeline.evaluate(async (element) => { + const scrollContainer = element as HTMLDivElement; + const anchor = scrollContainer.querySelector( + '[data-message-id="paged-history-160"]', + ); + if (!anchor) { + throw new Error("Oldest loaded message was not rendered."); + } + + scrollContainer.scrollTop = 0; + scrollContainer.dispatchEvent(new Event("scroll", { bubbles: true })); + + const startTop = anchor.getBoundingClientRect().top; + let maxDelta = 0; + let pendingFrame = false; + const sampleAnchor = () => { + pendingFrame = false; + const currentAnchor = scrollContainer.querySelector( + '[data-message-id="paged-history-160"]', + ); + if (!currentAnchor) { + return; + } + const top = currentAnchor.getBoundingClientRect().top; + maxDelta = Math.max(maxDelta, Math.abs(top - startTop)); + }; + const observer = new MutationObserver(() => { + if (pendingFrame) { + return; + } + pendingFrame = true; + requestAnimationFrame(sampleAnchor); + }); + observer.observe(scrollContainer, { childList: true, subtree: true }); + + await new Promise((resolve) => { + const deadline = performance.now() + 1_000; + const waitForOlderHistory = () => { + if ( + scrollContainer.textContent?.includes("Paged history seed 000") || + performance.now() >= deadline + ) { + resolve(); + return; + } + requestAnimationFrame(waitForOlderHistory); + }; + requestAnimationFrame(waitForOlderHistory); + }); + await new Promise((resolve) => + requestAnimationFrame(() => resolve()), + ); + await new Promise((resolve) => + requestAnimationFrame(() => resolve()), + ); + observer.disconnect(); + + return { + maxDelta, + scrollTop: scrollContainer.scrollTop, + }; + }); + + await expect(timeline).toContainText("Paged history seed 000"); + + expect(flickerMaxDelta.scrollTop).toBeGreaterThan(0); + expect(flickerMaxDelta.maxDelta).toBeLessThanOrEqual(2); +}); + test("does not shift the timeline when the composer grows", async ({ page, }) => { From 6cde5ea2105bec66f2719e2ae5c9a26dffe5bbe6 Mon Sep 17 00:00:00 2001 From: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 Date: Fri, 12 Jun 2026 22:57:44 -0400 Subject: [PATCH 2/2] fix(desktop): bypass restoreScrollPosition lock in prepend path useLoadOlderOnScroll captured the anchor correctly in a layout effect, but routed the restore through useTimelineScrollManager.restoreScrollPosition, which schedules a 2-rAF locked-write loop. That lock is correct for the ResizeObserver path (where layout may settle across frames) but wrong for prepend: it fights live wheel input for 2-3 frames after every fetchOlder, producing the up/down/snap-to-random behavior Tyler reported on PR 1025. Replace the call with a single synchronous pre-paint scrollTop write from the existing useLayoutEffect. Drop the now-unused prop pass-through from MessageTimeline. Verified in Playwright with instrumented scrollTop setter on continuous wheel-up across 500 messages: before: 8 user->other snap-back writes within 50ms, 12 divergences >50px after: 0 snap-back writes, 4 divergences (all = prepended content height, not snap-backs) The existing PR 1025 smoke (keeps viewport anchored after settle) still passes; it measured after-settle anchor position, which was already correct. What was broken was during-scroll lock contention, which that test does not measure. Co-authored-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta Signed-off-by: npub1cc3ha7z055mu0rwwu7806t2wt8mj3pvu0uv5mfp2c50dahaqhczshdalg6 --- .../src/features/messages/ui/MessageTimeline.tsx | 2 -- .../features/messages/ui/useLoadOlderOnScroll.ts | 13 ++++++------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 1fa347a20..b02c921d8 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -137,7 +137,6 @@ export const MessageTimeline = React.memo(function MessageTimeline({ highlightedMessageId, isAtBottom, newMessageCount, - restoreScrollPosition, scrollToBottom, syncScrollState, } = useTimelineScrollManager({ @@ -177,7 +176,6 @@ export const MessageTimeline = React.memo(function MessageTimeline({ fetchOlder, hasOlderMessages, isLoading, - restoreScrollPosition, scrollContainerRef, sentinelRef: topSentinelRef, }); diff --git a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts index 2d1ee9b79..86ea47d1c 100644 --- a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts +++ b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts @@ -4,7 +4,6 @@ type UseLoadOlderOnScrollOptions = { fetchOlder?: () => Promise; hasOlderMessages: boolean; isLoading: boolean; - restoreScrollPosition: (scrollTop: number) => void; scrollContainerRef: React.RefObject; sentinelRef: React.RefObject; }; @@ -18,19 +17,14 @@ export function useLoadOlderOnScroll({ fetchOlder, hasOlderMessages, isLoading, - restoreScrollPosition, scrollContainerRef, sentinelRef, }: UseLoadOlderOnScrollOptions) { - const restoreScrollPositionRef = React.useRef(restoreScrollPosition); const [, scheduleRestore] = React.useReducer((count: number) => count + 1, 0); const pendingRestoreRef = React.useRef<{ messageId: string; top: number; } | null>(null); - React.useEffect(() => { - restoreScrollPositionRef.current = restoreScrollPosition; - }); React.useLayoutEffect(() => { const pendingRestore = pendingRestoreRef.current; @@ -49,7 +43,12 @@ export function useLoadOlderOnScroll({ const delta = anchor.getBoundingClientRect().top - pendingRestore.top; if (delta !== 0) { - restoreScrollPositionRef.current(container.scrollTop + delta); + // Single synchronous pre-paint write. We deliberately do NOT route this + // through useTimelineScrollManager.restoreScrollPosition: that helper + // schedules a 2-rAF locked-write loop (correct for ResizeObserver-driven + // resizes that may settle across frames, wrong for prepend), which + // fights live wheel input for 2–3 frames after every fetchOlder. + container.scrollTop = container.scrollTop + delta; } });