diff --git a/apps/app/src/components/secondary-panel/SecondaryPanelHostLayoutContext.ts b/apps/app/src/components/secondary-panel/SecondaryPanelHostLayoutContext.ts index 0cd146fda..01e637b1f 100644 --- a/apps/app/src/components/secondary-panel/SecondaryPanelHostLayoutContext.ts +++ b/apps/app/src/components/secondary-panel/SecondaryPanelHostLayoutContext.ts @@ -12,9 +12,14 @@ import { createContext } from "react"; * - The right-edge pane headers reserve the window toggle's corner footprint * only while the panel is closed; open, the toggle overlays the panel's own * chrome and the pane actions sit flush at the pane edge. + * - A maximized thread temporarily suppresses the logically open panel without + * mutating its persisted visibility, so restoring the thread restores the + * previous panel layout. */ export interface SecondaryPanelHostLayout { isOpen: boolean; + /** The panel remains logically open but is hidden while a thread is full screen. */ + isSuppressed: boolean; } export const SecondaryPanelHostLayoutContext = diff --git a/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.collapseControl.test.tsx b/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.collapseControl.test.tsx index 6f8ea30a1..937d07330 100644 --- a/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.collapseControl.test.tsx +++ b/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.collapseControl.test.tsx @@ -41,11 +41,10 @@ function renderPanel(args: { ); } -// The conversation-collapse control is the ONLY way back once the conversation -// is hidden — there is no standalone rail to click. If the panel ever stops -// rendering it in the collapsed state, the conversation becomes unrecoverable -// without a keyboard shortcut, so pin both halves of the disclosure pair. -describe("ThreadSecondaryPanel conversation collapse control", () => { +// The full-screen control is the ONLY way back once the conversation is hidden +// — there is no standalone rail to click. Pin both halves of the same-slot +// expansion pair so a full-screen tab can always restore its prior layout. +describe("ThreadSecondaryPanel full-screen control", () => { it("expands the panel while the conversation is shown", () => { const onToggleConversationCollapse = vi.fn(); const view = renderPanel({ @@ -53,8 +52,8 @@ describe("ThreadSecondaryPanel conversation collapse control", () => { onToggleConversationCollapse, }); - const control = view.getByRole("button", { name: "Expand right panel" }); - expect(control.getAttribute("aria-expanded")).toBe("true"); + const control = view.getByRole("button", { name: "Full Screen" }); + expect(control.getAttribute("aria-pressed")).toBe("false"); fireEvent.click(control); expect(onToggleConversationCollapse).toHaveBeenCalledTimes(1); @@ -67,8 +66,8 @@ describe("ThreadSecondaryPanel conversation collapse control", () => { onToggleConversationCollapse, }); - const control = view.getByRole("button", { name: "Restore conversation" }); - expect(control.getAttribute("aria-expanded")).toBe("false"); + const control = view.getByRole("button", { name: "Exit Full Screen" }); + expect(control.getAttribute("aria-pressed")).toBe("true"); fireEvent.click(control); expect(onToggleConversationCollapse).toHaveBeenCalledTimes(1); diff --git a/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.test.ts b/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.test.ts index ea71d1ded..a5cea8e96 100644 --- a/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.test.ts +++ b/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.test.ts @@ -62,7 +62,7 @@ describe("getReservedInlinePanelToggleClassName", () => { }); }); -// BB-46: while the conversation is collapsed inside the split-workspace host and +// BB-46: while the panel is full screen inside the split-workspace host and // the main sidebar is collapsed, the panel is the window's flush top-left // surface, so its leading toolbar shares the title-bar row with the macOS // traffic lights and the pinned sidebar trigger. Without the reserve the @@ -77,7 +77,7 @@ describe("resolveCollapsedPanelTrafficLightReserveClassName", () => { reserveMacosTrafficLights: true, }; - it("reserves the safe area for the collapsed-left / expanded-right split host case", () => { + it("reserves the safe area for the panel full-screen split-host case", () => { expect(resolveCollapsedPanelTrafficLightReserveClassName(base)).toBe( MACOS_COLLAPSED_TOP_LEFT_RESERVE_CLASS, ); diff --git a/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.tsx b/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.tsx index 2170108f1..7a57909c7 100644 --- a/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.tsx +++ b/apps/app/src/components/secondary-panel/ThreadSecondaryPanel.tsx @@ -14,6 +14,7 @@ import { Icon } from "@bb/shared-ui/icon"; import { EmptyStatePanel } from "@bb/shared-ui/empty-state"; import { Panel, PanelResizeHandle } from "react-resizable-panels"; import { Button } from "@bb/shared-ui/button"; +import { HEADER_PANE_ACTION_ICON_BUTTON_CLASS } from "@/components/layout/AppPageHeader"; import { CHROME_SUBTLE_ICON_BUTTON_FOREGROUND_CLASS } from "@/components/ui/chromeStyleTokens"; import { COARSE_POINTER_COMPACT_ICON_BUTTON_CLASS, @@ -270,9 +271,8 @@ export interface ThreadSecondaryPanelProps { isConversationCollapsed: boolean; /** * Toggles {@link isConversationCollapsed}. On a wide viewport the panel header - * renders the expand/restore-conversation control in one slot (immediately - * left of the hide-panel button), so the collapsed conversation is restored - * from the same place it was collapsed. Unused in the drawer/compact layout, + * renders the full-screen/exit-full-screen control (immediately left of the + * hide-panel button) in both states. Unused in the drawer/compact layout, * which cannot collapse the conversation. */ onToggleConversationCollapse: () => void; @@ -401,19 +401,23 @@ export function ThreadSecondaryPanel({ }, [handleSecondaryPanelResize, onPanelResize], ); + const hostLayout = useContext(SecondaryPanelHostLayoutContext); const handlePanelCollapse = useCallback(() => { + if (hostLayout?.isSuppressed) { + return; + } if (!hasPanelExpandedRef.current) { return; } hasPanelExpandedRef.current = false; onCollapse(); - }, [onCollapse]); + }, [hostLayout?.isSuppressed, onCollapse]); // Inside a window-level host, the Panel's mount size must follow the // window's panel visibility: this pane's own persisted state can lag one // commit behind the host's alignment, and the group re-applies defaultSize // after mount — a stale-closed value would collapse the just-opened panel. - const hostLayout = useContext(SecondaryPanelHostLayoutContext); - const isLayoutOpen = hostLayout?.isOpen ?? isOpen; + const isLayoutOpen = + (hostLayout?.isOpen ?? isOpen) && !hostLayout?.isSuppressed; const activeFixedPanel = resolveActiveFixedPanel({ activeTab, canUseGitUi }) ?? "thread-info"; const isDiffPanelActive = activeFixedPanel === "git-diff"; @@ -632,7 +636,7 @@ export function ThreadSecondaryPanel({ onClick={() => onPanelChange("thread-info")} title="Thread info" usesDesktopChrome={usesDesktopChrome} - activeTreatment={isConversationCollapsed ? "underline" : "fill"} + activeTreatment="fill" /> ) : null} {shouldShowGitDiffTab ? ( @@ -649,7 +653,7 @@ export function ThreadSecondaryPanel({ onClick={() => onPanelChange("git-diff")} title="Diff" usesDesktopChrome={usesDesktopChrome} - activeTreatment={isConversationCollapsed ? "underline" : "fill"} + activeTreatment="fill" /> ) : null} {visibleFileTabs && visibleFileTabs.length > 0 ? ( @@ -657,7 +661,7 @@ export function ThreadSecondaryPanel({ fileTabs={visibleFileTabs} onReorderTab={onFileTabReorder} usesDesktopChrome={usesDesktopChrome} - activeTreatment={isConversationCollapsed ? "underline" : "fill"} + activeTreatment="fill" /> ) : null} {showNewTabButton ? ( @@ -677,13 +681,14 @@ export function ThreadSecondaryPanel({ variant="ghost" size="icon" className={cn( - COARSE_POINTER_COMPACT_ICON_BUTTON_CLASS, + HEADER_PANE_ACTION_ICON_BUTTON_CLASS, + CHROME_SUBTLE_ICON_BUTTON_FOREGROUND_CLASS, "shrink-0", usesDesktopChrome && MACOS_WINDOW_NO_DRAG_CLASS, )} onClick={conversationCollapseControl.onClick} aria-label={conversationCollapseControl.label} - aria-expanded={conversationCollapseControl.isExpanded} + aria-pressed={conversationCollapseControl.isFullScreen} > diff --git a/apps/app/src/components/secondary-panel/panelToggleControlState.test.ts b/apps/app/src/components/secondary-panel/panelToggleControlState.test.ts index 75989cb41..61953efa0 100644 --- a/apps/app/src/components/secondary-panel/panelToggleControlState.test.ts +++ b/apps/app/src/components/secondary-panel/panelToggleControlState.test.ts @@ -11,7 +11,7 @@ describe("resolveShowPanelControl", () => { expect(state.action).toBe("show-panel"); expect(state.label).toBe("Show right panel"); - expect(state.isExpanded).toBe(false); + expect(state.isFullScreen).toBe(false); // The recognizable panel icon reads as "open the right side panel". expect(state.iconName).toBe("PanelRight"); @@ -28,10 +28,9 @@ describe("resolveConversationCollapseControl", () => { onToggleConversationCollapse, }); - expect(state.action).toBe("expand-panel"); - expect(state.label).toBe("Expand right panel"); - // The conversation is currently expanded; clicking collapses it. - expect(state.isExpanded).toBe(true); + expect(state.action).toBe("enter-full-screen"); + expect(state.label).toBe("Full Screen"); + expect(state.isFullScreen).toBe(false); // The shared four-arrow glyph clearly expands the panel to fill the canvas. expect(state.iconName).toBe("Maximize2"); @@ -46,9 +45,9 @@ describe("resolveConversationCollapseControl", () => { onToggleConversationCollapse, }); - expect(state.action).toBe("restore-conversation"); - expect(state.label).toBe("Restore conversation"); - expect(state.isExpanded).toBe(false); + expect(state.action).toBe("exit-full-screen"); + expect(state.label).toBe("Exit Full Screen"); + expect(state.isFullScreen).toBe(true); // The matching four-arrow collapse glyph restores the split layout. expect(state.iconName).toBe("Minimize2"); diff --git a/apps/app/src/components/secondary-panel/panelToggleControlState.ts b/apps/app/src/components/secondary-panel/panelToggleControlState.ts index 412ccdc27..61adbadb8 100644 --- a/apps/app/src/components/secondary-panel/panelToggleControlState.ts +++ b/apps/app/src/components/secondary-panel/panelToggleControlState.ts @@ -1,7 +1,7 @@ export type PanelToggleAction = | "show-panel" - | "expand-panel" - | "restore-conversation"; + | "enter-full-screen" + | "exit-full-screen"; /** * Icon names the toggle can render. A subset of the Icon component's `IconName` @@ -13,11 +13,10 @@ interface PanelToggleActionPresentation { label: string; iconName: PanelToggleIconName; /** - * `aria-expanded` reflects whether the conversation pane is currently - * expanded (shown). The collapse toggle flips it; the "show panel" button is - * never an expanded disclosure. + * Whether the action is currently presenting the panel in full-screen mode. + * This drives the toggle button's `aria-pressed` state. */ - isExpanded: boolean; + isFullScreen: boolean; } /** @@ -30,36 +29,32 @@ interface PanelToggleActionPresentation { * reads as "open the right side panel" — matching the * in-panel hide button. Lives in the conversation * header, only while the panel is closed. - * expand-panel → conversation shown: expand the right panel so it - * fills the content area. Renders the shared four-arrow - * expand glyph. Lives in the panel header. - * restore-conversation → conversation collapsed: restore it. Renders the - * matching four-arrow collapse glyph. Lives in the - * panel header, in the same slot the expand action - * occupies, so the pair toggles in place. + * enter-full-screen → expand the right panel to fill the content area. + * exit-full-screen → restore the previous thread-and-panel layout. + * Both actions stay in the panel header so the control transforms in place. */ const PANEL_TOGGLE_ACTION_PRESENTATION = { "show-panel": { label: "Show right panel", iconName: "PanelRight", - isExpanded: false, + isFullScreen: false, }, - "expand-panel": { - label: "Expand right panel", + "enter-full-screen": { + label: "Full Screen", iconName: "Maximize2", - isExpanded: true, + isFullScreen: false, }, - "restore-conversation": { - label: "Restore conversation", + "exit-full-screen": { + label: "Exit Full Screen", iconName: "Minimize2", - isExpanded: false, + isFullScreen: true, }, } as const satisfies Record; export interface PanelToggleControlState { action: PanelToggleAction; label: string; - isExpanded: boolean; + isFullScreen: boolean; iconName: PanelToggleIconName; onClick: () => void; } @@ -98,8 +93,8 @@ export function resolveConversationCollapseControl({ onToggleConversationCollapse, }: ResolveConversationCollapseControlArgs): PanelToggleControlState { const action: PanelToggleAction = isConversationCollapsed - ? "restore-conversation" - : "expand-panel"; + ? "exit-full-screen" + : "enter-full-screen"; return { action, ...PANEL_TOGGLE_ACTION_PRESENTATION[action], diff --git a/apps/app/src/components/sidebar/sidebarRowClasses.test.ts b/apps/app/src/components/sidebar/sidebarRowClasses.test.ts new file mode 100644 index 000000000..05d56ca23 --- /dev/null +++ b/apps/app/src/components/sidebar/sidebarRowClasses.test.ts @@ -0,0 +1,12 @@ +import { describe, expect, it } from "vitest"; +import { CONTEXT_SELECTION_SURFACE_CLASS } from "@/components/ui/context-selection"; +import { SIDEBAR_ROW_SELECTED_STATE_CLASS } from "./sidebarRowClasses"; + +describe("sidebar selected thread styling", () => { + it("uses the shared active-context surface", () => { + expect(SIDEBAR_ROW_SELECTED_STATE_CLASS).toContain( + CONTEXT_SELECTION_SURFACE_CLASS, + ); + expect(CONTEXT_SELECTION_SURFACE_CLASS).toBe("bg-state-active"); + }); +}); diff --git a/apps/app/src/components/ui/context-selection.ts b/apps/app/src/components/ui/context-selection.ts index 7ac1fe68c..b21ffe37a 100644 --- a/apps/app/src/components/ui/context-selection.ts +++ b/apps/app/src/components/ui/context-selection.ts @@ -2,7 +2,7 @@ * Shared selected surface for the three views of the active thread context: * its left-sidebar row, focused split tab, and active right-panel tool tab. */ -export const CONTEXT_SELECTION_SURFACE_CLASS = "bg-muted"; +export const CONTEXT_SELECTION_SURFACE_CLASS = "bg-state-active"; /** Shared title treatment for split panes outside the active thread context. */ export const CONTEXT_INACTIVE_TEXT_CLASS = "text-muted-foreground/60"; diff --git a/apps/app/src/components/ui/tab-pill.test.tsx b/apps/app/src/components/ui/tab-pill.test.tsx index 6fd3f25c8..1eac9acd5 100644 --- a/apps/app/src/components/ui/tab-pill.test.tsx +++ b/apps/app/src/components/ui/tab-pill.test.tsx @@ -46,7 +46,7 @@ describe("TabPill", () => { name: "Show thread info panel", }); expect(tab.getAttribute("aria-pressed")).toBe("true"); - expect(tab.parentElement?.classList).toContain("bg-muted"); + expect(tab.parentElement?.classList).toContain("bg-state-active"); expect(screen.getByText("Info").classList).toContain("sr-only"); }); @@ -64,7 +64,7 @@ describe("TabPill", () => { const shell = screen.getByRole("button", { name: "Browser" }).parentElement; expect(shell?.classList).toContain("after:h-0.5"); - expect(shell?.classList).not.toContain("bg-muted"); + expect(shell?.classList).not.toContain("bg-state-active"); }); it("holds associated controls until overlapping pointer and keyboard intent both end", () => { diff --git a/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx b/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx index 9fe354b27..5d2e1db48 100644 --- a/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx +++ b/apps/app/src/views/thread-detail/SplitThreadArea.test.tsx @@ -48,7 +48,7 @@ const threadStore = vi.hoisted( const experimentState = vi.hoisted(() => ({ enabled: true })); const viewportState = vi.hoisted(() => ({ compact: false })); const sidebarState = vi.hoisted(() => ({ showing: true })); -const collapsedRailState = vi.hoisted(() => ({ +const panelFullScreenState = vi.hoisted(() => ({ isMainCollapsed: false, })); const panelGroupLayoutState = vi.hoisted(() => ({ layout: [100, 0] })); @@ -185,7 +185,7 @@ vi.mock("./ThreadDetailView", () => ({ () => ({ composerHost, contentKey: threadId, - isMainCollapsed: collapsedRailState.isMainCollapsed, + isMainCollapsed: panelFullScreenState.isMainCollapsed, isOpen: isPanelOpen, panel: (
@@ -472,7 +472,7 @@ beforeEach(() => { experimentState.enabled = true; viewportState.compact = false; sidebarState.showing = true; - collapsedRailState.isMainCollapsed = false; + panelFullScreenState.isMainCollapsed = false; panelGroupLayoutState.layout = [100, 0]; commandHandlers.clear(); commandPresentationState.isModifierHeld = false; @@ -535,8 +535,8 @@ describe("SplitThreadArea", () => { }); }); - it("removes the collapsed-thread rail while maximized and keeps the restore control", async () => { - collapsedRailState.isMainCollapsed = true; + it("temporarily replaces panel full screen with a clean thread full screen", async () => { + panelFullScreenState.isMainCollapsed = true; renderSplitArea({ path: threadPath("thr-a"), layout: twoPaneLayout("pane-1"), @@ -545,15 +545,16 @@ describe("SplitThreadArea", () => { expect(screen.queryByTestId("mock-collapsed-thread-rail")).toBeNull(); fireEvent.click(screen.getByTestId("maximize-thr-a")); - expect(screen.queryByTestId("mock-collapsed-thread-rail")).toBeNull(); expect(screen.getByTestId("maximize-thr-a").textContent).toBe("restore"); await waitFor(() => { - expect(panelGroupLayoutState.layout[0]).toBeGreaterThan(0); - expect(panelGroupLayoutState.layout[1]).toBeLessThan(100); + expect(panelGroupLayoutState.layout).toEqual([100, 0]); }); fireEvent.click(screen.getByTestId("maximize-thr-a")); expect(screen.queryByTestId("mock-collapsed-thread-rail")).toBeNull(); + await waitFor(() => { + expect(panelGroupLayoutState.layout).toEqual([0, 100]); + }); }); it("preserves a hidden pane's mounted scroll position through restore", async () => { @@ -902,7 +903,7 @@ describe("SplitThreadArea", () => { "[data-pane-header-focus-tab]", ); expect(focusedTab).not.toBeNull(); - expect(focusedTab?.classList).toContain("bg-muted"); + expect(focusedTab?.classList).toContain("bg-state-active"); expect(focusedTab?.classList).not.toContain("shadow-sm"); expect(screen.getByText("New thread").classList).toContain("font-normal"); expect(screen.getByText("New thread").classList).not.toContain( diff --git a/apps/app/src/views/thread-detail/SplitWorkspaceSecondaryPanelHost.tsx b/apps/app/src/views/thread-detail/SplitWorkspaceSecondaryPanelHost.tsx index ef8edb255..0e91edcf9 100644 --- a/apps/app/src/views/thread-detail/SplitWorkspaceSecondaryPanelHost.tsx +++ b/apps/app/src/views/thread-detail/SplitWorkspaceSecondaryPanelHost.tsx @@ -124,11 +124,15 @@ export function SplitWorkspaceSecondaryPanelHost({ // Both panels register with the group asynchronously on mount; until they // have, setLayout throws and defaultSize already encodes this state. if (group.getLayout().length !== 2) return; + if (isPaneMaximized) { + group.setLayout([MAIN_PANEL_OPEN_SIZE_PERCENT, 0]); + return; + } if (!isOpen) { group.setLayout([MAIN_PANEL_OPEN_SIZE_PERCENT, 0]); return; } - if (model?.isMainCollapsed && !isPaneMaximized) { + if (model?.isMainCollapsed) { group.setLayout([0, MAIN_PANEL_OPEN_SIZE_PERCENT]); return; } @@ -179,8 +183,8 @@ export function SplitWorkspaceSecondaryPanelHost({ const toggleLabel = isOpen ? "Hide right panel" : "Show right panel"; const hostLayout = useMemo( - () => ({ isOpen }), - [isOpen], + () => ({ isOpen, isSuppressed: isPaneMaximized }), + [isOpen, isPaneMaximized], ); return ( @@ -196,6 +200,7 @@ export function SplitWorkspaceSecondaryPanelHost({ data-testid="split-workspace-panel-toggle" className={cn( "absolute right-2.5 top-2.5 z-40", + isPaneMaximized && "hidden", // This overlay already owns positioning and stacking. Use only the // raw app-region token: MACOS_WINDOW_NO_DRAG_CLASS adds `relative // z-50`, which tailwind-merge would resolve against `absolute` and @@ -238,11 +243,13 @@ export function SplitWorkspaceSecondaryPanelHost({ collapsible collapsedSize={0} defaultSize={ - model?.isMainCollapsed && !isPaneMaximized - ? 0 - : isOpen - ? MAIN_PANEL_OPEN_SIZE_PERCENT - panelWidthPercent - : MAIN_PANEL_OPEN_SIZE_PERCENT + isPaneMaximized + ? MAIN_PANEL_OPEN_SIZE_PERCENT + : model?.isMainCollapsed + ? 0 + : isOpen + ? MAIN_PANEL_OPEN_SIZE_PERCENT - panelWidthPercent + : MAIN_PANEL_OPEN_SIZE_PERCENT } minSize={MAIN_PANEL_MIN_SIZE_PERCENT} order={1} @@ -252,10 +259,7 @@ export function SplitWorkspaceSecondaryPanelHost({ )} > {/* Panel renders a plain block; the split tree sizes itself with - flex-1, so restore a full-height flex context for it. `relative` - makes this the containing block for a maximized pane's - `absolute inset-0`, so fullscreening a pane fills the main panel - without covering the open secondary panel to its right. */} + flex-1, so restore a full-height flex context for it. */}
{children}
diff --git a/apps/app/src/views/thread-detail/ThreadDetailHeader.test.tsx b/apps/app/src/views/thread-detail/ThreadDetailHeader.test.tsx index 61b4f0d50..6f744b327 100644 --- a/apps/app/src/views/thread-detail/ThreadDetailHeader.test.tsx +++ b/apps/app/src/views/thread-detail/ThreadDetailHeader.test.tsx @@ -142,9 +142,7 @@ describe("ThreadDetailHeader", () => { expect(screen.getByText("Responsive menu actions")).not.toBeNull(); const closePane = screen.getByRole("button", { name: "Close pane" }); expect(closePane.classList).toContain("header-pane-action-button"); - const closeIcon = closePane.querySelector( - '[data-icon="CloseThreadPane"]', - ); + const closeIcon = closePane.querySelector('[data-icon="CloseThreadPane"]'); expect(closeIcon).not.toBeNull(); expect(closeIcon?.querySelectorAll("path")).toHaveLength(1); expect(closeIcon?.querySelector("path")?.getAttribute("d")).toContain( @@ -250,7 +248,7 @@ describe("ThreadDetailHeader", () => { "[data-pane-header-focus-tab]", ); expect(focusedTab).not.toBeNull(); - expect(focusedTab?.classList).toContain("bg-muted"); + expect(focusedTab?.classList).toContain("bg-state-active"); expect(focusedTab?.classList).not.toContain("shadow-sm"); expect(container.querySelector("[data-app-page-header-dim]")).toBeNull(); const activeTitle = screen.getByText("Focused thread");