From 096a31f8d8088aba04d6405267dd77c89376ff80 Mon Sep 17 00:00:00 2001 From: ss-dev-00 Date: Fri, 21 Aug 2026 18:16:17 -0700 Subject: [PATCH 1/2] feat(desktop): make the agent work viewer a peer of the thread surface MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The channel agent activity pane was a read-only side pane with no share of the thread surface's presentation: it could not take the focus-mode drawer, had no layout control, and kept its own back arrow as the only way out. That made the viewer feel like a subordinate inspector rather than a place you read an agent's work. Route the viewer through the same focus drawer the thread uses. One resolver (`resolveFocusDrawerSurface`) decides the drawer's occupant using the same precedence as ChannelPane's auxiliary render chain, so the drawer and the split pane cannot disagree about who is on screen. `useFocusDrawerSurfaces` owns the state that follows from that resolution — drawer presence, which surface receives dismissal, and a layout toggle per surface — keeping ChannelPane under the file-size ratchet. User-visible: the viewer gets the layout toggle and honours the shared `threadViewMode` preference (choosing a layout here chooses it for threads too), gets a centered reading column in focus mode, and is dismissed by the drawer's scrim sliver or Escape instead of a competing header arrow. The read-only boundaries are unchanged: no composer, scope label stays "Activity · #". Reading position is deliberately not preserved across a layout switch — the viewer snaps to latest. Transcript rows use `content-visibility: auto` with an over-reserved `contain-intrinsic-size`, so the scroll height shrinks as rows paint after the reflow and a restored offset slides out from under the reader (instrumented: a correct pin at scrollTop 1122 was then chased to 0 by late resize corrections). Snapping is the behaviour we can actually keep until transcript rows reserve their real heights. Signed-off-by: ss-dev-00 Co-authored-by: Bradley Axen Signed-off-by: Bradley Axen --- desktop/playwright.config.ts | 1 + .../channels/lib/focusDrawerSurface.test.mjs | 94 +++++++ .../channels/lib/focusDrawerSurface.ts | 62 +++++ .../channels/ui/AgentSessionThreadPanel.tsx | 58 +++- .../src/features/channels/ui/ChannelPane.tsx | 82 ++++-- .../channels/ui/FocusThreadDrawer.tsx | 12 +- .../channels/ui/useFocusDrawerSurfaces.ts | 107 ++++++++ .../channels/ui/useThreadViewModeSwitch.ts | 50 +++- .../tests/e2e/agent-viewer-focus-mode.spec.ts | 248 ++++++++++++++++++ 9 files changed, 677 insertions(+), 37 deletions(-) create mode 100644 desktop/src/features/channels/lib/focusDrawerSurface.test.mjs create mode 100644 desktop/src/features/channels/lib/focusDrawerSurface.ts create mode 100644 desktop/src/features/channels/ui/useFocusDrawerSurfaces.ts create mode 100644 desktop/tests/e2e/agent-viewer-focus-mode.spec.ts diff --git a/desktop/playwright.config.ts b/desktop/playwright.config.ts index ff8a0e7703b..16c609f7879 100644 --- a/desktop/playwright.config.ts +++ b/desktop/playwright.config.ts @@ -92,6 +92,7 @@ export default defineConfig({ "**/thread-reply-anchor-roleplay.spec.ts", "**/threadpane-ultrawide.spec.ts", "**/thread-focus-mode.spec.ts", + "**/agent-viewer-focus-mode.spec.ts", "**/animated-avatar.spec.ts", "**/reminders.spec.ts", "**/reminder-click-repro.spec.ts", diff --git a/desktop/src/features/channels/lib/focusDrawerSurface.test.mjs b/desktop/src/features/channels/lib/focusDrawerSurface.test.mjs new file mode 100644 index 00000000000..d61cd58db20 --- /dev/null +++ b/desktop/src/features/channels/lib/focusDrawerSurface.test.mjs @@ -0,0 +1,94 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { resolveFocusDrawerSurface } from "./focusDrawerSurface.ts"; + +const SPLIT_WITH_AGENT = { + channelManagementOpen: false, + hasAgentSession: true, + hasThread: false, + isFocusPreferred: true, + useSplitAuxiliaryPane: true, +}; + +test("the selected agent surface is focus-eligible", () => { + assert.equal(resolveFocusDrawerSurface(SPLIT_WITH_AGENT), "agent-session"); +}); + +test("split remains the default presentation for both surfaces", () => { + assert.equal( + resolveFocusDrawerSurface({ + ...SPLIT_WITH_AGENT, + isFocusPreferred: false, + }), + null, + ); + assert.equal( + resolveFocusDrawerSurface({ + ...SPLIT_WITH_AGENT, + hasAgentSession: false, + hasThread: true, + isFocusPreferred: false, + }), + null, + ); +}); + +test("narrow and overlay presentations are unchanged by focus mode", () => { + assert.equal( + resolveFocusDrawerSurface({ + ...SPLIT_WITH_AGENT, + useSplitAuxiliaryPane: false, + }), + null, + ); + assert.equal( + resolveFocusDrawerSurface({ + ...SPLIT_WITH_AGENT, + hasAgentSession: false, + hasThread: true, + useSplitAuxiliaryPane: false, + }), + null, + ); +}); + +test("no drawer without a surface to put in it", () => { + assert.equal( + resolveFocusDrawerSurface({ + ...SPLIT_WITH_AGENT, + hasAgentSession: false, + }), + null, + ); +}); + +test("precedence matches the render chain so the drawer cannot disagree with it", () => { + // ChannelPane renders management → thread → agent session. A thread and an + // agent session can both be resolvable at once (the panel keeps its selected + // agent while a thread opens), and only the thread renders. + assert.equal( + resolveFocusDrawerSurface({ + ...SPLIT_WITH_AGENT, + hasThread: true, + }), + "thread", + ); + // Channel management keeps its split pane in every view mode, so it outranks + // both drawers rather than being routed into one. + assert.equal( + resolveFocusDrawerSurface({ + ...SPLIT_WITH_AGENT, + channelManagementOpen: true, + hasThread: true, + }), + null, + ); + assert.equal( + resolveFocusDrawerSurface({ + ...SPLIT_WITH_AGENT, + channelManagementOpen: true, + }), + null, + ); +}); diff --git a/desktop/src/features/channels/lib/focusDrawerSurface.ts b/desktop/src/features/channels/lib/focusDrawerSurface.ts new file mode 100644 index 00000000000..f10cb27a937 --- /dev/null +++ b/desktop/src/features/channels/lib/focusDrawerSurface.ts @@ -0,0 +1,62 @@ +/** + * Which auxiliary surface owns the focus-mode drawer. + * + * Focus mode is a *presentation* of the right-hand auxiliary region, not a + * thread feature: the channel agent work viewer honours the same + * `threadViewMode` preference and reuses the same drawer. Only one surface can + * hold the drawer at a time, so the winner has to be resolved from the same + * precedence the render chain uses — otherwise the drawer and the split pane + * disagree about who is on screen. + */ + +/** `AnimatePresence` key shared by both agent-session layouts. + * + * The split pane and the focus drawer are two containers for one viewer, so + * presence belongs to the viewer rather than either container — keying them + * apart would make every layout switch read as a close followed by an open. + * Mirrors `THREAD_SURFACE_KEY`. + */ +export const AGENT_SESSION_SURFACE_KEY = "agent-session-surface"; + +export type FocusDrawerSurface = "agent-session" | "thread"; + +type FocusDrawerSurfaceOptions = { + /** + * Channel management is open *and* has a channel to manage — pass the same + * conjunction the render chain branches on, since it outranks both drawers. + */ + channelManagementOpen: boolean; + /** An agent work viewer has a resolved agent and channel to render. */ + hasAgentSession: boolean; + /** A thread panel or its skeleton is on screen. */ + hasThread: boolean; + /** The persisted `threadViewMode` preference selects the drawer. */ + isFocusPreferred: boolean; + /** False in the narrow single-column and overlay presentations. */ + useSplitAuxiliaryPane: boolean; +}; + +/** + * Resolves the focus drawer's occupant, or `null` when nothing should overlay. + * + * Precedence deliberately mirrors `ChannelPane`'s auxiliary render chain + * (management → thread → agent session). Keeping one ordering means a surface + * cannot be routed into the drawer while a higher-precedence surface is what + * actually renders. + */ +export function resolveFocusDrawerSurface({ + channelManagementOpen, + hasAgentSession, + hasThread, + isFocusPreferred, + useSplitAuxiliaryPane, +}: FocusDrawerSurfaceOptions): FocusDrawerSurface | null { + // The narrow and overlay presentations are unchanged by focus mode: they + // already fill the column, so there is no channel left to dim behind a scrim. + if (!isFocusPreferred || !useSplitAuxiliaryPane) return null; + // Channel management keeps its split pane in every view mode. + if (channelManagementOpen) return null; + if (hasThread) return "thread"; + if (hasAgentSession) return "agent-session"; + return null; +} diff --git a/desktop/src/features/channels/ui/AgentSessionThreadPanel.tsx b/desktop/src/features/channels/ui/AgentSessionThreadPanel.tsx index c1933f14bb7..22971153de9 100644 --- a/desktop/src/features/channels/ui/AgentSessionThreadPanel.tsx +++ b/desktop/src/features/channels/ui/AgentSessionThreadPanel.tsx @@ -28,6 +28,7 @@ import { cancelManagedAgentTurn } from "@/shared/api/agentControl"; import type { Channel } from "@/shared/api/types"; import { useEscapeKey } from "@/shared/hooks/useEscapeKey"; import { useIsThreadPanelOverlay } from "@/shared/hooks/use-mobile"; +import { cn } from "@/shared/lib/cn"; import { useNow } from "@/shared/lib/useNow"; import { AuxiliaryPanel } from "@/shared/layout/AuxiliaryPanel"; import { AuxiliaryPanelBody } from "@/shared/layout/AuxiliaryPanel"; @@ -67,6 +68,19 @@ type AgentSessionThreadPanelProps = { channel: Channel | null; channelId?: string | null; canInterruptTurn: boolean; + /** Constrains the reading column when presented as the focus drawer. */ + columnMaxWidthPx?: number; + /** Panel-owned control rendered ahead of the title (the layout toggle). */ + headerLeading?: React.ReactNode; + /** + * True when the panel is the focus-mode drawer's occupant. + * + * The drawer owns the panel's dismissal affordances, so this also turns on + * Escape (the drawer is modal over the channel), drops the header back arrow + * (the scrim sliver is the way back) and suppresses the panel's own slide-in + * so it does not compound with the drawer's. + */ + isFocusMode?: boolean; layout?: "standalone" | "split"; isSinglePanelView?: boolean; profiles?: UserProfileLookup; @@ -83,11 +97,31 @@ type AgentSessionThreadPanelProps = { transparentChrome?: boolean; }; +/** + * Scroll region of the agent work viewer. + * + * Named so the shared view-mode switch can read the top-visible transcript row + * out of it, the same way it reads the thread body. + */ +export const AGENT_SESSION_BODY_TEST_ID = "agent-session-transcript-body"; + +/** + * Centers the transcript column when a `columnMaxWidthPx` is supplied. + * + * Matches the thread panel's focus-mode column so the two drawers present one + * reading measure. The panel's own `px-3` body gutter already handles the split + * pane, so the wider inline padding only applies to the constrained column. + */ +const AGENT_SESSION_COLUMN_CLASS = "mx-auto w-full px-7"; + export function AgentSessionThreadPanel({ agent, canInterruptTurn, channel, channelId = null, + columnMaxWidthPx, + headerLeading, + isFocusMode = false, layout = "standalone", isSinglePanelView = false, profiles, @@ -98,6 +132,7 @@ export function AgentSessionThreadPanel({ }: AgentSessionThreadPanelProps) { const isLive = isManagedAgentActive(agent); const isOverlay = useIsThreadPanelOverlay(); + const hasConstrainedColumn = columnMaxWidthPx != null; const sessionChannelId = channelId ?? channel?.id ?? null; // Unified working signal, scoped to this panel's channel (or all channels // when the panel is unscoped) — observer turns primary, typing fallback. @@ -106,7 +141,9 @@ export function AgentSessionThreadPanel({ sessionChannelId, ); const canStopCurrentTurn = isWorking && canInterruptTurn; - useEscapeKey(onClose, isOverlay || isSinglePanelView); + // Focus mode is modal over the channel, so it owns Escape the same way the + // narrow and overlay presentations do. The split pane leaves Escape alone. + useEscapeKey(onClose, isOverlay || isSinglePanelView || isFocusMode); const scrollRef = React.useRef(null); const contentRef = React.useRef(null); @@ -416,7 +453,11 @@ export function AgentSessionThreadPanel({ align="start" backButtonAriaLabel="Back from activity" backButtonTestId="agent-session-back" - onBack={onBack} + leading={headerLeading} + // The focus drawer's scrim sliver is its way back to the channel, so the + // header arrow would be a second, competing exit. Mirrors the thread + // panel's focus-mode header. + onBack={isFocusMode ? undefined : onBack} >
-
+
agentSessionSelection.resolveSelectedAgentSession({ @@ -478,12 +463,37 @@ export const ChannelPane = React.memo(function ChannelPane({ }), [agentSessionAgents, openAgentSessionPubkey, profilePanelPubkey, profiles], ); + const hasAgentSessionPanel = Boolean(activeChannel && selectedAgent); + const { + channelIsCovered, + changeAgentViewMode, + changeThreadViewMode, + focusDrawerSurface, + layoutScrollTargetId, + markExitComplete, + resolveScrollTarget, + } = useFocusDrawerSurfaces({ + channelManagementOpen: channelManagementOpen && Boolean(activeChannel), + hasAgentSession: hasAgentSessionPanel, + hasThread: Boolean(threadHeadMessage) || shouldShowThreadSkeleton, + isFocusPreferred: threadViewMode === "focus", + onCloseAgentSession, + onCloseThread, + onThreadScrollTargetResolved, + threadHeadId: threadHeadMessage?.id ?? null, + threadScrollTargetId, + useSplitAuxiliaryPane, + }); + const useFocusThreadDrawer = focusDrawerSurface === "thread"; + const useFocusAgentDrawer = focusDrawerSurface === "agent-session"; const hasSplitAuxiliaryPane = useSplitAuxiliaryPane && (channelManagementOpen || Boolean(threadHeadMessage) || shouldShowThreadSkeleton || - Boolean(activeChannel && selectedAgent) || + // A surface presented as the focus drawer overlays the channel instead of + // splitting the row, so it must not reserve split width. + (hasAgentSessionPanel && !useFocusAgentDrawer) || Boolean(profilePanelPubkey)); const wrapAux = ( panel: React.ReactNode, @@ -516,6 +526,21 @@ export const ChannelPane = React.memo(function ChannelPane({ ) : ( wrapAux(panel, "message-thread-panel", { key: THREAD_SURFACE_KEY }) ); + const wrapAgentSessionPanel = (panel: React.ReactNode) => + useFocusAgentDrawer ? ( + + {panel} + + ) : ( + wrapAux(panel, "agent-session-thread-panel", { + key: AGENT_SESSION_SURFACE_KEY, + }) + ); const threadHeaderLeading = useSplitAuxiliaryPane ? ( ) : undefined; @@ -525,6 +550,16 @@ export const ChannelPane = React.memo(function ChannelPane({ isSinglePanelView, useSplitAuxiliaryPane, }); + // Same preference, same control: the viewer's toggle writes the shared + // `threadViewMode`, so choosing a layout here chooses it for threads too. + const agentSessionLayoutProps = getThreadPanelLayout({ + headerLeading: useSplitAuxiliaryPane ? ( + + ) : undefined, + isFocusDrawer: useFocusAgentDrawer, + isSinglePanelView, + useSplitAuxiliaryPane, + }); const timelineReplyHandler = activeChannel?.archivedAt || isHuddleTranscript ? undefined : onOpenThread; return ( @@ -897,18 +932,21 @@ export const ChannelPane = React.memo(function ChannelPane({ : null } channelId={effectiveAgentSessionChannelId} + columnMaxWidthPx={agentSessionLayoutProps.columnMaxWidthPx} + headerLeading={agentSessionLayoutProps.headerLeading} + isFocusMode={agentSessionLayoutProps.isFocusMode} isSinglePanelView={ - useSplitAuxiliaryPane ? false : isSinglePanelView + agentSessionLayoutProps.isSinglePanelView ?? false } - layout={useSplitAuxiliaryPane ? "split" : "standalone"} - transparentChrome={useSplitAuxiliaryPane} + layout={agentSessionLayoutProps.layout} + transparentChrome={agentSessionLayoutProps.transparentChrome} profiles={profiles} onBack={onBackFromAgentSession} onClose={onCloseAgentSession} widthPx={threadPanelWidthPx} /> ); - return wrapAux(panel, "agent-session-thread-panel"); + return wrapAgentSessionPanel(panel); })() ) : profilePanelPubkey ? ( (() => { diff --git a/desktop/src/features/channels/ui/FocusThreadDrawer.tsx b/desktop/src/features/channels/ui/FocusThreadDrawer.tsx index 1aaad0e6093..098a43689e0 100644 --- a/desktop/src/features/channels/ui/FocusThreadDrawer.tsx +++ b/desktop/src/features/channels/ui/FocusThreadDrawer.tsx @@ -9,6 +9,15 @@ import { getThreadViewMode } from "@/features/channels/lib/threadViewModePrefere import { cn } from "@/shared/lib/cn"; type FocusThreadDrawerProps = { + /** + * Names the surface inside the drawer for assistive technology. + * + * Parameterised rather than hardcoded because the channel agent work viewer + * shares this drawer: the motion, scrim, Escape ownership and focus + * capture/restore below are one behaviour, and duplicating them per surface is + * how they drift apart. + */ + ariaLabel?: string; channelName: string; children: React.ReactNode; onClose: () => void; @@ -137,6 +146,7 @@ const REDUCED_MOTION_TRANSITION = { duration: 0.12, ease: "linear" } as const; * isolated. */ export function FocusThreadDrawer({ + ariaLabel = "Thread", channelName, children, onClose, @@ -220,7 +230,7 @@ export function FocusThreadDrawer({ // see the token for why a `border-l` cannot. "absolute inset-y-0 right-0 flex flex-col overflow-hidden rounded-l-2xl bg-background shadow-panel-left", )} - aria-label="Thread" + aria-label={ariaLabel} data-testid="focus-thread-drawer" ref={drawerRef} role="complementary" diff --git a/desktop/src/features/channels/ui/useFocusDrawerSurfaces.ts b/desktop/src/features/channels/ui/useFocusDrawerSurfaces.ts new file mode 100644 index 00000000000..7f5048fcab7 --- /dev/null +++ b/desktop/src/features/channels/ui/useFocusDrawerSurfaces.ts @@ -0,0 +1,107 @@ +import { + resolveFocusDrawerSurface, + type FocusDrawerSurface, +} from "@/features/channels/lib/focusDrawerSurface"; +import { AGENT_SESSION_BODY_TEST_ID } from "@/features/channels/ui/AgentSessionThreadPanel"; +import { useFocusDrawerPresence } from "@/features/channels/ui/useFocusDrawerPresence"; +import { useThreadViewModeSwitch } from "@/features/channels/ui/useThreadViewModeSwitch"; + +const AGENT_SESSION_BODY_SELECTOR = `[data-testid="${AGENT_SESSION_BODY_TEST_ID}"]`; + +type UseFocusDrawerSurfacesOptions = { + channelManagementOpen: boolean; + hasAgentSession: boolean; + hasThread: boolean; + isFocusPreferred: boolean; + onCloseAgentSession: () => void; + onCloseThread: () => void; + onThreadScrollTargetResolved: () => void; + threadHeadId: string | null; + threadScrollTargetId: string | null; + useSplitAuxiliaryPane: boolean; +}; + +type ThreadViewModeSwitchHandler = ReturnType< + typeof useThreadViewModeSwitch +>["changeThreadViewMode"]; + +type UseFocusDrawerSurfacesResult = { + /** Keeps the covered channel section inert while the drawer is on screen. */ + channelIsCovered: boolean; + changeAgentViewMode: ThreadViewModeSwitchHandler; + changeThreadViewMode: ThreadViewModeSwitchHandler; + focusDrawerSurface: FocusDrawerSurface | null; + layoutScrollTargetId: string | null; + /** Hand to the auxiliary `AnimatePresence` so the drawer can finish exiting. */ + markExitComplete: () => void; + resolveScrollTarget: (settledMessageId?: string) => void; +}; + +/** + * Resolves which auxiliary surface occupies the focus drawer, and wires the + * per-surface presentation state that follows from it. + * + * Threads and the agent work viewer are two occupants of one drawer, so the + * drawer's presence, dismissal ownership and layout toggles all have to be + * decided together from a single resolution — deciding them separately is how + * the drawer and the split pane end up disagreeing about who is on screen. + */ +export function useFocusDrawerSurfaces({ + channelManagementOpen, + hasAgentSession, + hasThread, + isFocusPreferred, + onCloseAgentSession, + onCloseThread, + onThreadScrollTargetResolved, + threadHeadId, + threadScrollTargetId, + useSplitAuxiliaryPane, +}: UseFocusDrawerSurfacesOptions): UseFocusDrawerSurfacesResult { + const focusDrawerSurface = resolveFocusDrawerSurface({ + channelManagementOpen, + hasAgentSession, + hasThread, + isFocusPreferred, + useSplitAuxiliaryPane, + }); + // Whichever surface holds the drawer owns its dismissal, so presence tracks + // the drawer as one overlay and closes the surface actually on screen. + const closeFocusDrawerSurface = + focusDrawerSurface === "agent-session" + ? onCloseAgentSession + : onCloseThread; + const { channelIsCovered, markExitComplete } = useFocusDrawerPresence( + focusDrawerSurface !== null, + closeFocusDrawerSurface, + ); + const { changeThreadViewMode, layoutScrollTargetId, resolveScrollTarget } = + useThreadViewModeSwitch({ + activeThreadHeadId: threadHeadId, + externalScrollTargetId: threadScrollTargetId, + onExternalTargetResolved: onThreadScrollTargetResolved, + onModeChange: markExitComplete, + }); + // The viewer gets its own switch instance so the toggle it renders writes the + // shared preference without borrowing the thread's captured anchor. It + // deliberately does not preserve reading position — see + // `preserveReadingPosition`. + const { changeThreadViewMode: changeAgentViewMode } = useThreadViewModeSwitch( + { + activeThreadHeadId: null, + bodySelector: AGENT_SESSION_BODY_SELECTOR, + onModeChange: markExitComplete, + preserveReadingPosition: false, + }, + ); + + return { + channelIsCovered, + changeAgentViewMode, + changeThreadViewMode, + focusDrawerSurface, + layoutScrollTargetId, + markExitComplete, + resolveScrollTarget, + }; +} diff --git a/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts b/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts index 8dd41cfc51e..5efeb1a04e4 100644 --- a/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts +++ b/desktop/src/features/channels/ui/useThreadViewModeSwitch.ts @@ -48,19 +48,47 @@ export function getScopedLayoutScrollTargetId({ : null; } +/** Scroll region whose top-visible row is preserved across a mode switch. */ +export const THREAD_BODY_SELECTOR = '[data-testid="message-thread-body"]'; + type ThreadViewModeSwitchOptions = { + /** + * Identity of the surface the captured anchor belongs to. + * + * A row id only means something inside the surface it was read from, so the + * anchor is dropped when this changes. Threads pass the open thread head; + * surfaces that do not preserve a reading position pass `null`. + */ activeThreadHeadId: string | null; - externalScrollTargetId: string | null; - onExternalTargetResolved: () => void; + /** + * Scroll region to read the anchor from, and to focus when the switch was not + * keyboard-driven. Defaults to the channel thread body. + */ + bodySelector?: string; + externalScrollTargetId?: string | null; + onExternalTargetResolved?: () => void; onModeChange?: (mode: ThreadViewMode) => void; + /** + * Whether the reader's row is restored after the switch. + * + * Threads restore it. The agent work viewer deliberately does not: its + * transcript rows use `content-visibility: auto` with an over-reserved + * `contain-intrinsic-size`, so the scroll height shrinks as rows paint after + * the layout change and a restored position slides out from under the reader. + * Snapping to the latest activity is the honest behaviour until transcript + * rows reserve their real heights. + */ + preserveReadingPosition?: boolean; }; -/** Preserves the reply being read while the thread changes presentation. */ +/** Preserves the row being read while a surface changes presentation. */ export function useThreadViewModeSwitch({ activeThreadHeadId, - externalScrollTargetId, + bodySelector = THREAD_BODY_SELECTOR, + externalScrollTargetId = null, onExternalTargetResolved, onModeChange, + preserveReadingPosition = true, }: ThreadViewModeSwitchOptions) { const [layoutScrollTarget, setLayoutScrollTarget] = React.useState(null); @@ -77,10 +105,10 @@ export function useThreadViewModeSwitch({ const changeThreadViewMode = React.useCallback( (mode: ThreadViewMode, restoreFocus: boolean) => { - const body = document.querySelector( - '[data-testid="message-thread-body"]', - ); - const anchorId = findTopVisibleThreadMessageId(body); + const body = document.querySelector(bodySelector); + const anchorId = preserveReadingPosition + ? findTopVisibleThreadMessageId(body) + : null; setLayoutScrollTarget( anchorId && activeThreadHeadId @@ -95,13 +123,13 @@ export function useThreadViewModeSwitch({ .querySelector( restoreFocus ? '[data-testid="thread-view-mode-toggle"]' - : '[data-testid="message-thread-body"]', + : bodySelector, ) ?.focus({ preventScroll: true }); }); }); }, - [activeThreadHeadId, onModeChange], + [activeThreadHeadId, bodySelector, onModeChange, preserveReadingPosition], ); const resolveScrollTarget = React.useCallback( @@ -110,7 +138,7 @@ export function useThreadViewModeSwitch({ externalTargetId: externalScrollTargetId, layoutTargetId: layoutScrollTargetId, }); - if (resolution.resolveExternal) onExternalTargetResolved(); + if (resolution.resolveExternal) onExternalTargetResolved?.(); if (settledMessageId) { setLayoutScrollTarget((current) => current?.threadHeadId === activeThreadHeadId && diff --git a/desktop/tests/e2e/agent-viewer-focus-mode.spec.ts b/desktop/tests/e2e/agent-viewer-focus-mode.spec.ts new file mode 100644 index 00000000000..d4fae7cd353 --- /dev/null +++ b/desktop/tests/e2e/agent-viewer-focus-mode.spec.ts @@ -0,0 +1,248 @@ +import { expect, test } from "@playwright/test"; + +import { installMockBridge, TEST_IDENTITIES } from "../helpers/bridge"; + +const AGENT_PUBKEY = TEST_IDENTITIES.tyler.pubkey; +const CHANNEL_ID = "94a444a4-c0a3-5966-ab05-530c6ddc2301"; // #agents +const BASE_TIMESTAMP = Date.parse("2025-06-15T12:00:00Z"); +const TRANSCRIPT_ROW_COUNT = 40; +/** Newest display block the grouper emits for the seeded transcript. */ +const LATEST_ROW_ID = `turn:turn-${TRANSCRIPT_ROW_COUNT - 1}`; + +const MANAGED_AGENTS = [ + { + pubkey: AGENT_PUBKEY, + name: "Observer Agent", + status: "running" as const, + channelNames: ["agents"], + }, +]; + +/** + * Seeds enough distinct turns to make the transcript taller than its scroll + * region, so a preserved reading position is observable rather than incidental. + * + * One `turnId` per event: the grouper emits a `turn:` display block per + * turn, which is exactly the `data-message-id` the layout switch anchors on. + */ +async function seedLongTranscript(page: import("@playwright/test").Page) { + await page.waitForFunction( + () => typeof window.__BUZZ_E2E_SEED_OBSERVER_EVENTS__ === "function", + null, + { timeout: 10_000 }, + ); + await page.evaluate( + ({ pubkey, channelId, baseTimestamp, rowCount }) => { + const events = Array.from({ length: rowCount }, (_unused, index) => ({ + seq: index + 1, + timestamp: new Date(baseTimestamp + index * 1_000).toISOString(), + kind: "acp_read", + agentIndex: 0, + channelId, + sessionId: "session-focus", + turnId: `turn-${index}`, + payload: { + method: "session/update", + params: { + sessionId: "session-focus", + update: { + sessionUpdate: "agent_message_chunk", + messageId: `message-${index}`, + content: { + type: "text", + text: `Activity row ${index}: a deliberately long line of assistant output so that changing the reading measure forces the transcript to reflow and re-measure every row below it.`, + }, + }, + }, + }, + })); + window.__BUZZ_E2E_SEED_OBSERVER_EVENTS__?.({ + agentPubkey: pubkey, + events, + }); + }, + { + pubkey: AGENT_PUBKEY, + channelId: CHANNEL_ID, + baseTimestamp: BASE_TIMESTAMP, + rowCount: TRANSCRIPT_ROW_COUNT, + }, + ); +} + +async function openAgentsChannel(page: import("@playwright/test").Page) { + await page.getByTestId("channel-agents").click(); + await expect(page.getByTestId("chat-title")).toHaveText("agents"); +} + +/** + * Opens the viewer through the profile panel's activity ingress. + * + * With a seeded transcript the panel renders the live-activity embed rather than + * the empty "Activity log" row, so the ingress is the embed's open button. The + * panel may already be open underneath a just-closed viewer (closing the viewer + * does not close the profile pane), so only open it when it is absent — + * re-clicking the message row would toggle it shut. + */ +async function openAgentViewer(page: import("@playwright/test").Page) { + const profilePanel = page.getByTestId("user-profile-panel"); + if ((await profilePanel.count()) === 0) { + const messageRow = page + .getByTestId("message-row") + .filter({ has: page.getByText("Observer Agent", { exact: false }) }); + await expect(messageRow.first()).toBeVisible({ timeout: 8_000 }); + await messageRow.first().getByRole("button").first().click(); + } + await expect(profilePanel).toBeVisible({ timeout: 10_000 }); + + const openActivity = profilePanel.getByRole("button", { + name: /Open full activity/, + }); + await expect(openActivity).toBeVisible({ timeout: 10_000 }); + await openActivity.click(); + + const panel = page.getByTestId("agent-session-thread-panel"); + await expect(panel).toBeVisible({ timeout: 10_000 }); + return panel; +} + +/** + * Scrolls off the tail so a subsequent snap-to-latest is observable rather than + * incidental. + */ +async function scrollAwayFromBottom( + body: import("@playwright/test").Locator, +): Promise { + await expect + .poll( + async () => + await body.evaluate((element) => { + const maxScrollTop = element.scrollHeight - element.clientHeight; + if (maxScrollTop <= 0) return false; + + const targetScrollTop = Math.floor(maxScrollTop * 0.4); + element.scrollTop = targetScrollTop; + element.dispatchEvent(new Event("scroll", { bubbles: true })); + return Math.abs(element.scrollTop - targetScrollTop) <= 1; + }), + { timeout: 15_000 }, + ) + .toBe(true); +} + +/** + * Asserts the transcript is showing the newest activity. + * + * The viewer deliberately snaps to latest across a layout switch instead of + * restoring the reader's row: transcript rows use `content-visibility: auto` + * with an over-reserved `contain-intrinsic-size`, so the scroll height shrinks + * as rows paint after the reflow and any restored offset slides out from under + * the reader. Snapping is the behaviour we can actually keep. + */ +async function expectSnappedToLatest( + page: import("@playwright/test").Page, + body: import("@playwright/test").Locator, +) { + await expect( + page.locator(`[data-message-id="${LATEST_ROW_ID}"]`), + ).toBeVisible({ timeout: 10_000 }); + await expect + .poll( + async () => + await body.evaluate( + (element) => + element.scrollHeight - element.clientHeight - element.scrollTop, + ), + { timeout: 10_000 }, + ) + .toBeLessThanOrEqual(32); +} + +test("the agent work viewer shares the thread's focus drawer and dismissal", async ({ + page, +}) => { + await page.setViewportSize({ width: 1280, height: 720 }); + await page.addInitScript(() => { + localStorage.setItem("buzz.channels.threadViewMode", "focus"); + }); + await installMockBridge(page, { managedAgents: MANAGED_AGENTS }); + await page.goto("/", { waitUntil: "domcontentloaded" }); + await seedLongTranscript(page); + await openAgentsChannel(page); + + const panel = await openAgentViewer(page); + + // The viewer is the drawer's occupant, named for itself rather than "Thread". + const channel = page.getByTestId("channel-drop-zone"); + const drawer = page.getByTestId("focus-thread-drawer"); + await expect(drawer).toBeVisible(); + await expect(drawer).toHaveAttribute("aria-label", "Agent activity"); + await expect(channel).toHaveAttribute("inert", ""); + // The scrim sliver is the way back, so the header arrow must not compete. + await expect(page.getByTestId("agent-session-back")).toHaveCount(0); + // Read-only and channel-scoped boundaries are unchanged by the presentation. + await expect(page.getByTestId("agent-session-scope-label")).toHaveText( + "Activity · #agents", + ); + await expect(panel.getByTestId("message-input")).toHaveCount(0); + + const body = page.getByTestId("agent-session-transcript-body"); + await scrollAwayFromBottom(body); + + // focus → split, driven by the shared toggle in the viewer's header. + await page + .getByRole("button", { name: "Show thread beside channel" }) + .click(); + await expect(drawer).toHaveCount(0); + await expect(channel).not.toHaveAttribute("inert", ""); + await expect(page.getByTestId("agent-session-thread-panel")).toBeVisible(); + await expectSnappedToLatest(page, body); + + // split → focus: the transcript re-pins to latest on the return trip too. + await scrollAwayFromBottom(body); + await page.getByRole("button", { name: "Expand thread" }).click(); + await expect(drawer).toBeVisible(); + await expect(channel).toHaveAttribute("inert", ""); + await expectSnappedToLatest(page, body); + + // Escape closes the AGENT SESSION. Were it wired to thread close, the drawer + // would stay up with the viewer still inside it. + await page.keyboard.press("Escape"); + await expect(page.getByTestId("focus-thread-drawer-overlay")).toHaveCount(0); + await expect(page.getByTestId("agent-session-thread-panel")).toHaveCount(0); + await expect(channel).not.toHaveAttribute("inert", ""); + + // Scrim click dismisses the same surface. + await openAgentViewer(page); + await expect(drawer).toBeVisible(); + await page + .getByTestId("focus-thread-drawer-scrim") + .click({ position: { x: 24, y: 200 } }); + await expect(page.getByTestId("focus-thread-drawer-overlay")).toHaveCount(0); + await expect(page.getByTestId("agent-session-thread-panel")).toHaveCount(0); + await expect(channel).not.toHaveAttribute("inert", ""); + + // Sidebar background dismissal is the overlay presentation's, and it too must + // reach the viewer rather than a thread. + await openAgentViewer(page); + await expect(drawer).toBeVisible(); + await page + .getByTestId("app-sidebar-scroll-anchor") + .evaluate((element) => (element as HTMLElement).click()); + await expect(page.getByTestId("focus-thread-drawer-overlay")).toHaveCount(0); + await expect(page.getByTestId("agent-session-thread-panel")).toHaveCount(0); +}); + +test("narrow viewers do not offer an unavailable layout switch", async ({ + page, +}) => { + await page.setViewportSize({ width: 860, height: 720 }); + await installMockBridge(page, { managedAgents: MANAGED_AGENTS }); + await page.goto("/", { waitUntil: "domcontentloaded" }); + await seedLongTranscript(page); + await openAgentsChannel(page); + + await openAgentViewer(page); + await expect(page.getByTestId("focus-thread-drawer")).toHaveCount(0); + await expect(page.getByTestId("thread-view-mode-toggle")).toHaveCount(0); +}); From 66b3e23a7a5ba8fed582e6537d43a32305687c1e Mon Sep 17 00:00:00 2001 From: ss-quality-00 <75789fffd704a5265cd65462cfc263f44561872fbebcea3a131a2a5f511465d2@buzz.block.builderlab.xyz> Date: Fri, 21 Aug 2026 18:30:20 -0700 Subject: [PATCH 2/2] refactor(desktop): clarify agent viewer layout Give the shared layout control surface-specific accessible names and cover the viewer's centered reading measure and URL-backed history behavior. Co-authored-by: ss-quality-00 <75789fffd704a5265cd65462cfc263f44561872fbebcea3a131a2a5f511465d2@buzz.block.builderlab.xyz> Signed-off-by: ss-quality-00 <75789fffd704a5265cd65462cfc263f44561872fbebcea3a131a2a5f511465d2@buzz.block.builderlab.xyz> --- .../src/features/channels/ui/ChannelPane.tsx | 5 ++- .../channels/ui/ThreadViewModeToggle.tsx | 34 +++++++-------- .../tests/e2e/agent-viewer-focus-mode.spec.ts | 41 +++++++++++++++++-- 3 files changed, 59 insertions(+), 21 deletions(-) diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 899fe6352c3..c8a6b6f0899 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -554,7 +554,10 @@ export const ChannelPane = React.memo(function ChannelPane({ // `threadViewMode`, so choosing a layout here chooses it for threads too. const agentSessionLayoutProps = getThreadPanelLayout({ headerLeading: useSplitAuxiliaryPane ? ( - + ) : undefined, isFocusDrawer: useFocusAgentDrawer, isSinglePanelView, diff --git a/desktop/src/features/channels/ui/ThreadViewModeToggle.tsx b/desktop/src/features/channels/ui/ThreadViewModeToggle.tsx index 2f85691e192..96019f9fabe 100644 --- a/desktop/src/features/channels/ui/ThreadViewModeToggle.tsx +++ b/desktop/src/features/channels/ui/ThreadViewModeToggle.tsx @@ -28,43 +28,43 @@ const THREAD_VIEW_MODE_TOGGLE = { focus: { // Viewing the drawer → offer the pane. icon: Columns2, - label: "Show thread beside channel", + label: (surface: string) => `Show ${surface} beside channel`, target: "split", }, split: { // Viewing the pane → offer the drawer. icon: PanelRightOpen, - label: "Expand thread", + label: (surface: string) => `Expand ${surface}`, target: "focus", }, } as const; +type ThreadViewModeToggleProps = { + /** Accessible noun for the surface sharing the thread layout preference. */ + surfaceLabel?: string; + onChange: (mode: ThreadViewMode, restoreFocus: boolean) => void; +}; + /** - * Switches an open thread between the focus drawer and the split pane. + * Switches an open auxiliary surface between the focus drawer and split pane. * - * Writes straight to the persisted preference, so the control doubles as the - * setting: choosing a layout here is choosing how threads open from now on. That - * is the intended behaviour — the place you form the opinion is the place you are - * looking at the thread, not a settings page — and it is why the label names an - * action rather than a state. - * - * A tooltip is mandatory, not decoration. A lone toggle showing its target is the - * conventional pattern and still routinely misread as showing the current state; - * the glyph cannot disambiguate itself, so the label has to. + * Threads and the agent work viewer intentionally share this control and its + * persisted preference. `surfaceLabel` keeps the action truthful when the + * current occupant is not a thread. */ export function ThreadViewModeToggle({ onChange, -}: { - onChange: (mode: ThreadViewMode, restoreFocus: boolean) => void; -}) { + surfaceLabel = "thread", +}: ThreadViewModeToggleProps) { const viewMode = useThreadViewMode(); const { icon: Icon, label, target } = THREAD_VIEW_MODE_TOGGLE[viewMode]; + const accessibleLabel = label(surfaceLabel); return ( - {label} + {accessibleLabel} ); } diff --git a/desktop/tests/e2e/agent-viewer-focus-mode.spec.ts b/desktop/tests/e2e/agent-viewer-focus-mode.spec.ts index d4fae7cd353..332585e3cfe 100644 --- a/desktop/tests/e2e/agent-viewer-focus-mode.spec.ts +++ b/desktop/tests/e2e/agent-viewer-focus-mode.spec.ts @@ -171,6 +171,7 @@ test("the agent work viewer shares the thread's focus drawer and dismissal", asy await openAgentsChannel(page); const panel = await openAgentViewer(page); + await expect(page).toHaveURL(new RegExp(`agentSession=${AGENT_PUBKEY}`)); // The viewer is the drawer's occupant, named for itself rather than "Thread". const channel = page.getByTestId("channel-drop-zone"); @@ -187,11 +188,32 @@ test("the agent work viewer shares the thread's focus drawer and dismissal", asy await expect(panel.getByTestId("message-input")).toHaveCount(0); const body = page.getByTestId("agent-session-transcript-body"); + const readingColumn = body.locator(":scope > div").nth(1); + await expect + .poll(async () => { + const [drawerBox, columnBox] = await Promise.all([ + drawer.boundingBox(), + readingColumn.boundingBox(), + ]); + if (!drawerBox || !columnBox) return null; + return { + centered: + Math.abs( + columnBox.x + + columnBox.width / 2 - + (drawerBox.x + drawerBox.width / 2), + ) <= 1, + withinReadingMeasure: columnBox.width <= 880, + }; + }) + .toEqual({ centered: true, withinReadingMeasure: true }); await scrollAwayFromBottom(body); - // focus → split, driven by the shared toggle in the viewer's header. + // focus → split, driven by the shared toggle in the viewer's header. Its + // accessible name describes the viewer, not the thread that shares the + // preference. await page - .getByRole("button", { name: "Show thread beside channel" }) + .getByRole("button", { name: "Show activity beside channel" }) .click(); await expect(drawer).toHaveCount(0); await expect(channel).not.toHaveAttribute("inert", ""); @@ -200,7 +222,7 @@ test("the agent work viewer shares the thread's focus drawer and dismissal", asy // split → focus: the transcript re-pins to latest on the return trip too. await scrollAwayFromBottom(body); - await page.getByRole("button", { name: "Expand thread" }).click(); + await page.getByRole("button", { name: "Expand activity" }).click(); await expect(drawer).toBeVisible(); await expect(channel).toHaveAttribute("inert", ""); await expectSnappedToLatest(page, body); @@ -210,6 +232,19 @@ test("the agent work viewer shares the thread's focus drawer and dismissal", asy await page.keyboard.press("Escape"); await expect(page.getByTestId("focus-thread-drawer-overlay")).toHaveCount(0); await expect(page.getByTestId("agent-session-thread-panel")).toHaveCount(0); + await expect(page).not.toHaveURL(/agentSession=/); + await expect(channel).not.toHaveAttribute("inert", ""); + + // Browser history restores the channel-scoped viewer and its persisted focus + // presentation; closing it again must remove only the viewer URL state. + await page.goBack(); + await expect(page.getByTestId("agent-session-thread-panel")).toBeVisible(); + await expect(drawer).toBeVisible(); + await expect(page.getByTestId("agent-session-scope-label")).toHaveText( + "Activity · #agents", + ); + await page.goForward(); + await expect(page.getByTestId("agent-session-thread-panel")).toHaveCount(0); await expect(channel).not.toHaveAttribute("inert", ""); // Scrim click dismisses the same surface.