diff --git a/desktop/src/app/routes/ChannelRouteScreen.test.mjs b/desktop/src/app/routes/ChannelRouteScreen.test.mjs new file mode 100644 index 00000000000..d1dbcfd5d0d --- /dev/null +++ b/desktop/src/app/routes/ChannelRouteScreen.test.mjs @@ -0,0 +1,47 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + getValidatedRouteThreadRootId, + hasValidRouteThreadIntent, + isRouteEventForChannel, +} from "./ChannelRouteScreen.tsx"; + +function event(id, tags = [["h", "channel"]]) { + return { + id, + pubkey: "author", + created_at: 1, + kind: 9, + tags, + content: "hello", + sig: "signature", + }; +} + +test("a top-level route only accepts its own id as thread root", () => { + const target = event("target"); + assert.equal(getValidatedRouteThreadRootId(target, "target"), "target"); + assert.equal(getValidatedRouteThreadRootId(target, "unrelated"), null); + assert.equal(getValidatedRouteThreadRootId(target, null), null); +}); + +test("a reply route derives its containing root", () => { + const target = event("reply", [ + ["h", "channel"], + ["e", "root", "", "root"], + ["e", "root", "", "reply"], + ]); + assert.equal(getValidatedRouteThreadRootId(target, null), "root"); + assert.equal(getValidatedRouteThreadRootId(target, "root"), "root"); + assert.equal(getValidatedRouteThreadRootId(target, "unrelated-root"), null); + assert.equal(hasValidRouteThreadIntent(target, null), true); + assert.equal(hasValidRouteThreadIntent(target, "root"), true); + assert.equal(hasValidRouteThreadIntent(target, "unrelated-root"), false); +}); + +test("route events must belong to the routed channel", () => { + assert.equal(isRouteEventForChannel(event("target"), "channel"), true); + assert.equal(isRouteEventForChannel(event("target"), "other-channel"), false); + assert.equal(isRouteEventForChannel(event("target", []), "channel"), false); +}); diff --git a/desktop/src/app/routes/ChannelRouteScreen.tsx b/desktop/src/app/routes/ChannelRouteScreen.tsx index d4626d2c6fa..984920f6165 100644 --- a/desktop/src/app/routes/ChannelRouteScreen.tsx +++ b/desktop/src/app/routes/ChannelRouteScreen.tsx @@ -45,14 +45,47 @@ function getReplyParentId(event: RelayEvent): string | null { return getThreadReference(event.tags).parentId; } +export function isRouteEventForChannel( + event: RelayEvent, + channelId: string, +): boolean { + return event.tags.some((tag) => tag[0] === "h" && tag[1] === channelId); +} + +export function getValidatedRouteThreadRootId( + targetEvent: RelayEvent, + targetThreadRootId: string | null, +): string | null { + const targetThreadRef = getThreadReference(targetEvent.tags); + if (getReplyParentId(targetEvent) === null) { + return targetThreadRootId === targetEvent.id ? targetThreadRootId : null; + } + const derivedRootId = targetThreadRef.rootId ?? null; + return targetThreadRootId === null || targetThreadRootId === derivedRootId + ? derivedRootId + : null; +} + +export function hasValidRouteThreadIntent( + targetEvent: RelayEvent, + targetThreadRootId: string | null, +): boolean { + return ( + getReplyParentId(targetEvent) === null || + targetThreadRootId === null || + getValidatedRouteThreadRootId(targetEvent, targetThreadRootId) !== null + ); +} + async function fetchRouteTargetEvents( + channelId: string, eventIds: string[], targetMessageId: string | null, targetThreadRootId: string | null, ): Promise { const eventsById = new Map(); const addEvent = (event: RelayEvent | null) => { - if (event) { + if (event && isRouteEventForChannel(event, channelId)) { eventsById.set(event.id, event); } }; @@ -66,12 +99,17 @@ async function fetchRouteTargetEvents( const targetEvent = targetMessageId ? (eventsById.get(targetMessageId) ?? null) : null; - if (!targetEvent) { + if ( + !targetEvent || + !hasValidRouteThreadIntent(targetEvent, targetThreadRootId) + ) { return [...eventsById.values()]; } - const targetThreadRef = getThreadReference(targetEvent.tags); - const threadRootId = targetThreadRootId ?? targetThreadRef.rootId ?? null; + const threadRootId = getValidatedRouteThreadRootId( + targetEvent, + targetThreadRootId, + ); if (threadRootId && !eventsById.has(threadRootId)) { addEvent(await fetchRouteEvent(threadRootId)); } @@ -85,7 +123,7 @@ async function fetchRouteTargetEvents( ) { const parentEvent = eventsById.get(parentId) ?? (await fetchRouteEvent(parentId)); - if (!parentEvent) { + if (!parentEvent || !isRouteEventForChannel(parentEvent, channelId)) { break; } @@ -130,7 +168,9 @@ export function ChannelRouteScreen({ RelayEvent[] >(() => { const cachedTarget = getCachedSearchHitEvent(targetMessageId); - return cachedTarget ? [cachedTarget] : []; + return cachedTarget && isRouteEventForChannel(cachedTarget, channelId) + ? [cachedTarget] + : []; }); // Reset spliced target events when the channel context changes (channel @@ -166,7 +206,7 @@ export function ChannelRouteScreen({ } const cachedTarget = getCachedSearchHitEvent(targetMessageId); - if (cachedTarget) { + if (cachedTarget && isRouteEventForChannel(cachedTarget, channelId)) { setTargetMessageEvents((currentEvents) => currentEvents.some((event) => event.id === cachedTarget.id) ? currentEvents @@ -174,14 +214,17 @@ export function ChannelRouteScreen({ ); } - const eventIds = [ - targetMessageId, - targetThreadRootId && targetThreadRootId !== targetMessageId - ? targetThreadRootId - : null, - ].filter((eventId): eventId is string => eventId !== null); + // The selected message is authoritative. Load it first so the helper can + // validate any supplied thread relationship before fetching another event. + // A thread-only route has no selected message to validate against. + const eventIds = targetMessageId + ? [targetMessageId] + : targetThreadRootId + ? [targetThreadRootId] + : []; void fetchRouteTargetEvents( + channelId, eventIds, targetMessageId, targetThreadRootId, @@ -200,7 +243,7 @@ export function ChannelRouteScreen({ return () => { isCancelled = true; }; - }, [selectedPostId, targetMessageId, targetThreadRootId]); + }, [channelId, selectedPostId, targetMessageId, targetThreadRootId]); if ( !activeChannel && @@ -234,6 +277,7 @@ export function ChannelRouteScreen({ targetForumReplyId={targetReplyId} targetMessageEvents={targetMessageEvents} targetMessageId={targetMessageId} + targetThreadRootId={targetThreadRootId} /> ); } diff --git a/desktop/src/features/channels/ui/ChannelScreen.tsx b/desktop/src/features/channels/ui/ChannelScreen.tsx index 240a9ad70c1..38c8321024d 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.tsx +++ b/desktop/src/features/channels/ui/ChannelScreen.tsx @@ -89,6 +89,7 @@ import { useChannelUnreadState } from "./useChannelUnreadState"; import type { ChannelScreenProps } from "./ChannelScreen.types"; import { GuardedChannelPane } from "./GuardedChannelPane"; import { useNavigationGuard } from "./useNavigationGuard"; + const EMPTY_RELAY_EVENTS: RelayEvent[] = []; export function ChannelScreen({ activeChannel, @@ -101,6 +102,7 @@ export function ChannelScreen({ targetForumReplyId, targetMessageEvents, targetMessageId, + ...routeTargets }: ChannelScreenProps) { const queryClient = useQueryClient(); const { goHome } = useAppNavigation(); @@ -632,9 +634,6 @@ export function ChannelScreen({ isPlaceholderData: messagesQuery.isPlaceholderData, dataLength: messagesQuery.data?.length ?? null, }, - // A persisted head only counts as hydrated when it has rows to paint - // (channelHeadCache.ts), so this bypass never settles onto an empty - // placeholder while the authoritative refresh is still in flight. hasSettledThisChannel || (activeChannelId !== null && hasPersistedHydratedChannel(queryClient, activeChannelId)), @@ -672,6 +671,7 @@ export function ChannelScreen({ setThreadReplyTargetId, setThreadScrollTargetId, targetMessageId, + targetThreadRootId: routeTargets.targetThreadRootId, timelineMessages, }); useThreadTargetSync({ diff --git a/desktop/src/features/channels/ui/ChannelScreen.types.ts b/desktop/src/features/channels/ui/ChannelScreen.types.ts index 371af6faf5d..766f53eeb74 100644 --- a/desktop/src/features/channels/ui/ChannelScreen.types.ts +++ b/desktop/src/features/channels/ui/ChannelScreen.types.ts @@ -22,4 +22,11 @@ export type ChannelScreenProps = { targetForumReplyId: string | null; targetMessageEvents: RelayEvent[]; targetMessageId: string | null; + /** + * Thread root requested by the navigation source (`?threadRootId`, or the + * `?thread` panel param on deep links). Deciding input for top-level route + * targets: present → open the thread panel at that root; absent → the + * target is shown in the main timeline only. + */ + targetThreadRootId: string | null; }; diff --git a/desktop/src/features/channels/ui/useChannelRouteTarget.lifecycle.test.mjs b/desktop/src/features/channels/ui/useChannelRouteTarget.lifecycle.test.mjs new file mode 100644 index 00000000000..0b274c58c2f --- /dev/null +++ b/desktop/src/features/channels/ui/useChannelRouteTarget.lifecycle.test.mjs @@ -0,0 +1,66 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { JSDOM } from "jsdom"; + +const dom = new JSDOM(""); +globalThis.window = dom.window; +globalThis.document = dom.window.document; +Object.defineProperty(globalThis, "navigator", { + configurable: true, + value: dom.window.navigator, +}); +globalThis.HTMLElement = dom.window.HTMLElement; +globalThis.IS_REACT_ACT_ENVIRONMENT = true; + +const React = await import("react"); +const { act } = React; +const { createRoot } = await import("react-dom/client"); +const { useChannelRouteTarget } = await import("./useChannelRouteTarget.ts"); + +const target = { + id: "target", + author: "alice", + body: "hello", + createdAt: 1, + depth: 0, + parentId: null, + rootId: null, + tags: [], + time: "now", +}; + +function Harness({ calls, threadRootId }) { + useChannelRouteTarget({ + activeChannel: { id: "channel", channelType: "stream" }, + activeChannelId: "channel", + closeAgentSession: () => calls.push("close-agent"), + requireThreadEditResolution: () => true, + setEditTargetId: () => {}, + setExpandedThreadReplyIds: () => {}, + setOpenThreadHeadId: (id) => calls.push(`open:${id}`), + setProfilePanelPubkey: () => {}, + setThreadReplyTargetId: () => {}, + setThreadScrollTargetId: () => {}, + targetMessageId: "target", + targetThreadRootId: threadRootId, + timelineMessages: [target], + }); + return null; +} + +test("the same top-level target can advance from timeline-only to open-thread", async () => { + const calls = []; + const root = createRoot(document.createElement("div")); + await act(async () => { + root.render(React.createElement(Harness, { calls, threadRootId: null })); + }); + assert.deepEqual(calls, []); + + await act(async () => { + root.render( + React.createElement(Harness, { calls, threadRootId: "target" }), + ); + }); + assert.deepEqual(calls, ["close-agent", "open:target"]); + await act(async () => root.unmount()); +}); diff --git a/desktop/src/features/channels/ui/useChannelRouteTarget.test.mjs b/desktop/src/features/channels/ui/useChannelRouteTarget.test.mjs new file mode 100644 index 00000000000..54fdd0dd6f9 --- /dev/null +++ b/desktop/src/features/channels/ui/useChannelRouteTarget.test.mjs @@ -0,0 +1,153 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { getRouteTargetPanelAction } from "./useChannelRouteTarget.ts"; + +function makeMessage(overrides = {}) { + return { + id: "target", + author: "alice", + body: "hello", + createdAt: 1, + depth: 0, + parentId: null, + rootId: null, + tags: [], + time: "now", + ...overrides, + }; +} + +function byId(...messages) { + return new Map(messages.map((message) => [message.id, message])); +} + +test("top-level target without threadRootId stays in the main timeline", () => { + const root = makeMessage(); + assert.deepEqual(getRouteTargetPanelAction(root, null, byId(root)), { + kind: "main-timeline-only", + }); +}); + +test("top-level target with a mismatched threadRootId stays in the timeline", () => { + const root = makeMessage(); + assert.deepEqual( + getRouteTargetPanelAction(root, "unrelated-root", byId(root)), + { kind: "main-timeline-only" }, + ); +}); + +test("top-level target with an explicit threadRootId opens its thread panel", () => { + const root = makeMessage(); + assert.deepEqual(getRouteTargetPanelAction(root, root.id, byId(root)), { + kind: "open-thread", + expandedReplyIds: new Set(), + replyTargetId: root.id, + scrollTargetId: null, + threadHeadId: root.id, + }); +}); + +test("reply target without threadRootId opens the derived thread", () => { + const root = makeMessage({ id: "root" }); + const reply = makeMessage({ + id: "reply", + parentId: "root", + rootId: "root", + depth: 1, + }); + assert.deepEqual(getRouteTargetPanelAction(reply, null, byId(root, reply)), { + kind: "open-thread", + expandedReplyIds: new Set(), + replyTargetId: "root", + scrollTargetId: "reply", + threadHeadId: "root", + }); +}); + +test("reply target with its derived threadRootId opens the thread", () => { + const root = makeMessage({ id: "root" }); + const reply = makeMessage({ + id: "reply", + parentId: "root", + rootId: "root", + depth: 1, + }); + assert.deepEqual( + getRouteTargetPanelAction(reply, root.id, byId(root, reply)), + { + kind: "open-thread", + expandedReplyIds: new Set(), + replyTargetId: "root", + scrollTargetId: "reply", + threadHeadId: "root", + }, + ); +}); + +test("reply target with a mismatched threadRootId does not open the loaded derived thread", () => { + const root = makeMessage({ id: "root" }); + const reply = makeMessage({ + id: "reply", + parentId: "root", + rootId: "root", + depth: 1, + }); + assert.deepEqual( + getRouteTargetPanelAction(reply, "unrelated-root", byId(root, reply)), + { kind: "none" }, + ); +}); + +test("nested reply target expands its intermediate ancestors", () => { + const root = makeMessage({ id: "root" }); + const mid = makeMessage({ + id: "mid", + parentId: "root", + rootId: "root", + depth: 1, + }); + const leaf = makeMessage({ + id: "leaf", + parentId: "mid", + rootId: "root", + depth: 2, + }); + assert.deepEqual( + getRouteTargetPanelAction(leaf, null, byId(root, mid, leaf)), + { + kind: "open-thread", + expandedReplyIds: new Set(["mid"]), + replyTargetId: "root", + scrollTargetId: "leaf", + threadHeadId: "root", + }, + ); +}); + +test("broadcast reply target is not a panel action", () => { + const root = makeMessage({ id: "root" }); + const broadcast = makeMessage({ + id: "broadcast", + parentId: "root", + rootId: "root", + depth: 1, + tags: [["broadcast", "1"]], + }); + assert.deepEqual( + getRouteTargetPanelAction(broadcast, null, byId(root, broadcast)), + { kind: "none" }, + ); +}); + +test("reply whose thread head is not loaded yet defers", () => { + const reply = makeMessage({ + id: "reply", + parentId: "missing-root", + rootId: "missing-root", + depth: 1, + }); + assert.deepEqual(getRouteTargetPanelAction(reply, null, byId(reply)), { + kind: "none", + }); +}); diff --git a/desktop/src/features/channels/ui/useChannelRouteTarget.ts b/desktop/src/features/channels/ui/useChannelRouteTarget.ts index 39e8a6688d5..e099db26eeb 100644 --- a/desktop/src/features/channels/ui/useChannelRouteTarget.ts +++ b/desktop/src/features/channels/ui/useChannelRouteTarget.ts @@ -37,6 +37,76 @@ function getThreadRouteTarget( return { expandedReplyIds, threadHeadId }; } +export type RouteTargetPanelAction = + | { kind: "none" } + | { kind: "main-timeline-only" } + | { + kind: "open-thread"; + expandedReplyIds: Set; + replyTargetId: string; + scrollTargetId: string | null; + threadHeadId: string; + }; + +/** + * Decides what a message route target does to the thread panel. + * + * - Top-level target without an explicit `threadRootId` (inbox message rows, + * desktop notifications, search hits, `buzz://` root links): the + * main-timeline scroll + highlight is the entire navigation. Opening the + * reply panel here would show an empty "no replies" pane instead of the + * message in its own context — the exact defect this guards against. + * - Top-level target with an explicit `threadRootId` (inbox "Open full + * thread", thread-draft auto-send, channel-activity rows): the surface + * asked for the thread, so the panel opens at that root. + * - Reply target: the panel opens at the thread head, scrolled to the reply. + * - `none`: not actionable yet (broadcast reply, or the thread head is not + * loaded) — the caller retries when more messages arrive. + * + * Exported as a pure function so the routing contract is unit-testable + * without mounting the hook. + */ +export function getRouteTargetPanelAction( + targetMessage: TimelineMessage, + targetThreadRootId: string | null, + messageById: ReadonlyMap, +): RouteTargetPanelAction { + if (!targetMessage.parentId) { + if (!targetThreadRootId || targetThreadRootId !== targetMessage.id) { + return { kind: "main-timeline-only" }; + } + return { + kind: "open-thread", + expandedReplyIds: new Set(), + replyTargetId: targetMessage.id, + scrollTargetId: null, + threadHeadId: targetMessage.id, + }; + } + + const derivedRootId = targetMessage.rootId ?? targetMessage.parentId; + if (targetThreadRootId !== null && targetThreadRootId !== derivedRootId) { + return { kind: "none" }; + } + + if (isBroadcastReply(targetMessage.tags ?? [])) { + return { kind: "none" }; + } + + const routeTarget = getThreadRouteTarget(targetMessage, messageById); + if (!routeTarget) { + return { kind: "none" }; + } + + return { + kind: "open-thread", + expandedReplyIds: routeTarget.expandedReplyIds, + replyTargetId: routeTarget.threadHeadId, + scrollTargetId: targetMessage.id, + threadHeadId: routeTarget.threadHeadId, + }; +} + function getRouteMainTimelineTargetId( targetMessageId: string | null, targetMessage: TimelineMessage | null, @@ -64,6 +134,7 @@ export function useChannelRouteTarget({ setThreadReplyTargetId, setThreadScrollTargetId, targetMessageId, + targetThreadRootId, timelineMessages, }: { activeChannel: Channel | null; @@ -77,6 +148,7 @@ export function useChannelRouteTarget({ setThreadReplyTargetId: React.Dispatch>; setThreadScrollTargetId: React.Dispatch>; targetMessageId: string | null; + targetThreadRootId: string | null; timelineMessages: TimelineMessage[]; }) { const timelineMessageById = React.useMemo( @@ -98,50 +170,43 @@ export function useChannelRouteTarget({ return; } - const targetKey = `${activeChannelId ?? "none"}:${targetMessageId}`; - if (handledThreadRouteTargetRef.current !== targetKey) { - handledThreadRouteTargetRef.current = null; - } - + const targetMessage = timelineMessageById.get(targetMessageId) ?? null; if ( - handledThreadRouteTargetRef.current === targetKey || + !targetMessage || !activeChannel || activeChannel.channelType === "forum" ) { return; } - const targetMessage = timelineMessageById.get(targetMessageId) ?? null; - if (!targetMessage) { - return; - } - - if (!targetMessage.parentId) { - if (!requireThreadEditResolution()) { - return; - } - closeAgentSession(); - setProfilePanelPubkey(null, { replace: true }); - setEditTargetId(null); - // Root message links open the reply panel. Navigation is refused before - // this route target is accepted when another composer owns a dirty edit. - setOpenThreadHeadId(targetMessage.id, { replace: true }); - setThreadReplyTargetId(targetMessage.id); - setThreadScrollTargetId(null); - setExpandedThreadReplyIds(new Set()); - handledThreadRouteTargetRef.current = targetKey; + const action = getRouteTargetPanelAction( + targetMessage, + targetThreadRootId, + timelineMessageById, + ); + if (action.kind === "none") { return; } - if (isBroadcastReply(targetMessage.tags ?? [])) { + // Dedupe the complete normalized action, not just the selected row. The + // same top-level message can first arrive as a timeline-only target and + // later be re-targeted with a validated request to open its full thread. + const actionKey = + action.kind === "main-timeline-only" + ? action.kind + : `${action.kind}:${action.threadHeadId}:${action.replyTargetId}:${action.scrollTargetId ?? "none"}`; + const targetKey = `${activeChannelId ?? "none"}:${targetMessageId}:${actionKey}`; + if (handledThreadRouteTargetRef.current === targetKey) { return; } + handledThreadRouteTargetRef.current = null; - const routeTarget = getThreadRouteTarget( - targetMessage, - timelineMessageById, - ); - if (!routeTarget) { + if (action.kind === "main-timeline-only") { + // Top-level target with no requested thread: the main-timeline + // scroll/highlight (mainTimelineTargetMessageId) is the whole + // navigation. Mark handled so a later timeline update cannot + // re-process this target. + handledThreadRouteTargetRef.current = targetKey; return; } if (!requireThreadEditResolution()) { @@ -153,10 +218,10 @@ export function useChannelRouteTarget({ // back should leave the deep link, not strip the panel from it. setProfilePanelPubkey(null, { replace: true }); setEditTargetId(null); - setOpenThreadHeadId(routeTarget.threadHeadId, { replace: true }); - setThreadReplyTargetId(routeTarget.threadHeadId); - setThreadScrollTargetId(targetMessageId); - setExpandedThreadReplyIds(routeTarget.expandedReplyIds); + setOpenThreadHeadId(action.threadHeadId, { replace: true }); + setThreadReplyTargetId(action.replyTargetId); + setThreadScrollTargetId(action.scrollTargetId); + setExpandedThreadReplyIds(action.expandedReplyIds); handledThreadRouteTargetRef.current = targetKey; }, [ activeChannel, @@ -170,6 +235,7 @@ export function useChannelRouteTarget({ setThreadReplyTargetId, setThreadScrollTargetId, targetMessageId, + targetThreadRootId, timelineMessageById, ]); diff --git a/desktop/src/features/messages/ui/MessageRow.tsx b/desktop/src/features/messages/ui/MessageRow.tsx index 536631b02d3..9b48c4359eb 100644 --- a/desktop/src/features/messages/ui/MessageRow.tsx +++ b/desktop/src/features/messages/ui/MessageRow.tsx @@ -55,6 +55,7 @@ import { MessageMetaSegments, } from "./MessageHeader"; import { MessageTimestamp } from "./MessageTimestamp"; +import { ROUTE_TARGET_HIGHLIGHT_CLASS } from "./routeTargetHighlight"; import { SentFromThreadLine } from "./SentFromThreadLine"; import { WaveMessageAttachment } from "./WaveMessageAttachment"; import { Tooltip, TooltipContent, TooltipTrigger } from "@/shared/ui/tooltip"; @@ -892,9 +893,7 @@ export const MessageRow = React.memo( "flex gap-2.5", isDisplayedAsContinuation ? "items-center" : "items-start", hasActiveReminder ? "bg-blue-500/10" : "", - highlighted - ? "-mx-4 rounded-none px-6 before:absolute before:-inset-y-1.5 before:inset-x-0 before:animate-[route-target-highlight-fade_2s_ease-out_forwards] before:bg-primary/10 before:content-[''] motion-reduce:before:animate-none sm:-mx-6 sm:px-8" - : "", + highlighted ? ROUTE_TARGET_HIGHLIGHT_CLASS : "", )} data-message-id={message.id} data-testid="message-row" diff --git a/desktop/src/features/messages/ui/MessageTimeline.tsx b/desktop/src/features/messages/ui/MessageTimeline.tsx index fa8bb4e9f6d..e47d3f77d01 100644 --- a/desktop/src/features/messages/ui/MessageTimeline.tsx +++ b/desktop/src/features/messages/ui/MessageTimeline.tsx @@ -361,7 +361,9 @@ const MessageTimelineBase = React.forwardRef< scrollContainerRef: activeScrollContainerRef, splitPanelOpen: splitThreadPanelOpen, targetMessageId, + topBoundaryReached: renderedHistoryExhausted, virtualCancelBottomIntent: timelineVirtualizerApi?.cancelBottomIntent, + virtualScrollBy: timelineVirtualizerApi?.scrollBy, virtualScrollToMessage: timelineVirtualizerApi?.scrollToMessage, virtualScrollToBottom: timelineVirtualizerApi?.scrollToBottom, virtualSettleAtBottom: timelineVirtualizerApi?.settleAtBottom, @@ -473,8 +475,9 @@ const MessageTimelineBase = React.forwardRef< [prepareForOwnMessage, scrollToBottom, timelineVirtualizerApi], ); - // Jump-to-message is purely DOM-based now: all loaded rows are mounted, so - // `scrollToMessage` always finds the target row. No virtualizer convergence. + // Jump-to-message reports `centered` once the row is in the DOM and placed; + // `pending` means the virtualizer accepted the jump and the row commits on a + // later render, so callers retry on range change rather than re-querying. const jumpToMessage = React.useCallback( (messageId: string, options?: { behavior?: ScrollBehavior }) => { return scrollToMessage(messageId, { highlight: true, ...options }); @@ -535,28 +538,19 @@ const MessageTimelineBase = React.forwardRef< } pendingSearchTargetRef.current = null; prevSearchActiveRef.current = searchActiveMessageId; - if (!jumpToMessage(searchActiveMessageId, { behavior: "smooth" })) { + if ( + jumpToMessage(searchActiveMessageId, { behavior: "smooth" }) !== + "centered" + ) { pendingSearchTargetRef.current = searchActiveMessageId; } }, [jumpToMessage, searchActiveMessageId, showTimelineSkeleton]); - // biome-ignore lint/correctness/useExhaustiveDependencies: deferredMessages and virtualizerRenderVersion are intentional retry triggers — a search hit may be spliced into messages asynchronously, and in virtualized mode a phase-1 index jump only realizes the row; retry when the rendered range changes so the DOM-visible path can center and highlight it. + // biome-ignore lint/correctness/useExhaustiveDependencies: deferredMessages and virtualizerRenderVersion are intentional retry triggers — a search hit may be spliced into messages asynchronously (`missing`), and in virtualized mode an index jump only realizes the row (`pending`); retry when the rendered range changes so the DOM path can center and highlight it. React.useEffect(() => { const target = pendingSearchTargetRef.current; if (!target || showTimelineSkeleton) return; - if ( - useTimelineVirtualizer && - !activeScrollContainerRef.current?.querySelector( - `[data-message-id="${CSS.escape(target)}"]`, - ) - ) { - // Phase 1: ask the virtualizer to realize the match's index. The retry effect - // runs again on range change and the DOM-visible path does the actual - // center + highlight once the row exists. - void jumpToMessage(target, { behavior: "auto" }); - return; - } - if (jumpToMessage(target, { behavior: "auto" })) { + if (jumpToMessage(target, { behavior: "auto" }) === "centered") { pendingSearchTargetRef.current = null; } }, [ diff --git a/desktop/src/features/messages/ui/TimelineMessageList.tsx b/desktop/src/features/messages/ui/TimelineMessageList.tsx index d7ef78ea04b..695bafa73e2 100644 --- a/desktop/src/features/messages/ui/TimelineMessageList.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageList.tsx @@ -33,6 +33,7 @@ import { UnreadDivider } from "./UnreadDivider"; import { useTimelineRetention } from "./useTimelineRetention"; import { useUpwardPaginationWheel } from "./useUpwardPaginationWheel"; import { useVirtualizedBottomSettle } from "./useVirtualizedBottomSettle"; +import { getVirtualMessageScrollOptions } from "./virtualMessageScroll"; export type TimelineVirtualizerApi = { cancelBottomIntent: () => void; @@ -42,6 +43,7 @@ export type TimelineVirtualizerApi = { messageId: string, options?: { behavior?: ScrollBehavior }, ) => boolean; + scrollBy: (offset: number) => void; }; type TimelineMessageListProps = { @@ -670,13 +672,19 @@ function VirtualizedTimelineRows({ settleAtBottom(); }, settleAtBottom, - scrollToMessage(messageId) { + scrollToMessage(messageId, options) { cancelBottomSettle(); const index = messageItemIndexByIdRef.current.get(messageId); if (index === undefined) return false; - listRef.current?.scrollToIndex(index, { align: "center" }); + listRef.current?.scrollToIndex( + index, + getVirtualMessageScrollOptions(options?.behavior), + ); return true; }, + scrollBy(offset) { + listRef.current?.scrollBy(offset); + }, }; onVirtualizerApiChange(api); return () => onVirtualizerApiChange(null); diff --git a/desktop/src/features/messages/ui/TimelineMessageRow.tsx b/desktop/src/features/messages/ui/TimelineMessageRow.tsx index 283760fb021..3ad05b41246 100644 --- a/desktop/src/features/messages/ui/TimelineMessageRow.tsx +++ b/desktop/src/features/messages/ui/TimelineMessageRow.tsx @@ -9,6 +9,7 @@ import type { UserProfileLookup } from "@/features/profile/lib/identity"; import { cn } from "@/shared/lib/cn"; import { MessageRow } from "./MessageRow"; import { MessageThreadSummaryRow } from "./MessageThreadSummaryRow"; +import { ROUTE_TARGET_HIGHLIGHT_CLASS } from "./routeTargetHighlight"; import { SystemMessageRow } from "./SystemMessageRow"; type ToggleReaction = ( @@ -137,8 +138,7 @@ export function MessageRowItem({
; + contentRef: React.RefObject; + channelId?: string | null; + isLoading: boolean; + messages: Array<{ id: string }>; + splitPanelOpen?: boolean; + targetMessageId?: string | null; + highlightTargetMessage?: boolean; + pinTargetCentered?: boolean; + topBoundaryReached?: boolean; + onTargetReached?: (messageId: string) => void; + onTargetSettled?: (messageId: string) => void; + virtualCancelBottomIntent?: () => void; + virtualScrollToMessage?: ( + messageId: string, + options?: { behavior?: ScrollBehavior }, + ) => boolean; + virtualScrollBy?: (offset: number) => void; + virtualScrollToBottom?: (behavior?: ScrollBehavior) => void; + virtualSettleAtBottom?: () => void; + virtualizerOwnsPrependAnchoring?: boolean; + virtualizerRenderVersion?: number; +}; + +export type UseAnchoredScrollResult = { + onScroll: () => void; + isAtBottom: boolean; + newMessageCount: number; + highlightedMessageId: string | null; + scrollToBottom: (behavior?: ScrollBehavior) => void; + settleAtBottomAfterLayout: () => boolean; + scrollToBottomOnNextUpdate: () => void; + scrollToMessage: ( + messageId: string, + options?: { highlight?: boolean; behavior?: ScrollBehavior }, + ) => ScrollToMessageResult; + onVirtualizerAtBottomStateChange: (atBottom: boolean) => void; +}; diff --git a/desktop/src/features/messages/ui/routeTargetHighlight.ts b/desktop/src/features/messages/ui/routeTargetHighlight.ts new file mode 100644 index 00000000000..be99346535c --- /dev/null +++ b/desktop/src/features/messages/ui/routeTargetHighlight.ts @@ -0,0 +1,14 @@ +/** + * Classes that paint the "you were sent here" tint on a timeline row. + * + * The tint is drawn on a `before:` pseudo-element sized to the row's own + * hover pill (same `rounded-2xl` radius, no margin/padding change), so turning + * the highlight on or off never alters the row's geometry. In the virtualized + * timeline a geometry change would rewrap text, change the row's measured + * height, and make Virtua nudge the scroll position — once on arrival and + * again when the highlight clears. + * + * Hosts must be `relative` and use the `rounded-2xl` hover pill geometry. + */ +export const ROUTE_TARGET_HIGHLIGHT_CLASS = + "before:pointer-events-none before:absolute before:inset-0 before:rounded-2xl before:animate-[route-target-highlight-fade_2s_ease-out_forwards] before:bg-primary/10 before:content-[''] motion-reduce:before:animate-none"; diff --git a/desktop/src/features/messages/ui/targetRowCentering.ts b/desktop/src/features/messages/ui/targetRowCentering.ts new file mode 100644 index 00000000000..a078f7383ad --- /dev/null +++ b/desktop/src/features/messages/ui/targetRowCentering.ts @@ -0,0 +1,66 @@ +const CENTERED_ROW_TOLERANCE_PX = 2; + +function resolveCssLength(value: string) { + const parsed = Number.parseFloat(value); + if (!Number.isFinite(parsed)) return 0; + return value.trim().endsWith("rem") + ? parsed * + Number.parseFloat(getComputedStyle(document.documentElement).fontSize) + : parsed; +} + +function getUsableViewportBounds(container: HTMLDivElement) { + const containerRect = container.getBoundingClientRect(); + const styles = getComputedStyle(container); + return { + bottom: + containerRect.bottom - + resolveCssLength(styles.getPropertyValue("--composer-overlay-height")), + top: + containerRect.top + + resolveCssLength(styles.getPropertyValue("--channel-top-chrome-height")), + }; +} + +export function getTargetRowCenterOffset( + row: Element, + container: HTMLDivElement, +) { + const rowRect = row.getBoundingClientRect(); + const viewport = getUsableViewportBounds(container); + return ( + (rowRect.top + rowRect.bottom) / 2 - (viewport.top + viewport.bottom) / 2 + ); +} + +/** + * A virtualized jump is complete only when the row's midpoint reaches the + * viewport midpoint. Two pixels absorb fractional layout and Virtua's rounded + * scroll offsets. Boundary rows are the intentional exceptions: the list + * clamps the oldest row to the physical ceiling and the newest row to the + * physical floor, where exact centering is impossible. + */ +export function isTargetRowCentered( + row: Element, + container: HTMLDivElement, + boundary: "none" | "top" | "bottom", + isAtBottom: (container: HTMLDivElement) => boolean, +) { + const rowRect = row.getBoundingClientRect(); + if (rowRect.bottom - rowRect.top <= 0) return false; + if ( + Math.abs(getTargetRowCenterOffset(row, container)) <= + CENTERED_ROW_TOLERANCE_PX + ) { + return true; + } + const viewport = getUsableViewportBounds(container); + const rowIsVisible = + rowRect.bottom > viewport.top && rowRect.top < viewport.bottom; + if (boundary === "top") return rowIsVisible && container.scrollTop <= 0; + return boundary === "bottom" && rowIsVisible && isAtBottom(container); +} + +export function targetRowNeedsCenterCorrection(offset: number) { + return Math.abs(offset) > CENTERED_ROW_TOLERANCE_PX; +} diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs index ee3fec1a988..d7ff4fa3de5 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs +++ b/desktop/src/features/messages/ui/useAnchoredScroll.lifecycle.test.mjs @@ -148,11 +148,16 @@ function installDOMShim() { } installDOMShim(); +globalThis.getComputedStyle = () => ({ + fontSize: "16px", + getPropertyValue: () => "0px", +}); import React from "react"; import { act } from "react"; import { createRoot } from "react-dom/client"; +import { isTargetRowCentered } from "./targetRowCentering.ts"; import { useAnchoredScroll } from "./useAnchoredScroll.ts"; import { useVirtualizedBottomSettle } from "./useVirtualizedBottomSettle.ts"; @@ -266,6 +271,37 @@ function BottomStateHarness({ return null; } +function VirtualScrollBehaviorHarness({ + messages = [{ id: "selected" }], + refs, +}) { + const lastRunMessageCount = React.useRef(null); + const anchored = useAnchoredScroll({ + channelId: "conversation", + contentRef: refs.content, + isLoading: false, + messages, + scrollContainerRef: refs.scroller, + virtualizerOwnsPrependAnchoring: true, + virtualScrollBy: (offset) => { + refs.scrollOffsets.push(offset); + refs.rowTop -= offset; + }, + virtualScrollToMessage: (messageId, options) => { + refs.targetJumps.push({ messageId, options }); + return true; + }, + }); + React.useLayoutEffect(() => { + if (lastRunMessageCount.current === messages.length) return; + lastRunMessageCount.current = messages.length; + refs.targetResult = anchored.scrollToMessage("selected", { + behavior: "smooth", + }); + }, [anchored.scrollToMessage, messages.length, refs]); + return null; +} + function VirtualTargetHarness({ refs }) { const didRun = React.useRef(false); const bottomApi = useVirtualizedBottomSettle( @@ -281,7 +317,10 @@ function VirtualTargetHarness({ refs }) { scrollContainerRef: refs.scroller, virtualCancelBottomIntent: bottomApi.cancel, virtualizerOwnsPrependAnchoring: true, - virtualScrollToMessage: () => true, + virtualScrollToMessage: (messageId) => { + refs.targetJumps.current.push(messageId); + return true; + }, }); React.useLayoutEffect(() => { if (didRun.current) return; @@ -522,7 +561,180 @@ test("user interaction releases and retires a pending pinned target", async () = await act(async () => root.unmount()); }); -test("mounted virtual target retires bottom intent before direct centering", async () => { +test("boundary-clamped targets settle only at their matching physical edge", () => { + const container = document.createElement("div"); + container.scrollTop = 0; + container.getBoundingClientRect = () => ({ bottom: 400, top: 0 }); + const row = { + getBoundingClientRect: () => ({ bottom: 40, height: 40, top: 0 }), + }; + const isAtBottom = () => false; + + assert.equal(isTargetRowCentered(row, container, "top", isAtBottom), true); + assert.equal(isTargetRowCentered(row, container, "none", isAtBottom), false); + assert.equal( + isTargetRowCentered(row, container, "bottom", isAtBottom), + false, + ); + + row.getBoundingClientRect = () => ({ + bottom: 2_740, + height: 40, + top: 2_700, + }); + assert.equal( + isTargetRowCentered(row, container, "top", isAtBottom), + false, + "an unrendered indexed jump can report scrollTop zero before the row arrives", + ); + + row.getBoundingClientRect = () => ({ bottom: 40, height: 40, top: 0 }); + container.scrollTop = 1; + assert.equal(isTargetRowCentered(row, container, "top", isAtBottom), false); + assert.equal( + isTargetRowCentered(row, container, "bottom", () => true), + true, + ); +}); + +test("virtual search scrolling stays smooth only for an already-rendered target", async () => { + for (const rendered of [true, false]) { + const content = document.createElement("div"); + const scroller = document.createElement("div"); + scroller.clientHeight = 400; + scroller.scrollHeight = 1_000; + scroller.scrollTop = 0; + scroller.getBoundingClientRect = () => ({ bottom: 400, top: 0 }); + const row = { + getBoundingClientRect: () => ({ + bottom: refs.rowTop + 40, + height: 40, + top: refs.rowTop, + }), + }; + scroller.querySelector = () => (rendered ? row : null); + scroller.querySelectorAll = () => []; + scroller.appendChild(content); + const refs = { + content: { current: content }, + scroller: { current: scroller }, + rowTop: rendered ? 180 : 1_000, + scrollOffsets: [], + targetJumps: [], + targetResult: null, + }; + const root = createRoot(document.createElement("div")); + + await act(async () => { + root.render(React.createElement(VirtualScrollBehaviorHarness, { refs })); + }); + + assert.deepEqual(refs.targetJumps, [ + { + messageId: "selected", + options: { behavior: rendered ? "smooth" : "auto" }, + }, + ]); + assert.equal(refs.targetResult, rendered ? "centered" : "pending"); + await act(async () => root.unmount()); + } +}); + +test("a pending virtual jump is retried when the indexed message model grows", async () => { + const content = document.createElement("div"); + const scroller = document.createElement("div"); + scroller.clientHeight = 400; + scroller.scrollHeight = 1_000; + scroller.scrollTop = 0; + scroller.getBoundingClientRect = () => ({ bottom: 400, top: 0 }); + scroller.querySelector = () => null; + scroller.querySelectorAll = () => []; + scroller.appendChild(content); + const refs = { + content: { current: content }, + scroller: { current: scroller }, + rowTop: 1_000, + scrollOffsets: [], + targetJumps: [], + targetResult: null, + }; + const root = createRoot(document.createElement("div")); + + await act(async () => { + root.render( + React.createElement(VirtualScrollBehaviorHarness, { + messages: [{ id: "selected" }], + refs, + }), + ); + }); + await act(async () => { + root.render( + React.createElement(VirtualScrollBehaviorHarness, { + messages: [{ id: "selected" }, { id: "later" }], + refs, + }), + ); + }); + + assert.equal(refs.targetJumps.length, 2); + assert.deepEqual( + refs.targetJumps.map(({ messageId }) => messageId), + ["selected", "selected"], + ); + await act(async () => root.unmount()); +}); + +test("virtual centering corrects rendered geometry on the following frame", async () => { + const previousGetComputedStyle = globalThis.getComputedStyle; + globalThis.getComputedStyle = (element) => ({ + fontSize: "16px", + getPropertyValue: (name) => + element === document.documentElement + ? "0px" + : name === "--composer-overlay-height" + ? "20px" + : "0px", + }); + const content = document.createElement("div"); + const scroller = document.createElement("div"); + scroller.clientHeight = 400; + scroller.scrollHeight = 1_000; + scroller.scrollTop = 0; + scroller.getBoundingClientRect = () => ({ bottom: 400, top: 0 }); + const refs = { + content: { current: content }, + scroller: { current: scroller }, + rowTop: 180, + scrollOffsets: [], + targetJumps: [], + targetResult: null, + }; + const row = { + getBoundingClientRect: () => ({ + bottom: refs.rowTop + 40, + height: 40, + top: refs.rowTop, + }), + }; + scroller.querySelector = () => row; + scroller.querySelectorAll = () => []; + scroller.appendChild(content); + const root = createRoot(document.createElement("div")); + + await act(async () => { + root.render(React.createElement(VirtualScrollBehaviorHarness, { refs })); + }); + assert.equal(refs.targetResult, "pending"); + await act(async () => new Promise((resolve) => setTimeout(resolve, 0))); + assert.deepEqual(refs.scrollOffsets, [10]); + assert.equal(refs.rowTop, 170); + + await act(async () => root.unmount()); + globalThis.getComputedStyle = previousGetComputedStyle; +}); + +test("mounted virtual target retires bottom intent and delegates the jump to the virtualizer", async () => { const resizeObservers = []; globalThis.ResizeObserver = class { constructor(callback) { @@ -545,7 +757,7 @@ test("mounted virtual target retires bottom intent before direct centering", asy scroller.scrollHeight = 1_000; scroller.scrollTop = 0; scroller.getBoundingClientRect = () => ({ bottom: 400, top: 0 }); - const targetContentTop = 250; + const targetContentTop = 180; const row = { getBoundingClientRect: () => ({ bottom: targetContentTop - scroller.scrollTop + 40, @@ -555,7 +767,9 @@ test("mounted virtual target retires bottom intent before direct centering", asy }; scroller.querySelector = () => row; scroller.querySelectorAll = () => []; + const directScrollWrites = []; scroller.scrollTo = ({ top }) => { + directScrollWrites.push(top); scroller.scrollTop = top; }; @@ -571,6 +785,7 @@ test("mounted virtual target retires bottom intent before direct centering", asy }, }, scroller: { current: scroller }, + targetJumps: { current: [] }, targetResult: { current: null }, }; const root = createRoot(document.createElement("div")); @@ -579,8 +794,12 @@ test("mounted virtual target retires bottom intent before direct centering", asy }); assert.deepEqual(bottomWrites, [{ index: 4, options: { align: "end" } }]); - assert.equal(refs.targetResult.current, true); - assert.equal(row.getBoundingClientRect().top, 180); + // The virtualizer is the only scroll writer: the hook hands it the jump and + // never races it with a direct `scrollTo` that its in-flight correction + // would overwrite. The row is already settled in view, so it is handled. + assert.deepEqual(refs.targetJumps.current, ["selected"]); + assert.deepEqual(directScrollWrites, []); + assert.equal(refs.targetResult.current, "centered"); const bottomGeometryObserver = resizeObservers.find( (observer) => observer.targets.includes(content) && observer.targets.includes(scroller), @@ -589,11 +808,6 @@ test("mounted virtual target retires bottom intent before direct centering", asy bottomGeometryObserver.callback(); await act(async () => new Promise((resolve) => setTimeout(resolve, 0))); - assert.equal( - row.getBoundingClientRect().top, - 180, - "target remains centered after later virtual geometry activity", - ); assert.equal(bottomWrites.length, 1, "geometry cannot re-pin to bottom"); await act(async () => root.unmount()); }); diff --git a/desktop/src/features/messages/ui/useAnchoredScroll.ts b/desktop/src/features/messages/ui/useAnchoredScroll.ts index 0bfcb3b3e2f..f307ce6ae3a 100644 --- a/desktop/src/features/messages/ui/useAnchoredScroll.ts +++ b/desktop/src/features/messages/ui/useAnchoredScroll.ts @@ -8,6 +8,17 @@ import { shouldSettleForSplitPanel, shouldSettleVirtualizedBottom, } from "./anchoredScrollPolicy"; +import { + getTargetRowCenterOffset, + isTargetRowCentered, + targetRowNeedsCenterCorrection, +} from "./targetRowCentering"; +import type { + AnchorState, + ScrollToMessageResult, + UseAnchoredScrollOptions, + UseAnchoredScrollResult, +} from "./anchoredScrollTypes"; import { useVirtualizedViewportResize } from "./useVirtualizedViewportResize"; /** @@ -18,77 +29,6 @@ import { useVirtualizedViewportResize } from "./useVirtualizedViewportResize"; */ const AT_BOTTOM_THRESHOLD_PX = 32; -type AnchorState = - | { kind: "at-bottom" } - | { kind: "message"; messageId: string; topOffset: number } - | { kind: "pinned-center"; messageId: string; contentTop: 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; - /** 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: Array<{ id: string }>; - splitPanelOpen?: boolean; - - /** When set, scroll to this message on mount and on change. */ - targetMessageId?: string | null; - /** Whether a targeted message should pulse after scrolling to it. */ - highlightTargetMessage?: boolean; - /** 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, - options?: { behavior?: ScrollBehavior }, - ) => boolean; - /** Imperative virtualizer-owned bottom jump, used only when virtualizer mode is active. */ - virtualScrollToBottom?: (behavior?: ScrollBehavior) => void; - virtualSettleAtBottom?: () => void; - /** When active, the virtualizer owns prepend compensation and bottom-state synchronization. */ - virtualizerOwnsPrependAnchoring?: boolean; - /** Bumps when a virtualized range changes, so pending target/search retries can re-check newly mounted DOM. */ - virtualizerRenderVersion?: number; -}; - -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; - /** Re-pins after a layout owner changes trailing geometry. Returns true when - * the hook handled the settlement, including a preserved pinned target. */ - settleAtBottomAfterLayout: () => boolean; - /** 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; - /** Syncs the hook's bottom affordances from a virtualizer-owned scroller. */ - onVirtualizerAtBottomStateChange: (atBottom: boolean) => void; -}; - function isAtBottomNow( container: Pick< HTMLDivElement, @@ -158,9 +98,11 @@ export function useAnchoredScroll({ targetMessageId = null, highlightTargetMessage = true, pinTargetCentered = false, + topBoundaryReached = false, onTargetReached, onTargetSettled, virtualCancelBottomIntent, + virtualScrollBy, virtualScrollToMessage, virtualScrollToBottom, virtualSettleAtBottom, @@ -209,6 +151,14 @@ export function useAnchoredScroll({ const isWritingScrollRef = React.useRef(false); const programmaticScrollRafRef = React.useRef(null); const targetSettleRafRef = React.useRef(null); + const targetRetryRafRef = React.useRef(null); + const virtualTargetJumpRef = React.useRef<{ + messageId: string; + messageCount: number; + } | null>(null); + const virtualTargetCorrectionAppliedRef = React.useRef(false); + const targetCorrectionRafRef = React.useRef(null); + const [targetRetryVersion, setTargetRetryVersion] = React.useState(0); // Reset everything when the channel changes — the layout effect that runs // immediately after this reset is responsible for either jumping to bottom @@ -238,6 +188,16 @@ export function useAnchoredScroll({ cancelAnimationFrame(targetSettleRafRef.current); targetSettleRafRef.current = null; } + if (targetRetryRafRef.current !== null) { + cancelAnimationFrame(targetRetryRafRef.current); + targetRetryRafRef.current = null; + } + if (targetCorrectionRafRef.current !== null) { + cancelAnimationFrame(targetCorrectionRafRef.current); + targetCorrectionRafRef.current = null; + } + virtualTargetJumpRef.current = null; + virtualTargetCorrectionAppliedRef.current = false; if (highlightTimeoutRef.current !== null) { window.clearTimeout(highlightTimeoutRef.current); highlightTimeoutRef.current = null; @@ -432,55 +392,115 @@ export function useAnchoredScroll({ ( messageId: string, options: { highlight?: boolean; behavior?: ScrollBehavior } = {}, - ): boolean => { + ): ScrollToMessageResult => { const container = scrollContainerRef.current; - if (!container) return false; + if (!container) return "missing"; const el = container.querySelector( `[data-message-id="${messageId}"]`, ); + if (virtualizerOwnsPrependAnchoring && !virtualScrollToMessage) + return "pending"; // Wait for Virtua's imperative API. if (virtualizerOwnsPrependAnchoring && virtualScrollToMessage) { - // Target navigation owns the viewport before any movement strategy is - // chosen. The already-mounted fast path centers the DOM node directly - // and would otherwise leave durable bottom intent armed. + // Virtua is the sole scroll writer; a new jump cancels its initial + // bottom settle and the cancel call prevents later re-pinning. virtualCancelBottomIntent?.(); - if (el) { - const rect = el.getBoundingClientRect(); - const containerRect = container.getBoundingClientRect(); - const isInViewport = - rect.top >= containerRect.top && - rect.bottom <= containerRect.bottom; - if (!isInViewport) { - if (!virtualScrollToMessage(messageId, { behavior: "auto" })) { - return false; - } - anchorRef.current = { kind: "message", messageId, topOffset: 0 }; - setIsAtBottom(false); - return false; + const virtualScrollBehavior = el + ? (options.behavior ?? "auto") + : "auto"; + const rowIsVisible = el + ? (() => { + const rowRect = el.getBoundingClientRect(); + const containerRect = container.getBoundingClientRect(); + return ( + rowRect.bottom > containerRect.top && + rowRect.top < containerRect.bottom + ); + })() + : false; + const jumpMatchesCurrentModel = + virtualTargetJumpRef.current?.messageId === messageId && + virtualTargetJumpRef.current.messageCount === messages.length; + if (!jumpMatchesCurrentModel) { + if ( + !virtualScrollToMessage(messageId, { + behavior: virtualScrollBehavior, + }) + ) { + return "missing"; } - const centeredTop = (container.clientHeight - rect.height) / 2; - container.scrollTo({ - top: Math.max( - 0, - container.scrollTop + - (rect.top - containerRect.top) - - centeredTop, - ), - behavior: options.behavior ?? "auto", - }); - } else if ( - !virtualScrollToMessage(messageId, { - behavior: options.behavior ?? "auto", - }) + virtualTargetJumpRef.current = { + messageId, + messageCount: messages.length, + }; + virtualTargetCorrectionAppliedRef.current = false; + } + if ( + rowIsVisible && + !virtualTargetCorrectionAppliedRef.current && + targetCorrectionRafRef.current === null ) { - return false; + // Virtua first realizes and centers by index. Once the target row is + // rendered, wait one more frame for its measured geometry to land, + // then compensate for chrome that overlays the usable viewport. + targetCorrectionRafRef.current = requestAnimationFrame(() => { + targetCorrectionRafRef.current = null; + const settledContainer = scrollContainerRef.current; + const settledRow = settledContainer?.querySelector( + `[data-message-id="${CSS.escape(messageId)}"]`, + ); + if (!settledContainer || !settledRow) return; + const rowRect = settledRow.getBoundingClientRect(); + const containerRect = settledContainer.getBoundingClientRect(); + // Virtua may mount the requested row before its indexed jump has + // placed that row in the viewport. Do not turn that transient, + // offscreen geometry into a multi-thousand-pixel correction. + if ( + rowRect.bottom <= containerRect.top || + rowRect.top >= containerRect.bottom + ) { + setTargetRetryVersion((version) => version + 1); + return; + } + const correction = getTargetRowCenterOffset( + settledRow, + settledContainer, + ); + if (targetRowNeedsCenterCorrection(correction)) { + virtualScrollBy?.(correction); + } + virtualTargetCorrectionAppliedRef.current = true; + setTargetRetryVersion((version) => version + 1); + }); } anchorRef.current = { kind: "message", messageId, topOffset: 0 }; - setIsAtBottom(false); - if (el && options.highlight) highlightMessage(messageId); - return el !== null; + // Completion requires midpoint alignment or a confirmed boundary. + const targetIndex = messages.findIndex( + (message) => message.id === messageId, + ); + const targetBoundary = + targetIndex === 0 && topBoundaryReached + ? "top" + : targetIndex === messages.length - 1 + ? "bottom" + : "none"; + if ( + !el || + !isTargetRowCentered(el, container, targetBoundary, isAtBottomNow) + ) { + virtualizerAtBottomRef.current = false; + setIsAtBottom(false); + return "pending"; + } + const atBottom = isAtBottomNow(container); + virtualizerAtBottomRef.current = atBottom; + setIsAtBottom(atBottom); + if (options.highlight) highlightMessage(messageId); + virtualTargetJumpRef.current = null; + virtualTargetCorrectionAppliedRef.current = false; + return "centered"; } - if (!el) return false; + if (!el) return "missing"; const rect = el.getBoundingClientRect(); const containerRect = container.getBoundingClientRect(); @@ -532,13 +552,16 @@ export function useAnchoredScroll({ } if (options.highlight) highlightMessage(messageId); - return true; + return "centered"; }, [ highlightMessage, + messages, pinTargetCentered, + topBoundaryReached, scrollContainerRef, virtualCancelBottomIntent, + virtualScrollBy, virtualizerOwnsPrependAnchoring, writePinnedCenterScroll, virtualScrollToMessage, @@ -630,19 +653,21 @@ export function useAnchoredScroll({ }); }; 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: highlightTargetMessage, - }) - ) { + // A cold deep-link target is rarely in the DOM on this first commit: + // a virtualized list renders only a window, and a target outside the + // loaded history is fetched by id and spliced in a render or two + // later. Either way the post-mount target effect (keyed on `messages` + // and the rendered range) finishes the job — it is not handled yet. + // Only fall back to the bottom pin when the row is genuinely absent; + // pinning while the virtualizer is mid-jump re-arms durable bottom + // intent and strands the view at the floor with no highlight. + const result = scrollToMessageImperative(targetMessageId, { + highlight: highlightTargetMessage, + }); + if (result === "centered") { handledTargetIdRef.current = targetMessageId; onTargetReached?.(targetMessageId); - } else { + } else if (result === "missing") { pinToBottomOnMount(); } } else { @@ -876,7 +901,7 @@ export function useAnchoredScroll({ // *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` and `virtualizerRenderVersion` are intentional retry triggers, not values read by the effect body — the effect reads the DOM (querySelector), and we need it to re-run each time the message list or virtualized rendered range changes so a target spliced into older history gets centered once its row commits. + // biome-ignore lint/correctness/useExhaustiveDependencies: `messages` and `virtualizerRenderVersion` are intentional retry triggers, not values read by the effect body — `scrollToMessageImperative` reads the DOM, and we need the effect to re-run each time the message list or virtualized rendered range changes so a target spliced into older history (or windowed out of the DOM) gets centered once its row commits. React.useEffect(() => { if (!targetMessageId) { handledTargetIdRef.current = null; @@ -893,43 +918,37 @@ export function useAnchoredScroll({ if (!hasInitializedRef.current) return; // initial-mount path will handle. void virtualizerRenderVersion; - const container = scrollContainerRef.current; - if (!container) return; - const el = container.querySelector( - `[data-message-id="${targetMessageId}"]`, - ); - if (!el && virtualizerOwnsPrependAnchoring) { - if ( - scrollToMessageImperative(targetMessageId, { - highlight: highlightTargetMessage, - }) - ) { - handledTargetIdRef.current = targetMessageId; - onTargetReached?.(targetMessageId); - } - return; - } - if (!el) { - // Row not in the DOM yet. A cold deep-link target is fetched by id and - // spliced into `messages` a render or two later; this effect re-runs on - // each `messages` commit and retries until the row exists. - return; - } - handledTargetIdRef.current = targetMessageId; - scrollToMessageImperative(targetMessageId, { + // `pending` (virtualizer mid-jump) and `missing` (row not spliced in yet) + // both leave the target unhandled; the next `messages` or rendered-range + // commit re-runs this effect and retries until the row is centered. + const result = scrollToMessageImperative(targetMessageId, { highlight: highlightTargetMessage, }); - onTargetReached?.(targetMessageId); + if (result === "centered") { + if (targetRetryRafRef.current !== null) { + cancelAnimationFrame(targetRetryRafRef.current); + targetRetryRafRef.current = null; + } + handledTargetIdRef.current = targetMessageId; + onTargetReached?.(targetMessageId); + } else if (result === "pending" && targetRetryRafRef.current === null) { + // Virtua can finish correcting measured row offsets without changing its + // rendered range. Retry on the next frame so completion observes the + // final geometry rather than depending on an unrelated React render. + targetRetryRafRef.current = requestAnimationFrame(() => { + targetRetryRafRef.current = null; + setTargetRetryVersion((version) => version + 1); + }); + } }, [ highlightTargetMessage, isLoading, messages, onTargetReached, releasePinnedCenter, - scrollContainerRef, scrollToMessageImperative, targetMessageId, - virtualizerOwnsPrependAnchoring, + targetRetryVersion, virtualizerRenderVersion, ]); @@ -944,6 +963,12 @@ export function useAnchoredScroll({ if (targetSettleRafRef.current !== null) { cancelAnimationFrame(targetSettleRafRef.current); } + if (targetRetryRafRef.current !== null) { + cancelAnimationFrame(targetRetryRafRef.current); + } + if (targetCorrectionRafRef.current !== null) { + cancelAnimationFrame(targetCorrectionRafRef.current); + } }; }, []); diff --git a/desktop/src/features/messages/ui/virtualMessageScroll.test.mjs b/desktop/src/features/messages/ui/virtualMessageScroll.test.mjs new file mode 100644 index 00000000000..d5bc989d2dd --- /dev/null +++ b/desktop/src/features/messages/ui/virtualMessageScroll.test.mjs @@ -0,0 +1,15 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { getVirtualMessageScrollOptions } from "./virtualMessageScroll.ts"; + +test("maps smooth message navigation to Virtua's smooth option", () => { + assert.deepEqual(getVirtualMessageScrollOptions("smooth"), { + align: "center", + smooth: true, + }); + assert.deepEqual(getVirtualMessageScrollOptions("auto"), { + align: "center", + smooth: false, + }); +}); diff --git a/desktop/src/features/messages/ui/virtualMessageScroll.ts b/desktop/src/features/messages/ui/virtualMessageScroll.ts new file mode 100644 index 00000000000..1950605d621 --- /dev/null +++ b/desktop/src/features/messages/ui/virtualMessageScroll.ts @@ -0,0 +1,8 @@ +export function getVirtualMessageScrollOptions( + behavior: ScrollBehavior | undefined, +) { + return { + align: "center" as const, + smooth: behavior === "smooth", + }; +} diff --git a/desktop/src/testing/e2eBridge.ts b/desktop/src/testing/e2eBridge.ts index 9002d751c8c..902766852f3 100644 --- a/desktop/src/testing/e2eBridge.ts +++ b/desktop/src/testing/e2eBridge.ts @@ -345,6 +345,8 @@ type E2eConfig = { honorChannelsKnownHash?: boolean; /** Number of seeded rows in the deep-history fixture. Defaults to 600. */ deepHistoryMessageCount?: number; + /** Opt-in numbered history for the alice-tyler deep-link scenario. */ + aliceTylerHistoryMessageCount?: number; feedReadError?: string; canvasReadError?: string; /** Delay (ms) for `apply_workspace` so e2e tests can observe the @@ -4506,24 +4508,42 @@ function getMockMessageStore(channelId: string): RelayEvent[] { sig: "mocksig".repeat(20).slice(0, 128), })), ] - : channelId === "feedf00d-0000-4000-8000-000000000007" + : channelId === "f48efb06-0c93-5025-aac9-2e646bb6bfa8" && + (getConfig()?.mock?.aliceTylerHistoryMessageCount ?? 0) > 0 ? (() => { - const count = getConfig()?.mock?.deepHistoryMessageCount ?? 600; + const count = + getConfig()?.mock?.aliceTylerHistoryMessageCount ?? 0; return Array.from({ length: count }, (_, index) => ({ - id: `mock-deep-history-${index}`, + id: `mock-alice-tyler-${index}`, pubkey: index % 2 === 0 ? ALICE_PUBKEY : MOCK_IDENTITY_PUBKEY, created_at: Math.floor(Date.now() / 1000) - (count - index) * 60, kind: 9, tags: [["h", channelId]], - content: - count > 600 - ? `Deep history message #${index}\n${"variable wrapped history ".repeat((index % 12) + 1)}` - : `Deep history message #${index}`, + content: `Alice and Tyler message #${index}`, sig: "mocksig".repeat(20).slice(0, 128), })); })() - : []; + : channelId === "feedf00d-0000-4000-8000-000000000007" + ? (() => { + const count = + getConfig()?.mock?.deepHistoryMessageCount ?? 600; + return Array.from({ length: count }, (_, index) => ({ + id: `mock-deep-history-${index}`, + pubkey: + index % 2 === 0 ? ALICE_PUBKEY : MOCK_IDENTITY_PUBKEY, + created_at: + Math.floor(Date.now() / 1000) - (count - index) * 60, + kind: 9, + tags: [["h", channelId]], + content: + count > 600 + ? `Deep history message #${index}\n${"variable wrapped history ".repeat((index % 12) + 1)}` + : `Deep history message #${index}`, + sig: "mocksig".repeat(20).slice(0, 128), + })); + })() + : []; mockMessages.set(channelId, seeded); return seeded; diff --git a/desktop/tests/e2e/messaging.spec.ts b/desktop/tests/e2e/messaging.spec.ts index d24c2744133..683d979a7fe 100644 --- a/desktop/tests/e2e/messaging.spec.ts +++ b/desktop/tests/e2e/messaging.spec.ts @@ -3668,12 +3668,8 @@ test("a refused message deep link retries after the thread edit is canceled", as const routedDestination = page .getByTestId("message-timeline") .locator(`[data-message-id="${destinationId}"]`); - await expect(threadPanel).toBeVisible(); - await expect(threadPanel.getByTestId("message-thread-head")).toContainText( - destinationRoot, - ); + await expect(threadPanel).not.toBeVisible(); await expect(routedDestination).toBeVisible(); - await expect(routedDestination).toHaveClass(/route-target-highlight-fade/); }); test("a refused sent-from-thread link preserves the edit and retries after cancel", async ({ @@ -3828,10 +3824,12 @@ test("a refused search result preserves the edit and retries after cancel", asyn await page.getByTestId("search-dialog-input").fill(destinationRoot); await destinationResult.click(); await expect(page).not.toHaveURL(threadUrl); - await expect(threadPanel.getByTestId("message-thread-head")).toContainText( - destinationRoot, + const routedDestination = timeline.locator( + `[data-message-id="${destinationRootId}"]`, ); - await expect(page).toHaveURL(new RegExp(`thread=${destinationRootId}`)); + await expect(threadPanel).not.toBeVisible(); + await expect(routedDestination).toBeVisible(); + await expect(routedDestination).toHaveClass(/route-target-highlight-fade/); }); test("a refused forum search result preserves the edit and retries after cancel", async ({ diff --git a/desktop/tests/e2e/navigation.spec.ts b/desktop/tests/e2e/navigation.spec.ts index d19849ea17c..a087afdfffa 100644 --- a/desktop/tests/e2e/navigation.spec.ts +++ b/desktop/tests/e2e/navigation.spec.ts @@ -8,8 +8,16 @@ const WATERCOLOR_CHANNEL_ID = "a27e1ee9-76a6-5bdf-a5d5-1d85610dad11"; const FORUM_POST_ID = "mock-forum-release-thread"; const FORUM_REPLY_ID = "mock-forum-release-reply"; -test.beforeEach(async ({ page }) => { - await installMockBridge(page); +const DM_DEEP_LINK_HISTORY_TEST = + "cold deep link to a top-level DM message stays in the timeline"; + +test.beforeEach(async ({ page }, testInfo) => { + await installMockBridge( + page, + testInfo.title === DM_DEEP_LINK_HISTORY_TEST + ? { aliceTylerHistoryMessageCount: 80 } + : undefined, + ); }); /** @@ -643,7 +651,7 @@ test("composer Buzz chip labels wrap without orphaning their icons", async ({ ).toBeGreaterThan(1); }); -test("message links to visible root messages open the thread panel", async ({ +test("message links to visible root messages highlight them in the main timeline", async ({ page, }) => { await page.goto("/"); @@ -833,16 +841,139 @@ test("message links to visible root messages open the thread panel", async ({ }), ) .toBe(link); +}); - await rootThreadLink.click({ button: "right" }); - await linkMenu.getByRole("button", { name: "Open link" }).click(); +// Cold deep links arrive from outside the channel (Home inbox "Open in +// channel", search, notifications): the timeline mounts *with* the target +// already in the route. `#deep-history` is long enough that the virtualizer +// only renders the newest rows on first commit, so the target row is in the +// loaded window but not yet in the DOM — the state the in-channel link tests +// above never reach. +const DEEP_HISTORY_CHANNEL_ID = "feedf00d-0000-4000-8000-000000000007"; - const threadPanel = page.getByTestId("message-thread-panel"); - await expect(threadPanel).toBeVisible(); - await expect(page).toHaveURL(/thread=mock-general-welcome/); - await expect(threadPanel.getByTestId("message-thread-head")).toContainText( - "Welcome to general", +async function expectColdDeepLinkLandsOnTarget( + page: import("@playwright/test").Page, + messageId: string, + { expectCentered = true }: { expectCentered?: boolean } = {}, +) { + await page.goto("/"); + await expect(page.getByTestId("home-inbox-list")).toBeVisible(); + + // Same-document hash navigation — the route the Home inbox / search hand + // off to, without reloading the app shell. + await page.goto( + `/#/channels/${DEEP_HISTORY_CHANNEL_ID}?messageId=${messageId}`, ); + await expect(page.getByTestId("chat-title")).toHaveText("deep-history"); + + const timeline = page.getByTestId("message-timeline"); + const targetRow = timeline.locator(`[data-message-id="${messageId}"]`); + // The highlight is applied only once the hook has seen the row settled in + // the viewport, so it is the strongest signal that the jump completed. It + // fades after 2s — assert it before anything that can wait on layout. + if (expectCentered) { + await expect + .poll(() => + targetRow.evaluate((row) => { + const timeline = row.closest('[data-testid="message-timeline"]'); + if (!timeline) return Number.POSITIVE_INFINITY; + const rowRect = row.getBoundingClientRect(); + const timelineRect = timeline.getBoundingClientRect(); + const styles = getComputedStyle(timeline); + const rootFontSize = Number.parseFloat( + getComputedStyle(document.documentElement).fontSize, + ); + const resolveLength = (value: string) => { + const parsed = Number.parseFloat(value); + if (!Number.isFinite(parsed)) return 0; + return value.trim().endsWith("rem") + ? parsed * rootFontSize + : parsed; + }; + const topInset = resolveLength( + styles.getPropertyValue("--channel-top-chrome-height"), + ); + const bottomInset = resolveLength( + styles.getPropertyValue("--composer-overlay-height"), + ); + return Math.abs( + (rowRect.top + rowRect.bottom) / 2 - + (timelineRect.top + + topInset + + timelineRect.bottom - + bottomInset) / + 2, + ); + }), + ) + .toBeLessThanOrEqual(2); + } + await expect(targetRow).toHaveClass(/route-target-highlight-fade/); + await expect(targetRow).toBeInViewport(); + // Top-level targets resolve in the main timeline only. + await expect(page.getByTestId("message-thread-panel")).not.toBeVisible(); + // Reaching the target consumes the route param so a later channel visit + // doesn't re-scroll to stale history. + await expect(page).toHaveURL( + new RegExp(`#/channels/${DEEP_HISTORY_CHANNEL_ID}$`), + ); + // The target must still be in view once the route param is cleared — the + // list's first-commit bottom settle and the mount-time resize pass must not + // win over the target jump. + await page.waitForTimeout(500); + await expect(targetRow).toBeInViewport(); + return targetRow; +} + +test("cold deep link to a message in virtualized history scrolls to and highlights it", async ({ + page, +}) => { + // Index 500 is inside the cold-load window and ~100 rows above the bottom, + // so it is neither initially rendered nor boundary-clamped. The jump must + // win over the list's own first-commit bottom settle and land at midpoint. + await expectColdDeepLinkLandsOnTarget(page, "mock-deep-history-500"); +}); + +test("cold deep link to a top-level DM message stays in the timeline", async ({ + page, +}) => { + const dmChannelId = "f48efb06-0c93-5025-aac9-2e646bb6bfa8"; + const messageId = "mock-alice-tyler-40"; + await page.goto(`/#/channels/${dmChannelId}`); + await expect(page.getByTestId("chat-title")).toHaveText("alice-tyler"); + const seededTarget = await page.evaluate(async (eventId) => { + const invoke = window.__BUZZ_E2E_INVOKE_MOCK_COMMAND__; + if (!invoke) return null; + const eventJson = await invoke("get_event", { eventId }); + return typeof eventJson === "string" ? JSON.parse(eventJson) : eventJson; + }, messageId); + expect(seededTarget).toMatchObject({ + id: messageId, + content: "Alice and Tyler message #40", + }); + + await page.goto(`/#/channels/${dmChannelId}?messageId=${messageId}`); + await expect(page.getByTestId("chat-title")).toHaveText("alice-tyler"); + + const targetRow = page + .getByTestId("message-timeline") + .locator(`[data-message-id="${messageId}"]`); + // Positive control: prove the opt-in fixture supplied the intended target + // before asserting the route-specific highlight and panel behavior. + await expect(targetRow).toContainText("Alice and Tyler message #40"); + await expect(targetRow).toHaveClass(/route-target-highlight-fade/); + await expect(page.getByTestId("message-thread-panel")).not.toBeVisible(); + await expect(page).toHaveURL(new RegExp(`#/channels/${dmChannelId}$`)); +}); + +test("cold deep link to the newest message highlights it at the bottom", async ({ + page, +}) => { + // The boundary-clamped case: the target is the last message. It must still + // highlight, and the view stays at the floor instead of centering. + await expectColdDeepLinkLandsOnTarget(page, "mock-deep-history-599", { + expectCentered: false, + }); }); test("direct-message tooltip metadata stays on one physical line", async ({ @@ -940,7 +1071,7 @@ test("message links explain when preview metadata is unavailable", async ({ await expect(unavailableTooltip).toHaveCount(0); }); -test("message links reopen a closed thread when the same messageId is already in the URL", async ({ +test("root message links re-target the main timeline when the same messageId was already in the URL", async ({ page, }) => { await page.goto( @@ -948,14 +1079,20 @@ test("message links reopen a closed thread when the same messageId is already in ); await expect(page.getByTestId("chat-title")).toHaveText("general"); + // A top-level deep link never opens the reply panel — the message is shown + // highlighted in its own context instead. const threadPanel = page.getByTestId("message-thread-panel"); - await expect(threadPanel).toBeVisible(); - await expect(threadPanel.getByTestId("message-thread-head")).toContainText( - "Welcome to general", - ); - - await threadPanel.getByRole("button", { name: "Close panel" }).click(); await expect(threadPanel).not.toBeVisible(); + const welcomeRow = page + .getByTestId("message-timeline") + .locator('[data-message-id="mock-general-welcome"]'); + await expect(welcomeRow).toBeVisible(); + await expect(welcomeRow).toHaveClass(/route-target-highlight-fade/); + + // Once the target is reached the messageId param clears; re-activating a + // link to the same root must route again instead of being swallowed + // (regression guard from the original reopen-a-closed-thread test). + await expect(page).not.toHaveURL(/messageId=/); const link = "buzz://message?channel=9a1657ac-f7aa-5db0-b632-d8bbeb6dfb50&id=mock-general-welcome"; @@ -975,24 +1112,30 @@ test("message links reopen a closed thread when the same messageId is already in await expect(rootThreadLink).toHaveText("general"); await rootThreadLink.click(); - await expect(threadPanel).toBeVisible(); - await expect(threadPanel.getByTestId("message-thread-head")).toContainText( - "Welcome to general", - ); + await expect(threadPanel).not.toBeVisible(); + await expect(welcomeRow).toHaveClass(/route-target-highlight-fade/); }); test("message deep links survive reload", async ({ page }) => { - await page.goto( - `/#/channels/${ENGINEERING_CHANNEL_ID}?messageId=mock-engineering-shipped`, - ); + const deepLinkUrl = `/#/channels/${ENGINEERING_CHANNEL_ID}?messageId=mock-engineering-shipped`; + await page.goto(deepLinkUrl); await expect(page.getByTestId("chat-title")).toHaveText("engineering"); await expect(page.getByTestId("message-timeline")).toContainText( "Engineering shipped the desktop build.", ); + // Once the target is centered the messageId param is consumed (cleared via + // onTargetReached so re-activating the same link is never swallowed), and a + // top-level target no longer pins a `thread` param either — so a bare + // reload lands on the plain channel. + await expect(page).not.toHaveURL(/messageId=/); await page.reload(); + await expect(page.getByTestId("chat-title")).toHaveText("engineering"); + // The deep-link URL itself stays valid: loading it again cold re-resolves + // and re-splices the target message. + await page.goto(deepLinkUrl); await expect(page.getByTestId("chat-title")).toHaveText("engineering"); await expect(page.getByTestId("message-timeline")).toContainText( "Engineering shipped the desktop build.", diff --git a/desktop/tests/e2e/scroll-history.spec.ts b/desktop/tests/e2e/scroll-history.spec.ts index 0e1d4cfed6f..dd0185fa857 100644 --- a/desktop/tests/e2e/scroll-history.spec.ts +++ b/desktop/tests/e2e/scroll-history.spec.ts @@ -933,15 +933,17 @@ test("unified channel search opens rows regardless of history position", async ( // 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". + // because the find-bar -> active-match -> virtualizer path is async + // (state update, then an indexed jump that may need to render the row). + // 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). + // Tolerance: 1px on each edge for sub-pixel rounding. The 5s budget leaves + // room for the virtualizer to realize and measure the distant ALPHA row + // (~180 rows above the initial bottom position). Distant unrendered search + // targets intentionally jump instantly; only already-rendered targets keep + // smooth scrolling for spatial context. const waitForRowInViewport = async (needle: string) => timeline.evaluate((timelineEl, n) => { return new Promise<{ diff --git a/desktop/tests/helpers/bridge.ts b/desktop/tests/helpers/bridge.ts index 2637b94a808..f074598b935 100644 --- a/desktop/tests/helpers/bridge.ts +++ b/desktop/tests/helpers/bridge.ts @@ -277,6 +277,8 @@ type MockBridgeOptions = { honorChannelsKnownHash?: boolean; /** Number of seeded rows in the deep-history fixture. Defaults to 600. */ deepHistoryMessageCount?: number; + /** Opt-in numbered history for the alice-tyler deep-link scenario. */ + aliceTylerHistoryMessageCount?: number; feedReadError?: string; canvasReadError?: string; /** Delay (ms) for `apply_workspace`; see e2eBridge mock config. */