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/11] 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/11] 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/11] 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/11] 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/11] 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/11] 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/11] =?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/11] 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 e8500c43976c57e70b3360b99faef3c9a554a7c6 Mon Sep 17 00:00:00 2001 From: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta Date: Thu, 18 Jun 2026 18:29:04 -0400 Subject: [PATCH 09/11] perf(desktop): flatten timeline render rows Co-authored-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta Signed-off-by: npub1mprnacetjua2xx3p5eddmhxyk6wv929ymm5py8kd2xfxurxahspqqlgyta --- .../messages/lib/videoReviewContext.test.mjs | 59 +++ .../messages/lib/videoReviewContext.ts | 32 ++ .../messages/ui/TimelineMessageList.tsx | 473 +++++++++++------- 3 files changed, 375 insertions(+), 189 deletions(-) diff --git a/desktop/src/features/messages/lib/videoReviewContext.test.mjs b/desktop/src/features/messages/lib/videoReviewContext.test.mjs index e3f988fc1..de35d53ce 100644 --- a/desktop/src/features/messages/lib/videoReviewContext.test.mjs +++ b/desktop/src/features/messages/lib/videoReviewContext.test.mjs @@ -3,6 +3,7 @@ import test from "node:test"; import { buildVideoReviewCommentsByRootId, + buildVideoReviewCommentsForRoot, buildVideoReviewContextForMessage, hasVideoAttachment, } from "./videoReviewContext.ts"; @@ -99,6 +100,64 @@ test("buildVideoReviewCommentsByRootId includes nested descendants", () => { ); }); +test("buildVideoReviewCommentsForRoot returns descendants for one root", () => { + const video = message({ + id: "video", + body: "![video](https://relay/media/a.mp4)", + createdAt: 1, + }); + const otherVideo = message({ + id: "other-video", + body: "![video](https://relay/media/b.mp4)", + createdAt: 2, + }); + const firstComment = message({ + id: "first-comment", + body: "[00:01] tighten this", + createdAt: 4, + parentId: "video", + rootId: "video", + }); + const nestedReply = message({ + id: "nested-reply", + body: "agreed", + createdAt: 5, + parentId: "first-comment", + rootId: "video", + }); + const earlierComment = message({ + id: "earlier-comment", + body: "[00:00] opener", + createdAt: 3, + parentId: "video", + rootId: "video", + }); + const otherComment = message({ + id: "other-comment", + body: "different root", + createdAt: 6, + parentId: "other-video", + rootId: "other-video", + }); + + const comments = buildVideoReviewCommentsForRoot( + [ + video, + otherVideo, + firstComment, + nestedReply, + earlierComment, + otherComment, + ], + "video", + ); + + assert.deepEqual( + comments.map((comment) => comment.id), + ["earlier-comment", "first-comment", "nested-reply"], + ); +}); + test("buildVideoReviewContextForMessage posts against the source video", async () => { const video = message({ id: "video", diff --git a/desktop/src/features/messages/lib/videoReviewContext.ts b/desktop/src/features/messages/lib/videoReviewContext.ts index 6d7b63a98..8d0798db4 100644 --- a/desktop/src/features/messages/lib/videoReviewContext.ts +++ b/desktop/src/features/messages/lib/videoReviewContext.ts @@ -61,6 +61,38 @@ export function buildVideoReviewCommentsByRootId( return commentsByRootId; } +export function buildVideoReviewCommentsForRoot( + messages: TimelineMessage[], + rootId: string, +): TimelineMessage[] { + const messageById = new Map(messages.map((message) => [message.id, message])); + const comments: TimelineMessage[] = []; + + for (const message of messages) { + let ancestorId = message.parentId ?? null; + let hops = 0; + const maxHops = messages.length + 1; + + while (ancestorId && hops < maxHops) { + if (ancestorId === rootId) { + comments.push(message); + break; + } + ancestorId = messageById.get(ancestorId)?.parentId ?? null; + hops += 1; + } + } + + comments.sort((left, right) => { + if (left.createdAt !== right.createdAt) { + return left.createdAt - right.createdAt; + } + return left.id.localeCompare(right.id); + }); + + return comments; +} + export function buildVideoReviewContextForMessage({ channelId, channelName, diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index aaf9f9f7e..23af7112a 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -1,15 +1,19 @@ import * as React from "react"; -import { formatDayHeading } from "@/features/messages/lib/dateFormatters"; +import { + formatDayHeading, + isSameDay, + startOfLocalDaySeconds, +} from "@/features/messages/lib/dateFormatters"; import { buildMainTimelineEntries, shouldRenderUnreadDivider, } from "@/features/messages/lib/threadPanel"; import { - buildVideoReviewCommentsByRootId, + buildVideoReviewCommentsForRoot, buildVideoReviewContextForMessage, + hasVideoAttachment, } 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"; @@ -65,6 +69,76 @@ type TimelineMessageListProps = { threadUnreadCounts?: ReadonlyMap; }; +type TimelineDayRow = { + key: string; + label: string; + type: "day"; +}; + +type TimelineUnreadRow = { + key: string; + type: "unread"; +}; + +type TimelineMessageRowModel = { + key: string; + message: TimelineMessage; + summary: ReturnType[number]["summary"]; + type: "message"; +}; + +type TimelineRenderRow = + | TimelineDayRow + | TimelineUnreadRow + | TimelineMessageRowModel; + +function buildTimelineRenderRows({ + firstUnreadMessageId, + messages, +}: { + firstUnreadMessageId: string | null; + messages: TimelineMessage[]; +}): TimelineRenderRow[] { + const entries = buildMainTimelineEntries(messages); + const rows: TimelineRenderRow[] = []; + let previousMessage: TimelineMessage | null = null; + + for (let i = 0; i < entries.length; i++) { + const { message, summary } = entries[i]; + const messageRenderKey = message.renderKey ?? message.id; + + if ( + !previousMessage || + !isSameDay(previousMessage.createdAt, message.createdAt) + ) { + rows.push({ + key: `day-${startOfLocalDaySeconds(message.createdAt)}`, + label: formatDayHeading(message.createdAt), + type: "day", + }); + } + + // The unread "New" divider only marks a read/unread boundary when there is + // a message above the first unread. When the first unread is the first + // rendered top-level entry (fresh/never-read channel), there is nothing + // above to separate from, so it is suppressed. + if (shouldRenderUnreadDivider(i, message.id, firstUnreadMessageId)) { + rows.push({ key: `unread:${messageRenderKey}`, type: "unread" }); + } + + rows.push({ + key: `msg:${messageRenderKey}`, + message, + summary, + type: "message", + }); + + previousMessage = message; + } + + return rows; +} + export const TimelineMessageList = React.memo(function TimelineMessageList({ agentPubkeys, channelId, @@ -91,209 +165,230 @@ export const TimelineMessageList = React.memo(function TimelineMessageList({ threadUnreadCounts, unfollowThreadById, }: TimelineMessageListProps) { - const entries = React.useMemo( - () => buildMainTimelineEntries(messages), - [messages], + const rows = React.useMemo( + () => buildTimelineRenderRows({ firstUnreadMessageId, messages }), + [firstUnreadMessageId, messages], ); - const reviewCommentsByRootId = React.useMemo( - () => buildVideoReviewCommentsByRootId(messages), - [messages], - ); - // Contexts are memoized per message id so MessageRow/Markdown memo - // comparisons hold across unrelated timeline re-renders (typing - // indicators, presence updates) — a fresh context object per render would - // defeat the memo and re-render every video message on every pass. - const videoReviewContextById = React.useMemo(() => { - const contexts = new Map< - string, - NonNullable> - >(); - for (const message of messages) { - const comments = reviewCommentsByRootId.get(message.id) ?? []; - const context = buildVideoReviewContextForMessage({ - channelId, - channelName, - channelType, - comments, - isSendingVideoReviewComment, - message, - onSendVideoReviewComment, - onToggleReaction, - profiles, - }); - if (context) { - contexts.set(message.id, context); + return rows.map((row) => ( + + )); +}); + +type TimelineRenderRowViewProps = Omit< + TimelineMessageListProps, + "firstUnreadMessageId" | "messages" | "personaLookup" +> & { + allMessages?: TimelineMessage[]; + row: TimelineRenderRow; +}; + +const TimelineRenderRowView = React.memo(function TimelineRenderRowView({ + agentPubkeys, + allMessages, + channelId, + channelName, + channelType, + currentPubkey, + followThreadById, + highlightedMessageId = null, + isFollowingThreadById, + isSendingVideoReviewComment = false, + messageFooters, + onDelete, + onEdit, + onMarkUnread, + onReply, + onSendVideoReviewComment, + onToggleReaction, + profiles, + searchActiveMessageId = null, + searchMatchingMessageIds, + searchQuery, + row, + threadUnreadCounts, + unfollowThreadById, +}: TimelineRenderRowViewProps) { + const messageForContext = row.type === "message" ? row.message : null; + const videoReviewContext = React.useMemo(() => { + if (!allMessages || !messageForContext) { + return undefined; } - return contexts; + + return buildVideoReviewContextForMessage({ + channelId, + channelName, + channelType, + comments: buildVideoReviewCommentsForRoot( + allMessages, + messageForContext.id, + ), + isSendingVideoReviewComment, + message: messageForContext, + onSendVideoReviewComment, + onToggleReaction, + profiles, + }); }, [ + allMessages, channelId, channelName, channelType, isSendingVideoReviewComment, - messages, + messageForContext, onSendVideoReviewComment, onToggleReaction, 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], - ), - ); - - for (let i = 0; i < entries.length; i++) { - const { message, summary } = entries[i]; - const messageRenderKey = message.renderKey ?? message.id; + if (row.type === "day") { + return ( +
+ +
+ ); + } - const dayBoundary = dayGroupBoundariesByStartIndex.get(i); - if (dayBoundary) { - currentDayGroup = { - key: dayBoundary.key, - label: formatDayHeading(message.createdAt), - elements: [], - }; - dayGroups.push(currentDayGroup); - } + if (row.type === "unread") { + return ; + } - // 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( - , - ); - } + const { message, summary } = row; - 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; + if (message.kind === KIND_SYSTEM_MESSAGE) { + const footer = messageFooters?.[message.id] ?? null; + return ( +
+ + {footer} +
+ ); + } - currentDayGroup?.elements.push( -
- - {footer} -
, - ); - } + if (summary && onReply) { + const footer = messageFooters?.[message.id] ?? null; + 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; + const footer = messageFooters?.[message.id] ?? null; + + return ( +
+ + {footer} +
+ ); }); From 9b18ad01ab8ad41b83cd29dbcb59978a96b7bf4f Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Thu, 18 Jun 2026 20:06:38 -0400 Subject: [PATCH 10/11] fix(desktop): balance intro spacing around day divider, self-center hairline The flat-render merge left the day row wrapped in a section with gap-2.5 py-1 and a before:top-[19px] hairline, plus mb-0.5 on both intro cards. That asymmetry broke expectIntroBalancedAroundDayDivider (gap diff 2 > 1). Move the hairline onto DayDivider's own before: pseudo-element, centered with top-1/2 + -translate-y-1/2 so it no longer depends on a magic 19px offset; drop the now-purposeless single-child wrapper; remove the compensating mb-0.5 from both intro cards. Spacing is symmetric, the line decoration is preserved and self-contained, no magic pixels remain. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- desktop/src/features/messages/ui/DayDivider.tsx | 2 +- desktop/src/features/messages/ui/MessageTimeline.tsx | 4 ++-- desktop/src/features/messages/ui/TimelineMessageList.tsx | 6 +----- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/desktop/src/features/messages/ui/DayDivider.tsx b/desktop/src/features/messages/ui/DayDivider.tsx index a76b8a7f1..5356e6320 100644 --- a/desktop/src/features/messages/ui/DayDivider.tsx +++ b/desktop/src/features/messages/ui/DayDivider.tsx @@ -2,7 +2,7 @@ export function DayDivider({ label }: { label: string }) { return (
diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index f9d7d1716..29da7d214 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -344,7 +344,7 @@ const MessageTimelineBase = React.forwardRef< ) : null} {activeDirectMessageIntro ? (
- -
- ); + return ; } if (row.type === "unread") { From f652745b9437f5b4e67195ee99ae8fb329084027 Mon Sep 17 00:00:00 2001 From: npub1qyvc0c5kl4gqv2fd97fsk46tu378sqgy35vc83rvgfwne90sel7s0ed67d <011987e296fd5006292d2f930b574be47c7801048d1983c46c425d3c95f0cffd@sprout-oss.stage.blox.sqprod.co> Date: Thu, 18 Jun 2026 20:12:10 -0400 Subject: [PATCH 11/11] fix(desktop): save smooth-scroll destination offset so restore doesn't fight it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit scrollToMessageImperative read el.getBoundingClientRect() immediately after scrollIntoView. With behavior:"smooth" the animation is async, so the rect still reflects the pre-animation position — the anchor was saved at the row's start-of-scroll offset. Any messages commit during the ~300ms animation re-ran restoreAnchorToMessage, found the row mid-flight, computed a delta against the stale offset, and scrollBy'd it back toward the start. Find-in-channel next-match in a live channel jerked back. Compute the clamped destination offset analytically before kicking off the animation (block:"center" lands the row at (clientHeight - height)/2, clamped to the real scroll range), and save that as the anchor. Mid-animation restoration is now a no-op because the saved offset is the eventual offset. No timers, no listeners. Co-authored-by: Tyler Longwell Signed-off-by: Tyler Longwell --- .../features/messages/ui/useAnchoredScroll.ts | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index abd382058..ec67c7138 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -297,21 +297,37 @@ export function useAnchoredScroll({ ); if (!el) return false; + const rect = el.getBoundingClientRect(); + const containerRect = container.getBoundingClientRect(); + const currentTopOffset = rect.top - containerRect.top; + const centeredTopOffset = (container.clientHeight - rect.height) / 2; + const maxScrollTop = Math.max( + 0, + container.scrollHeight - container.clientHeight, + ); + const targetScrollTop = Math.min( + maxScrollTop, + Math.max(0, container.scrollTop + currentTopOffset - centeredTopOffset), + ); + const targetTopOffset = + currentTopOffset - (targetScrollTop - container.scrollTop); + 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; + // `scrollIntoView({ behavior: "smooth" })` starts an async animation, so + // measuring after the call can still return the pre-animation position. + // Save the clamped destination offset instead; otherwise a concurrent + // render/ResizeObserver restore can fight the smooth scroll back toward + // where it started. anchorRef.current = { kind: "message", messageId, - topOffset: rect.top - containerTop, + topOffset: targetTopOffset, }; - setIsAtBottom(isAtBottomNow(container)); + setIsAtBottom(maxScrollTop - targetScrollTop <= AT_BOTTOM_THRESHOLD_PX); if (options.highlight) { if (highlightTimeoutRef.current !== null) {