From 0f32f49045c4e2d81deae176b2f8842db20e9e50 Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Thu, 18 Jun 2026 12:36:44 -0400 Subject: [PATCH 01/15] fix(desktop): single-owner anchored scroll for dynamic loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the two competing scroll writers (useTimelineScrollManager + useLoadOlderOnScroll) with one anchor-based primitive, useAnchoredScroll, owned by both the main timeline and the thread panel. The prior design had two hooks both mutating scrollTop on prepend, which is the root of the timeline-jumping bug: a fetch-older restore and a scroll-anchor adjustment would fight over the same frame. The new hook keeps a single anchor (the row the reader's eye is on, picked by a top-crossing walk) and restores it relative to its prior top offset after every render — prepends, appends, image loads, and embed expansions all flow through that one path. Also fixes three concrete bugs surfaced by the ported E2E suite: - Deep-link targets in older history: the post-mount target effect bailed once when the row wasn't loaded yet and never retried. It now keys on `messages` and re-runs until the spliced-in row commits, then centers. - Root-message deep-link reopen: the initial-mount target path scrolled and marked the target handled but never fired `onTargetReached`, so the `messageId` URL param stuck and re-clicking the same link was a no-op. The initial path now fires the callback too, matching the post-mount one. - Inline image height reservation: images never read their NIP-92 `dim` tag, so a tall image grew from ~0 on load and shoved the timeline. We now stamp intrinsic width/height so the row reserves space before decode. Verification: tsc clean, biome (764 files) clean, 989/989 unit tests, scroll-history 6/6, full e2e smoke 253 passed (3 failures all reproduce on a clean main checkout — pre-existing, unrelated to this change). Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- desktop/playwright.config.ts | 1 + .../src/features/messages/lib/messageLink.ts | 2 +- .../messages/ui/MessageThreadPanel.tsx | 46 +- .../features/messages/ui/MessageTimeline.tsx | 48 +- .../features/messages/ui/useAnchoredScroll.ts | 544 ++++++++++ .../messages/ui/useLoadOlderOnScroll.ts | 92 -- .../messages/ui/useTimelineScrollManager.ts | 464 --------- desktop/src/shared/ui/markdown.tsx | 46 +- desktop/src/testing/e2eBridge.ts | 89 +- desktop/tests/e2e/scroll-history.spec.ts | 941 ++++++++++++++++++ desktop/tests/helpers/bridge.ts | 2 + 11 files changed, 1657 insertions(+), 618 deletions(-) create mode 100644 desktop/src/features/messages/ui/useAnchoredScroll.ts delete mode 100644 desktop/src/features/messages/ui/useLoadOlderOnScroll.ts delete mode 100644 desktop/src/features/messages/ui/useTimelineScrollManager.ts create mode 100644 desktop/tests/e2e/scroll-history.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index 8256c70c9..1025cec35 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -49,6 +49,7 @@ export default defineConfig({ "**/animated-avatar-screenshots.spec.ts", "**/reminders-screenshots.spec.ts", "**/virtualization-screenshots.spec.ts", + "**/scroll-history.spec.ts", ], use: { ...devices["Desktop Chrome"], diff --git a/desktop/src/features/messages/lib/messageLink.ts b/desktop/src/features/messages/lib/messageLink.ts index 02e892aaf..b8fa89999 100644 --- a/desktop/src/features/messages/lib/messageLink.ts +++ b/desktop/src/features/messages/lib/messageLink.ts @@ -16,7 +16,7 @@ export type MessageLinkInput = { * * Currently emitted into the URL but not consumed by the click handler * or deep-link listener — both route via `goChannel(channelId, - * { messageId })` and let `useTimelineScrollManager` resolve the target. + * { messageId })` and let `useAnchoredScroll` resolve the target. * Reserved for future "open in thread view" routing. */ threadRootId?: string | null; diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index c8d8f99ea..06b12a274 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -31,7 +31,7 @@ import { MessageThreadSummaryRow } from "./MessageThreadSummaryRow"; import { TypingIndicatorRow } from "./TypingIndicatorRow"; import { UnreadDivider } from "./UnreadDivider"; import { useComposerHeightPadding } from "./useComposerHeightPadding"; -import { useTimelineScrollManager } from "./useTimelineScrollManager"; +import { useAnchoredScroll } from "./useAnchoredScroll"; import { selectDeferredListRenderState } from "@/features/messages/lib/timelineSnapshot"; type MessageThreadPanelProps = { @@ -306,6 +306,11 @@ export function MessageThreadPanel({ widthPx, }: 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; @@ -338,8 +343,7 @@ export function MessageThreadPanel({ // (`scrollIntoView`), so it must stay consistent with what's actually painted. // You can't scroll to a reply that hasn't committed yet. The thread pane gets // this no-tearing guarantee for free by routing through the same - // `useTimelineScrollManager` (and its `timelineSnapshot` helpers) as the main - // timeline. + // `useAnchoredScroll` primitive as the main timeline. const deferredThreadReplies = React.useDeferredValue( threadReplies, EMPTY_THREAD_REPLIES, @@ -359,22 +363,18 @@ export function MessageThreadPanel({ [deferredThreadReplies], ); - const { - bottomAnchorRef, - contentRef, - isAtBottom, - newMessageCount, - scrollToBottom, - syncScrollState, - } = useTimelineScrollManager({ - channelId: threadHeadId, - // Wait for deferred replies to commit before scroll-init (else rows mount un-scrolled). - isLoading: repliesRenderState === "pending", - messages: threadMessages, - onTargetReached: onScrollTargetResolved, - scrollContainerRef: threadBodyRef, - targetMessageId: scrollTargetId, - }); + const { isAtBottom, newMessageCount, onScroll, scrollToBottom } = + useAnchoredScroll({ + channelId: threadHeadId, + contentRef: threadContentRef, + // Wait for deferred replies to commit before scroll-init (else rows mount un-scrolled). + isLoading: repliesRenderState === "pending", + messages: threadMessages, + onTargetReached: onScrollTargetResolved, + scrollContainerRef: threadBodyRef, + sentinelRef: threadTopSentinelRef, + targetMessageId: scrollTargetId, + }); if (!threadHead) { return null; @@ -383,15 +383,16 @@ export function MessageThreadPanel({ const threadScrollRegion = (
-
+
+
diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 6e549bc8c..c3ea9b3ee 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -17,8 +17,7 @@ import { UnreadPill, unreadCountLabel } from "@/shared/ui/UnreadPill"; import { UserAvatar } from "@/shared/ui/UserAvatar"; import { TimelineSkeleton, useTimelineSkeletonRows } from "./TimelineSkeleton"; import { TimelineMessageList } from "./TimelineMessageList"; -import { useLoadOlderOnScroll } from "./useLoadOlderOnScroll"; -import { useTimelineScrollManager } from "./useTimelineScrollManager"; +import { useAnchoredScroll } from "./useAnchoredScroll"; export type MessageTimelineHandle = { scrollToBottomOnNextUpdate: () => void; @@ -161,6 +160,7 @@ const MessageTimelineBase = React.forwardRef< ) { const internalScrollRef = React.useRef(null); const scrollContainerRef = externalScrollRef ?? internalScrollRef; + const contentRef = React.useRef(null); const topSentinelRef = React.useRef(null); // Gate the heavy timeline render (each row runs a synchronous @@ -203,22 +203,23 @@ const MessageTimelineBase = React.forwardRef< const showMessageList = timelineBodySurface === "list"; const { - bottomAnchorRef, - contentRef, highlightedMessageId, isAtBottom, newMessageCount, - restoreScrollPosition, + onScroll, scrollToBottom, scrollToBottomOnNextUpdate, scrollToMessage, - syncScrollState, - } = useTimelineScrollManager({ + } = useAnchoredScroll({ channelId, + contentRef, + fetchOlder, + hasOlderMessages, isLoading: showTimelineSkeleton, messages: deferredMessages, onTargetReached, scrollContainerRef, + sentinelRef: topSentinelRef, targetMessageId, }); @@ -263,9 +264,10 @@ const MessageTimelineBase = React.forwardRef< } }, [firstUnreadMessageId, scrollToMessage]); - // Scroll to the active search match when it changes. + // 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. const prevSearchActiveRef = React.useRef(null); - // biome-ignore lint/correctness/useExhaustiveDependencies: scrollContainerRef is a stable React ref React.useEffect(() => { if (showTimelineSkeleton) return; if ( @@ -276,26 +278,8 @@ const MessageTimelineBase = React.forwardRef< return; } prevSearchActiveRef.current = searchActiveMessageId; - - const container = scrollContainerRef.current; - if (!container) return; - - const el = container.querySelector( - `[data-message-id="${searchActiveMessageId}"]`, - ); - if (el) { - el.scrollIntoView({ block: "center", behavior: "smooth" }); - } - }, [searchActiveMessageId, showTimelineSkeleton]); - - useLoadOlderOnScroll({ - fetchOlder, - hasOlderMessages, - isLoading: showTimelineSkeleton, - restoreScrollPosition, - scrollContainerRef, - sentinelRef: topSentinelRef, - }); + scrollToMessage(searchActiveMessageId, { behavior: "smooth" }); + }, [scrollToMessage, searchActiveMessageId, showTimelineSkeleton]); const timelineSkeletonRows = useTimelineSkeletonRows({ channelId, @@ -323,12 +307,12 @@ const MessageTimelineBase = React.forwardRef< ) : null}
) : null}
- -
diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts new file mode 100644 index 000000000..f3acdd935 --- /dev/null +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -0,0 +1,544 @@ +import * as React from "react"; + +import type { TimelineMessage } from "@/features/messages/types"; + +/** + * Distance (in CSS pixels) below which we consider the scroll position + * "at the bottom" of the message list. Tight enough that the user has to + * actually scroll down to re-pin; permissive enough to tolerate sub-pixel + * rounding from the layout engine. + */ +const AT_BOTTOM_THRESHOLD_PX = 32; + +type AnchorState = + | { kind: "at-bottom" } + | { kind: "message"; messageId: string; topOffset: number }; + +type UseAnchoredScrollOptions = { + /** Scroll container. Owned by the parent so external refs still compose. */ + scrollContainerRef: React.RefObject; + /** Inner content element — must wrap every renderable row, including the + * sentinel and bottom anchor. Used to schedule layout work on resize. */ + contentRef: React.RefObject; + /** 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. */ + isLoading: boolean; + /** 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; + /** When set, scroll to and highlight this message on mount and on change. */ + targetMessageId?: string | null; + onTargetReached?: (messageId: string) => void; +}; + +type UseAnchoredScrollResult = { + /** Pass through to the scroll container's `onScroll`. */ + onScroll: () => void; + /** True when the user is within `AT_BOTTOM_THRESHOLD_PX` of the bottom. */ + isAtBottom: boolean; + /** Number of new messages that have arrived while the user is not at the + * bottom. Cleared when the user returns to the bottom. */ + newMessageCount: number; + /** Message id that should pulse a highlight (target/active-search). */ + highlightedMessageId: string | null; + /** Imperative: scroll to bottom. */ + scrollToBottom: (behavior?: ScrollBehavior) => void; + /** Arm a one-shot scroll-to-bottom that fires on the next appended message + * (used by the composer's send flow). */ + scrollToBottomOnNextUpdate: () => void; + /** Imperative: scroll a specific message into view; optionally pulse it. + * Returns true if the row was found and scrolled, false otherwise. */ + scrollToMessage: ( + messageId: string, + options?: { highlight?: boolean; behavior?: ScrollBehavior }, + ) => boolean; +}; + +function isAtBottomNow(container: HTMLDivElement) { + return ( + container.scrollHeight - container.clientHeight - container.scrollTop <= + AT_BOTTOM_THRESHOLD_PX + ); +} + +/** + * Pick an anchor for the current scroll position. + * + * Top-crossing walk: chronological children, top-down. The first + * `data-message-id` row whose bottom edge has crossed below the container + * top is the anchor — that's the row the reader's eye is on when they've + * scrolled up through history. `topOffset` is the row's top relative to + * the container's top and may be negative when the row straddles the edge. + * + * If no such row exists (e.g. nothing scrolled past the top, list shorter + * than the viewport, etc.) the anchor is `at-bottom`. + * + * Algorithm credit: Sami's [13] in the buzz-bugs scroll-redesign thread, + * supersedes the Matrix-style bottom-up walk in [7]. The top-crossing + * choice is what keeps the row the reader is *reading* fixed under + * in-viewport reflow (image-load, embed expansion). + */ +function computeAnchor(container: HTMLDivElement): AnchorState { + if (isAtBottomNow(container)) { + return { kind: "at-bottom" }; + } + + const containerTop = container.getBoundingClientRect().top; + const rows = container.querySelectorAll("[data-message-id]"); + + for (let i = 0; i < rows.length; i++) { + const row = rows[i]; + const rect = row.getBoundingClientRect(); + if (rect.bottom > containerTop) { + const messageId = row.dataset.messageId; + if (messageId) { + return { + kind: "message", + messageId, + topOffset: rect.top - containerTop, + }; + } + } + } + + return { kind: "at-bottom" }; +} + +/** + * Find the rendered message id that is closest in chronological order to + * the anchor, scanning forward in `messages`. Used as the fallback when the + * anchor's row is gone post-render (e.g. message deleted). + */ +function findNearestNewerMessageId( + container: HTMLDivElement, + messages: TimelineMessage[], + anchorId: string, +): string | null { + const anchorIndex = messages.findIndex((m) => m.id === anchorId); + if (anchorIndex < 0) return null; + + for (let i = anchorIndex + 1; i < messages.length; i++) { + const candidate = messages[i]; + const el = container.querySelector(`[data-message-id="${candidate.id}"]`); + if (el) return candidate.id; + } + return null; +} + +export function useAnchoredScroll({ + scrollContainerRef, + contentRef, + sentinelRef, + channelId, + isLoading, + messages, + fetchOlder, + hasOlderMessages = false, + targetMessageId = null, + onTargetReached, +}: 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 + // restoration). useState would force re-renders we don't want. + const anchorRef = React.useRef({ kind: "at-bottom" }); + const [isAtBottom, setIsAtBottom] = React.useState(true); + const [newMessageCount, setNewMessageCount] = React.useState(0); + const [highlightedMessageId, setHighlightedMessageId] = React.useState< + string | null + >(null); + + const hasInitializedRef = React.useRef(false); + const prevLastMessageIdRef = React.useRef(undefined); + const prevMessageCountRef = React.useRef(0); + const fetchingOlderRef = React.useRef(false); + const handledTargetIdRef = 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 + // appends, we snap to bottom even if they had scrolled up to read history. + // Consumed (and cleared) by the next append in the restoration effect. + const forceBottomOnNextAppendRef = React.useRef(false); + + // Reset everything when the channel changes — the layout effect that runs + // immediately after this reset is responsible for either jumping to bottom + // or to the target message for the new channel. + // biome-ignore lint/correctness/useExhaustiveDependencies: channelId is intentionally the sole trigger — we want this effect to fire exactly when the channel changes (and on mount). + React.useLayoutEffect(() => { + anchorRef.current = { kind: "at-bottom" }; + setIsAtBottom(true); + setNewMessageCount(0); + setHighlightedMessageId(null); + hasInitializedRef.current = false; + prevLastMessageIdRef.current = undefined; + prevMessageCountRef.current = 0; + fetchingOlderRef.current = false; + handledTargetIdRef.current = null; + forceBottomOnNextAppendRef.current = false; + if (highlightTimeoutRef.current !== null) { + window.clearTimeout(highlightTimeoutRef.current); + highlightTimeoutRef.current = null; + } + }, [channelId]); + + const scrollToBottomImperative = React.useCallback( + (behavior: ScrollBehavior = "auto") => { + const container = scrollContainerRef.current; + if (!container) return; + anchorRef.current = { kind: "at-bottom" }; + container.scrollTo({ top: container.scrollHeight, behavior }); + setIsAtBottom(true); + setNewMessageCount(0); + }, + [scrollContainerRef], + ); + + // Arm a one-shot: the next append snaps to bottom regardless of where the + // user is. The consumer calls this right before sending so their own + // outbound message pulls the view down even if they'd scrolled up. + const scrollToBottomOnNextUpdate = React.useCallback(() => { + forceBottomOnNextAppendRef.current = true; + }, []); + + const scrollToMessageImperative = React.useCallback( + ( + messageId: string, + options: { highlight?: boolean; behavior?: ScrollBehavior } = {}, + ): boolean => { + const container = scrollContainerRef.current; + if (!container) return false; + const el = container.querySelector( + `[data-message-id="${messageId}"]`, + ); + if (!el) return false; + + el.scrollIntoView({ + block: "center", + behavior: options.behavior ?? "auto", + }); + + // After the scroll, the user's anchor row is this message at its new + // top-relative offset. Recompute so layout-effect restoration matches. + const rect = el.getBoundingClientRect(); + const containerTop = container.getBoundingClientRect().top; + anchorRef.current = { + kind: "message", + messageId, + topOffset: rect.top - containerTop, + }; + setIsAtBottom(isAtBottomNow(container)); + + if (options.highlight) { + if (highlightTimeoutRef.current !== null) { + window.clearTimeout(highlightTimeoutRef.current); + } + setHighlightedMessageId(messageId); + highlightTimeoutRef.current = window.setTimeout(() => { + setHighlightedMessageId((current) => + current === messageId ? null : current, + ); + highlightTimeoutRef.current = null; + }, 2_000); + } + return true; + }, + [scrollContainerRef], + ); + + // 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. + const onScroll = React.useCallback(() => { + const container = scrollContainerRef.current; + if (!container) return; + anchorRef.current = computeAnchor(container); + const atBottom = anchorRef.current.kind === "at-bottom"; + setIsAtBottom((prev) => (prev === atBottom ? prev : atBottom)); + if (atBottom) { + setNewMessageCount(0); + } + }, [scrollContainerRef]); + + // --------------------------------------------------------------------------- + // 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. + // --------------------------------------------------------------------------- + React.useLayoutEffect(() => { + const container = scrollContainerRef.current; + if (!container) return; + + // First render after a reset (channel switch or initial mount): jump + // to the requested target message, or to the bottom by default. + if (!hasInitializedRef.current) { + if (isLoading) return; + if (targetMessageId) { + // A cold deep-link target may not be in the DOM on this first + // commit — the route screen fetches it by id and splices it in a + // render or two later. If centering fails now, leave the timeline at + // its default position and let the post-mount target effect (keyed on + // `messages`) retry once the row lands, rather than marking it handled. + if (scrollToMessageImperative(targetMessageId, { highlight: true })) { + handledTargetIdRef.current = targetMessageId; + // Consumers clear the route target (`messageId` URL param) on this + // callback. The post-mount target effect below also fires it, but + // for a target already in the DOM on first commit that effect bails + // (handled ref is set), so the initial path must fire it too — else + // the param sticks and re-clicking the same deep link is a no-op. + onTargetReached?.(targetMessageId); + } else { + scrollToBottomImperative("auto"); + } + } else { + scrollToBottomImperative("auto"); + } + hasInitializedRef.current = true; + prevLastMessageIdRef.current = messages[messages.length - 1]?.id; + prevMessageCountRef.current = messages.length; + return; + } + + const anchor = anchorRef.current; + const lastMessage = messages[messages.length - 1]; + const prevLastId = prevLastMessageIdRef.current; + const prevCount = prevMessageCountRef.current; + const newLatestArrived = + lastMessage !== undefined && lastMessage.id !== prevLastId; + + // One-shot: an outbound send armed `scrollToBottomOnNextUpdate`. When the + // resulting append lands, snap to bottom regardless of the current anchor, + // then clear the flag. Bail before the anchored branch so the user's own + // message pulls the view down. + if (newLatestArrived && forceBottomOnNextAppendRef.current) { + forceBottomOnNextAppendRef.current = false; + container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); + anchorRef.current = { kind: "at-bottom" }; + setIsAtBottom(true); + setNewMessageCount(0); + prevLastMessageIdRef.current = lastMessage?.id; + prevMessageCountRef.current = messages.length; + return; + } + + if (anchor.kind === "at-bottom") { + // Stick to bottom. Use scrollTo to avoid relying on scroll anchoring. + container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); + if (newLatestArrived) setNewMessageCount(0); + } else { + // Anchored to a specific message. Find it; if it's gone, fall back to + // the nearest newer rendered message; if that's also gone, give up + // and pin to bottom. + let anchorEl = container.querySelector( + `[data-message-id="${anchor.messageId}"]`, + ); + let usedAnchor: AnchorState = anchor; + if (!anchorEl) { + const fallbackId = findNearestNewerMessageId( + container, + messages, + anchor.messageId, + ); + if (fallbackId) { + anchorEl = container.querySelector( + `[data-message-id="${fallbackId}"]`, + ); + if (anchorEl) { + usedAnchor = { + kind: "message", + messageId: fallbackId, + topOffset: anchor.topOffset, + }; + } + } + } + + if (anchorEl) { + const containerTop = container.getBoundingClientRect().top; + const currentTop = anchorEl.getBoundingClientRect().top - containerTop; + const delta = currentTop - usedAnchor.topOffset; + if (Math.abs(delta) > 0.5) { + // `scrollBy` is intentional over `scrollTop = ...`: relative + // adjustment composes with whatever the browser's own scroll + // anchoring did, and it doesn't fight a smooth-scroll in flight. + container.scrollBy(0, delta); + } + anchorRef.current = usedAnchor; + } else { + // Anchor message and all subsequent rendered messages are gone. + // Last-resort fallback so the user doesn't end up stranded. + container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); + anchorRef.current = { kind: "at-bottom" }; + setIsAtBottom(true); + } + + if (newLatestArrived) { + const added = Math.max(1, messages.length - prevCount); + setNewMessageCount((current) => current + added); + } + } + + prevLastMessageIdRef.current = lastMessage?.id; + prevMessageCountRef.current = messages.length; + }, [ + isLoading, + messages, + onTargetReached, + scrollContainerRef, + scrollToBottomImperative, + scrollToMessageImperative, + 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, or an embed + // expands, the row positions shift. A ResizeObserver fires a synthetic + // scroll so the anchor is recomputed; the layout effect on the next + // render will then restore. We deliberately do NOT call scrollTo here — + // that would fight the anchor restoration. + // --------------------------------------------------------------------------- + React.useEffect(() => { + const content = contentRef.current; + if (!content || typeof ResizeObserver === "undefined") return; + const observer = new ResizeObserver(() => { + const container = scrollContainerRef.current; + if (!container) return; + // If we're stuck-to-bottom, just stay there. Otherwise let the next + // layout effect handle restoration via the existing anchor. + if (anchorRef.current.kind === "at-bottom") { + container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); + } + }); + observer.observe(content); + return () => observer.disconnect(); + }, [contentRef, scrollContainerRef]); + + // --------------------------------------------------------------------------- + // Target message handling (deep link, jump-to-reply, etc.). Distinct from + // the initial-mount target above — this handles changes after the first + // render. + // + // A deep-link target may live in older history that isn't in the DOM when + // the route param first changes. The route screen fetches the target event + // by id and splices it into `messages` asynchronously, so its row appears a + // render or two later. We therefore key this effect on `messages` and bail + // *without* marking the target handled until its row actually exists — each + // subsequent message commit re-runs the effect and retries the centering. + // --------------------------------------------------------------------------- + // biome-ignore lint/correctness/useExhaustiveDependencies: `messages` is an intentional trigger, not a read — the effect reads the DOM (querySelector), and we need it to re-run each time the rendered row set changes so a target spliced into older history gets centered once its row commits. + React.useEffect(() => { + if (!targetMessageId) { + handledTargetIdRef.current = null; + return; + } + if (handledTargetIdRef.current === targetMessageId || isLoading) return; + if (!hasInitializedRef.current) return; // initial-mount path will handle. + + const container = scrollContainerRef.current; + if (!container) return; + const el = container.querySelector( + `[data-message-id="${targetMessageId}"]`, + ); + if (!el) return; // Row not rendered yet; a later `messages` commit retries. + handledTargetIdRef.current = targetMessageId; + scrollToMessageImperative(targetMessageId, { highlight: true }); + onTargetReached?.(targetMessageId); + }, [ + isLoading, + messages, + onTargetReached, + scrollContainerRef, + scrollToMessageImperative, + targetMessageId, + ]); + + React.useEffect(() => { + return () => { + if (highlightTimeoutRef.current !== null) { + window.clearTimeout(highlightTimeoutRef.current); + } + }; + }, []); + + return { + onScroll, + isAtBottom, + newMessageCount, + highlightedMessageId, + scrollToBottom: scrollToBottomImperative, + scrollToBottomOnNextUpdate, + scrollToMessage: scrollToMessageImperative, + }; +} diff --git a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts b/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts deleted file mode 100644 index 73efbd3fc..000000000 --- a/desktop/src/features/messages/ui/useLoadOlderOnScroll.ts +++ /dev/null @@ -1,92 +0,0 @@ -import * as React from "react"; - -type UseLoadOlderOnScrollOptions = { - fetchOlder?: () => Promise; - hasOlderMessages: boolean; - isLoading: boolean; - restoreScrollPosition: (scrollTop: number) => void; - scrollContainerRef: React.RefObject; - sentinelRef: React.RefObject; -}; - -/** - * 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, -}: UseLoadOlderOnScrollOptions) { - const restoreScrollPositionRef = React.useRef(restoreScrollPosition); - React.useEffect(() => { - restoreScrollPositionRef.current = restoreScrollPosition; - }); - - 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 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, - ]); -} diff --git a/desktop/src/features/messages/ui/useTimelineScrollManager.ts b/desktop/src/features/messages/ui/useTimelineScrollManager.ts deleted file mode 100644 index af8b66c88..000000000 --- a/desktop/src/features/messages/ui/useTimelineScrollManager.ts +++ /dev/null @@ -1,464 +0,0 @@ -import * as React from "react"; - -import { - isNearBottom, - resolveDeepLinkTarget, - selectLatestMessageAutoScrollBehavior, - selectLatestMessageKey, -} from "@/features/messages/lib/timelineSnapshot"; -import type { TimelineMessage } from "@/features/messages/types"; - -type UseTimelineScrollManagerOptions = { - channelId?: string | null; - isLoading: boolean; - messages: TimelineMessage[]; - onTargetReached?: (messageId: string) => void; - scrollContainerRef: React.RefObject; - targetMessageId?: string | null; -}; - -type PinToBottomOptions = { - clearNewMessageCount?: boolean; -}; - -export function useTimelineScrollManager({ - channelId, - isLoading, - messages, - onTargetReached, - scrollContainerRef, - targetMessageId, -}: UseTimelineScrollManagerOptions) { - const timelineRef = scrollContainerRef; - const contentRef = React.useRef(null); - const bottomAnchorRef = React.useRef(null); - const hasInitializedRef = React.useRef(false); - const shouldStickToBottomRef = React.useRef(true); - const isAtBottomRef = React.useRef(true); - const isProgrammaticBottomScrollRef = React.useRef(false); - const previousTimelineHeightRef = React.useRef(null); - const previousScrollTopRef = React.useRef(0); - const lockedScrollTopRef = React.useRef(null); - const previousLastMessageKeyRef = React.useRef(undefined); - const previousMessageCountRef = React.useRef(0); - const handledTargetMessageIdRef = React.useRef(null); - const scrollToBottomOnNextUpdateRef = React.useRef(false); - // Mirror isLoading into a ref so the ResizeObservers (which subscribe once) - // can skip reacting while the skeleton is up — reacting to height churn under - // a streaming-in list is what makes the timeline thrash on entry. - const isLoadingRef = React.useRef(isLoading); - isLoadingRef.current = isLoading; - const [isAtBottom, setIsAtBottom] = React.useState(true); - const [highlightedMessageId, setHighlightedMessageId] = React.useState< - string | null - >(null); - const [newMessageCount, setNewMessageCount] = React.useState(0); - - const resetScrollTracking = React.useCallback(() => { - hasInitializedRef.current = false; - shouldStickToBottomRef.current = true; - isAtBottomRef.current = true; - isProgrammaticBottomScrollRef.current = false; - previousTimelineHeightRef.current = null; - previousScrollTopRef.current = 0; - lockedScrollTopRef.current = null; - previousLastMessageKeyRef.current = undefined; - previousMessageCountRef.current = 0; - handledTargetMessageIdRef.current = null; - scrollToBottomOnNextUpdateRef.current = false; - setIsAtBottom(true); - setHighlightedMessageId(null); - setNewMessageCount(0); - }, []); - - const pinToBottom = React.useCallback( - ({ clearNewMessageCount = false }: PinToBottomOptions = {}) => { - shouldStickToBottomRef.current = true; - isAtBottomRef.current = true; - setIsAtBottom((current) => (current ? current : true)); - - if (clearNewMessageCount) { - setNewMessageCount(0); - } - }, - [], - ); - - const setObservedBottomState = React.useCallback((atBottom: boolean) => { - shouldStickToBottomRef.current = atBottom; - isAtBottomRef.current = atBottom; - setIsAtBottom((current) => (current === atBottom ? current : atBottom)); - - if (atBottom) { - setNewMessageCount(0); - } - }, []); - - const unpinFromBottom = React.useCallback((scrollTop: number) => { - shouldStickToBottomRef.current = false; - isAtBottomRef.current = false; - isProgrammaticBottomScrollRef.current = false; - previousScrollTopRef.current = scrollTop; - setIsAtBottom(false); - }, []); - - // biome-ignore lint/correctness/useExhaustiveDependencies: channelId is intentionally the sole trigger — we reset all scroll state when the channel changes - React.useLayoutEffect(() => { - resetScrollTracking(); - }, [channelId, resetScrollTracking]); - - const latestMessage = - messages.length > 0 ? messages[messages.length - 1] : undefined; - const latestMessageKey = selectLatestMessageKey(messages); - - const scrollToBottomOnNextUpdate = React.useCallback(() => { - scrollToBottomOnNextUpdateRef.current = true; - }, []); - - // biome-ignore lint/correctness/useExhaustiveDependencies: timelineRef is a stable React ref passed from the parent — its identity never changes - const syncScrollState = React.useCallback(() => { - const timeline = timelineRef.current; - if (!timeline) { - return; - } - - const scrollTop = lockedScrollTopRef.current ?? timeline.scrollTop; - const atBottom = isNearBottom(timeline); - const movedAwayFromBottom = scrollTop + 1 < previousScrollTopRef.current; - - if (isProgrammaticBottomScrollRef.current) { - previousScrollTopRef.current = scrollTop; - - if (movedAwayFromBottom) { - isProgrammaticBottomScrollRef.current = false; - } else if (!atBottom) { - pinToBottom(); - return; - } else { - isProgrammaticBottomScrollRef.current = false; - pinToBottom({ clearNewMessageCount: true }); - return; - } - } - - if (shouldStickToBottomRef.current && !atBottom && !movedAwayFromBottom) { - previousScrollTopRef.current = scrollTop; - pinToBottom({ clearNewMessageCount: true }); - return; - } - - previousScrollTopRef.current = scrollTop; - setObservedBottomState(atBottom); - }, [pinToBottom, setObservedBottomState]); - - // biome-ignore lint/correctness/useExhaustiveDependencies: timelineRef is a stable React ref — its identity never changes - const restoreScrollPosition = React.useCallback( - (scrollTop: number) => { - const timeline = timelineRef.current; - - if (!timeline) { - return; - } - - isProgrammaticBottomScrollRef.current = false; - lockedScrollTopRef.current = scrollTop; - - const restore = (remainingFrames: number) => { - timeline.scrollTop = scrollTop; - - if (remainingFrames > 0) { - requestAnimationFrame(() => { - restore(remainingFrames - 1); - }); - return; - } - - lockedScrollTopRef.current = null; - previousScrollTopRef.current = timeline.scrollTop; - syncScrollState(); - }; - - restore(2); - }, - [syncScrollState], - ); - - // biome-ignore lint/correctness/useExhaustiveDependencies: timelineRef is a stable React ref — its identity never changes - const scrollToBottom = React.useCallback( - (behavior: ScrollBehavior) => { - const timeline = timelineRef.current; - - if (!timeline) { - return; - } - - isProgrammaticBottomScrollRef.current = true; - - const alignToBottom = (nextBehavior: ScrollBehavior) => { - bottomAnchorRef.current?.scrollIntoView({ - block: "end", - behavior: nextBehavior, - }); - timeline.scrollTo({ - top: timeline.scrollHeight, - behavior: nextBehavior, - }); - }; - - alignToBottom(behavior); - lockedScrollTopRef.current = null; - previousScrollTopRef.current = timeline.scrollTop; - pinToBottom({ clearNewMessageCount: true }); - - if (behavior === "smooth") { - requestAnimationFrame(() => { - previousScrollTopRef.current = timeline.scrollTop; - syncScrollState(); - }); - return; - } - - const settleAlignment = (remainingFrames: number) => { - requestAnimationFrame(() => { - alignToBottom("auto"); - previousScrollTopRef.current = timeline.scrollTop; - - if (remainingFrames > 0) { - settleAlignment(remainingFrames - 1); - return; - } - - syncScrollState(); - }); - }; - - settleAlignment(2); - }, - [pinToBottom, syncScrollState], - ); - - // biome-ignore lint/correctness/useExhaustiveDependencies: timelineRef is a stable React ref — its identity never changes - React.useEffect(() => { - const timeline = timelineRef.current; - - if (!timeline || typeof ResizeObserver === "undefined") { - return; - } - - previousTimelineHeightRef.current = timeline.clientHeight; - previousScrollTopRef.current = timeline.scrollTop; - - const observer = new ResizeObserver(([entry]) => { - const previousTimelineHeight = previousTimelineHeightRef.current; - const nextTimelineHeight = entry.contentRect.height; - previousTimelineHeightRef.current = nextTimelineHeight; - - // Track height while loading, but don't scroll — the init layout-effect - // owns the first scroll once content settles. - if (isLoadingRef.current) { - return; - } - - if ( - previousTimelineHeight === null || - Math.abs(nextTimelineHeight - previousTimelineHeight) < 1 - ) { - return; - } - - if (shouldStickToBottomRef.current || isAtBottomRef.current) { - scrollToBottom("auto"); - return; - } - - restoreScrollPosition(previousScrollTopRef.current); - }); - - observer.observe(timeline); - - return () => { - observer.disconnect(); - }; - }, [restoreScrollPosition, scrollToBottom]); - - React.useEffect(() => { - const content = contentRef.current; - - if (!content || typeof ResizeObserver === "undefined") { - return; - } - - const observer = new ResizeObserver(() => { - if (isLoadingRef.current) { - return; - } - if (shouldStickToBottomRef.current) { - scrollToBottom("auto"); - return; - } - - syncScrollState(); - }); - - observer.observe(content); - - return () => { - observer.disconnect(); - }; - }, [scrollToBottom, syncScrollState]); - - React.useLayoutEffect(() => { - if (!hasInitializedRef.current) { - if (isLoading) { - return; - } - - if (targetMessageId) { - const timeline = timelineRef.current; - unpinFromBottom(timeline?.scrollTop ?? 0); - } else { - scrollToBottom("auto"); - } - hasInitializedRef.current = true; - previousLastMessageKeyRef.current = latestMessageKey; - previousMessageCountRef.current = messages.length; - return; - } - - const previousLastMessageKey = previousLastMessageKeyRef.current; - const previousMessageCount = previousMessageCountRef.current; - const hasNewLatestMessage = - latestMessage !== undefined && - latestMessageKey !== previousLastMessageKey; - - if (!hasNewLatestMessage) { - previousLastMessageKeyRef.current = latestMessageKey; - previousMessageCountRef.current = messages.length; - return; - } - - const shouldHonorExplicitBottomRequest = - scrollToBottomOnNextUpdateRef.current; - scrollToBottomOnNextUpdateRef.current = false; - - const autoScrollBehavior = selectLatestMessageAutoScrollBehavior({ - hasExplicitBottomRequest: shouldHonorExplicitBottomRequest, - isAtBottom: isAtBottomRef.current, - shouldStickToBottom: shouldStickToBottomRef.current, - targetMessageId, - }); - - if (autoScrollBehavior) { - scrollToBottom(autoScrollBehavior); - } else { - setNewMessageCount((current) => { - const addedMessages = Math.max( - 1, - messages.length - previousMessageCount, - ); - return current + addedMessages; - }); - } - - previousLastMessageKeyRef.current = latestMessageKey; - previousMessageCountRef.current = messages.length; - }, [ - isLoading, - latestMessage, - latestMessageKey, - messages.length, - scrollToBottom, - targetMessageId, - timelineRef, - unpinFromBottom, - ]); - - // biome-ignore lint/correctness/useExhaustiveDependencies: timelineRef is a stable React ref — its identity never changes - const scrollToMessage = React.useCallback( - (messageId: string) => { - const timeline = timelineRef.current; - if (!timeline) { - return false; - } - - const targetElement = timeline.querySelector( - `[data-message-id="${messageId}"]`, - ); - if (!targetElement) { - return false; - } - - unpinFromBottom(timeline.scrollTop); - setHighlightedMessageId(messageId); - setNewMessageCount(0); - - const alignToTarget = (remainingFrames: number) => { - targetElement.scrollIntoView({ - block: "center", - behavior: "auto", - }); - previousScrollTopRef.current = timeline.scrollTop; - - if (remainingFrames > 0) { - requestAnimationFrame(() => { - alignToTarget(remainingFrames - 1); - }); - return; - } - - onTargetReached?.(messageId); - }; - - alignToTarget(2); - - window.setTimeout(() => { - setHighlightedMessageId((current) => - current === messageId ? null : current, - ); - }, 2_000); - - return true; - }, - [onTargetReached, unpinFromBottom], - ); - - React.useEffect(() => { - if (!targetMessageId) { - handledTargetMessageIdRef.current = null; - setHighlightedMessageId(null); - return; - } - - if (handledTargetMessageIdRef.current === targetMessageId || isLoading) { - return; - } - - // Deep-link decision delegated to a pure, lib-tested helper: only attempt the - // jump once the target actually exists in THIS (deferred) snapshot. If it - // doesn't, the row hasn't committed yet — bail and let the next snapshot that - // includes it drive the jump. This reads the same `messages` snapshot the - // list rendered, which closes the tearing race. - if (!resolveDeepLinkTarget(messages, targetMessageId).resolved) { - return; - } - - if (!scrollToMessage(targetMessageId)) { - return; - } - - handledTargetMessageIdRef.current = targetMessageId; - }, [isLoading, messages, scrollToMessage, targetMessageId]); - - return { - bottomAnchorRef, - contentRef, - highlightedMessageId, - isAtBottom, - newMessageCount, - restoreScrollPosition, - scrollToBottom, - scrollToBottomOnNextUpdate, - scrollToMessage, - syncScrollState, - }; -} diff --git a/desktop/src/shared/ui/markdown.tsx b/desktop/src/shared/ui/markdown.tsx index 516ffceb4..dc5b60248 100644 --- a/desktop/src/shared/ui/markdown.tsx +++ b/desktop/src/shared/ui/markdown.tsx @@ -144,6 +144,32 @@ function aspectRatioFromDim(dim?: string): number | undefined { return width / height; } +/** + * Parse a NIP-92 `dim` value ("WxH") into intrinsic pixel dimensions. Used to + * stamp explicit `width`/`height` attributes on inline images so the browser + * reserves aspect-ratio-correct layout space *before* the image decodes. This + * is what keeps the timeline from jumping when a tall image loads late — the + * row's height is known at first paint instead of growing from ~0 on load. + */ +function dimensionsFromDim( + dim?: string, +): { width: number; height: number } | undefined { + if (!dim) return undefined; + const match = dim.match(/^(\d+)x(\d+)$/i); + if (!match) return undefined; + const width = Number(match[1]); + const height = Number(match[2]); + if ( + !Number.isFinite(width) || + !Number.isFinite(height) || + width <= 0 || + height <= 0 + ) { + return undefined; + } + return { width, height }; +} + function isInsideHiddenSpoiler(element: Element): boolean { return ( element.closest('.buzz-spoiler[data-spoiler][data-revealed="false"]') !== @@ -1004,10 +1030,12 @@ function ImageZoomOverlay({ */ function ImageBlock({ alt, + dim, resolvedSrc, src, }: { alt: string | undefined; + dim?: string; resolvedSrc: string | undefined; src: string | undefined; }) { @@ -1093,6 +1121,12 @@ function ImageBlock({ const closeMenu = React.useCallback(() => setMenu(null), []); useDismissImageContextMenu(Boolean(menu), closeMenu); + // Intrinsic dimensions from the NIP-92 `dim` tag, stamped as width/height + // attributes so the browser reserves aspect-ratio space before the image + // decodes. Without this the row grows from ~0 on load and shoves the + // timeline — the exact reflow the anchored-scroll restore then has to fight. + const intrinsicDimensions = dimensionsFromDim(dim); + const handleContextMenu = (e: React.MouseEvent) => { e.preventDefault(); if (isInsideHiddenSpoiler(e.currentTarget)) return; @@ -1153,9 +1187,11 @@ function ImageBlock({ alt={alt} className="block max-h-64 max-w-sm rounded-xl object-contain" data-spoiler-media-size={hiddenSpoilerMediaSize ? "" : undefined} + height={intrinsicDimensions?.height} ref={imageRef} src={resolvedSrc} style={spoilerMediaStyle} + width={intrinsicDimensions?.width} onLoad={(event) => updateSpoilerMediaSize(event.currentTarget)} /> @@ -1860,9 +1896,15 @@ function createMarkdownComponents( ); } + const entry = src ? imetaByUrl?.get(src) : undefined; return ( - + ); }, @@ -2094,7 +2136,7 @@ function MarkdownInner({ (link: ParsedMessageLink) => { // Always route through `goChannel` with `messageId` set: the channel // route already handles scroll-into-view + highlight via - // `useTimelineScrollManager` + `getEventById` backfill, and works for + // `useAnchoredScroll` + `getEventById` backfill, and works for // both stream-message replies and forum threads. Detecting "the thread // root is a forum post" up front would require an event lookup we don't // currently have synchronously; the brief explicitly allows skipping diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index a97172df7..7d225a063 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -72,6 +72,9 @@ type E2eConfig = { feedReadError?: string; canvasReadError?: string; openDmDelayMs?: number; + /** Delay (ms) applied to older-history (`history-` subId) fetches so e2e + * tests can observe the in-flight prepend window. 0/undefined = instant. */ + historyDelayMs?: number; profileReadDelayMs?: number; profileReadError?: string; profileUpdateError?: string; @@ -593,6 +596,17 @@ declare global { extraTags?: string[][]; createdAt?: number; }) => RelayEvent; + /** Prepend `count` synthetic older messages to a channel's mock store so + * an older-history fetch has something to paginate. Mirrors how the real + * relay backfills history. Returns the created events. */ + __BUZZ_E2E_PREPEND_MOCK_HISTORY__?: (input: { + channelName: string; + count: number; + startIndex?: number; + lineCount?: number; + createdAtStart?: number; + emit?: boolean; + }) => RelayEvent[]; __BUZZ_E2E_EMIT_MOCK_TYPING__?: (input: { channelName: string; pubkey?: string; @@ -2267,6 +2281,60 @@ function getMockMessageStore(channelId: string): RelayEvent[] { return seeded; } +function prependMockHistory(input: { + channelName: string; + count: number; + startIndex?: number; + lineCount?: number; + createdAtStart?: number; + emit?: boolean; +}) { + const channel = mockChannels.find( + (candidate) => candidate.name === input.channelName, + ); + if (!channel) { + throw new Error(`Unknown mock channel: ${input.channelName}`); + } + + const store = getMockMessageStore(channel.id); + const earliestCreatedAt = store.reduce( + (earliest, event) => Math.min(earliest, event.created_at), + Math.floor(Date.now() / 1000), + ); + const createdAtStart = + input.createdAtStart ?? earliestCreatedAt - input.count - 1; + const startIndex = input.startIndex ?? 0; + const lineCount = input.lineCount ?? 1; + + const events = Array.from({ length: input.count }, (_, offset) => { + const index = startIndex + offset; + const body = Array.from( + { length: lineCount }, + (_unused, lineIndex) => `mock older ${index} line ${lineIndex + 1}`, + ).join("\n"); + + return createMockEvent( + 9, + body, + [["h", channel.id]], + ALICE_PUBKEY, + createdAtStart + offset, + `mock-older-${channel.name}-${index}`.replace(/[^a-zA-Z0-9]/g, ""), + ); + }); + + store.unshift(...events); + store.sort((left, right) => left.created_at - right.created_at); + + if (input.emit) { + for (const event of events) { + emitMockLiveEvent(channel.id, event); + } + } + + return events; +} + function emitMockHistory( socket: MockSocket, subId: string, @@ -2290,10 +2358,24 @@ function emitMockHistory( .slice(0, filter.limit ?? 50) .sort((left, right) => left.created_at - right.created_at); - for (const event of events) { - sendWsText(socket.handler, ["EVENT", subId, event]); + const emit = () => { + for (const event of events) { + sendWsText(socket.handler, ["EVENT", subId, event]); + } + sendWsText(socket.handler, ["EOSE", subId]); + }; + + // Optionally pace older-history fetches so e2e tests can observe the + // in-flight prepend window (scroll up, abandon, etc.). Scoped to + // `history-` subscriptions — the prefix `relayClientSession` uses for + // older-message pagination — so live/initial subscriptions stay instant. + const delayMs = getConfig()?.mock?.historyDelayMs ?? 0; + if (delayMs > 0 && subId.startsWith("history-")) { + window.setTimeout(emit, delayMs); + return; } - sendWsText(socket.handler, ["EOSE", subId]); + + emit(); } function emitMockLiveEvent(channelId: string, event: RelayEvent) { @@ -6049,6 +6131,7 @@ export function maybeInstallE2eTauriMocks() { createdAt, ); }; + window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ = prependMockHistory; window.__BUZZ_E2E_EMIT_MOCK_TYPING__ = ({ channelName, pubkey }) => { const channel = mockChannels.find( (candidate) => candidate.name === channelName, diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts new file mode 100644 index 000000000..70d9daa83 --- /dev/null +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -0,0 +1,941 @@ +import { expect, test } from "@playwright/test"; + +import { installMockBridge } from "../helpers/bridge"; + +async function getTimelineMetrics(page: import("@playwright/test").Page) { + return page.getByTestId("message-timeline").evaluate((element) => { + const timeline = element as HTMLDivElement; + + return { + clientHeight: timeline.clientHeight, + scrollHeight: timeline.scrollHeight, + scrollTop: timeline.scrollTop, + }; + }); +} + +async function getFirstVisibleMessage(page: import("@playwright/test").Page) { + return page.getByTestId("message-timeline").evaluate((element) => { + const timeline = element as HTMLDivElement; + const timelineRect = timeline.getBoundingClientRect(); + const messages = Array.from( + timeline.querySelectorAll("[data-message-id]"), + ); + + for (const message of messages) { + const rect = message.getBoundingClientRect(); + if (rect.bottom <= timelineRect.top || rect.top >= timelineRect.bottom) { + continue; + } + + return { + id: message.dataset.messageId ?? "", + text: message.textContent?.replace(/\s+/g, " ").slice(0, 80) ?? "", + top: rect.top - timelineRect.top, + }; + } + + return null; + }); +} + +async function getMessagePosition( + page: import("@playwright/test").Page, + messageId: string, +) { + return page.getByTestId("message-timeline").evaluate((element, id) => { + const timeline = element as HTMLDivElement; + const message = timeline.querySelector( + `[data-message-id="${CSS.escape(id)}"]`, + ); + if (!message) { + return null; + } + + return { + id, + top: + message.getBoundingClientRect().top - + timeline.getBoundingClientRect().top, + }; + }, messageId); +} + +test("preserves user scroll while older channel history loads", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" && + typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function", + ); + + await page.evaluate(() => { + for (let index = 0; index < 40; index += 1) { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `visible current ${index}\nsecond line ${index}`, + }); + } + window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ + channelName: "general", + count: 250, + lineCount: 3, + }); + }); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).toContainText("visible current 39"); + + // Initial load should receive enough history to make the page scrollable. + // Delay only the next history request, so the test isolates pagination while + // the user is actively scrolling. + await page.evaluate(() => { + window.__BUZZ_E2E__ = { + ...window.__BUZZ_E2E__, + mock: { + ...window.__BUZZ_E2E__?.mock, + historyDelayMs: 1_000, + }, + }; + }); + + await page.waitForFunction(() => { + const element = document.querySelector( + '[data-testid="message-timeline"]', + ) as HTMLDivElement | null; + return element && element.scrollHeight > element.clientHeight + 1000; + }); + + // Move away from the bottom before jumping near the top; otherwise the + // timeline's sticky-bottom guard can intentionally pin the first upward jump. + const beforeFetch = await getTimelineMetrics(page); + await timeline.evaluate((element) => { + const timelineElement = element as HTMLDivElement; + timelineElement.scrollTop = timelineElement.scrollHeight; + timelineElement.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + await page.waitForTimeout(50); + + const nearTop = await timeline.evaluate((element) => { + const timelineElement = element as HTMLDivElement; + timelineElement.scrollTop = 180; + timelineElement.dispatchEvent(new Event("scroll", { bubbles: true })); + return timelineElement.scrollTop; + }); + expect(nearTop).toBeLessThan(260); + + await page.waitForTimeout(100); + const duringFetch = await timeline.evaluate((element) => { + const timelineElement = element as HTMLDivElement; + timelineElement.scrollTop = timelineElement.scrollTop + 160; + timelineElement.dispatchEvent(new Event("scroll", { bubbles: true })); + return timelineElement.scrollTop; + }); + expect(duringFetch).toBeGreaterThan(nearTop); + const anchorDuringFetch = await getFirstVisibleMessage(page); + expect(anchorDuringFetch).not.toBeNull(); + + await expect + .poll( + async () => { + const [anchor, metrics] = await Promise.all([ + getMessagePosition(page, anchorDuringFetch?.id ?? ""), + getTimelineMetrics(page), + ]); + if (metrics.scrollHeight <= beforeFetch.scrollHeight + 1000) { + return Number.POSITIVE_INFINITY; + } + return anchor + ? Math.abs(anchor.top - (anchorDuringFetch?.top ?? 0)) + : Number.POSITIVE_INFINITY; + }, + { + timeout: 3_000, + }, + ) + .toBeLessThanOrEqual(2); +}); + +// Criterion 2: abandon-to-bottom mid-fetch. +// +// This catches the live-anchor vs transaction-anchor race that made the old +// design untrustworthy. The user begins an older-history fetch, then changes +// their mind and jumps back to bottom before the fetch resolves. When the +// prepended messages finally land, the timeline must remain pinned to the +// bottom -- it must NOT teleport upward to "restore" the anchor row the user +// already abandoned. +// +// Baseline note (recorded against main): this test PASSES on main. Probe +// geometry shows the existing useLoadOlderOnScroll restore would write +// scrollTop = capturedScrollTop(180) + delta(~9736) = ~9916, but the actual +// post-prepend scrollTop is at the true bottom (~29600). The bottom-stick +// guard in useTimelineScrollManager wins the race against the older-restore +// callback when the user has returned to bottom -- by accident of ordering, +// not by design. The Virtuoso replacement must keep this user-observable +// contract while removing the race-prone two-writer architecture: any +// implementation where firstItemIndex/anchor-restore can override the +// at-bottom state will fail this test. That is exactly what we want to +// prevent regressing. +// +// Black-box assertions only: +// (a) timeline geometry: scrollTop + clientHeight >= scrollHeight - 2 +// (still at bottom after prepend) +// (b) the last [data-message-id] row's bottom is within 2px of the +// timeline's bottom edge (last message stayed visible at the floor) +test("does not teleport upward when user abandons fetch by jumping to bottom", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" && + typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function", + ); + + await page.evaluate(() => { + for (let index = 0; index < 40; index += 1) { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `visible current ${index}\nsecond line ${index}`, + }); + } + window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ + channelName: "general", + count: 250, + lineCount: 3, + }); + }); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + const timeline = page.getByTestId("message-timeline"); + // Setup gate: confirm the channel rendered at least one message row. + // The original draft asserted toContainText("visible current 39") -- + // i.e. the last seeded message text must be in the timeline -- which + // accidentally encodes a non-virtualized contract. A Virtuoso-rendered + // timeline mounts a window of rows; the LAST seeded row may not be in + // that window if the implementation doesn't start at the bottom (which + // is itself a separate concern, validated below by `baseline` being + // pinned to scrollHeight). + await expect(timeline.locator("[data-message-id]").first()).toBeVisible(); + + // Pace the next history fetch so we have a deterministic window to abandon + // it. The window must be longer than the time required for the wheel-up + // step + the post-wheel waitForTimeout, otherwise the fetch resolves + // mid-wheel and the "abandon mid-fetch" semantic disappears (it becomes + // "scroll up, observe prepend land, scroll back to bottom" -- a different + // test). 5s is comfortably longer than the wheel-up loop in practice. + await page.evaluate(() => { + window.__BUZZ_E2E__ = { + ...window.__BUZZ_E2E__, + mock: { + ...window.__BUZZ_E2E__?.mock, + historyDelayMs: 5_000, + }, + }; + }); + + // Wait until the timeline is scrollable before we start the dance. + await page.waitForFunction(() => { + const element = document.querySelector( + '[data-testid="message-timeline"]', + ) as HTMLDivElement | null; + return element ? element.scrollHeight > element.clientHeight + 1000 : false; + }); + + // Start at bottom (the timeline should already be sticky-bottom on first + // load; assert it so the abandon step has a meaningful target to return to). + const baseline = await getTimelineMetrics(page); + expect(baseline.scrollTop + baseline.clientHeight).toBeGreaterThanOrEqual( + baseline.scrollHeight - 2, + ); + + // Scroll up to trigger the older-history fetch. We drive this through + // real wheel input rather than `scrollTop = 180; dispatchEvent("scroll")` + // because that direct-mutation pattern only works against a naive + // scroll container -- a virtualizer like Virtuoso can intercept or + // re-assert its tracked scroll position, leaving the outer element's + // scrollTop pinned to the bottom even after the write. mouse.wheel + // dispatches a real WheelEvent that whichever element owns scrolling + // must honor. Wheel up in 2000-delta chunks (the browser applies its + // own delta scaling so the actual scrolled distance is typically a + // fraction of this) until scrollTop crosses below 500 or we hit a + // step cap. + await timeline.hover(); + for (let attempt = 0; attempt < 32; attempt += 1) { + const metrics = await getTimelineMetrics(page); + if (metrics.scrollTop < 500) { + break; + } + await page.mouse.wheel(0, -2000); + } + await page.waitForTimeout(150); + + const duringFetch = await getTimelineMetrics(page); + // Sanity: we did move away from the bottom and the prepend has NOT yet + // resolved (otherwise the abandon scenario doesn't exist). + expect(duringFetch.scrollTop).toBeLessThan(500); + expect(duringFetch.scrollHeight).toBeLessThanOrEqual( + baseline.scrollHeight + 100, + ); + + // Abandon: jump back to bottom while the fetch is still in flight. + // We click the in-app "Jump to latest" button rather than writing + // scrollTop = scrollHeight directly. Same rationale as the upward + // wheel above: a virtualizer may not honor a raw scrollTop write, + // but the application's own scroll-to-bottom path is exactly what + // a real user would invoke and is part of the user-observable + // surface for both scroller implementations. + // + // The button triggers `scrollToBottom("smooth")` which animates the + // scroll, so we poll for the at-bottom condition rather than waiting + // a fixed interval. Cap at 2s; well inside our 5s historyDelayMs + // window so the prepend is still in flight when this resolves. + await page.getByTestId("message-scroll-to-latest").click(); + await expect + .poll( + async () => { + const m = await getTimelineMetrics(page); + return m.scrollTop + m.clientHeight >= m.scrollHeight - 2; + }, + { timeout: 2_000 }, + ) + .toBe(true); + + const afterAbandon = await getTimelineMetrics(page); + expect( + afterAbandon.scrollTop + afterAbandon.clientHeight, + ).toBeGreaterThanOrEqual(afterAbandon.scrollHeight - 2); + + // Now wait for the prepend to resolve. scrollHeight grows when older + // messages land; we poll for that growth and then assert we are STILL + // at the bottom (no upward teleport to the abandoned anchor). + // + // Timeout: 6s. The wheel-up + smooth-abandon path can burn 2-3s of + // the 5s historyDelayMs window before this poll begins, so the + // prepend may not land for another 2-3s. 6s leaves margin. + await expect + .poll( + async () => { + const metrics = await getTimelineMetrics(page); + return metrics.scrollHeight > afterAbandon.scrollHeight + 1000 + ? "resolved" + : "pending"; + }, + { timeout: 6_000 }, + ) + .toBe("resolved"); + + const afterPrepend = await getTimelineMetrics(page); + // (a) Geometry: timeline still pinned to bottom. + expect( + afterPrepend.scrollTop + afterPrepend.clientHeight, + ).toBeGreaterThanOrEqual(afterPrepend.scrollHeight - 2); + + // (b) DOM: the last rendered [data-message-id] sits within 2px of the + // timeline's bottom edge. This catches a class of bugs where the geometry + // looks bottom-pinned but the row landed off-screen due to padding/ + // composer-spacer drift. + const lastRowOffset = await timeline.evaluate((element) => { + const t = element as HTMLDivElement; + const messages = Array.from( + t.querySelectorAll("[data-message-id]"), + ); + if (messages.length === 0) { + return null; + } + const last = messages[messages.length - 1]; + const timelineRect = t.getBoundingClientRect(); + const rowRect = last.getBoundingClientRect(); + return timelineRect.bottom - rowRect.bottom; + }); + expect(lastRowOffset).not.toBeNull(); + // Allow up to one composer-height worth of slack here: the composer overlay + // can legitimately float above the timeline's clientHeight floor. The + // important regression we are catching is "last row teleported hundreds + // of pixels up", not "last row sits 40px above the floor due to overlay". + // If the new design uses a Footer spacer matching composer height, this + // value should be small. We assert <= 200 to be tolerant of design choice + // while still catching teleport-class regressions. + expect(lastRowOffset as number).toBeLessThanOrEqual(200); +}); + +const REAL_BUZZ_BUGS_IMAGE_SHA = + "ff2862080bac3d009f97cad4bb94e6efec328eaaee058a405e854acd49fc1483"; +const REAL_BUZZ_BUGS_IMAGE_URL = `https://sprout-oss.stage.blox.sqprod.co/media/${REAL_BUZZ_BUGS_IMAGE_SHA}.png`; +const REAL_BUZZ_BUGS_IMAGE_TAG = [ + "imeta", + `url ${REAL_BUZZ_BUGS_IMAGE_URL}`, + "m image/png", + `x ${REAL_BUZZ_BUGS_IMAGE_SHA}`, + "size 26257", + "dim 951x244", + "filename image.png", +] as string[]; + +test("reserves real buzz-bugs imeta image height before image loads", async ({ + page, +}) => { + await page.route("**/media/**", () => new Promise(() => {})); + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + + await page.evaluate( + ({ content, extraTags }) => { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content, + extraTags, + }); + }, + { + content: `this setting gets reverted on every update\n![image](${REAL_BUZZ_BUGS_IMAGE_URL})`, + extraTags: [REAL_BUZZ_BUGS_IMAGE_TAG], + }, + ); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + const image = page.getByAltText("image").last(); + const rect = await image.evaluate((element) => { + const img = element as HTMLImageElement; + const box = img.getBoundingClientRect(); + return { + attrHeight: img.getAttribute("height"), + attrWidth: img.getAttribute("width"), + height: box.height, + offsetHeight: img.offsetHeight, + offsetWidth: img.offsetWidth, + width: box.width, + }; + }); + expect(rect.attrWidth).toBe("951"); + expect(rect.attrHeight).toBe("244"); + expect(rect.offsetHeight).toBeGreaterThan(80); +}); + +// Criterion 3: target-after-backfill via deep-link. +// +// When the user opens a deep-link URL like /channels/?messageId= +// for a message that lives in older history (not in the initial render +// window), the timeline must scroll the target into view and apply the +// highlight treatment. This validates the target-resolution path independent +// of the user manually scrolling up to find the message. +// +// Black-box assertions only (per Mari's refinement): +// (a) target row's [data-message-id] is in the DOM +// (b) target row's bounding rect is inside the timeline's bounding rect +// AND within ~half a viewport of the timeline's vertical center +// (c) target row's className includes the highlight token +// ("route-target-highlight-fade") -- this is the user-visible +// highlight effect, applied via the `highlighted` prop in MessageRow +// +// No `onTargetReached` probe is used; visible-centered-highlighted is the +// stable user-observable contract. +test("deep-link to a message in older history scrolls and highlights it", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" && + typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function", + ); + + // Seed the channel with a small live window plus a large prepended + // history block. Capture the prepended event ids so we can pick a target + // that sits well outside the initial-render window. + const prependedIds: string[] = await page.evaluate(() => { + for (let index = 0; index < 40; index += 1) { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `visible current ${index}\nsecond line ${index}`, + }); + } + const events = window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ + channelName: "general", + count: 250, + lineCount: 3, + }); + return (events ?? []).map((event) => event.id); + }); + expect(prependedIds.length).toBeGreaterThanOrEqual(100); + + // Pick a target from the OLDER half of the prepended block. The initial + // history slice on channel open is limited to ~50 events; anything in + // the older half is guaranteed to be outside the first render window. + // prependedIds are emitted in chronological order (older first), so the + // first quarter is reliably old. + const targetId = prependedIds[Math.floor(prependedIds.length / 8)]; + expect(targetId).toBeTruthy(); + + // Open the channel via the sidebar click (same pattern as criterion-1). + // The dev server has no SPA fallback, so a direct page.goto into a deep + // /channels/?messageId= URL 404s, and a synthetic popstate isn't + // enough to make Tanstack Router's matcher pick up the new route. Going + // through the live UI navigation puts us inside the channel route with + // the router properly mounted; then we only need to update the search + // param to test the target-resolution contract. + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).toContainText("visible current 39"); + + // Now push ?messageId= onto the existing /channels/ URL. + // This is the contract under test: when the route's messageId search + // param changes (which happens both on deep-link arrival and on in-app + // navigation like clicking a reply quote), the timeline must resolve the + // target, scroll to it, and apply the highlight -- even when the target + // is in not-yet-loaded older history. + // + // The app uses a hash history (createHashHistory), so the router's + // location -- pathname AND search params -- lives inside the URL hash + // fragment (#/channels/?messageId=...), NOT in window.location.search. + // We must therefore rewrite the hash, not the top-level query string, or + // Tanstack Router never sees the param and targetMessageId stays null. + await page.evaluate((targetId) => { + const hash = window.location.hash.replace(/^#/, "") || "/"; + const [path, query = ""] = hash.split("?"); + const params = new URLSearchParams(query); + params.set("messageId", targetId); + const nextHash = `#${path}?${params.toString()}`; + window.history.pushState({}, "", `${window.location.pathname}${nextHash}`); + window.dispatchEvent(new HashChangeEvent("hashchange")); + window.dispatchEvent(new PopStateEvent("popstate")); + }, targetId); + + // Wait for the target row to appear in the DOM. Note: data-message-id + // values are nostr event ids (lowercase hex), so no CSS-escape is needed + // for the Playwright locator. The `evaluate` block below runs in + // browser context and can use CSS.escape directly. + 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. + 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, + }; + }, targetId); + expect(placement).not.toBeNull(); + const p = placement as NonNullable; + + // Row is inside the timeline viewport vertically. + expect(p.rowTopRelative).toBeGreaterThanOrEqual(0); + expect(p.rowBottomRelative).toBeLessThanOrEqual(p.timelineHeight + 1); + + // Row's center is within half-a-viewport of the timeline's vertical center. + // This catches "scrolled but at the very top/bottom" regressions while + // tolerating different centering strategies (smooth scrollIntoView with + // block: "center" vs Virtuoso scrollToIndex({ align: "center" })). + const rowCenter = (p.rowTopRelative + p.rowBottomRelative) / 2; + const timelineCenter = p.timelineHeight / 2; + expect(Math.abs(rowCenter - timelineCenter)).toBeLessThanOrEqual( + p.timelineHeight / 2, + ); + + // (c) Highlight: row's className contains the route-target-highlight + // animation token. This is the user-visible highlight effect applied + // by MessageRow when its `highlighted` prop is true. + expect(p.className).toContain("route-target-highlight-fade"); +}); + +// Criterion 5: search-active match enters the timeline viewport and +// carries the active-match highlight class, even when the match is far +// from the current scroll position. +// +// In a virtualized list, "active match" cannot be implemented via +// querySelector('[data-message-id=...]') + scrollIntoView, because rows +// outside the rendered window are not in the DOM. The contract this test +// pins (regardless of virtualization strategy): +// +// For any user-driven find-bar match selection, the matched row must +// enter the timeline viewport and carry the route-target-highlight-fade +// className. +// +// Test method (black-box only): +// 1. Seed the channel with a generic message bulk plus two distinct +// needles: NEEDLE-ALPHA early in the history, NEEDLE-BRAVO later. +// 2. Open the channel; user is at the bottom (most recent). +// 3. Open the find bar via Cmd/Ctrl+F. Type the ALPHA needle. +// 4. Assert: the row matching ALPHA is in the timeline viewport and +// carries the highlight class. +// 5. Replace the query with the BRAVO needle. +// 6. Assert the same two properties on the BRAVO row. +// +// Centeredness is intentionally NOT asserted -- see the comment on +// `assertInViewportAndHighlighted` below for why edge-bias is correct +// browser behavior, not a scroll regression. +// +// On main (non-virtualized) this is expected to pass: querySelector + +// scrollIntoView in MessageTimeline.tsx:165-174 finds the row because all +// loaded messages are in the DOM. The blind spot -- which this test will +// catch on the Virtuoso branch if not handled -- is recycling: any +// implementation where rows outside the rendered window are removed from +// the DOM must route active-match selection through a key->index map, +// not querySelector. +// +// Per Mari's baseline-either-way rule, recording the contract here is +// valuable even though main happens to satisfy it by construction. +test("find-bar active match scrolls and highlights row regardless of position", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + + const ALPHA = "NEEDLE-ALPHA-c7b3"; + const BRAVO = "NEEDLE-BRAVO-9f21"; + const TOTAL = 200; + const ALPHA_INDEX = 20; // near the top after backfill + const BRAVO_INDEX = 110; // mid-channel + + await page.evaluate( + ({ total, alpha, bravo, alphaIndex, bravoIndex }) => { + for (let i = 0; i < total; i += 1) { + let body = `filler message ${i}`; + if (i === alphaIndex) { + body = `filler message ${i}\n${alpha}`; + } else if (i === bravoIndex) { + body = `filler message ${i}\n${bravo}`; + } + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: body, + }); + } + }, + { + total: TOTAL, + alpha: ALPHA, + bravo: BRAVO, + alphaIndex: ALPHA_INDEX, + bravoIndex: BRAVO_INDEX, + }, + ); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + const timeline = page.getByTestId("message-timeline"); + // Setup gate: confirm the channel rendered at least one message row + // AND the user is at the bottom of the channel before opening the + // find bar. The original draft asserted toContainText( + // `filler message ${TOTAL - 1}`) -- i.e. the last seeded message + // text must be in the timeline -- which accidentally encodes a + // non-virtualized contract (a virtualized timeline mounts only a + // window of rows; the LAST seeded row may not be in that window). + // + // The at-bottom geometry assertion is load-bearing for this test: + // the find-bar contract is "active match scrolls a non-visible row + // into view." If the test started at a position where ALPHA was + // already mounted/visible, the scroll-into-view path would be a + // no-op and the test would pass vacuously. Pinning the start to + // the bottom of the channel guarantees the matched row is far from + // initial scroll position regardless of how the implementation + // sizes its initial render window. + await expect(timeline.locator("[data-message-id]").first()).toBeVisible(); + await expect + .poll(async () => { + const m = await timeline.evaluate((el) => { + const t = el as HTMLDivElement; + return { + scrollTop: t.scrollTop, + clientHeight: t.clientHeight, + scrollHeight: t.scrollHeight, + }; + }); + return m.scrollTop + m.clientHeight >= m.scrollHeight - 2; + }) + .toBe(true); + + // Open the find bar. The shortcut handler uses platform-standard + // primary modifier (Meta on macOS, Control elsewhere). Playwright's + // ControlOrMeta abstracts this for us. + await page.keyboard.press("ControlOrMeta+f"); + await expect(page.getByTestId("channel-find-bar")).toBeVisible(); + + const input = page.getByPlaceholder("Find in channel"); + + // Poll for the row matching `needle` to settle inside the timeline + // viewport, then return its placement + className. Polling is required + // because the find-bar -> active-match -> scrollIntoView path is async + // (state update, then a smooth scroll). Locator `toBeVisible` only + // checks DOM-visible (display/visibility), not in-viewport, so it + // can't be used as the wait condition for "the scroll completed". + // + // Tolerance: 1px on each edge for sub-pixel rounding. The 5s budget + // accommodates browsers honoring smooth-scroll over long distances + // (initial scroll position is the bottom of a 200-message channel; + // the ALPHA row is ~180 rows up). + const waitForRowInViewport = async (needle: string) => + timeline.evaluate((timelineEl, n) => { + return new Promise<{ + rowTopRelative: number; + rowBottomRelative: number; + timelineHeight: number; + className: string; + }>((resolve, reject) => { + const deadline = performance.now() + 5_000; + const t = timelineEl as HTMLDivElement; + const tick = () => { + const rows = Array.from( + t.querySelectorAll("[data-message-id]"), + ); + const row = rows.find((r) => r.textContent?.includes(n)) ?? null; + if (row) { + const tRect = t.getBoundingClientRect(); + const rRect = row.getBoundingClientRect(); + const top = rRect.top - tRect.top; + const bottom = rRect.bottom - tRect.top; + const height = tRect.height; + if (top >= -1 && bottom <= height + 1) { + resolve({ + rowTopRelative: top, + rowBottomRelative: bottom, + timelineHeight: height, + className: row.className, + }); + return; + } + } + if (performance.now() > deadline) { + reject( + new Error( + `row matching "${n}" did not enter timeline viewport within 5s`, + ), + ); + return; + } + requestAnimationFrame(tick); + }; + tick(); + }); + }, needle); + + const assertInViewportAndHighlighted = (placement: { + rowTopRelative: number; + rowBottomRelative: number; + timelineHeight: number; + className: string; + }) => { + // User-observable contract: row enters the timeline's own viewport and + // carries the active-match highlight class. We intentionally do NOT + // assert centeredness: `scrollIntoView({ block: 'center' })` legitimately + // biases toward a list edge when the target is near the start or end + // (e.g., the ALPHA index-20 row of a 200-message channel lands at the + // top because the browser cannot center an item that has fewer rows + // above it than half the viewport). Near-edge bias is correct browser + // behavior, not a scroll-contract regression; tightening on + // centeredness here would couple the test to scroll-anchor heuristics + // rather than the user-observable invariant. + expect(placement.rowTopRelative).toBeGreaterThanOrEqual(-1); + expect(placement.rowBottomRelative).toBeLessThanOrEqual( + placement.timelineHeight + 1, + ); + expect(placement.className).toContain("route-target-highlight-fade"); + }; + + // --- Phase 1: ALPHA --- + await input.fill(ALPHA); + // Sanity check: the active match should resolve and the matching row + // should land in the DOM. visibility != in-viewport here -- we follow + // up with `waitForRowInViewport` to enforce the placement contract. + const alphaRow = timeline.locator(`[data-message-id]`).filter({ + hasText: ALPHA, + }); + await expect(alphaRow).toBeVisible({ timeout: 5_000 }); + + const a = await waitForRowInViewport(ALPHA); + assertInViewportAndHighlighted(a); + + // --- Phase 2: BRAVO --- + // Replace the query. The active-match id changes, which should drive + // a fresh scroll + highlight on the BRAVO row. + await input.fill(BRAVO); + const bravoRow = timeline.locator(`[data-message-id]`).filter({ + hasText: BRAVO, + }); + await expect(bravoRow).toBeVisible({ timeout: 5_000 }); + + const b = await waitForRowInViewport(BRAVO); + assertInViewportAndHighlighted(b); +}); + +// Criterion 6 (composer half): expanding the composer (multi-line input) +// must not push the bottom row out of the user-visible area of the +// timeline when the user is following the bottom. On main the composer +// is rendered as an overlay (`absolute inset-x-0 bottom-0`) on top of +// the timeline, and the timeline reserves bottom padding to keep the +// last message clear of the composer. The contract this test pins +// (regardless of overlay-vs-sibling layout strategy): +// +// While the user is at the bottom of the timeline, growing the +// composer from one line to several lines must keep the last +// message's bottom edge at-or-above the composer's top edge +// (within a small tolerance). The bottom row must not slide +// underneath the composer chrome. +// +// Test method (black-box only): +// 1. Seed the channel with enough messages to make the timeline +// scrollable. The last message text is unique so we can address +// it. +// 2. Open the channel. The view starts at the bottom by default. +// 3. Record the gap between the last message's bottom edge and the +// composer's top edge. The bottom row must sit at-or-above the +// composer top (gap >= 0). +// 4. Focus the composer and press Shift+Enter several times to grow +// it into a multi-line state. +// 5. Wait for the composer to actually grow (its bounding rect grows +// taller, equivalently its top moves up). +// 6. Assert the bottom-row-to-composer-top gap is still >= 0 +// (within tolerance). The timeline must have either auto-scrolled +// to follow output or kept enough bottom padding to clear the +// enlarged composer. +// +// Tolerance: 4px. Accounts for sub-pixel rendering and a single rAF +// of lag between composer resize and follow-output. We intentionally +// don't require equality -- the invariant is "the user can still see +// the bottom row clear of composer chrome," not "glued to a specific +// pixel." +test("composer expansion does not push bottom row out of viewport", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + + // Seed enough messages that the timeline is scrollable. The bottom + // message has a distinct text so we can resolve its row even if the + // virtualizer recycles surrounding rows. + const TOTAL = 60; + const BOTTOM_NEEDLE = "BOTTOM-NEEDLE-3a91"; + await page.evaluate( + ({ total, bottom }) => { + for (let i = 0; i < total; i += 1) { + const body = i === total - 1 ? bottom : `filler message ${i}`; + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: body, + }); + } + }, + { total: TOTAL, bottom: BOTTOM_NEEDLE }, + ); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).toContainText(BOTTOM_NEEDLE); + + // Geometry probe: report the gap between the bottom row's bottom edge + // and the composer's top edge, plus the composer's own height so we + // can verify the composer-driven growth separately. + const probe = async () => + page.evaluate((needle) => { + const t = document.querySelector( + '[data-testid="message-timeline"]', + ); + const c = document.querySelector( + '[data-testid="message-composer"]', + ); + if (!t || !c) { + return { found: false as const }; + } + const rows = Array.from( + t.querySelectorAll("[data-message-id]"), + ); + const bottomRow = rows.find((r) => r.textContent?.includes(needle)); + if (!bottomRow) { + return { found: false as const }; + } + const rRect = bottomRow.getBoundingClientRect(); + const cRect = c.getBoundingClientRect(); + return { + found: true as const, + // Positive or zero means the row's bottom sits at-or-above + // the composer's top. Negative means the row has been pushed + // underneath the composer overlay. + gapAboveComposer: cRect.top - rRect.bottom, + composerHeight: cRect.height, + }; + }, BOTTOM_NEEDLE); + + const before = await probe(); + expect(before.found).toBe(true); + if (!before.found) return; + // Sanity: starting state has the bottom row clear of the composer. + expect(before.gapAboveComposer).toBeGreaterThanOrEqual(-4); + + // Grow the composer. Shift+Enter inserts a hard break in the Tiptap + // editor without sending; six line breaks plus typed text reliably + // grows the composer past its single-line min-height. The inner + // editor scroll container is height-capped at max-h-32, so growth + // stops at ~one extra row of UI height after the cap is reached -- + // that's still enough to verify the contract: even modest composer + // growth must not occlude the bottom row. + const input = page.getByTestId("message-input"); + await input.click(); + for (let i = 0; i < 6; i += 1) { + await page.keyboard.type(`line ${i}`); + await page.keyboard.press("Shift+Enter"); + } + await page.keyboard.type("line 6"); + + // Wait for the composer growth to propagate through layout. The + // composer's outer bounding height must visibly grow. This is the + // smallest sanity guard against the test tautologically passing + // when the composer didn't actually grow (e.g., focus failed). + await expect + .poll( + async () => { + const p = await probe(); + return p.found ? p.composerHeight : Number.NaN; + }, + { timeout: 5_000 }, + ) + .toBeGreaterThan(before.composerHeight + 4); + + // The invariant: bottom row must still be clear of the composer + // overlay. Either the timeline auto-scrolled to follow output, or + // the reserved bottom padding grew with the composer. Both are + // acceptable user-observable states. + const after = await probe(); + expect(after.found).toBe(true); + if (!after.found) return; + expect(after.gapAboveComposer).toBeGreaterThanOrEqual(-4); +}); diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index 1d79e3cf4..8f6c04a3e 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -96,6 +96,8 @@ type MockBridgeOptions = { feedReadError?: string; canvasReadError?: string; openDmDelayMs?: number; + /** Delay (ms) for older-history fetches; see e2eBridge mock config. */ + historyDelayMs?: number; profileReadDelayMs?: number; profileReadError?: string; profileUpdateError?: string; From 2a56e08eb9af9f8544d5542de23830a5dda141de Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Thu, 18 Jun 2026 12:56:40 -0400 Subject: [PATCH 02/15] fix(desktop): restore overflow-anchor:none on live scroll containers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit useAnchoredScroll is meant to be the single owner of scrollTop, but both live scroll containers had lost the [overflow-anchor:none] class (it survived only on the thread loading skeleton, which never scrolls). With it gone, Chromium's native scroll-anchoring heuristic re-engaged and applied its own scrollTop correction on prepend — picking its own anchor element — while the hook applied a second scrollBy correction on top. When the two anchors diverged the corrections stacked, producing the residual jiggle that survived the single-owner rewrite. Restoring the class on both live containers (MessageTimeline timeline + MessageThreadPanel body) hands scroll ownership back to the hook alone. This is a real-wheel-only symptom: native scroll anchoring barely fires under synthetic scrollTop= writes, so the headless suite stayed green while a manual macOS scroll pass surfaced it. scroll-history e2e 6/6, tsc clean. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- desktop/src/features/messages/ui/MessageThreadPanel.tsx | 2 +- desktop/src/features/messages/ui/MessageTimeline.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index 06b12a274..cb3bc0b5a 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -383,7 +383,7 @@ export function MessageThreadPanel({ const threadScrollRegion = (
Date: Thu, 18 Jun 2026 13:35:16 -0400 Subject: [PATCH 03/15] fix(desktop): keep deep-linked message spliced after route target clears MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ChannelRouteScreen splices a getEventById-fetched target into targetMessageEvents so a deep link works in a channel whose feed doesn't already contain the message. The fetch effect wiped those events whenever the route target cleared — and onTargetReached clears the messageId URL param the moment the row is centered — so the only copy of the deep-linked message vanished and the timeline went blank. Reset spliced events on channel/forum-post change only (guarded effect keyed on channelId/selectedPostId, seeded with the mount key so first-commit cache seeds survive), and have the fetch effect merge cached/fetched events additively instead of overwriting. e2eBridge: route the mock-engineering-shipped known event by 'h' tag so getChannelIdFromTags resolves it. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- desktop/src/app/routes/ChannelRouteScreen.tsx | 32 +++++++++++++++++-- desktop/src/testing/e2eBridge.ts | 2 +- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/desktop/src/app/routes/ChannelRouteScreen.tsx b/desktop/src/app/routes/ChannelRouteScreen.tsx index ccfec1506..fc94ca8e5 100644 --- a/desktop/src/app/routes/ChannelRouteScreen.tsx +++ b/desktop/src/app/routes/ChannelRouteScreen.tsx @@ -39,18 +39,46 @@ export function ChannelRouteScreen({ return cachedTarget ? [cachedTarget] : []; }); + // Reset spliced target events when the channel context changes (channel + // switch or entering/leaving a forum post). Tied to channel identity rather + // than the route target so clearing the `messageId` param mid-channel keeps + // the deep-linked row in view. Seeded with the mount key so the initial + // cache-seeded events survive first commit; only a genuine channel change + // clears them. Declared before the fetch effect so a channel switch clears + // stale events before the new target is fetched. + const previousResetKeyRef = React.useRef( + `${channelId}::${selectedPostId ?? ""}`, + ); + React.useEffect(() => { + const resetKey = `${channelId}::${selectedPostId ?? ""}`; + if (previousResetKeyRef.current === resetKey) return; + previousResetKeyRef.current = resetKey; + setTargetMessageEvents([]); + }, [channelId, selectedPostId]); + React.useEffect(() => { let isCancelled = false; + // Don't wipe already-spliced target events just because the route target + // cleared (e.g. `onTargetReached` clears the `messageId` URL param once the + // row is centered). In a channel whose feed doesn't already contain the + // deep-linked message, the spliced event is the only copy — dropping it on + // param-clear blanks the timeline. Resetting on channel / forum-post change + // is handled by the effect below; here we only fetch when there's a target. if ((!targetMessageId && !targetThreadRootId) || selectedPostId) { - setTargetMessageEvents([]); return () => { isCancelled = true; }; } const cachedTarget = getCachedSearchHitEvent(targetMessageId); - setTargetMessageEvents(cachedTarget ? [cachedTarget] : []); + if (cachedTarget) { + setTargetMessageEvents((currentEvents) => + currentEvents.some((event) => event.id === cachedTarget.id) + ? currentEvents + : [...currentEvents, cachedTarget], + ); + } const eventIds = [ targetMessageId, diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 7d225a063..37de50c5e 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -5710,7 +5710,7 @@ async function handleGetEvent( "bb22a5299220cad76ffd46190ccbeede8ab5dc260faa28b6e5a2cb31b9aff260", created_at: Math.floor(Date.now() / 1000) - 42 * 60, kind: 9, - tags: [["e", "1c7e1c02-87bb-5e88-b2da-5a7a9432d0c9"]], + tags: [["h", "1c7e1c02-87bb-5e88-b2da-5a7a9432d0c9"]], content: "Engineering shipped the desktop build.", sig: "mocksig".repeat(20).slice(0, 128), }, From 02efffc956124cde3d8084cad8c8d911fa1c3677 Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Thu, 18 Jun 2026 14:34:14 -0400 Subject: [PATCH 04/15] fix(desktop): restore anchor when fetch-older spinner toggles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The older-history fetch spinner renders above the anchor row but toggles on its own render commit (messages unchanged). The anchor-restoration layout effect was keyed only on `messages`, so it never re-ran when the spinner appeared or disappeared — leaving the spinner's height as an uncorrected shift above the reader's eye, the residual flicker on prepend. Thread isFetchingOlder into useAnchoredScroll as an extra restoration trigger so the existing scrollBy correction fires on the spinner toggle too, making the anchor the single owner of every layout change above the fold. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- desktop/src/features/messages/ui/MessageTimeline.tsx | 1 + desktop/src/features/messages/ui/useAnchoredScroll.ts | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index 2ef6d453d..f9d7d1716 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -215,6 +215,7 @@ const MessageTimelineBase = React.forwardRef< contentRef, fetchOlder, hasOlderMessages, + isFetchingOlder, isLoading: showTimelineSkeleton, messages: deferredMessages, onTargetReached, diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index f3acdd935..adc65b344 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -34,6 +34,13 @@ type UseAnchoredScrollOptions = { * 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; @@ -142,6 +149,7 @@ export function useAnchoredScroll({ messages, fetchOlder, hasOlderMessages = false, + isFetchingOlder = false, targetMessageId = null, onTargetReached, }: UseAnchoredScrollOptions): UseAnchoredScrollResult { @@ -272,6 +280,7 @@ export function useAnchoredScroll({ // before the render. This is the single mechanism for keeping scroll // stable across prepends, appends, image loads, embed expansions, etc. // --------------------------------------------------------------------------- + // 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; @@ -388,6 +397,7 @@ export function useAnchoredScroll({ prevLastMessageIdRef.current = lastMessage?.id; prevMessageCountRef.current = messages.length; }, [ + isFetchingOlder, isLoading, messages, onTargetReached, From 53c580871eeea58249493e92a7213e4a0096953c Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Thu, 18 Jun 2026 15:49:11 -0400 Subject: [PATCH 05/15] fix(desktop): key timeline day sections by calendar day, not first message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each day group renders as a `
` whose React key was the exact `createdAt` of its first message. Scrolling up prepends older messages, and when a batch landed on a calendar day already on screen, the first message of that day changed — flipping the section key. React can't diff a changed key, so it unmounted and remounted the entire day section, all its rows torn down and rebuilt above the reader's eye. That whole-section remount is the residual flicker on scroll-up: the anchor restore was correcting a full teardown instead of a clean prepend. Key the section by the local start-of-day of its messages, which is stable across same-day prepends, so an older message grows an existing section's children (stably-keyed rows reorder, not remount) instead of replacing the section. Fold the duplicate key derivation in the render loop into the lib boundary's `key`, which was already documented as "stable" but wasn't. Pure helper `startOfLocalDaySeconds` plus lib tests for day-key stability across a prepend and separation across calendar days. tsc, biome, 40 lib unit, scroll-history e2e 6/6 green. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- .../messages/lib/dateFormatters.test.mjs | 20 +++++++++++++++ .../features/messages/lib/dateFormatters.ts | 12 +++++++++ .../messages/lib/timelineSnapshot.test.mjs | 25 +++++++++++++++++++ .../features/messages/lib/timelineSnapshot.ts | 10 +++++--- .../messages/ui/TimelineMessageList.tsx | 14 ++++++----- 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/desktop/src/features/messages/lib/dateFormatters.test.mjs b/desktop/src/features/messages/lib/dateFormatters.test.mjs index 7d533dbe3..260b7a358 100644 --- a/desktop/src/features/messages/lib/dateFormatters.test.mjs +++ b/desktop/src/features/messages/lib/dateFormatters.test.mjs @@ -5,6 +5,7 @@ import { formatDayHeading, formatShortMonthDayOrdinal, formatThreadSummaryLastReplyTime, + startOfLocalDaySeconds, } from "./dateFormatters.ts"; function localUnixSeconds(year, monthIndex, day) { @@ -126,3 +127,22 @@ test("formatDayHeading includes the year for other years", () => { `${weekday(date)}, May 19th, ${year}`, ); }); + +test("startOfLocalDaySeconds collapses a day's timestamps to one value", () => { + const morning = new Date(2026, 5, 14, 8, 30, 15).getTime() / 1_000; + const evening = new Date(2026, 5, 14, 23, 59, 59).getTime() / 1_000; + const midnight = new Date(2026, 5, 14, 0, 0, 0).getTime() / 1_000; + + assert.equal(startOfLocalDaySeconds(morning), midnight); + assert.equal(startOfLocalDaySeconds(evening), midnight); +}); + +test("startOfLocalDaySeconds separates adjacent calendar days", () => { + const lateOn14 = new Date(2026, 5, 14, 23, 0, 0).getTime() / 1_000; + const earlyOn15 = new Date(2026, 5, 15, 1, 0, 0).getTime() / 1_000; + + assert.notEqual( + startOfLocalDaySeconds(lateOn14), + startOfLocalDaySeconds(earlyOn15), + ); +}); diff --git a/desktop/src/features/messages/lib/dateFormatters.ts b/desktop/src/features/messages/lib/dateFormatters.ts index 344be9bf5..c0a3904d8 100644 --- a/desktop/src/features/messages/lib/dateFormatters.ts +++ b/desktop/src/features/messages/lib/dateFormatters.ts @@ -78,6 +78,18 @@ export function isSameDay(a: number, b: number): boolean { return isSameDayDate(new Date(a * 1_000), new Date(b * 1_000)); } +/** + * Unix-seconds timestamp of local midnight for the calendar day containing + * `unixSeconds`. Two timestamps on the same calendar day map to the same value, + * so it is a stable identifier for a day group that does not shift when an + * older message is prepended into that day. + */ +export function startOfLocalDaySeconds(unixSeconds: number): number { + const date = new Date(unixSeconds * 1_000); + date.setHours(0, 0, 0, 0); + return Math.floor(date.getTime() / 1_000); +} + /** Short month + ordinal day, e.g. "May 19th". */ export function formatShortMonthDayOrdinal(unixSeconds: number): string { return formatMonthDayOrdinal( diff --git a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs index dc0b5aaf4..be0c291ab 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.test.mjs +++ b/desktop/src/features/messages/lib/timelineSnapshot.test.mjs @@ -212,6 +212,31 @@ test("buildDayGroupBoundaries: group counts always sum to message count", () => assert.equal(total, messages.length); }); +test("buildDayGroupBoundaries: same-day group key is stable across a prepend", () => { + // The day section is keyed by this value; if it changes when an older + // message lands on the same calendar day, React remounts the whole section + // on every scroll-up prepend — the timeline flicker. The key must depend on + // the calendar day, not the first message's exact timestamp. + const before = buildDayGroupBoundaries([ + message({ id: "b", createdAt: dayAt(2026, 6, 14, 9) }), + message({ id: "c", createdAt: dayAt(2026, 6, 14, 10) }), + ]); + const afterPrepend = buildDayGroupBoundaries([ + message({ id: "a", createdAt: dayAt(2026, 6, 14, 8) }), + message({ id: "b", createdAt: dayAt(2026, 6, 14, 9) }), + message({ id: "c", createdAt: dayAt(2026, 6, 14, 10) }), + ]); + assert.equal(before[0].key, afterPrepend[0].key); +}); + +test("buildDayGroupBoundaries: distinct calendar days get distinct keys", () => { + const groups = buildDayGroupBoundaries([ + message({ id: "a", createdAt: dayAt(2026, 6, 13, 12) }), + message({ id: "b", createdAt: dayAt(2026, 6, 14, 12) }), + ]); + assert.notEqual(groups[0].key, groups[1].key); +}); + // --- jump-to-message deep links ---------------------------------------------- test("resolveDeepLinkTarget: unresolved with no target", () => { diff --git a/desktop/src/features/messages/lib/timelineSnapshot.ts b/desktop/src/features/messages/lib/timelineSnapshot.ts index e63c2ea0d..8cf3b1a1d 100644 --- a/desktop/src/features/messages/lib/timelineSnapshot.ts +++ b/desktop/src/features/messages/lib/timelineSnapshot.ts @@ -11,7 +11,7 @@ */ import type { TimelineMessage } from "@/features/messages/types"; -import { isSameDay } from "./dateFormatters"; +import { isSameDay, startOfLocalDaySeconds } from "./dateFormatters"; /** Distance (px) from the bottom within which the timeline counts as "at bottom". */ export const BOTTOM_THRESHOLD_PX = 72; @@ -88,7 +88,11 @@ export function selectLatestMessageAutoScrollBehavior({ /** A single day boundary in the timeline: where it starts and how many messages it covers. */ export type DayGroupBoundary = { - /** Stable key for the day section. */ + /** + * Stable key for the day section: the local start-of-day of the messages it + * covers, so prepending an older message into an already-rendered day reuses + * the same key instead of remounting the whole `
`. + */ key: string; /** Index into `messages` of the first message in this day. */ startIndex: number; @@ -114,7 +118,7 @@ export function buildDayGroupBoundaries( if (!prev || !isSameDay(prev.createdAt, message.createdAt)) { boundaries.push({ - key: `day-${message.createdAt}`, + key: `day-${startOfLocalDaySeconds(message.createdAt)}`, startIndex: i, count: 1, headingTimestamp: message.createdAt, diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index 9f66b161e..aaf9f9f7e 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -146,11 +146,12 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ // Day-divider decision delegated to a pure, lib-tested helper: a new group // starts at index 0 and whenever a message falls on a different calendar day - // than the one before it. We index the boundary start positions so the render - // loop below stays a straight walk while the grouping logic lives in `lib/`. - const dayGroupStartIndices = new Set( + // than the one before it. We index the boundaries by start position so the + // render loop below stays a straight walk while the grouping logic — and the + // prepend-stable section key — lives in `lib/`. + const dayGroupBoundariesByStartIndex = new Map( buildDayGroupBoundaries(entries.map((entry) => entry.message)).map( - (boundary) => boundary.startIndex, + (boundary) => [boundary.startIndex, boundary], ), ); @@ -158,9 +159,10 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ const { message, summary } = entries[i]; const messageRenderKey = message.renderKey ?? message.id; - if (dayGroupStartIndices.has(i)) { + const dayBoundary = dayGroupBoundariesByStartIndex.get(i); + if (dayBoundary) { currentDayGroup = { - key: `day-${message.createdAt}`, + key: dayBoundary.key, label: formatDayHeading(message.createdAt), elements: [], }; From feb9ac424369a3b8e0fcb945bbf33a4bead13f08 Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Thu, 18 Jun 2026 16:10:46 -0400 Subject: [PATCH 06/15] fix(desktop): page older history until N visible rows, not a fixed message count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scrolling up fetched a fixed 100-message batch and stopped. Because thread replies collapse into their parent's summary and non-content events never render, a reply-heavy window could fetch 100 events but grow the timeline by only a handful of rows — making one wheel-up feel like it did nothing. fetchOlder now pages in batches until at least MIN_TOP_LEVEL_ROWS_PER_FETCH (10) *visible* top-level rows have been added, or history is exhausted. A new pure helper, countTopLevelTimelineRows, counts rows the same way buildMainTimelineEntries renders them: content kinds, minus deletions, top-level or broadcast replies only. The dedup/exhaustion guards already in place terminate the loop when the window stops advancing. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- .../lib/formatTimelineMessages.test.mjs | 96 ++++++++++++++++++- .../messages/lib/formatTimelineMessages.ts | 46 ++++++++- .../messages/useFetchOlderMessages.ts | 78 ++++++++++----- 3 files changed, 195 insertions(+), 25 deletions(-) diff --git a/desktop/src/features/messages/lib/formatTimelineMessages.test.mjs b/desktop/src/features/messages/lib/formatTimelineMessages.test.mjs index fa7232614..6526d19f8 100644 --- a/desktop/src/features/messages/lib/formatTimelineMessages.test.mjs +++ b/desktop/src/features/messages/lib/formatTimelineMessages.test.mjs @@ -1,7 +1,10 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { formatTimelineMessages } from "./formatTimelineMessages.ts"; +import { + countTopLevelTimelineRows, + formatTimelineMessages, +} from "./formatTimelineMessages.ts"; const HEX64_A = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; @@ -100,3 +103,94 @@ test("deletion target with non-hex `e` tag value is ignored", () => { "malformed deletion tag should not match anything", ); }); + +// --------------------------------------------------------------------------- +// countTopLevelTimelineRows — the unit fetch-older pages by. Must match the +// rows `buildMainTimelineEntries` would actually render: top-level content +// events, minus deletions, with thread replies collapsed into their parent. +// --------------------------------------------------------------------------- + +function hex64(char) { + return char.repeat(64); +} + +function message(id, overrides = {}) { + return { + id, + pubkey: PUBKEY_A, + kind: 9, + created_at: 1_700_000_000, + content: "hi", + tags: [["h", CHANNEL_ID]], + sig: "sig", + ...overrides, + }; +} + +function reply(id, parentId, overrides = {}) { + return message(id, { + tags: [ + ["h", CHANNEL_ID], + ["e", parentId, "", "reply"], + ], + ...overrides, + }); +} + +test("countTopLevelTimelineRows counts top-level messages", () => { + const events = [ + message(hex64("1")), + message(hex64("2")), + message(hex64("3")), + ]; + assert.equal(countTopLevelTimelineRows(events), 3); +}); + +test("countTopLevelTimelineRows ignores collapsed thread replies", () => { + const root = hex64("1"); + const events = [ + message(root), + reply(hex64("2"), root), + reply(hex64("3"), root), + ]; + // Two replies collapse into the root's summary → one visible row. + assert.equal(countTopLevelTimelineRows(events), 1); +}); + +test("countTopLevelTimelineRows counts broadcast replies as top-level", () => { + const root = hex64("1"); + const broadcast = reply(hex64("2"), root, { + tags: [ + ["h", CHANNEL_ID], + ["e", root, "", "reply"], + ["broadcast", "1"], + ], + }); + assert.equal(countTopLevelTimelineRows([message(root), broadcast]), 2); +}); + +test("countTopLevelTimelineRows excludes deleted messages", () => { + const target = hex64("1"); + const events = [ + message(target), + message(hex64("2")), + deletionEvent(9005, target, { id: hex64("9") }), + ]; + assert.equal(countTopLevelTimelineRows(events), 1); +}); + +test("countTopLevelTimelineRows ignores non-content kinds (reactions)", () => { + const reaction = { + id: hex64("9"), + pubkey: PUBKEY_B, + kind: 7, + created_at: 1_700_000_001, + content: "+", + tags: [ + ["h", CHANNEL_ID], + ["e", hex64("1")], + ], + sig: "sig", + }; + assert.equal(countTopLevelTimelineRows([message(hex64("1")), reaction]), 1); +}); diff --git a/desktop/src/features/messages/lib/formatTimelineMessages.ts b/desktop/src/features/messages/lib/formatTimelineMessages.ts index 6b18a90be..5cb474c0d 100644 --- a/desktop/src/features/messages/lib/formatTimelineMessages.ts +++ b/desktop/src/features/messages/lib/formatTimelineMessages.ts @@ -9,7 +9,10 @@ import type { TimelineMessage, TimelineReaction, } from "@/features/messages/types"; -import { getThreadReference } from "@/features/messages/lib/threading"; +import { + getThreadReference, + isBroadcastReply, +} from "@/features/messages/lib/threading"; import { resolveUserLabel, type UserProfileLookup, @@ -66,6 +69,47 @@ function getDeletionTargets(tags: string[][]) { .map((tag) => tag[1]); } +/** + * Count the *visible top-level rows* a raw event window would render in the + * main channel timeline — the same unit `buildMainTimelineEntries` produces. + * + * This is deliberately NOT `events.length`: thread replies collapse into their + * parent's summary row, deleted events disappear, and non-content kinds + * (reactions, edits, deletions) never render as their own row. A history batch + * heavy with replies can add 100 events but only a handful of rows, which is + * why fetch-older counts rows here, not messages, when deciding how far to page. + * + * Mirrors the two filters that bound the rendered list: + * 1. `formatTimelineMessages` keeps content kinds that aren't deletion targets. + * 2. `buildMainTimelineEntries` keeps entries that are top-level + * (`parentId == null`) or broadcast replies. + */ +export function countTopLevelTimelineRows(events: RelayEvent[]): number { + const deletedEventIds = new Set(); + for (const event of events) { + if ( + event.kind === KIND_DELETION || + event.kind === KIND_NIP29_DELETE_EVENT + ) { + for (const targetId of getDeletionTargets(event.tags)) { + deletedEventIds.add(targetId); + } + } + } + + let count = 0; + for (const event of events) { + if (!isTimelineContentEvent(event) || deletedEventIds.has(event.id)) { + continue; + } + const { parentId } = getThreadReference(event.tags); + if (parentId == null || isBroadcastReply(event.tags)) { + count += 1; + } + } + return count; +} + function getReactionTargetId(tags: string[][]) { for (let index = tags.length - 1; index >= 0; index -= 1) { const tag = tags[index]; diff --git a/desktop/src/features/messages/useFetchOlderMessages.ts b/desktop/src/features/messages/useFetchOlderMessages.ts index 0203199a4..ad4d39d9f 100644 --- a/desktop/src/features/messages/useFetchOlderMessages.ts +++ b/desktop/src/features/messages/useFetchOlderMessages.ts @@ -1,6 +1,7 @@ import { useCallback, useRef, useState } from "react"; import { useQueryClient } from "@tanstack/react-query"; +import { countTopLevelTimelineRows } from "@/features/messages/lib/formatTimelineMessages"; import { channelMessagesKey, mergeTimelineHistoryMessages, @@ -10,6 +11,14 @@ import type { Channel, RelayEvent } from "@/shared/api/types"; const OLDER_MESSAGES_BATCH_SIZE = 100; +// One scroll-up should advance the timeline by a predictable, *visible* amount. +// Because thread replies collapse into their parent and non-content events +// never render, a single 100-message batch can add far fewer rows than that — +// so we page in additional batches until at least this many top-level rows have +// been added (or history runs out). Counting rows, not messages, keeps a +// reply-heavy window from feeling like the fetch did nothing. +const MIN_TOP_LEVEL_ROWS_PER_FETCH = 10; + export function useFetchOlderMessages(channel: Channel | null) { const queryClient = useQueryClient(); const channelId = channel?.id ?? null; @@ -43,39 +52,62 @@ export function useFetchOlderMessages(channel: Channel | null) { return; } - // Use the oldest timestamp directly — `until` is inclusive so the relay will - // return the boundary message again, but `sortMessages` deduplicates by id. - // Subtracting 1 risks skipping messages that share the same second. - const oldestTimestamp = currentMessages[0].created_at; isFetchingOlderRef.current = true; setIsFetchingOlder(true); + // Page in batches until the timeline has gained at least + // MIN_TOP_LEVEL_ROWS_PER_FETCH *visible* rows or history is exhausted. + // A single batch is fetched first; only reply-heavy windows that fall short + // of the floor loop again. Each batch re-reads the oldest timestamp from the + // cache so successive `until` values keep walking backward without gaps. + const baselineRowCount = countTopLevelTimelineRows(currentMessages); try { - const olderMessages = await relayClient.fetchChannelHistoryBefore( - channelId, - oldestTimestamp, - OLDER_MESSAGES_BATCH_SIZE, - ); - - if (olderMessages.length < OLDER_MESSAGES_BATCH_SIZE) { - hasOlderMessagesRef.current = false; - setHasOlderMessages(false); - } + while (hasOlderMessagesRef.current) { + const messagesBeforeFetch = + queryClient.getQueryData(queryKey) ?? []; + if (messagesBeforeFetch.length === 0) { + break; + } - if (olderMessages.length > 0) { - queryClient.setQueryData(queryKey, (current = []) => - mergeTimelineHistoryMessages(current, olderMessages), + // Use the oldest timestamp directly — `until` is inclusive so the relay + // will return the boundary message again, but `sortMessages` + // deduplicates by id. Subtracting 1 risks skipping messages that share + // the same second. + const oldestTimestamp = messagesBeforeFetch[0].created_at; + const olderMessages = await relayClient.fetchChannelHistoryBefore( + channelId, + oldestTimestamp, + OLDER_MESSAGES_BATCH_SIZE, ); - const updatedMessages = - queryClient.getQueryData(queryKey) ?? []; - if ( - updatedMessages.length > 0 && - updatedMessages[0].created_at === oldestTimestamp - ) { + if (olderMessages.length < OLDER_MESSAGES_BATCH_SIZE) { hasOlderMessagesRef.current = false; setHasOlderMessages(false); } + + if (olderMessages.length > 0) { + queryClient.setQueryData(queryKey, (current = []) => + mergeTimelineHistoryMessages(current, olderMessages), + ); + + const updatedMessages = + queryClient.getQueryData(queryKey) ?? []; + if ( + updatedMessages.length > 0 && + updatedMessages[0].created_at === oldestTimestamp + ) { + hasOlderMessagesRef.current = false; + setHasOlderMessages(false); + } + } + + const rowsGained = + countTopLevelTimelineRows( + queryClient.getQueryData(queryKey) ?? [], + ) - baselineRowCount; + if (rowsGained >= MIN_TOP_LEVEL_ROWS_PER_FETCH) { + break; + } } } catch (error) { console.error("Failed to fetch older messages", channelId, error); From 5a42d3706a87f7b5a35827f9ae5157a2c28f23df Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Thu, 18 Jun 2026 16:38:22 -0400 Subject: [PATCH 07/15] =?UTF-8?q?test(desktop):=20scroll-smoothness=20perf?= =?UTF-8?q?=20harness=20=E2=80=94=20steady-state=20+=20prepend=20cost?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An instrument, not a gate. Seeds a busy channel against the mock bridge and measures the main-thread cost the headless correctness suite cannot feel. Two measurements: - Fast-wheel scroll of the bounded ~200-row window: compositor-cheap (~0.2ms layout + 1.3ms recalc over a 241-frame, 12k-px burst). - Prepend re-render cost while scrolled up: ~9-13ms main-thread tick, attributable to the O(rendered-rows) parent walk (videoReviewContext rebuild + day-group boundaries + element construction), NOT layout and NOT leaf-row reconciliation (MessageRow memo bails correctly). Anchor holds at 0px drift on this path. Scope limit: measures Chromium reconciliation, not the WKWebView compositor feel — that remains the real-Tauri macOS pass. Run: pnpm build && npx playwright test --config=playwright.perf.config.ts Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- desktop/playwright.perf.config.ts | 23 ++ desktop/tests/e2e/scroll-smoothness.perf.ts | 311 ++++++++++++++++++++ 2 files changed, 334 insertions(+) create mode 100644 desktop/playwright.perf.config.ts create mode 100644 desktop/tests/e2e/scroll-smoothness.perf.ts diff --git a/desktop/playwright.perf.config.ts b/desktop/playwright.perf.config.ts new file mode 100644 index 000000000..6fde2c28f --- /dev/null +++ b/desktop/playwright.perf.config.ts @@ -0,0 +1,23 @@ +import { defineConfig, devices } from "@playwright/test"; + +export default defineConfig({ + testDir: "./tests/e2e", + timeout: 60_000, + retries: 0, + workers: 1, + reporter: [["list"]], + use: { baseURL: "http://127.0.0.1:4173" }, + projects: [ + { + name: "perf", + testMatch: ["**/*.perf.ts"], + use: { ...devices["Desktop Chrome"] }, + }, + ], + webServer: { + command: "python3 -m http.server 4173 -d dist", + cwd: ".", + reuseExistingServer: true, + url: "http://127.0.0.1:4173", + }, +}); diff --git a/desktop/tests/e2e/scroll-smoothness.perf.ts b/desktop/tests/e2e/scroll-smoothness.perf.ts new file mode 100644 index 000000000..06fec8bae --- /dev/null +++ b/desktop/tests/e2e/scroll-smoothness.perf.ts @@ -0,0 +1,311 @@ +import { expect, test } from "@playwright/test"; + +import { installMockBridge } from "../helpers/bridge"; + +/** + * Scroll-smoothness measurement harness. + * + * This is NOT a pass/fail correctness test — it's an instrument. It seeds a + * busy channel, mounts the full history into the DOM, drives a synthetic + * fast-wheel scroll up through it, and measures the *main-thread layout work* + * that scroll triggers — the cost the headless correctness suite cannot feel + * (it polls only the final position). + * + * WHY LAYOUT COST, NOT FRAME CADENCE: under Playwright's headed Chromium there + * is no vsync throttle, so requestAnimationFrame fires far faster than a real + * 60Hz display and "frame interval" tells you nothing about paint jank. The + * honest, engine-agnostic signal is the cumulative layout / style-recalc time + * Chromium spends servicing the scroll burst, read via CDP Performance metrics. + * More live DOM rows + per-row reflow on scroll == more layout cost == the jank + * a user feels. That is exactly the axis virtualization / content-visibility + * attack, so it's the axis worth a baseline. + * + * SCOPE LIMIT (stated honestly): this measures Chromium's main-thread layout + * cost. It does NOT measure the WKWebView compositor / stale-scrollTop race + * Sami flagged as the dominant *feel* hazard on the shipped Tauri app — that + * only reproduces in the real desktop shell and is Tyler's real-wheel pass. + * + * Run headed to watch it: + * pnpm build && npx playwright test --config=playwright.perf.config.ts --headed + */ + +const SEED_ROWS = 600; // a genuinely busy channel, fully mounted +const LINES_PER_ROW = 3; + +type Sample = { + rowCount: number; + scrollSpan: number; + frames: number; +}; + +type Metrics = { layoutMs: number; recalcMs: number; layoutCount: number }; + +async function readMetrics( + client: import("@playwright/test").CDPSession, +): Promise { + const { metrics } = (await client.send("Performance.getMetrics")) as { + metrics: Array<{ name: string; value: number }>; + }; + const m = (name: string) => metrics.find((x) => x.name === name)?.value ?? 0; + return { + // CDP reports durations in seconds; convert to ms. + layoutMs: m("LayoutDuration") * 1000, + recalcMs: m("RecalcStyleDuration") * 1000, + layoutCount: m("LayoutCount"), + }; +} + +test("MEASURE: fast-wheel scroll-up layout cost on a busy un-virtualized timeline", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" && + typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function", + ); + + await page.evaluate( + ({ rows, lines }) => { + // Emit the entire busy channel as live messages so the FULL list mounts + // into the DOM up front — no dependence on the app's fetch-older + // pagination (which caps per-request and is a separate axis). This is the + // un-virtualized DOM we're here to stress. + for (let i = 0; i < rows; i += 1) { + const body = Array.from( + { length: lines }, + (_u, l) => `busy row ${i} line ${l + 1}`, + ).join("\n"); + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: body, + }); + } + }, + { rows: SEED_ROWS, lines: LINES_PER_ROW }, + ); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + const timeline = page.getByTestId("message-timeline"); + + // Confirm the list is mounted before measuring. Capture the actual mounted + // count — we don't assume all SEED_ROWS render (the app may cap/window). + await page.waitForFunction(() => { + const el = document.querySelector( + '[data-testid="message-timeline"]', + ) as HTMLDivElement | null; + return !!el && el.querySelectorAll("[data-message-id]").length > 50; + }); + await page.waitForTimeout(500); // let live emits settle + + const client = await page.context().newCDPSession(page); + await client.send("Performance.enable"); + + // Reset to bottom, settle, then snapshot metrics around the scroll burst. + await timeline.evaluate((element) => { + const el = element as HTMLDivElement; + el.scrollTop = el.scrollHeight; + el.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + await page.waitForTimeout(100); + + const before = await readMetrics(client); + + const sample = await timeline.evaluate(async (element): Promise => { + const el = element as HTMLDivElement; + el.scrollTop = el.scrollHeight; + const startTop = el.scrollTop; + let frames = 0; + + await new Promise((resolve) => { + let elapsed = 0; + let last = performance.now(); + const DURATION_MS = 2_000; + const PX_PER_FRAME = 50; // brisk human flick + + const step = (now: number) => { + elapsed += now - last; + last = now; + frames += 1; + el.dispatchEvent( + new WheelEvent("wheel", { + deltaY: -PX_PER_FRAME, + bubbles: true, + cancelable: true, + }), + ); + el.scrollTop = Math.max(0, el.scrollTop - PX_PER_FRAME); + el.dispatchEvent(new Event("scroll", { bubbles: true })); + if (elapsed < DURATION_MS && el.scrollTop > 0) { + requestAnimationFrame(step); + } else { + resolve(); + } + }; + requestAnimationFrame(step); + }); + + return { + rowCount: el.querySelectorAll("[data-message-id]").length, + scrollSpan: startTop - el.scrollTop, + frames, + }; + }); + + const after = await readMetrics(client); + await client.send("Performance.disable"); + + const layoutDelta = after.layoutMs - before.layoutMs; + const recalcDelta = after.recalcMs - before.recalcMs; + const layoutCountDelta = after.layoutCount - before.layoutCount; + const perScroll = sample.frames > 0 ? sample.frames : 1; + + /* eslint-disable no-console */ + console.log("\n=== SCROLL SMOOTHNESS BASELINE (Chromium layout cost) ==="); + console.log(`rows mounted (live DOM): ${sample.rowCount}`); + console.log( + `scroll span covered: ${Math.round(sample.scrollSpan)}px`, + ); + console.log(`scroll frames driven: ${sample.frames}`); + console.log(`layout time over burst: ${layoutDelta.toFixed(1)}ms`); + console.log(`style-recalc time over burst: ${recalcDelta.toFixed(1)}ms`); + console.log(`forced layouts (count delta): ${layoutCountDelta}`); + console.log( + `avg layout+recalc per frame: ${((layoutDelta + recalcDelta) / perScroll).toFixed(3)}ms`, + ); + console.log("(>~8ms/frame main-thread work is where 120Hz starts to drop)"); + console.log("=========================================================\n"); + /* eslint-enable no-console */ + + // Instrument, not a gate — just confirm it exercised the full list. + expect(sample.rowCount).toBeGreaterThan(100); + expect(sample.scrollSpan).toBeGreaterThan(500); +}); + +/** + * MEASURE: prepend re-render cost — the one event-cost the team had no number + * for. Steady-state scroll of the bounded 200-row window is compositor-cheap + * (see the test above). The felt jank, if any, lives at the *prepend*: when + * older rows land while you're scrolled up, the whole memoized + * TimelineMessageList re-renders AND the anchor scrollBy fires, all on one + * main-thread tick. If that tick exceeds the frame budget mid-wheel, the + * compositor stalls and you feel it. + * + * This drives the genuine path: seed older history, scroll off-bottom, then + * prepend a small batch via the live-event ingest (the same WS path the relay + * uses), and measure (a) the synchronous wall-time of the tick that flushes + * the React commit + layout, and (b) the CDP layout/recalc cost attributed to + * it. It also records whether the anchor held (scrollTop preserved within the + * row that was under the eye) — a non-restore here is the in-viewport-shift + * bug, not a perf cost. + * + * SCOPE LIMIT: same as above — Chromium main-thread cost, not WKWebView feel. + */ +test("MEASURE: prepend re-render cost while scrolled up (the untested event-cost)", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => + typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" && + typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function", + ); + + // Seed the channel at roughly the real data-window cap (CHANNEL_HISTORY_LIMIT + // = 200) so the prepend re-renders a full-size list, the worst realistic case. + const SEED = 200; + await page.evaluate( + ({ rows, lines }) => { + for (let i = 0; i < rows; i += 1) { + const body = Array.from( + { length: lines }, + (_u, l) => `seed row ${i} line ${l + 1}`, + ).join("\n"); + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: body, + }); + } + }, + { rows: SEED, lines: LINES_PER_ROW }, + ); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + const timeline = page.getByTestId("message-timeline"); + + await page.waitForFunction(() => { + const el = document.querySelector( + '[data-testid="message-timeline"]', + ) as HTMLDivElement | null; + return !!el && el.querySelectorAll("[data-message-id]").length > 50; + }); + await page.waitForTimeout(500); + + // Scroll up off the bottom so the anchor is a mid-list row, not "at-bottom" + // (the at-bottom path corrects differently). Land roughly in the middle. + await timeline.evaluate((element) => { + const el = element as HTMLDivElement; + el.scrollTop = Math.floor(el.scrollHeight * 0.4); + el.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + await page.waitForTimeout(200); + + const client = await page.context().newCDPSession(page); + await client.send("Performance.enable"); + + const before = await readMetrics(client); + const scrollTopBefore = await timeline.evaluate( + (el) => (el as HTMLDivElement).scrollTop, + ); + + // Prepend a small older batch through the live ingest path and time the tick + // that flushes the resulting React commit + layout + anchor restore. + const tickMs = await page.evaluate(async (count) => { + const t0 = performance.now(); + window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({ + channelName: "general", + count, + lineCount: 3, + emit: true, + }); + // Force a layout flush so the synchronous commit + anchor scrollBy + reflow + // are all accounted before we stop the clock. Two rAFs span the commit and + // the subsequent paint-prep tick. + await new Promise((r) => requestAnimationFrame(() => r())); + await new Promise((r) => requestAnimationFrame(() => r())); + return performance.now() - t0; + }, 10); + + const after = await readMetrics(client); + const scrollTopAfter = await timeline.evaluate( + (el) => (el as HTMLDivElement).scrollTop, + ); + const rowCountAfter = await timeline.evaluate( + (el) => (el as HTMLDivElement).querySelectorAll("[data-message-id]").length, + ); + await client.send("Performance.disable"); + + const layoutDelta = after.layoutMs - before.layoutMs; + const recalcDelta = after.recalcMs - before.recalcMs; + const anchorDrift = scrollTopAfter - scrollTopBefore; + + /* eslint-disable no-console */ + console.log("\n=== PREPEND RE-RENDER COST (10 older rows, scrolled up) ==="); + console.log(`rows after prepend (live DOM): ${rowCountAfter}`); + console.log(`tick wall-time (commit+layout): ${tickMs.toFixed(2)}ms`); + console.log(`layout time attributed: ${layoutDelta.toFixed(1)}ms`); + console.log(`style-recalc time attributed: ${recalcDelta.toFixed(1)}ms`); + console.log( + `anchor drift (scrollTop delta): ${anchorDrift.toFixed(1)}px (0 = held)`, + ); + console.log("(tick > ~8ms during active wheel is where 120Hz stalls)"); + console.log("===========================================================\n"); + /* eslint-enable no-console */ + + // Instrument, not a gate — just confirm the prepend actually happened. + expect(rowCountAfter).toBeGreaterThan(50); +}); From 78ac6caf19b379627cfb71f601354ee84dac4a11 Mon Sep 17 00:00:00 2001 From: npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc Date: Thu, 18 Jun 2026 16:39:26 -0400 Subject: [PATCH 08/15] fix(desktop): restore anchor on in-viewport reflow when scrolled up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ResizeObserver in useAnchoredScroll only re-pinned the scroller when the user was at-bottom — when scrolled up reading older history and a row above the reading row reflowed (link-card decode, async embed expand, late font load, markdown that expands), the anchor row shifted on them and nothing restored it. The PR description claimed image and embed loads all flow through the anchor, but on a careful read only NIP-92 imeta images are actually covered (their dim is reserved before decode, so no resize fires). Every other in-viewport content growth fell into the gap. Extract the anchor-restoration primitive — find the row (with the nearest-newer fallback already used for prepends), measure its current top, scrollBy the delta — into a shared restoreAnchorToMessage helper, and call it from both the layout effect and the ResizeObserver. One primitive serves the React-driven path (post-commit, on messages / spinner change) and the non-React-driven path (image decode, embed expand, font load), preserving the single-owner invariant. A messagesRef is mirrored from the layout effect so the observer reads the same list the DOM was last rendered from without resubscribing on every commit. E2E coverage: scroll-history e2e adds an in-viewport reflow case that seeds a scrollable channel, scrolls to a mid position, captures the top-crossing row's offset, programmatically grows a row above the anchor via style.minHeight, and asserts the anchor's offset is unchanged within 2px after the observer fires. Confirmed the assertion catches the pre-fix behavior (80px drift) by reverting the implementation and re-running. tsc, biome (764 files), 998/998 unit, scroll-history e2e 7/7 green. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- .../features/messages/ui/useAnchoredScroll.ts | 158 ++++++++++++------ desktop/tests/e2e/scroll-history.spec.ts | 109 ++++++++++++ 2 files changed, 216 insertions(+), 51 deletions(-) diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index adc65b344..abd382058 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -140,6 +140,71 @@ function findNearestNewerMessageId( return null; } +/** + * Restore a message-kind anchor's on-screen offset after a layout shift. + * + * Finds the anchor row (or the nearest newer rendered row if the anchor + * itself was removed), measures its current top-relative offset, and + * `scrollBy(0, delta)` if the offset has drifted. Returns the new anchor + * state the caller should write back: + * - `{ kind: "message", ... }` — anchor (or its fallback) is in the DOM + * and now sits at its previous offset. + * - `{ kind: "at-bottom" }` — anchor and all newer rendered rows are gone; + * caller should pin to the bottom and update at-bottom state. + * + * `scrollBy` is intentional over `scrollTop = ...`: relative adjustment + * composes with the browser's own scroll anchoring and doesn't fight a + * smooth-scroll in flight. Same rationale as the layout-effect restore. + * + * Used by both the post-commit layout effect (prepend / append / spinner + * toggle / etc.) and the ResizeObserver (in-viewport reflow from image + * decode, embed expansion, font load). Keeping them on one primitive + * preserves the single-owner invariant of the hook. + */ +function restoreAnchorToMessage( + container: HTMLDivElement, + messages: TimelineMessage[], + anchor: Extract, +): AnchorState { + let anchorEl = container.querySelector( + `[data-message-id="${anchor.messageId}"]`, + ); + let usedAnchor: AnchorState = anchor; + if (!anchorEl) { + const fallbackId = findNearestNewerMessageId( + container, + messages, + anchor.messageId, + ); + if (fallbackId) { + anchorEl = container.querySelector( + `[data-message-id="${fallbackId}"]`, + ); + if (anchorEl) { + usedAnchor = { + kind: "message", + messageId: fallbackId, + topOffset: anchor.topOffset, + }; + } + } + } + + if (!anchorEl) { + // Anchor message and all subsequent rendered messages are gone. + container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); + return { kind: "at-bottom" }; + } + + const containerTop = container.getBoundingClientRect().top; + const currentTop = anchorEl.getBoundingClientRect().top - containerTop; + const delta = currentTop - usedAnchor.topOffset; + if (Math.abs(delta) > 0.5) { + container.scrollBy(0, delta); + } + return usedAnchor; +} + export function useAnchoredScroll({ scrollContainerRef, contentRef, @@ -157,6 +222,11 @@ export function useAnchoredScroll({ // both on scroll (commit-time read) and in the layout effect (post-render // restoration). useState would force re-renders we don't want. const anchorRef = React.useRef({ kind: "at-bottom" }); + // Latest `messages` mirrored to a ref so the ResizeObserver effect can read + // the current list without re-subscribing the observer on every commit + // (which would also drop any in-flight resize callbacks). Kept fresh by a + // layout effect below so the read is consistent with what's in the DOM. + const messagesRef = React.useRef(messages); const [isAtBottom, setIsAtBottom] = React.useState(true); const [newMessageCount, setNewMessageCount] = React.useState(0); const [highlightedMessageId, setHighlightedMessageId] = React.useState< @@ -285,6 +355,12 @@ export function useAnchoredScroll({ const container = scrollContainerRef.current; if (!container) return; + // Mirror the current messages list into the ref read by the + // ResizeObserver's restore path. Must happen before any early return so + // a non-React layout shift sees the same array the next restoration + // would use. + messagesRef.current = messages; + // First render after a reset (channel switch or initial mount): jump // to the requested target message, or to the bottom by default. if (!hasInitializedRef.current) { @@ -342,49 +418,13 @@ export function useAnchoredScroll({ container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); if (newLatestArrived) setNewMessageCount(0); } else { - // Anchored to a specific message. Find it; if it's gone, fall back to - // the nearest newer rendered message; if that's also gone, give up - // and pin to bottom. - let anchorEl = container.querySelector( - `[data-message-id="${anchor.messageId}"]`, - ); - let usedAnchor: AnchorState = anchor; - if (!anchorEl) { - const fallbackId = findNearestNewerMessageId( - container, - messages, - anchor.messageId, - ); - if (fallbackId) { - anchorEl = container.querySelector( - `[data-message-id="${fallbackId}"]`, - ); - if (anchorEl) { - usedAnchor = { - kind: "message", - messageId: fallbackId, - topOffset: anchor.topOffset, - }; - } - } - } - - if (anchorEl) { - const containerTop = container.getBoundingClientRect().top; - const currentTop = anchorEl.getBoundingClientRect().top - containerTop; - const delta = currentTop - usedAnchor.topOffset; - if (Math.abs(delta) > 0.5) { - // `scrollBy` is intentional over `scrollTop = ...`: relative - // adjustment composes with whatever the browser's own scroll - // anchoring did, and it doesn't fight a smooth-scroll in flight. - container.scrollBy(0, delta); - } - anchorRef.current = usedAnchor; - } else { - // Anchor message and all subsequent rendered messages are gone. - // Last-resort fallback so the user doesn't end up stranded. - container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); - anchorRef.current = { kind: "at-bottom" }; + // Anchored to a specific message. The shared helper finds it (with a + // nearest-newer fallback if the row was removed) and corrects the + // offset via `scrollBy`. If both the anchor and all newer rendered + // rows are gone, it pins to the bottom and returns `at-bottom`. + const restored = restoreAnchorToMessage(container, messages, anchor); + anchorRef.current = restored; + if (restored.kind === "at-bottom") { setIsAtBottom(true); } @@ -473,11 +513,16 @@ export function useAnchoredScroll({ ]); // --------------------------------------------------------------------------- - // Content resize: when fonts load late, an image decodes, or an embed - // expands, the row positions shift. A ResizeObserver fires a synthetic - // scroll so the anchor is recomputed; the layout effect on the next - // render will then restore. We deliberately do NOT call scrollTo here — - // that would fight the anchor restoration. + // 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 + // layout-effect doesn't fire), the anchor row's on-screen offset drifts. + // + // When stuck-to-bottom we re-pin to bottom. When anchored to a message we + // call the same restore primitive the layout effect uses, so an in-viewport + // reflow above the reader's eye shifts back into place. Without this, + // anything that resizes without changing `messages` (link-card decode, + // async embed expand, late font load, markdown that expands) silently + // pushes the reading row around. // --------------------------------------------------------------------------- React.useEffect(() => { const content = contentRef.current; @@ -485,10 +530,21 @@ export function useAnchoredScroll({ const observer = new ResizeObserver(() => { const container = scrollContainerRef.current; if (!container) return; - // If we're stuck-to-bottom, just stay there. Otherwise let the next - // layout effect handle restoration via the existing anchor. - if (anchorRef.current.kind === "at-bottom") { + const anchor = anchorRef.current; + if (anchor.kind === "at-bottom") { container.scrollTo({ top: container.scrollHeight, behavior: "auto" }); + return; + } + // Use the same restore primitive as the layout effect so the + // single-owner model holds across non-React-driven layout shifts. + const restored = restoreAnchorToMessage( + container, + messagesRef.current, + anchor, + ); + anchorRef.current = restored; + if (restored.kind === "at-bottom") { + setIsAtBottom(true); } }); observer.observe(content); diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts index 70d9daa83..71e6b1b7a 100644 --- a/desktop/tests/e2e/scroll-history.spec.ts +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -939,3 +939,112 @@ test("composer expansion does not push bottom row out of viewport", async ({ if (!after.found) return; expect(after.gapAboveComposer).toBeGreaterThanOrEqual(-4); }); + +// Criterion 8: in-viewport content resize while scrolled up preserves the +// anchor row's position. +// +// The hook owns scroll position via `useAnchoredScroll`. When the user is +// scrolled up reading older history and a row *above* their reading row +// reflows (image decode without reserved dim metadata, link-card load, +// async embed expand, late font load, markdown that expands), the rows +// below shift on them. Before this test landed, the ResizeObserver only +// re-pinned when stuck-to-bottom; the scrolled-up case had no correction. +// +// The fix: the ResizeObserver calls the same anchor-restore primitive as +// the post-commit layout effect when anchored to a message. This test +// reproduces the scenario without touching React state — it directly +// grows a DOM row's height via a style override, which is exactly the +// kind of layout shift that previously had no correction (the messages +// array is unchanged, so the React layout effect never runs). +// +// Black-box assertions: the anchor row's top within the timeline must be +// unchanged (within 2px) before and after the synthetic above-anchor +// height growth. +test("in-viewport reflow above the anchor row does not push it down", async ({ + page, +}) => { + await installMockBridge(page); + await page.goto("/"); + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function", + ); + + // Seed enough rows that the timeline becomes scrollable with several + // rows above whatever we anchor to. + await page.evaluate(() => { + for (let index = 0; index < 60; index += 1) { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "general", + content: `resize-anchor row ${index}\nsecond line ${index}\nthird line ${index}`, + }); + } + }); + + await page.getByTestId("channel-general").click(); + await expect(page.getByTestId("chat-title")).toHaveText("general"); + const timeline = page.getByTestId("message-timeline"); + await expect(timeline).toContainText("resize-anchor row 59"); + + // Wait until the timeline is genuinely scrollable. + await page.waitForFunction(() => { + const element = document.querySelector( + '[data-testid="message-timeline"]', + ) as HTMLDivElement | null; + return element && element.scrollHeight > element.clientHeight + 800; + }); + + // Scroll to a middle position so we have rows on both sides of the anchor. + await timeline.evaluate((element) => { + const t = element as HTMLDivElement; + t.scrollTop = Math.floor(t.scrollHeight / 2); + t.dispatchEvent(new Event("scroll", { bubbles: true })); + }); + await page.waitForTimeout(50); + + // Capture the anchor row (top-crossing) and its baseline top within + // the timeline. This is the row the user is reading. + const baseline = await getFirstVisibleMessage(page); + expect(baseline).not.toBeNull(); + if (!baseline) return; + + // Find a rendered row *above* the anchor and grow its height. This + // mimics an in-viewport reflow (image decode, embed expansion) that + // does NOT change the messages array, so the React layout effect + // would not fire. The ResizeObserver is the only path that can + // correct the resulting shift. + const growthApplied = await timeline.evaluate((element, anchorId) => { + const t = element as HTMLDivElement; + const rows = Array.from( + t.querySelectorAll("[data-message-id]"), + ); + const anchorIndex = rows.findIndex( + (row) => row.dataset.messageId === anchorId, + ); + if (anchorIndex <= 0) return false; + // Pick a row a few above the anchor so the growth is clearly above + // the reader's eye, not at it. + const target = rows[Math.max(0, anchorIndex - 3)]; + if (!target) return false; + // 80px is well above the 0.5px noise floor in restoreAnchorToMessage + // and large enough to be a visible jump if uncorrected. + const currentHeight = target.getBoundingClientRect().height; + target.style.minHeight = `${currentHeight + 80}px`; + return true; + }, baseline.id); + expect(growthApplied).toBe(true); + + // ResizeObserver callbacks run asynchronously after layout. Poll the + // anchor row's position; it must converge back to (or stay at) its + // baseline top within ~2px. + await expect + .poll( + async () => { + const current = await getMessagePosition(page, baseline.id); + return current + ? Math.abs(current.top - baseline.top) + : Number.POSITIVE_INFINITY; + }, + { timeout: 3_000 }, + ) + .toBeLessThanOrEqual(2); +}); From 913facde71cb2585db890b545f5df2750cc3b286 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Wed, 17 Jun 2026 23:05:19 -0400 Subject: [PATCH 09/15] 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 | 424 +++++++++++------- .../ui/useConvergentScrollToMessage.ts | 166 +++++++ .../messages/ui/useLoadOlderOnScroll.ts | 157 +++++++ 13 files changed, 1212 insertions(+), 163 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 cca89f471..a6ab4bda5 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"; @@ -540,6 +543,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 aaf9f9f7e..4e91a01a8 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -2,19 +2,25 @@ import * as React from "react"; import { formatDayHeading } from "@/features/messages/lib/dateFormatters"; import { - buildMainTimelineEntries, - shouldRenderUnreadDivider, -} from "@/features/messages/lib/threadPanel"; + 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 { buildVideoReviewCommentsByRootId, buildVideoReviewContextForMessage, } from "@/features/messages/lib/videoReviewContext"; -import { buildDayGroupBoundaries } from "@/features/messages/lib/timelineSnapshot"; 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"; @@ -63,6 +69,13 @@ 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; }; export const TimelineMessageList = React.memo(function TimelineMessageList({ @@ -90,6 +103,9 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ searchQuery, threadUnreadCounts, unfollowThreadById, + scrollContainerRef, + onItems, + onVirtualizer, }: TimelineMessageListProps) { const entries = React.useMemo( () => buildMainTimelineEntries(messages), @@ -137,163 +153,259 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ profiles, reviewCommentsByRootId, ]); - const dayGroups: Array<{ - key: string; - label: string; - elements: React.ReactNode[]; - }> = []; - let currentDayGroup: (typeof dayGroups)[number] | null = null; - // Day-divider decision delegated to a pure, lib-tested helper: a new group - // starts at index 0 and whenever a message falls on a different calendar day - // than the one before it. We index the boundaries by start position so the - // render loop below stays a straight walk while the grouping logic — and the - // prepend-stable section key — lives in `lib/`. - const dayGroupBoundariesByStartIndex = new Map( - buildDayGroupBoundaries(entries.map((entry) => entry.message)).map( - (boundary) => [boundary.startIndex, boundary], - ), + // 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], ); - for (let i = 0; i < entries.length; i++) { - const { message, summary } = entries[i]; - const messageRenderKey = message.renderKey ?? message.id; + React.useEffect(() => { + onItems?.(itemsResult); + }, [itemsResult, onItems]); - const dayBoundary = dayGroupBoundariesByStartIndex.get(i); - if (dayBoundary) { - currentDayGroup = { - key: dayBoundary.key, - label: formatDayHeading(message.createdAt), - elements: [], - }; - dayGroups.push(currentDayGroup); - } + 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, + ], + ); - // 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)) { - currentDayGroup?.elements.push( - , - ); - } + return ( + + ); +}); + +function SystemRow({ + currentPubkey, + entry, + footer, + onToggleReaction, + profiles, +}: { + currentPubkey?: string; + entry: MainTimelineEntry; + footer: React.ReactNode; + onToggleReaction?: TimelineMessageListProps["onToggleReaction"]; + profiles?: UserProfileLookup; +}) { + return ( +
+ + {footer} +
+ ); +} - if (message.kind === KIND_SYSTEM_MESSAGE) { - const footer = messageFooters?.[message.id] ?? null; - currentDayGroup?.elements.push( -
- - {footer} -
, - ); - } else if (summary && onReply) { - const footer = messageFooters?.[message.id] ?? null; - const isHighlighted = message.id === highlightedMessageId; - currentDayGroup?.elements.push( -
- followThreadById(message.id) : undefined - } - onMarkUnread={onMarkUnread} - onToggleReaction={onToggleReaction} - onReply={onReply} - onUnfollowThread={ - unfollowThreadById - ? () => unfollowThreadById(message.id) - : undefined - } - profiles={profiles} - showDepthGuides={false} - videoReviewContext={videoReviewContextById.get(message.id)} - /> - - {footer} -
, - ); - } else { - const isSearchMatch = searchMatchingMessageIds?.has(message.id) ?? false; - const isSearchActive = message.id === searchActiveMessageId; - const footer = messageFooters?.[message.id] ?? null; +type MessageRowItemProps = Pick< + TimelineMessageListProps, + | "agentPubkeys" + | "channelId" + | "currentPubkey" + | "followThreadById" + | "highlightedMessageId" + | "isFollowingThreadById" + | "onDelete" + | "onEdit" + | "onMarkUnread" + | "onReply" + | "onToggleReaction" + | "profiles" + | "searchActiveMessageId" + | "searchMatchingMessageIds" + | "searchQuery" + | "threadUnreadCounts" + | "unfollowThreadById" +> & { + entry: MainTimelineEntry; + footer: React.ReactNode; + videoReviewContext: ReturnType; +}; - currentDayGroup?.elements.push( -
- - {footer} -
, - ); - } +function MessageRowItem({ + agentPubkeys, + channelId, + currentPubkey, + entry, + followThreadById, + footer, + highlightedMessageId, + isFollowingThreadById, + onDelete, + onEdit, + onMarkUnread, + onReply, + onToggleReaction, + profiles, + searchActiveMessageId, + searchMatchingMessageIds, + searchQuery, + threadUnreadCounts, + unfollowThreadById, + 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 isHighlighted = message.id === highlightedMessageId; + return ( +
+ followThreadById(message.id) : undefined + } + onMarkUnread={onMarkUnread} + onToggleReaction={onToggleReaction} + onReply={onReply} + onUnfollowThread={ + unfollowThreadById + ? () => unfollowThreadById(message.id) + : undefined + } + profiles={profiles} + showDepthGuides={false} + videoReviewContext={videoReviewContext} + /> + + {footer} +
+ ); } - return dayGroups.map((group) => ( -
- - {group.elements} -
- )); -}); + const isSearchMatch = searchMatchingMessageIds?.has(message.id) ?? false; + const isSearchActive = message.id === searchActiveMessageId; + + return ( +
+ + {footer} +
+ ); +} 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 fc54d84efba87e7d8ce2ccbe41c884cc7dce638d Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 01:42:07 -0400 Subject: [PATCH 10/15] 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 9c0eac5f41acccb5130a99ea183df0558bf59386 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 02:07:18 -0400 Subject: [PATCH 11/15] 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 4127d37bf61b1248d70b0d053daae65dcfdedbdf Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 18:21:39 -0400 Subject: [PATCH 12/15] 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); @@ -330,6 +341,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. @@ -347,10 +388,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; @@ -387,16 +429,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, @@ -409,6 +484,7 @@ export function useAnchoredScroll({ setIsAtBottom(true); setNewMessageCount(0); prevLastMessageIdRef.current = lastMessage?.id; + prevFirstMessageIdRef.current = firstMessage?.id; prevMessageCountRef.current = messages.length; return; } @@ -435,9 +511,9 @@ export function useAnchoredScroll({ } prevLastMessageIdRef.current = lastMessage?.id; + prevFirstMessageIdRef.current = firstMessage?.id; prevMessageCountRef.current = messages.length; }, [ - isFetchingOlder, isLoading, messages, onTargetReached, @@ -447,71 +523,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 @@ -532,7 +543,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 @@ -567,6 +588,7 @@ export function useAnchoredScroll({ React.useEffect(() => { if (!targetMessageId) { handledTargetIdRef.current = null; + convergingTargetIdRef.current = null; return; } if (handledTargetIdRef.current === targetMessageId || isLoading) return; @@ -577,7 +599,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); @@ -606,5 +644,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 4ac28bfd6782ac39e04b5498c2a1d2d85499b761 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 19:12:20 -0400 Subject: [PATCH 13/15] 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 f744697d2..347daf582 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -541,19 +541,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 fe489d81a87ac5c46cd5535b0644280633950dc4 Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 19:38:18 -0400 Subject: [PATCH 14/15] 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 1c214ba76..fc3fa1c28 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 347daf582..024e80faa 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); @@ -371,6 +385,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. @@ -549,6 +569,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 @@ -648,5 +675,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 49db96c0d8023b417a5e97abc7e47899c241c88e Mon Sep 17 00:00:00 2001 From: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 Date: Thu, 18 Jun 2026 19:50:01 -0400 Subject: [PATCH 15/15] 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 3437b0f18..64d36f547 100644 --- a/desktop/tests/e2e/channels.spec.ts +++ b/desktop/tests/e2e/channels.spec.ts @@ -268,7 +268,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); });