From cada3028ceda7ebdae61329f15b5364ca0d23dd4 Mon Sep 17 00:00:00 2001 From: thomaspblock Date: Wed, 26 Aug 2026 20:59:47 -0400 Subject: [PATCH] fix(desktop): keep project sheets independent from threads (#6901) ## Summary Project workspace sheets now slide in from the right as an independent layer over an open conversation thread instead of replacing the thread or rendering inside its panel. Closing the sheet restores the unchanged thread, Escape applies only to the topmost surface, and the covered thread's resize divider no longer leaks through the overlay. ### Related issue Related: #6602. No matching issue found. ### Testing - `pnpm --dir desktop exec biome check src/features/channels/ui/RightAuxiliaryPane.tsx tests/e2e/project-commit-detail.spec.ts` - `pnpm exec playwright test tests/e2e/project-commit-detail.spec.ts --project=smoke --grep "project workspace sheet stays independent from an open thread"` - Pre-push desktop formatting, file-size, typecheck, and unit-test checks --------- Signed-off-by: Thomas Petersen --- .../channels/ui/ChannelPane.helpers.test.mjs | 16 ++ .../channels/ui/ChannelPane.helpers.ts | 6 +- .../src/features/channels/ui/ChannelPane.tsx | 69 +++++--- .../channels/ui/FocusThreadDrawer.tsx | 61 ++++++- .../channels/ui/RightAuxiliaryPane.tsx | 2 +- .../channels/ui/ThreadPanelSurface.tsx | 66 ++++++++ .../channels/ui/useFocusDrawerPresence.ts | 18 ++- .../tests/e2e/project-commit-detail.spec.ts | 150 +++++++++++++++++- 8 files changed, 352 insertions(+), 36 deletions(-) create mode 100644 desktop/src/features/channels/ui/ThreadPanelSurface.tsx diff --git a/desktop/src/features/channels/ui/ChannelPane.helpers.test.mjs b/desktop/src/features/channels/ui/ChannelPane.helpers.test.mjs index 10a948e705..9d2cd6a32d 100644 --- a/desktop/src/features/channels/ui/ChannelPane.helpers.test.mjs +++ b/desktop/src/features/channels/ui/ChannelPane.helpers.test.mjs @@ -42,6 +42,22 @@ test("focus idle drawers yield to every higher-priority auxiliary surface", () = } }); +test("an explicit thread override keeps the idle panel in its own focus drawer", () => { + assert.equal( + shouldUseFocusIdleDrawer({ + channelManagementOpen: false, + hasAgentSession: false, + hasIdleAuxiliaryPanel: true, + hasIdlePanelCloseHandler: true, + hasProfilePanel: false, + hasThreadSurface: true, + overrideThread: true, + useSplitAuxiliaryPane: false, + }), + true, + ); +}); + test("getChannelIntroKind names project homes ahead of regular streams", () => { assert.equal(getChannelIntroKind(channel(), true), "project channel"); assert.equal(getChannelIntroKind(channel(), false), "regular channel"); diff --git a/desktop/src/features/channels/ui/ChannelPane.helpers.ts b/desktop/src/features/channels/ui/ChannelPane.helpers.ts index 695fef166f..f59568779d 100644 --- a/desktop/src/features/channels/ui/ChannelPane.helpers.ts +++ b/desktop/src/features/channels/ui/ChannelPane.helpers.ts @@ -10,6 +10,7 @@ export function shouldUseFocusIdleDrawer({ hasIdlePanelCloseHandler, hasProfilePanel, hasThreadSurface, + overrideThread = false, useSplitAuxiliaryPane, }: { channelManagementOpen: boolean; @@ -18,14 +19,15 @@ export function shouldUseFocusIdleDrawer({ hasIdlePanelCloseHandler: boolean; hasProfilePanel: boolean; hasThreadSurface: boolean; + overrideThread?: boolean; useSplitAuxiliaryPane: boolean; }): boolean { return ( - useSplitAuxiliaryPane && + (useSplitAuxiliaryPane || overrideThread) && !channelManagementOpen && !hasAgentSession && !hasProfilePanel && - !hasThreadSurface && + (!hasThreadSurface || overrideThread) && hasIdleAuxiliaryPanel && hasIdlePanelCloseHandler ); diff --git a/desktop/src/features/channels/ui/ChannelPane.tsx b/desktop/src/features/channels/ui/ChannelPane.tsx index 0d9a5eeb1e..38ebd9234a 100644 --- a/desktop/src/features/channels/ui/ChannelPane.tsx +++ b/desktop/src/features/channels/ui/ChannelPane.tsx @@ -30,6 +30,10 @@ import { AgentSessionThreadPanel } from "@/features/channels/ui/AgentSessionThre import { ChannelManagementAuxiliaryPanel } from "@/features/channels/ui/ChannelManagementAuxiliaryPanel"; import { IdleAuxiliaryPanel } from "@/features/channels/ui/IdleAuxiliaryPanel"; import { RightAuxiliaryPane } from "@/features/channels/ui/RightAuxiliaryPane"; +import { + ThreadPanelSurface, + useThreadPanelSurface, +} from "@/features/channels/ui/ThreadPanelSurface"; import { ThreadViewModeToggle } from "@/features/channels/ui/ThreadViewModeToggle"; import { FocusThreadDrawer } from "@/features/channels/ui/FocusThreadDrawer"; import { THREAD_SURFACE_KEY } from "@/features/channels/lib/threadFocusLayout"; @@ -419,10 +423,10 @@ export const ChannelPane = React.memo(function ChannelPane({ const isOverlay = useIsThreadPanelOverlay(); const useSplitAuxiliaryPane = !isSinglePanelView && !isOverlay; const threadViewMode = useThreadViewMode(); + const hasThreadSurface = + Boolean(threadHeadMessage) || shouldShowThreadSkeleton; const useFocusThreadDrawer = - threadViewMode === "focus" && - useSplitAuxiliaryPane && - (Boolean(threadHeadMessage) || shouldShowThreadSkeleton); + threadViewMode === "focus" && useSplitAuxiliaryPane && hasThreadSurface; const selectedAgent = React.useMemo( () => agentSessionSelection.resolveSelectedAgentSession({ @@ -435,19 +439,26 @@ export const ChannelPane = React.memo(function ChannelPane({ ); const hasIdleAuxiliary = Boolean(idleAuxiliaryPanel) && Boolean(onCloseIdleAuxiliaryPanel); + const priorityIdleAuxiliary = shouldPrioritizeIdleAuxiliary( + idleAuxiliaryOverridesThread, + hasIdleAuxiliary, + ); + const overlayIdleAuxiliaryOverThread = + priorityIdleAuxiliary && hasThreadSurface && !isOverlay; + const replaceThreadWithIdleAuxiliary = + priorityIdleAuxiliary && hasThreadSurface && isOverlay; const useFocusIdleDrawer = shouldUseFocusIdleDrawer({ channelManagementOpen, hasAgentSession: Boolean(activeChannel && selectedAgent), hasIdleAuxiliaryPanel: Boolean(idleAuxiliaryPanel), hasIdlePanelCloseHandler: Boolean(onCloseIdleAuxiliaryPanel), hasProfilePanel: Boolean(profilePanelPubkey), - hasThreadSurface: Boolean(threadHeadMessage) || shouldShowThreadSkeleton, + hasThreadSurface, + overrideThread: overlayIdleAuxiliaryOverThread, useSplitAuxiliaryPane, }); - const priorityIdleAuxiliary = shouldPrioritizeIdleAuxiliary( - idleAuxiliaryOverridesThread, - hasIdleAuxiliary, - ); + const showIdleAuxiliaryOverThread = + overlayIdleAuxiliaryOverThread && useFocusIdleDrawer; const { channelIsCovered, markExitComplete } = useFocusDrawerPresence( useFocusThreadDrawer || useFocusIdleDrawer, priorityIdleAuxiliary @@ -456,6 +467,10 @@ export const ChannelPane = React.memo(function ChannelPane({ ? onCloseThread : (onCloseIdleAuxiliaryPanel ?? onCloseThread), ); + const threadSurface = useThreadPanelSurface( + showIdleAuxiliaryOverThread, + markExitComplete, + ); const { changeThreadViewMode, layoutScrollTargetId, resolveScrollTarget } = useThreadViewModeSwitch({ activeThreadHeadId: threadHeadMessage?.id ?? null, @@ -506,19 +521,19 @@ export const ChannelPane = React.memo(function ChannelPane({ ) : ( {panel} ); - const wrapThreadPanel = (panel: React.ReactNode) => - useFocusThreadDrawer ? ( - - {panel} - - ) : ( - wrapAux(panel, "message-thread-panel", { key: THREAD_SURFACE_KEY }) - ); + const wrapThreadPanel = (panel: React.ReactNode) => ( + + {useFocusThreadDrawer ? panel : wrapAux(panel, "message-thread-panel")} + + ); const wrapIdlePanel = (panel: React.ReactNode) => useFocusIdleDrawer && onCloseIdleAuxiliaryPanel ? ( {panel} @@ -779,10 +795,8 @@ export const ChannelPane = React.memo(function ChannelPane({ } showTopBorder={false} /> - {/* The activity accessory is anchored in the dock's reserved - bottom rail, so fading it cannot change the observed - overlay height or move the conversation. Its natural - content height remains responsive. */} + {/* The reserved bottom rail keeps accessory fades from moving + the conversation while content remains responsive. */} - ) : priorityIdleAuxiliary && idleAuxiliarySurface ? ( + ) : replaceThreadWithIdleAuxiliary && idleAuxiliarySurface ? ( idleAuxiliarySurface ) : threadHeadMessage ? ( (() => { @@ -977,6 +991,9 @@ export const ChannelPane = React.memo(function ChannelPane({ idleAuxiliarySurface )} + + {showIdleAuxiliaryOverThread ? idleAuxiliarySurface : null} + ); }); diff --git a/desktop/src/features/channels/ui/FocusThreadDrawer.tsx b/desktop/src/features/channels/ui/FocusThreadDrawer.tsx index 626dbbddc2..82ecfe6440 100644 --- a/desktop/src/features/channels/ui/FocusThreadDrawer.tsx +++ b/desktop/src/features/channels/ui/FocusThreadDrawer.tsx @@ -11,10 +11,14 @@ import { cn } from "@/shared/lib/cn"; type FocusThreadDrawerProps = { channelName: string; children: React.ReactNode; + /** Prevent a covered drawer from handling Escape before its overlay. */ + escapeEnabled?: boolean; /** Accessible name for the drawer. Channel threads leave the default. */ label?: string; hasActiveEdit?: boolean; onClose: () => void; + /** Resolve an explicit focus target after this drawer has been dismissed. */ + restoreFocusTarget?: () => HTMLElement | null; }; /** @@ -120,6 +124,46 @@ const EXIT_TRANSITION = { */ const REDUCED_MOTION_TRANSITION = { duration: 0.12, ease: "linear" } as const; +function useViewportRightInsetPx( + overlayRef: React.RefObject, +) { + const [rightInsetPx, setRightInsetPx] = React.useState(0); + + React.useLayoutEffect(() => { + const layoutRoot = overlayRef.current?.parentElement; + if (!layoutRoot) return; + + const updateRightInset = () => { + const bounds = layoutRoot.getBoundingClientRect(); + const overflowPx = Math.max(0, bounds.right - window.innerWidth); + const nextRightInsetPx = Math.min(bounds.width, Math.ceil(overflowPx)); + setRightInsetPx((current) => + current === nextRightInsetPx ? current : nextRightInsetPx, + ); + }; + + updateRightInset(); + window.addEventListener("resize", updateRightInset); + + const observer = + typeof ResizeObserver === "undefined" + ? null + : new ResizeObserver(updateRightInset); + let ancestor: HTMLElement | null = layoutRoot; + while (ancestor) { + observer?.observe(ancestor); + ancestor = ancestor.parentElement; + } + + return () => { + observer?.disconnect(); + window.removeEventListener("resize", updateRightInset); + }; + }, [overlayRef]); + + return rightInsetPx; +} + /** * Right-anchored thread drawer that overlays the channel content area. * @@ -142,16 +186,22 @@ const REDUCED_MOTION_TRANSITION = { duration: 0.12, ease: "linear" } as const; export function FocusThreadDrawer({ channelName, children, + escapeEnabled = true, label = "Thread", hasActiveEdit = false, onClose, + restoreFocusTarget, }: FocusThreadDrawerProps) { const prefersReducedMotion = useReducedMotion(); const travelPx = prefersReducedMotion ? 0 : THREAD_FOCUS_DRAWER_TRAVEL_PX; const drawerRef = React.useRef(null); + const overlayRef = React.useRef(null); const previousFocusRef = React.useRef(null); + const viewportRightInsetPx = useViewportRightInsetPx(overlayRef); React.useEffect(() => { + if (!escapeEnabled) return; + function handleEscape(event: KeyboardEvent) { if (event.key !== "Escape") return; const target = event.target; @@ -171,7 +221,7 @@ export function FocusThreadDrawer({ return () => { window.removeEventListener("keydown", handleEscape, { capture: true }); }; - }, [hasActiveEdit, onClose]); + }, [escapeEnabled, hasActiveEdit, onClose]); React.useLayoutEffect(() => { previousFocusRef.current = @@ -183,6 +233,11 @@ export function FocusThreadDrawer({ return () => { const previousFocus = previousFocusRef.current; requestAnimationFrame(() => { + const explicitTarget = restoreFocusTarget?.(); + if (explicitTarget) { + explicitTarget.focus({ preventScroll: true }); + return; + } // A real dismissal keeps focus mode selected; a presentation switch // has already selected split mode and owns focus inside the new panel. if (getThreadViewMode() === "focus") { @@ -190,12 +245,14 @@ export function FocusThreadDrawer({ } }); }; - }, []); + }, [restoreFocusTarget]); return (
void; +}; + +/** Keeps a thread mounted while controlling its focus-drawer presentation. */ +export const ThreadPanelSurface = React.forwardRef< + HTMLDivElement, + ThreadPanelSurfaceProps +>(function ThreadPanelSurface( + { channelName, children, covered, hasActiveEdit, isFocusDrawer, onClose }, + ref, +) { + return ( +
+ {isFocusDrawer ? ( + + {children} + + ) : ( + children + )} +
+ ); +}); + +/** Supplies covered-thread lifecycle and focus ownership for an overlay drawer. */ +export function useThreadPanelSurface( + open: boolean, + onExitComplete: () => void, +) { + const ref = React.useRef(null); + const coverage = usePresenceCoverage(open); + const markExitComplete = React.useCallback(() => { + coverage.markExitComplete(); + onExitComplete(); + }, [coverage.markExitComplete, onExitComplete]); + const restoreFocusTarget = React.useCallback( + () => + ref.current?.querySelector( + '[data-testid="auxiliary-panel-close"]', + ) ?? null, + [], + ); + return { ...coverage, markExitComplete, ref, restoreFocusTarget }; +} diff --git a/desktop/src/features/channels/ui/useFocusDrawerPresence.ts b/desktop/src/features/channels/ui/useFocusDrawerPresence.ts index 271c867ae3..660e8f741c 100644 --- a/desktop/src/features/channels/ui/useFocusDrawerPresence.ts +++ b/desktop/src/features/channels/ui/useFocusDrawerPresence.ts @@ -2,22 +2,32 @@ import * as React from "react"; import { subscribeToFocusedThreadCloseRequest } from "@/features/channels/focusedThreadCloseRequest"; -/** Keeps the covered channel inert and owns external dismissal while open. */ -export function useFocusDrawerPresence(open: boolean, onClose: () => void) { +/** Retains coverage until the owning presence boundary completes its exit. */ +export function usePresenceCoverage(open: boolean) { const [present, setPresent] = React.useState(false); React.useEffect(() => { if (open) setPresent(true); }, [open]); + const markExitComplete = React.useCallback(() => setPresent(false), []); + return { + covered: open || present, + markExitComplete, + }; +} + +/** Keeps the covered channel inert and owns external dismissal while open. */ +export function useFocusDrawerPresence(open: boolean, onClose: () => void) { + const { covered, markExitComplete } = usePresenceCoverage(open); + React.useEffect(() => { if (!open) return; return subscribeToFocusedThreadCloseRequest(onClose); }, [onClose, open]); - const markExitComplete = React.useCallback(() => setPresent(false), []); return { - channelIsCovered: open || present, + channelIsCovered: covered, markExitComplete, }; } diff --git a/desktop/tests/e2e/project-commit-detail.spec.ts b/desktop/tests/e2e/project-commit-detail.spec.ts index 6c84a170a9..2ee454aefc 100644 --- a/desktop/tests/e2e/project-commit-detail.spec.ts +++ b/desktop/tests/e2e/project-commit-detail.spec.ts @@ -781,7 +781,7 @@ test("latest files commit opens its detail without a divider", async ({ await expect(page.getByTestId("project-commit-detail")).toBeVisible(); }); -test("project workspace sheet enters at its settled width", async ({ +test("project workspace sheet stays independent from an open thread", async ({ page, }) => { await enableProjectsFeature(page); @@ -792,6 +792,7 @@ test("project workspace sheet enters at its settled width", async ({ await page.getByTestId("create-project-name").fill("sheet-motion-demo"); await page.getByTestId("create-project-submit").click(); await expect(page.getByTestId("project-channel-home")).toBeVisible(); + await page.setViewportSize({ height: 720, width: 820 }); const summaryColumn = page.getByTestId("project-home-summary-column"); const resizeHandle = summaryColumn.getByTestId( @@ -822,6 +823,153 @@ test("project workspace sheet enters at its settled width", async ({ expect( Math.abs(settledDrawerWidth - enteringDrawerWidth), ).toBeLessThanOrEqual(1); + + await focusDrawer.getByTestId("auxiliary-panel-close").click(); + await expect(page.getByTestId("project-home-workspace-sheet")).toHaveCount(0); + await expect(page.getByTestId("project-home-summary-column")).toBeVisible(); + await waitForMockLiveSubscription(page, "sheet-motion-demo"); + const threadRootContent = "Workspace drawer thread root"; + await page.evaluate((content) => { + window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({ + channelName: "sheet-motion-demo", + content, + }); + }, threadRootContent); + const threadRoot = page + .getByTestId("message-timeline") + .getByTestId("message-row") + .filter({ hasText: threadRootContent }); + await expect(threadRoot).toBeVisible(); + await threadRoot.hover(); + await threadRoot.getByRole("button", { name: "Reply" }).click(); + await expect(page.getByTestId("message-thread-panel")).toBeVisible(); + + await page.getByTestId("project-home-context-tasks").click(); + await expect(page.getByTestId("project-home-workspace-sheet")).toBeVisible(); + const workspaceDrawer = page.getByTestId("focus-thread-drawer"); + await expect(workspaceDrawer).toHaveCount(1); + await expect( + workspaceDrawer.getByTestId("project-home-workspace-sheet"), + ).toBeVisible(); + await expect(page.getByTestId("message-thread-panel")).toHaveCount(1); + await expect(workspaceDrawer.getByTestId("message-thread-panel")).toHaveCount( + 0, + ); + const coveredThreadSurface = page.getByTestId("thread-surface"); + await expect(coveredThreadSurface).toHaveAttribute("inert", ""); + await expect(coveredThreadSurface).toHaveAttribute("aria-hidden", "true"); + const coveredSnapshot = await page.locator("body").ariaSnapshot(); + expect(coveredSnapshot).not.toContain(threadRootContent); + expect(coveredSnapshot).toContain("Tasks"); + expect(coveredSnapshot).toContain("Close panel"); + + const workspaceClose = workspaceDrawer.getByTestId("auxiliary-panel-close"); + await workspaceClose.focus(); + await expect(workspaceClose).toBeFocused(); + for (let index = 0; index < 8; index += 1) { + await page.keyboard.press("Tab"); + expect( + await page.evaluate( + () => + document.activeElement?.closest('[data-testid="thread-surface"]') !== + null, + ), + ).toBe(false); + } + + await workspaceClose.click(); + const exitingWorkspaceState = await page.evaluate(() => { + const workspaceSheet = document.querySelector( + '[data-testid="project-home-workspace-sheet"]', + ); + const threadSurface = document.querySelector( + '[data-testid="thread-surface"]', + ); + return { + threadAriaHidden: threadSurface?.getAttribute("aria-hidden"), + threadInert: threadSurface?.hasAttribute("inert") ?? false, + workspaceSheetMounted: workspaceSheet !== null, + }; + }); + expect(exitingWorkspaceState).toEqual({ + threadAriaHidden: "true", + threadInert: true, + workspaceSheetMounted: true, + }); + await expect(page.getByTestId("project-home-workspace-sheet")).toHaveCount(0); + await expect(page.getByTestId("message-thread-panel")).toBeVisible(); + const threadClose = coveredThreadSurface.getByTestId("auxiliary-panel-close"); + await expect(threadClose).toBeFocused(); + await expect(coveredThreadSurface).not.toHaveAttribute("inert", ""); + await expect(coveredThreadSurface).not.toHaveAttribute("aria-hidden", "true"); + + await page.evaluate(() => { + document.documentElement.style.fontSize = "140%"; + }); + await page.getByTestId("project-home-context-tasks").click(); + await expect(page.getByTestId("project-home-workspace-sheet")).toBeVisible(); + const enlargedTextWorkspaceClose = page + .getByTestId("focus-thread-drawer") + .getByTestId("auxiliary-panel-close"); + await expect + .poll(() => + enlargedTextWorkspaceClose.evaluate((element) => { + const bounds = element.getBoundingClientRect(); + return { + leftInsideViewport: bounds.left >= 0, + rightInsideViewport: bounds.right <= window.innerWidth, + viewportWidth: window.innerWidth, + }; + }), + ) + .toEqual({ + leftInsideViewport: true, + rightInsideViewport: true, + viewportWidth: 820, + }); + await enlargedTextWorkspaceClose.click(); + await expect(page.getByTestId("project-home-workspace-sheet")).toHaveCount(0); + await expect(page.getByTestId("message-thread-panel")).toBeVisible(); + await page.evaluate(() => { + document.documentElement.style.removeProperty("font-size"); + }); + + await page.setViewportSize({ height: 1080, width: 1920 }); + const splitThreadPane = page + .locator( + '[data-testid="message-thread-panel"]:has([data-testid="right-auxiliary-pane-resize-handle"])', + ) + .first(); + await expect(splitThreadPane).toBeVisible(); + const threadResizeHandle = splitThreadPane.getByTestId( + "right-auxiliary-pane-resize-handle", + ); + const threadResizeHandleBox = await threadResizeHandle.boundingBox(); + expect(threadResizeHandleBox).not.toBeNull(); + + await page.getByTestId("project-home-context-tasks").click(); + await expect(page.getByTestId("project-home-workspace-sheet")).toBeVisible(); + await waitForAnimations(page); + const workspaceCoversThreadDivider = await page.evaluate( + ({ x, y }) => { + return Boolean( + document + .elementFromPoint(x, y) + ?.closest('[data-testid="focus-thread-drawer"]'), + ); + }, + { + x: + (threadResizeHandleBox?.x ?? 0) + + (threadResizeHandleBox?.width ?? 0) / 2, + y: (threadResizeHandleBox?.y ?? 0) + 40, + }, + ); + expect(workspaceCoversThreadDivider).toBe(true); + await page.keyboard.press("Escape"); + await expect(page.getByTestId("project-home-workspace-sheet")).toHaveCount(0); + await expect(splitThreadPane).toBeVisible(); + await expect(threadClose).toBeFocused(); }); test("commit detail opens from the commits feed with a diff", async ({