From 6391d1e79f97f8630efa5b874fdc42338cef76e5 Mon Sep 17 00:00:00 2001 From: Wes Date: Mon, 27 Jul 2026 15:04:06 -0600 Subject: [PATCH 1/2] fix(desktop): preserve thread anchor through layout reflow Keep presentation-switch targets pinned until content resize correction has completed and the target is visible on the following paint frame. This avoids retiring the one-shot anchor before focus-to-split text reflow settles. Co-authored-by: Carl Signed-off-by: Wes --- .../src/features/channels/ui/ChannelPane.tsx | 5 +- .../ui/useThreadViewModeSwitch.test.mjs | 17 ++++ .../channels/ui/useThreadViewModeSwitch.ts | 23 ++++-- .../messages/ui/MessageThreadPanel.tsx | 4 + .../ui/useAnchoredScroll.lifecycle.test.mjs | 67 ++++++++++++++- .../features/messages/ui/useAnchoredScroll.ts | 81 +++++++++++++++++-- 6 files changed, 179 insertions(+), 18 deletions(-) diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index fea06e1b8e9..d607f102a5c 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -514,8 +514,6 @@ export const ChannelPane = React.memo(function ChannelPane({ const isOverlay = useIsThreadPanelOverlay(); const useSplitAuxiliaryPane = !isSinglePanelView && !isOverlay; const threadViewMode = useThreadViewMode(); - // Focus mode only replaces the wide split thread pane; narrow threads and - // other auxiliary panels keep their existing presentation. const useFocusThreadDrawer = threadViewMode === "focus" && useSplitAuxiliaryPane && @@ -881,7 +879,8 @@ export const ChannelPane = React.memo(function ChannelPane({ onExpandReplies={onExpandThreadReplies} onSelectReplyTarget={onSelectThreadReplyTarget} onSend={onSendThreadReply} - onScrollTargetResolved={resolveScrollTarget} + onScrollTargetResolved={() => resolveScrollTarget()} + onScrollTargetSettled={resolveScrollTarget} onToggleReaction={onToggleReaction} onUnfollowThread={onUnfollowThread} profiles={profiles} diff --git a/desktop/src/features/channels/ui/useThreadViewModeSwitch.test.mjs b/desktop/src/features/channels/ui/useThreadViewModeSwitch.test.mjs index 255492bf49a..06ae153d5c4 100644 --- a/desktop/src/features/channels/ui/useThreadViewModeSwitch.test.mjs +++ b/desktop/src/features/channels/ui/useThreadViewModeSwitch.test.mjs @@ -44,6 +44,23 @@ test("resolves both sources when a layout anchor matches the external target", ( ); }); +test("does not resolve a layout target that was never captured", () => { + assert.deepEqual( + getResolvedThreadTargets({ + externalTargetId: "reply-b", + layoutTargetId: null, + }), + { resolveExternal: true, resolveLayout: false }, + ); + assert.deepEqual( + getResolvedThreadTargets({ + externalTargetId: null, + layoutTargetId: null, + }), + { resolveExternal: true, resolveLayout: false }, + ); +}); + test("returns null without a mounted thread body or visible message", () => { assert.equal(findTopVisibleThreadMessageId(null), null); assert.equal( diff --git a/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts b/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts index 5493eada508..aaddbc95942 100644 --- a/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts +++ b/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts @@ -72,14 +72,21 @@ export function useThreadViewModeSwitch({ [onModeChange], ); - const resolveScrollTarget = React.useCallback(() => { - const resolution = getResolvedThreadTargets({ - externalTargetId: externalScrollTargetId, - layoutTargetId: layoutScrollTargetId, - }); - if (resolution.resolveLayout) setLayoutScrollTargetId(null); - if (resolution.resolveExternal) onExternalTargetResolved(); - }, [externalScrollTargetId, layoutScrollTargetId, onExternalTargetResolved]); + const resolveScrollTarget = React.useCallback( + (settledMessageId?: string) => { + const resolution = getResolvedThreadTargets({ + externalTargetId: externalScrollTargetId, + layoutTargetId: layoutScrollTargetId, + }); + if (resolution.resolveExternal) onExternalTargetResolved(); + if (settledMessageId) { + setLayoutScrollTargetId((current) => + current === settledMessageId ? null : current, + ); + } + }, + [externalScrollTargetId, layoutScrollTargetId, onExternalTargetResolved], + ); return { changeThreadViewMode, diff --git a/desktop/src/features/messages/ui/MessageThreadPanel.tsx b/desktop/src/features/messages/ui/MessageThreadPanel.tsx index 0cd4d67238a..ddf9b9dda1a 100644 --- a/desktop/src/features/messages/ui/MessageThreadPanel.tsx +++ b/desktop/src/features/messages/ui/MessageThreadPanel.tsx @@ -79,6 +79,7 @@ type MessageThreadPanelProps = ThreadPanelLayoutProps & { onMarkRead?: (message: TimelineMessage) => void; onExpandReplies: (message: TimelineMessage) => void; onScrollTargetResolved: () => void; + onScrollTargetSettled?: (messageId: string) => void; scrollTargetHighlights?: boolean; onSelectReplyTarget: (message: TimelineMessage) => void; onSend: ( @@ -207,6 +208,7 @@ export function MessageThreadPanel({ onMarkRead, onExpandReplies, onScrollTargetResolved, + onScrollTargetSettled, onSelectReplyTarget, onSend, onToggleReaction, @@ -488,6 +490,8 @@ export function MessageThreadPanel({ messages: threadMessages, highlightTargetMessage: scrollTargetHighlights, onTargetReached: onScrollTargetResolved, + onTargetSettled: onScrollTargetSettled, + pinTargetCentered: !scrollTargetHighlights, scrollContainerRef: threadBodyRef, targetMessageId: scrollTargetId, }); diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs index 2df5a22efe4..507f654cd0e 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs +++ b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs @@ -173,7 +173,7 @@ function makePinnedCenterNodes() { listener(event); }, getBoundingClientRect() { - return { top: 0 }; + return { bottom: this.clientHeight, top: 0 }; }, querySelector() { return row; @@ -232,12 +232,13 @@ function makePinnedCenterNodes() { }; } -function Harness({ channelId, refs }) { +function Harness({ channelId, onTargetSettled, refs }) { useAnchoredScroll({ channelId, contentRef: refs.content, isLoading: false, messages: [{ id: "selected" }], + onTargetSettled, pinTargetCentered: true, scrollContainerRef: refs.container, targetMessageId: "selected", @@ -312,6 +313,68 @@ test("channel change attaches pinned-center observers after refs mount", async ( }); }); +test("pinned target settles only after resize correction and a paint frame", async () => { + const refs = { + container: { current: null }, + content: { current: null }, + }; + const root = createRoot(document.createElement("div")); + const settled = []; + const nodes = makePinnedCenterNodes(); + refs.container.current = nodes.container; + refs.content.current = nodes.content; + + await act(async () => { + root.render( + React.createElement(Harness, { + channelId: "conversation", + onTargetSettled: (messageId) => settled.push(messageId), + refs, + }), + ); + }); + + assert.deepEqual(settled, [], "initial target reach is not settled"); + nodes.moveSelectedRowBy(96); + nodes.resizeObservers[0].callback(); + assert.deepEqual(settled, [], "resize callback waits for the paint frame"); + await act(async () => new Promise((resolve) => setTimeout(resolve, 0))); + + assert.deepEqual(settled, ["selected"]); + assert.deepEqual(nodes.container.scrollWrites, [96]); + await act(async () => root.unmount()); +}); + +test("user interaction releases and retires a pending pinned target", async () => { + const refs = { + container: { current: null }, + content: { current: null }, + }; + const root = createRoot(document.createElement("div")); + const settled = []; + const nodes = makePinnedCenterNodes(); + refs.container.current = nodes.container; + refs.content.current = nodes.content; + + await act(async () => { + root.render( + React.createElement(Harness, { + channelId: "conversation", + onTargetSettled: (messageId) => settled.push(messageId), + refs, + }), + ); + }); + await act(async () => nodes.container.dispatchEvent({ type: "wheel" })); + nodes.moveSelectedRowBy(96); + nodes.resizeObservers[0].callback(); + await act(async () => new Promise((resolve) => setTimeout(resolve, 0))); + + assert.deepEqual(settled, ["selected"]); + assert.deepEqual(nodes.container.scrollWrites, []); + await act(async () => root.unmount()); +}); + test("mounted virtual target retires bottom intent before direct centering", async () => { const resizeObservers = []; globalThis.ResizeObserver = class { diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index 34929fc26e7..1ed1a6741ef 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -109,6 +109,8 @@ type UseAnchoredScrollOptions = { /** Keeps a targeted message centered until the user deliberately scrolls. */ pinTargetCentered?: boolean; onTargetReached?: (messageId: string) => void; + /** Reports a pinned target after resize correction and one paint frame. */ + onTargetSettled?: (messageId: string) => void; virtualCancelBottomIntent?: () => void; virtualScrollToMessage?: ( messageId: string, @@ -230,6 +232,7 @@ export function useAnchoredScroll({ highlightTargetMessage = true, pinTargetCentered = false, onTargetReached, + onTargetSettled, virtualCancelBottomIntent, virtualScrollToMessage, virtualScrollToBottom, @@ -278,6 +281,7 @@ export function useAnchoredScroll({ const programmaticScrollTopRef = React.useRef(null); const isWritingScrollRef = React.useRef(false); const programmaticScrollRafRef = React.useRef(null); + const targetSettleRafRef = React.useRef(null); // Reset everything when the channel changes — the layout effect that runs // immediately after this reset is responsible for either jumping to bottom @@ -303,6 +307,10 @@ export function useAnchoredScroll({ cancelAnimationFrame(programmaticScrollRafRef.current); programmaticScrollRafRef.current = null; } + if (targetSettleRafRef.current !== null) { + cancelAnimationFrame(targetSettleRafRef.current); + targetSettleRafRef.current = null; + } if (highlightTimeoutRef.current !== null) { window.clearTimeout(highlightTimeoutRef.current); highlightTimeoutRef.current = null; @@ -382,6 +390,40 @@ export function useAnchoredScroll({ if (atBottom) setNewMessageCount(0); }, [scrollContainerRef]); + const schedulePinnedTargetSettle = React.useCallback( + (messageId: string) => { + if (!onTargetSettled) return; + if (targetSettleRafRef.current !== null) { + cancelAnimationFrame(targetSettleRafRef.current); + } + targetSettleRafRef.current = requestAnimationFrame(() => { + targetSettleRafRef.current = null; + const container = scrollContainerRef.current; + const anchor = anchorRef.current; + if ( + !container || + anchor.kind !== "pinned-center" || + anchor.messageId !== messageId + ) { + return; + } + const row = container.querySelector( + `[data-message-id="${CSS.escape(messageId)}"]`, + ); + if (!row) return; + const rowRect = row.getBoundingClientRect(); + const containerRect = container.getBoundingClientRect(); + if ( + rowRect.bottom > containerRect.top && + rowRect.top < containerRect.bottom + ) { + onTargetSettled(messageId); + } + }); + }, + [onTargetSettled, scrollContainerRef], + ); + const scrollToBottomImperative = React.useCallback( (behavior: ScrollBehavior = "auto") => { const container = scrollContainerRef.current; @@ -752,11 +794,11 @@ export function useAnchoredScroll({ isLoading, messages, onTargetReached, + repinPinnedCenter, scrollContainerRef, scrollToBottomImperative, scrollToMessageImperative, targetMessageId, - repinPinnedCenter, virtualScrollToBottom, virtualSettleAtBottom, virtualizerOwnsPrependAnchoring, @@ -779,6 +821,7 @@ export function useAnchoredScroll({ if (!container) return; if (anchorRef.current.kind === "pinned-center") { repinPinnedCenter(); + schedulePinnedTargetSettle(anchorRef.current.messageId); } else if ( anchorRef.current.kind === "at-bottom" && !virtualizerOwnsPrependAnchoring @@ -787,24 +830,43 @@ export function useAnchoredScroll({ } }); observer.observe(content); - return () => observer.disconnect(); + return () => { + observer.disconnect(); + if (targetSettleRafRef.current !== null) { + cancelAnimationFrame(targetSettleRafRef.current); + targetSettleRafRef.current = null; + } + }; }, [ channelId, contentRef, repinPinnedCenter, + schedulePinnedTargetSettle, scrollContainerRef, virtualizerOwnsPrependAnchoring, ]); // Pinned centers survive our own corrections but release as soon as the - // reader deliberately takes control of the scroll position. + // reader deliberately takes control of the scroll position or the caller + // retires the temporary target after layout settlement. + React.useEffect(() => { + if (!pinTargetCentered) releasePinnedCenter(); + }, [pinTargetCentered, releasePinnedCenter]); + // biome-ignore lint/correctness/useExhaustiveDependencies: channelId deliberately re-subscribes after a keyed or conditional scroll-container mount replaces ref.current. React.useEffect(() => { if (!pinTargetCentered) return; const container = scrollContainerRef.current; if (!container) return; - const handleUserInteraction = () => releasePinnedCenter(); + const handleUserInteraction = () => { + const pinnedMessageId = + anchorRef.current.kind === "pinned-center" + ? anchorRef.current.messageId + : null; + releasePinnedCenter(); + if (pinnedMessageId) onTargetSettled?.(pinnedMessageId); + }; container.addEventListener("wheel", handleUserInteraction, { passive: true, }); @@ -817,7 +879,13 @@ export function useAnchoredScroll({ container.removeEventListener("touchstart", handleUserInteraction); container.removeEventListener("keydown", handleUserInteraction); }; - }, [channelId, pinTargetCentered, releasePinnedCenter, scrollContainerRef]); + }, [ + channelId, + onTargetSettled, + pinTargetCentered, + releasePinnedCenter, + scrollContainerRef, + ]); // --------------------------------------------------------------------------- // Target message handling (deep link, jump-to-reply, etc.). Distinct from @@ -896,6 +964,9 @@ export function useAnchoredScroll({ if (programmaticScrollRafRef.current !== null) { cancelAnimationFrame(programmaticScrollRafRef.current); } + if (targetSettleRafRef.current !== null) { + cancelAnimationFrame(targetSettleRafRef.current); + } }; }, []); From 840e7a4be1a8e9c893967eebb7a62b2d62f62c3e Mon Sep 17 00:00:00 2001 From: Wes Date: Mon, 27 Jul 2026 15:11:11 -0600 Subject: [PATCH 2/2] fix(desktop): scope layout anchors to active thread Discard pending presentation-switch anchors as soon as their thread closes or is replaced so stale message IDs cannot mask the next thread's scroll target. Co-authored-by: Carl Signed-off-by: Wes --- .../src/features/channels/ui/ChannelPane.tsx | 1 + .../ui/useThreadViewModeSwitch.test.mjs | 33 +++++++++++ .../channels/ui/useThreadViewModeSwitch.ts | 56 ++++++++++++++++--- 3 files changed, 82 insertions(+), 8 deletions(-) diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index d607f102a5c..6fb7ff4ef3c 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -524,6 +524,7 @@ export const ChannelPane = React.memo(function ChannelPane({ ); const { changeThreadViewMode, layoutScrollTargetId, resolveScrollTarget } = useThreadViewModeSwitch({ + activeThreadHeadId: threadHeadMessage?.id ?? null, externalScrollTargetId: threadScrollTargetId, onExternalTargetResolved: onThreadScrollTargetResolved, onModeChange: markExitComplete, diff --git a/desktop/src/features/channels/ui/useThreadViewModeSwitch.test.mjs b/desktop/src/features/channels/ui/useThreadViewModeSwitch.test.mjs index 06ae153d5c4..9f40a00fbc2 100644 --- a/desktop/src/features/channels/ui/useThreadViewModeSwitch.test.mjs +++ b/desktop/src/features/channels/ui/useThreadViewModeSwitch.test.mjs @@ -4,6 +4,7 @@ import test from "node:test"; import { findTopVisibleThreadMessageId, getResolvedThreadTargets, + getScopedLayoutScrollTargetId, } from "./useThreadViewModeSwitch.ts"; function row(id, top, bottom) { @@ -61,6 +62,38 @@ test("does not resolve a layout target that was never captured", () => { ); }); +test("drops a captured layout target when the active thread closes or changes", () => { + const captured = { messageId: "reply-a", threadHeadId: "thread-a" }; + + assert.equal( + getScopedLayoutScrollTargetId({ + activeThreadHeadId: "thread-a", + layoutTarget: captured, + }), + "reply-a", + ); + assert.equal( + getScopedLayoutScrollTargetId({ + activeThreadHeadId: null, + layoutTarget: captured, + }), + null, + ); + const replacementLayoutTargetId = getScopedLayoutScrollTargetId({ + activeThreadHeadId: "thread-b", + layoutTarget: captured, + }); + assert.equal(replacementLayoutTargetId, null); + assert.deepEqual( + getResolvedThreadTargets({ + externalTargetId: "reply-b", + layoutTargetId: replacementLayoutTargetId, + }), + { resolveExternal: true, resolveLayout: false }, + "the stale anchor does not mask the replacement thread target", + ); +}); + test("returns null without a mounted thread body or visible message", () => { assert.equal(findTopVisibleThreadMessageId(null), null); assert.equal( diff --git a/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts b/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts index aaddbc95942..8dd41cfc51e 100644 --- a/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts +++ b/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts @@ -31,7 +31,25 @@ export function getResolvedThreadTargets({ }; } +type LayoutScrollTarget = { + messageId: string; + threadHeadId: string; +}; + +export function getScopedLayoutScrollTargetId({ + activeThreadHeadId, + layoutTarget, +}: { + activeThreadHeadId: string | null; + layoutTarget: LayoutScrollTarget | null; +}): string | null { + return layoutTarget?.threadHeadId === activeThreadHeadId + ? layoutTarget.messageId + : null; +} + type ThreadViewModeSwitchOptions = { + activeThreadHeadId: string | null; externalScrollTargetId: string | null; onExternalTargetResolved: () => void; onModeChange?: (mode: ThreadViewMode) => void; @@ -39,13 +57,23 @@ type ThreadViewModeSwitchOptions = { /** Preserves the reply being read while the thread changes presentation. */ export function useThreadViewModeSwitch({ + activeThreadHeadId, externalScrollTargetId, onExternalTargetResolved, onModeChange, }: ThreadViewModeSwitchOptions) { - const [layoutScrollTargetId, setLayoutScrollTargetId] = React.useState< - string | null - >(null); + const [layoutScrollTarget, setLayoutScrollTarget] = + React.useState(null); + const layoutScrollTargetId = getScopedLayoutScrollTargetId({ + activeThreadHeadId, + layoutTarget: layoutScrollTarget, + }); + + React.useEffect(() => { + setLayoutScrollTarget((current) => + current?.threadHeadId === activeThreadHeadId ? current : null, + ); + }, [activeThreadHeadId]); const changeThreadViewMode = React.useCallback( (mode: ThreadViewMode, restoreFocus: boolean) => { @@ -54,7 +82,11 @@ export function useThreadViewModeSwitch({ ); const anchorId = findTopVisibleThreadMessageId(body); - setLayoutScrollTargetId(anchorId); + setLayoutScrollTarget( + anchorId && activeThreadHeadId + ? { messageId: anchorId, threadHeadId: activeThreadHeadId } + : null, + ); onModeChange?.(mode); setThreadViewMode(mode); requestAnimationFrame(() => { @@ -69,7 +101,7 @@ export function useThreadViewModeSwitch({ }); }); }, - [onModeChange], + [activeThreadHeadId, onModeChange], ); const resolveScrollTarget = React.useCallback( @@ -80,12 +112,20 @@ export function useThreadViewModeSwitch({ }); if (resolution.resolveExternal) onExternalTargetResolved(); if (settledMessageId) { - setLayoutScrollTargetId((current) => - current === settledMessageId ? null : current, + setLayoutScrollTarget((current) => + current?.threadHeadId === activeThreadHeadId && + current.messageId === settledMessageId + ? null + : current, ); } }, - [externalScrollTargetId, layoutScrollTargetId, onExternalTargetResolved], + [ + activeThreadHeadId, + externalScrollTargetId, + layoutScrollTargetId, + onExternalTargetResolved, + ], ); return {