From de46fb03655db5ac083bfabaea4eb1656eeaab92 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 31 Jul 2026 23:48:56 -0500 Subject: [PATCH 1/5] fix(desktop): make the terminal toggle work wherever the terminal sits Dragging the terminal to the bottom stacks logs into its zone, and every tool-panel toggle broke in that stack. Boot revealed instead of leaving the tree alone: bindPaneCollapse ran setPaneCollapsed(id, !open), and `false` there fronts the pane. Logs binds last, so it stole the active tab from the persisted tree. Ctrl-` then asked to collapse a terminal that was no longer the active tab, the shared-zone branch declined by design, and the key did nothing until the stack was broken up. Boot now only ever collapses. The toggles also asked the wrong question. !$terminalTakeover.get() flips a boolean that has no idea what is on screen, so once anything else moved the pane the press spent itself re-asserting a value the store already held. toggleToolPane derives from the tree instead, and ctrl-`, Cmd+J's terminal fallback, the statusbar button and the logs palette row all route through it. The terminal cycle/close keys drop the same stale boolean. bindPaneCollapse moves into the tree store as bindToolPaneCollapse so the boot rule is testable against the real function rather than a copy. --- apps/desktop/src/app/contrib/controller.tsx | 48 +++---- apps/desktop/src/app/hooks/use-keybinds.ts | 25 ++-- .../app/shell/hooks/use-statusbar-items.tsx | 15 ++- .../src/components/pane-shell/tree/store.ts | 127 ++++++++++++++---- 4 files changed, 147 insertions(+), 68 deletions(-) diff --git a/apps/desktop/src/app/contrib/controller.tsx b/apps/desktop/src/app/contrib/controller.tsx index 9e01cde324..7243f7dc90 100644 --- a/apps/desktop/src/app/contrib/controller.tsx +++ b/apps/desktop/src/app/contrib/controller.tsx @@ -13,11 +13,12 @@ import { LayoutTreeRoot } from '@/components/pane-shell/tree/renderer' import type { DoubleTapContext } from '@/components/pane-shell/tree/renderer/drag-session' import { $layoutTree, + bindToolPaneCollapse, bindTreeSideVisibility, declareDefaultTree, dismissTreePane, dockPaneBeside, - markCollapsePane, + isToolPaneVisible, mirrorLayoutTree, paneRootSide, registerLayoutResetHandler, @@ -25,8 +26,8 @@ import { registerPaneOpener, resetLayoutTree, revealTreePane, - setPaneCollapsed, setTreePaneHidden, + toggleToolPane, watchContributedPanes } from '@/components/pane-shell/tree/store' import { SidebarProvider } from '@/components/ui/sidebar' @@ -175,7 +176,11 @@ registry.registerMany([ // staying collapsed behind the ⌃` toggle. height sizes the fixed track (a // single-pane zone declaring a height is a fixed track — the preset weight // is moot): a short deck, not a third of the window. - data: { placement: 'bottom', height: '20vh', minHeight: '7.5rem', maxHeight: '80vh', revealOnPreset: true }, + // + // NO minHeight: a tool panel drags all the way down to its collapsed + // header (the sash floors it at COLLAPSED_ZONE_PX and folds the zone to + // its rail there). A real floor left a sliver of unusable terminal. + data: { placement: 'bottom', height: '20vh', maxHeight: '80vh', revealOnPreset: true }, render: () => }, { @@ -236,7 +241,8 @@ registry.registerMany([ title: 'logs', // revealOnPreset: the Quad layout places logs, so applying it turns the // logs pane on (like a ⌘K "Toggle logs") instead of leaving it collapsed. - data: { placement: 'bottom', height: '20vh', minHeight: '7.5rem', maxHeight: '80vh', revealOnPreset: true }, + // No minHeight — same tool-panel rule as the terminal above. + data: { placement: 'bottom', height: '20vh', maxHeight: '80vh', revealOnPreset: true }, render: () => idle() } ]) @@ -488,27 +494,9 @@ function bindPaneVisibility( } } -// TOOL PANELS (terminal, logs): like bindPaneVisibility but the toggle COLLAPSES -// the zone to a persistent rail (tab stays) instead of hiding it — the -// IntelliJ/VS-Code tool-window model. Restore routes back through `open` (rail -// click / chevron) so ⌃`/the button stay truthful; Close removes the tab. -// -// OPEN goes through revealTreePane, not setPaneCollapsed: Close DISMISSES the -// pane, and setPaneCollapsed can't act on a pane that has left the tree — the -// toggle would flip its store with nothing coming back. revealTreePane -// un-dismisses and re-adopts. -function bindPaneCollapse( - paneId: string, - $open: { get(): boolean; listen(fn: (open: boolean) => void): void }, - close: () => void, - open: () => void -) { - markCollapsePane(paneId) - setPaneCollapsed(paneId, !$open.get()) - $open.listen(isOpen => (isOpen ? revealTreePane(paneId) : setPaneCollapsed(paneId, true))) - registerPaneCloser(paneId, close) - registerPaneOpener(paneId, open) -} +// TOOL PANELS (terminal, logs): the binding lives in the tree store — +// bindToolPaneCollapse — so the boot rule it encodes is testable against the +// real function instead of a copy. See its docblock for the semantics. // SIDES have one source of truth: the TREE. The legacy $panesFlipped flag is // DERIVED from where the sessions zone actually sits (TitlebarControls maps @@ -573,7 +561,7 @@ bindPaneVisibility( ) // ⌃` / statusbar toggle — the terminal COLLAPSES to a rail (tab stays), not // hides; PTYs stay alive while collapsed (see PersistentTerminal). -bindPaneCollapse( +bindToolPaneCollapse( 'terminal', $terminalTakeover, () => setTerminalTakeover(false), @@ -591,7 +579,7 @@ bindPaneVisibility('preview', $previewVisible, closeRightRail) // Logs are optional chrome: off by default, toggled from ⌘K, persisted. const $logsOpen = persistentAtom('hermes.desktop.logsOpen', false, Codecs.bool) -bindPaneCollapse( +bindToolPaneCollapse( 'logs', $logsOpen, () => $logsOpen.set(false), @@ -603,8 +591,10 @@ registry.register( label: 'Toggle logs', icon: FileText, keywords: ['logs', 'agent log', 'tail', 'debug'], - get: () => $logsOpen.get(), - set: enabled => $logsOpen.set(enabled) + // On-screen, not the store's boolean: logs stacks with the terminal, and + // behind its sibling's tab `$logsOpen` stays true while nothing is visible. + get: () => isToolPaneVisible('logs'), + set: () => toggleToolPane('logs') }) ) diff --git a/apps/desktop/src/app/hooks/use-keybinds.ts b/apps/desktop/src/app/hooks/use-keybinds.ts index dcd97cf29c..3b3677815c 100644 --- a/apps/desktop/src/app/hooks/use-keybinds.ts +++ b/apps/desktop/src/app/hooks/use-keybinds.ts @@ -2,9 +2,15 @@ import { useEffect, useRef } from 'react' import { useNavigate } from 'react-router' import { closeActiveTab } from '@/app/chat/close-tab' -import { $terminalTakeover, setTerminalTakeover } from '@/app/right-sidebar/store' +import { setTerminalTakeover } from '@/app/right-sidebar/store' import { closeActiveTerminal, createTerminal, cycleTerminal } from '@/app/right-sidebar/terminal/terminals' -import { activateTreeTabSlot, cycleTreeTabInFocusedZone, layoutHasRootSide } from '@/components/pane-shell/tree/store' +import { + activateTreeTabSlot, + cycleTreeTabInFocusedZone, + isToolPaneVisible, + layoutHasRootSide, + toggleToolPane +} from '@/components/pane-shell/tree/store' import { onReleaseTypingFocus } from '@/components/ui/keyboard-first' import { findBarClaimsCombo } from '@/lib/find-in-page' import { contributedKeybindHandler, PROFILE_SLOT_COUNT, SESSION_SLOT_COUNT } from '@/lib/keybinds/actions' @@ -184,22 +190,23 @@ export function useKeybinds(deps: KeybindRuntimeDeps): void { // terminal-on-bottom) would leave it a dead key, so it falls back to the // terminal there. The single "secondary panel" toggle. 'view.toggleRightSidebar': () => - layoutHasRootSide('right') ? toggleFileBrowserOpen() : setTerminalTakeover(!$terminalTakeover.get()), + layoutHasRootSide('right') ? toggleFileBrowserOpen() : toggleToolPane('terminal'), 'view.toggleReview': toggleReview, 'view.toggleStatusbar': toggleStatusbarVisible, 'view.showFiles': showFiles, - 'view.showTerminal': () => setTerminalTakeover(!$terminalTakeover.get()), + 'view.showTerminal': () => toggleToolPane('terminal'), // Create first so the pane's open-effect ensure sees a non-empty set and // doesn't also spawn one — net effect is exactly one fresh terminal. 'view.newTerminal': () => { createTerminal() setTerminalTakeover(true) }, - // Switch / close only act while the pane is open (no focus-scoping here, so - // this stands in for "terminal is showing"). - 'view.nextTerminal': () => $terminalTakeover.get() && cycleTerminal(1), - 'view.prevTerminal': () => $terminalTakeover.get() && cycleTerminal(-1), - 'view.closeTerminal': () => $terminalTakeover.get() && closeActiveTerminal(), + // Switch / close only act while the terminal is actually ON SCREEN — ask + // the tree, not the toggle store (which stays true behind a stacked + // sibling tab or a minimized zone). + 'view.nextTerminal': () => isToolPaneVisible('terminal') && cycleTerminal(1), + 'view.prevTerminal': () => isToolPaneVisible('terminal') && cycleTerminal(-1), + 'view.closeTerminal': () => isToolPaneVisible('terminal') && closeActiveTerminal(), 'view.flipPanes': togglePanesFlipped, // ⌘W: close the focused tab (terminal / preview target / zone tree tab). // On the main tab with session tabs stacked, it shifts the next one in — diff --git a/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx b/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx index 0aa3c69fe3..fa81c227ad 100644 --- a/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx +++ b/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx @@ -2,10 +2,10 @@ import { useStore } from '@nanostores/react' import { useCallback, useMemo } from 'react' import type { CommandCenterSection } from '@/app/command-center' -import { $terminalTakeover, setTerminalTakeover } from '@/app/right-sidebar/store' import { useApprovalModeStatusbarItem } from '@/app/shell/approval-mode-menu' import { ContextUsagePanel } from '@/app/shell/context-usage-panel' import { GatewayMenuPanel } from '@/app/shell/gateway-menu-panel' +import { $toolPaneVisible, toggleToolPane } from '@/components/pane-shell/tree/store' import { Codicon } from '@/components/ui/codicon' import { GlyphSpinner } from '@/components/ui/glyph-spinner' import { useI18n } from '@/i18n' @@ -92,7 +92,10 @@ export function useStatusbarItems({ const fileMenu = t.fileMenu const primaryActiveSessionId = useStore($activeSessionId) const activeGatewayProfile = useStore($activeGatewayProfile) - const terminalTakeover = useStore($terminalTakeover) + // What the button paints and flips is whether the terminal is ON SCREEN — + // the takeover store alone stays true behind a stacked sibling tab or a + // minimized zone, which lit the button for a pane the user couldn't see. + const terminalShowing = useStore($toolPaneVisible('terminal')) const primaryBusy = useStore($busy) const currentCwd = useStore($currentCwd) // Derive the workspace's project name from the already-cached project tree @@ -506,12 +509,12 @@ export function useStatusbarItems({ }, { actionId: 'view.showTerminal', - className: `w-7 justify-center px-0${terminalTakeover ? ' bg-accent/55 text-foreground' : ''}`, + className: `w-7 justify-center px-0${terminalShowing ? ' bg-accent/55 text-foreground' : ''}`, hidden: !chatOpen, icon: , id: 'terminal', - onSelect: () => setTerminalTakeover(!$terminalTakeover.get()), - title: terminalTakeover ? copy.hideTerminal : copy.showTerminal, + onSelect: () => toggleToolPane('terminal'), + title: terminalShowing ? copy.hideTerminal : copy.showTerminal, toggleLabel: copy.toggleTerminal, variant: 'action' }, @@ -533,7 +536,7 @@ export function useStatusbarItems({ requestGateway, sessionStartedAt, gatewayState, - terminalTakeover, + terminalShowing, turnStartedAt ] ) diff --git a/apps/desktop/src/components/pane-shell/tree/store.ts b/apps/desktop/src/components/pane-shell/tree/store.ts index 548ee352fe..9c9be382ba 100644 --- a/apps/desktop/src/components/pane-shell/tree/store.ts +++ b/apps/desktop/src/components/pane-shell/tree/store.ts @@ -4,7 +4,7 @@ * the persisted tree is the user's customization; reset returns to default. */ -import { atom, computed } from 'nanostores' +import { atom, computed, type ReadableAtom } from 'nanostores' import { SIDEBAR_COLLAPSE_MEDIA_QUERY } from '@/app/layout-constants' import { setPluginEnabled } from '@/contrib/plugins-store' @@ -1050,16 +1050,27 @@ function adoptContributedPanes(): void { const target = findGroupOfPane(next, anchor ?? '')?.id if (target) { + // Whether the DESTINATION zone's header was explicitly hidden, read + // BEFORE the insert — `insertAtGroup` pins `headerHidden: false` on a + // center drop (a stack you can't see is a trap), which is right for a + // drag but wrong for adoption into a zone whose bar the user hid. + const hostHeaderHidden = findGroup(next, target)?.headerHidden === true + // Silent adoption: don't front over the zone's active tab — a reveal does. next = insertAtGroup(next, target, pane.id, dock?.pos ?? 'center', dock?.before, false) ?? next // An adopted pane ARRIVES with its chip showing — a surprise zone with // zero chrome has no obvious handle to drag or close. (Explicit reveal; // the next structural op returns lone panes to the auto-hide default.) + // + // EXCEPT into a zone whose header the user explicitly hid: that's a + // standing preference about the zone, not a stale default. Without this + // the bar came back every time a tool panel was closed and toggled on + // again — Close dismisses the pane, the toggle re-adopts it through here. const landed = findGroupOfPane(next, pane.id) if (landed) { - next = setGroupHeaderHiddenOp(next, landed.id, false) + next = setGroupHeaderHiddenOp(next, landed.id, hostHeaderHidden) } } } @@ -1315,34 +1326,102 @@ export function setPaneCollapsed(paneId: string, collapsed: boolean) { } /** Restore a minimized tool pane the truthful way — through its store opener - * when bound (keeps ⌃`/titlebar toggles in sync), else just un-minimize + - * front. Used by the rail (tab / whole-rail click) and the header chevron. */ + * when bound (keeps ⌃`/titlebar toggles in sync), then reveal regardless. + * Used by the rail (tab / whole-rail click), the header chevron, and ⌃`. + * + * The opener is fire-and-forget because it may be a NO-OP: the store can + * already be `true` while the pane is off screen (the zone was minimized from + * the zone menu, the tab was closed with ⌘W, or a stacked sibling holds the + * active slot). nanostores don't fire listeners on a same-value `.set()`, so + * the bindPaneCollapse listener never runs. `revealTreePane` is idempotent and + * does the real work — un-dismiss, un-collapse the side, un-minimize, front. */ export function restoreTreePane(paneId: string) { - const open = paneOpeners[paneId] + paneOpeners[paneId]?.() + revealTreePane(paneId) +} - if (open) { - open() - - // The opener may be a no-op — the store was already true (zone minimized - // via the zone menu, not the toggle). nanostores don't fire listeners on - // a same-value .set(), so the bindPaneCollapse listener never runs and - // the zone stays minimized. Un-minimize directly when that happens. - const group = paneGroup(paneId) - - if (group?.minimized) { - setTreeGroupMinimized(group.id, false) - } - - revealTreePane(paneId) - - return +/** Is a tool pane actually ON SCREEN? In the tree, not dismissed or chrome + * hidden, its zone un-minimized, and holding its stack's active slot. */ +export function isToolPaneVisible(paneId: string): boolean { + if ($dismissedPanes.get().has(paneId) || $hiddenTreePanes.get().has(paneId)) { + return false } const group = paneGroup(paneId) - if (group) { - setTreeGroupMinimized(group.id, false) - activateTreePane(group.id, paneId) + return Boolean(group && !group.minimized && group.active === paneId) +} + +const toolPaneVisibleCache = new Map>() + +/** Reactive `isToolPaneVisible` for chrome that renders an on/off affordance + * (the statusbar's terminal button). Memoized per pane id so `useStore` + * subscriptions stay referentially stable across renders. */ +export function $toolPaneVisible(paneId: string): ReadableAtom { + let cached = toolPaneVisibleCache.get(paneId) + + if (!cached) { + cached = computed([$layoutTree, $dismissedPanes, $hiddenTreePanes], () => isToolPaneVisible(paneId)) + toolPaneVisibleCache.set(paneId, cached) + } + + return cached +} + +/** + * TOOL PANELS (terminal, logs): bind a pane's visibility STORE to the tree so + * its toggle COLLAPSES the zone to a persistent rail (the tab stays) instead of + * hiding it — the IntelliJ/VS-Code tool-window model. Restore routes back + * through `open` (rail click / chevron) so ⌃` and the statusbar button stay + * truthful; Close removes the tab. + * + * OPEN goes through `revealTreePane`, not `setPaneCollapsed`: Close DISMISSES + * the pane, and `setPaneCollapsed` is a no-op on a pane that has left the tree, + * so the toggle would flip its store with nothing coming back. `revealTreePane` + * un-dismisses and re-adopts. + * + * BOOT ONLY COLLAPSES — it must never reveal. `setPaneCollapsed(id, false)` + * fronts the pane in its stack, so binding two tool panels that are both "open" + * let the second one steal the active tab from the persisted tree. With + * terminal+logs stacked (what you get by dragging the terminal to the bottom), + * logs bound last and won the slot; ⌃` then asked to collapse a terminal that + * wasn't the active tab, the shared-zone branch declined, and the key read as + * dead until the stack was broken up. The persisted tree already records which + * tab was active — leave it alone. + */ +export function bindToolPaneCollapse( + paneId: string, + $open: { get(): boolean; listen(fn: (open: boolean) => void): void }, + close: () => void, + open: () => void +) { + markCollapsePane(paneId) + + if (!$open.get()) { + setPaneCollapsed(paneId, true) + } + + $open.listen(isOpen => (isOpen ? revealTreePane(paneId) : setPaneCollapsed(paneId, true))) + registerPaneCloser(paneId, close) + registerPaneOpener(paneId, open) +} + +/** + * ⌃` / the statusbar button / ⌘J's terminal fallback: ONE resolver for "flip + * this tool panel", derived from what is on screen rather than from the + * toggle's own boolean. + * + * A free-floating `!$open.get()` diverges from the tree the moment anything + * else moves the pane — stacked behind a sibling tab, minimized from the zone + * menu, closed with ⌘W — and then the toggle spends its press re-asserting a + * value the store already held, which reads as a dead key. Asking the tree + * instead means the first press always does the visible thing. + */ +export function toggleToolPane(paneId: string) { + if (isToolPaneVisible(paneId)) { + collapseTreePane(paneId) + } else { + restoreTreePane(paneId) } } From 4802d2d88d92bff9812d0544fd983b4daacea407 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 31 Jul 2026 23:49:11 -0500 Subject: [PATCH 2/5] fix(desktop): keep a hidden zone header hidden MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two independent clobbers threw the choice away, so "hide header" never survived a close/reopen cycle. normalize() deleted headerHidden whenever a zone dropped to one pane, on the grounds that a lone zone is headerless by default. It is — but the flag is the user's standing preference for that zone, not a redundant value, and dropping it meant the bar returned the moment a pane rejoined (close the stacked logs pane, toggle it back). Re-adoption then pinned headerHidden false unconditionally. That is right for a pane arriving somewhere new, where zero chrome leaves no handle to drag or close, and wrong for a zone whose bar the user deliberately hid. Adoption now carries the destination zone's own setting, read before the insert since insertAtGroup pins the flag itself on a center drop. --- .../src/components/pane-shell/tree/model.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/apps/desktop/src/components/pane-shell/tree/model.ts b/apps/desktop/src/components/pane-shell/tree/model.ts index 0834360b0a..7f61143bab 100644 --- a/apps/desktop/src/components/pane-shell/tree/model.ts +++ b/apps/desktop/src/components/pane-shell/tree/model.ts @@ -134,17 +134,18 @@ export function normalize(node: LayoutNode): LayoutNode | null { } const active = node.panes.includes(node.active) ? node.active : node.panes[0] - // A zone down to one pane clears a redundant HIDDEN override (the lone-pane - // default is already headerless) but KEEPS an explicit SHOWN override — - // once a zone has ever had a tab bar, closing back to one tab leaves it - // shown (sticky bar; the off switch is "Hide tab bar"). `false` survives. - const headerHidden = node.panes.length <= 1 && node.headerHidden !== false ? undefined : node.headerHidden - if (active === node.active && headerHidden === node.headerHidden) { + // NOTE: `headerHidden` is deliberately untouched here. A zone down to one + // pane is headerless by default anyway, so a stored `true` is visually + // redundant *while it's alone* — but normalize used to DROP it, which threw + // away the user's standing choice: the bar came back the moment a pane + // rejoined (close a stacked tool panel, toggle it back on). `false` is + // sticky for the mirror reason — once a zone has had a tab bar, it keeps it. + if (active === node.active) { return node } - return { ...node, active, headerHidden } + return { ...node, active } } const children: LayoutNode[] = [] From aca571ef49368f74fb6e004a77fd58b92f9d80ba Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 31 Jul 2026 23:49:28 -0500 Subject: [PATCH 3/5] feat(desktop): let a tool panel shrink to its collapsed header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The terminal and logs carried a 7.5rem minHeight, so dragging the seam down jammed against a floor with a sliver of unusable terminal still showing. Tool-panel zones now floor at COLLAPSED_ZONE_PX (the h-7 header strip) instead of the generic 80px, and releasing at that floor minimizes the zone rather than persisting the sliver — it folds to its collapsed header, vertical rail or horizontal strip depending on the parent axis. The sliver size is never written, so restoring returns the height it had before. --- .../pane-shell/tree/renderer/track-model.ts | 9 ++++ .../pane-shell/tree/renderer/tree-split.tsx | 44 ++++++++++++++++--- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts b/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts index 7dd10012aa..67dbdac932 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts +++ b/apps/desktop/src/components/pane-shell/tree/renderer/track-model.ts @@ -19,6 +19,15 @@ import type { FloatingAnchor } from './floating-rect' export const MIN_PANE_PX = 80 +/** + * The floor for a TOOL PANEL zone (terminal / logs) instead of `MIN_PANE_PX`. + * A tool panel is meant to be draggable down to nothing — the minimized rail + * (its header strip, `h-7`) is the smallest meaningful form, so the sash lets + * it shrink to exactly that and then collapses the zone rather than jamming + * against an 80px floor with a sliver of unusable content still showing. + */ +export const COLLAPSED_ZONE_PX = 28 + /** Optional CSS sizing a pane contributes (`data.width` / `data.minWidth`…). * Applied to the pane's GROUP along the axis of the split that contains it — * the same semantics as the app's `Pane width/minWidth/maxWidth` props: diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/tree-split.tsx b/apps/desktop/src/components/pane-shell/tree/renderer/tree-split.tsx index 378341552b..07a4e18a84 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/tree-split.tsx +++ b/apps/desktop/src/components/pane-shell/tree/renderer/tree-split.tsx @@ -22,12 +22,15 @@ import { $collapsedTreeSides, $hiddenTreePanes, $narrowViewport, + isCollapsePane, persistTree, presetSplitWeights, + setTreeGroupMinimized, setTreeSplitWeights } from '../store' import { + COLLAPSED_ZONE_PX, computedPx, cssMax, edgeFixedZone, @@ -43,6 +46,18 @@ import { } from './track-model' import { TreeNode } from './tree-node' +/** The single group id a subtree resolves to, or null when it holds several + * zones — the sash can only collapse a boundary that IS exactly one zone. */ +function groupIdOf(node: LayoutNode): null | string { + if (node.type === 'group') { + return node.id + } + + const ids = new Set(node.children.map(groupIdOf)) + + return ids.size === 1 ? [...ids][0] : null +} + /** * The size overrides for a fixed set of panes, referentially stable until one * of THEM changes. Sash drags churn `$paneStates` every frame; subscribing the @@ -197,6 +212,11 @@ export function TreeSplit({ node, root, rootRow }: { node: SplitNode; root?: boo // Clamps live on the zone's split-child WRAPPER (where we render them). const el = zoneEl?.parentElement ?? wrapper const cs = window.getComputedStyle(el) + // A tool panel (terminal / logs) may be dragged down to its collapsed + // header — the generic 80px floor is not its floor. Below that the + // release minimizes the zone instead of leaving a useless sliver. + const toolZone = allPaneIds(child).length > 0 && allPaneIds(child).every(isCollapsePane) + const floor = toolZone ? COLLAPSED_ZONE_PX : MIN_PANE_PX return { // EVERY shown pane of the zone: the zone's track is the max() of its @@ -206,8 +226,10 @@ export function TreeSplit({ node, root, rootRow }: { node: SplitNode; root?: boo paneIds: zone ? shownPaneIds(zone, trackCtx) : [], fixed: Boolean(zone), size: sizeOf(zoneEl ?? wrapper), - min: Math.max(MIN_PANE_PX, computedPx(horizontal ? cs.minWidth : cs.minHeight, 0)), - max: computedPx(horizontal ? cs.maxWidth : cs.maxHeight, Number.POSITIVE_INFINITY) + min: toolZone ? floor : Math.max(floor, computedPx(horizontal ? cs.minWidth : cs.minHeight, 0)), + max: computedPx(horizontal ? cs.maxWidth : cs.maxHeight, Number.POSITIVE_INFINITY), + collapseId: toolZone ? (zone?.id ?? groupIdOf(child)) : null, + floor } } @@ -310,9 +332,21 @@ export function TreeSplit({ node, root, rootRow }: { node: SplitNode; root?: boo resize.finish() if (lastShift !== null) { - // One store commit; the re-render rewrites `flex` and clears the - // preview overrides. - applyShift(lastShift) + // Dragged a tool panel down to its collapsed header? Fold the zone + // to its rail instead of persisting a sliver — and DON'T write the + // sliver size, so restoring brings back the size it had before. + const collapsedSide = + (a.collapseId && a0px + lastShift <= a.floor && a.collapseId) || + (b.collapseId && b0px - lastShift <= b.floor && b.collapseId) || + null + + if (collapsedSide) { + setTreeGroupMinimized(collapsedSide, true) + } else { + // One store commit; the re-render rewrites `flex` and clears the + // preview overrides. + applyShift(lastShift) + } } else { // Click without movement: nothing will re-render, so put the // wrappers' inline styles back exactly as React last wrote them. From 16431b8ab282d7579a5971c7278ca18bd077fe16 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Fri, 31 Jul 2026 23:49:28 -0500 Subject: [PATCH 4/5] test(desktop): cover the tool-panel toggle and hidden-header regressions --- .../pane-shell/tree/tool-pane-toggle.test.ts | 210 ++++++++++++++++++ 1 file changed, 210 insertions(+) create mode 100644 apps/desktop/src/components/pane-shell/tree/tool-pane-toggle.test.ts diff --git a/apps/desktop/src/components/pane-shell/tree/tool-pane-toggle.test.ts b/apps/desktop/src/components/pane-shell/tree/tool-pane-toggle.test.ts new file mode 100644 index 0000000000..8031fd6bab --- /dev/null +++ b/apps/desktop/src/components/pane-shell/tree/tool-pane-toggle.test.ts @@ -0,0 +1,210 @@ +import { atom } from 'nanostores' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { registry } from '@/contrib/registry' + +import { allPaneIds, group, split } from './model' +import { + $dismissedPanes, + $hiddenTreePanes, + $layoutTree, + bindToolPaneCollapse, + closeToolPane, + isToolPaneVisible, + setTreeGroupHeaderHidden, + toggleToolPane +} from './store' + +// Ground truth for "toggle terminal broke — ⌘J/⌘B work fine, but once I move +// the terminal around it just doesn't open/close anymore", and for "I have the +// header hidden and it keeps coming back". +// +// Both symptoms need the terminal STACKED with logs, which is the layout you +// get by dragging the terminal to the bottom: adoption stacks logs into the +// terminal's zone. A lone terminal never reproduced either bug, which is why +// they read as "randomly broken". + +const disposers: (() => void)[] = [] + +/** Bind through the REAL production function, with the ergonomics the + * controller gives it (closer/opener writing the same store). */ +function bindPaneCollapse(paneId: string, $open: ReturnType>) { + bindToolPaneCollapse( + paneId, + $open, + () => $open.set(false), + () => $open.set(true) + ) +} + +beforeEach(() => { + window.localStorage.clear() + $dismissedPanes.set(new Set()) + $hiddenTreePanes.set(new Set()) + + for (const [id, data] of [ + ['workspace', { placement: 'main', uncloseable: true }], + ['terminal', { placement: 'bottom' }], + ['logs', { placement: 'bottom' }] + ] as const) { + disposers.push(registry.register({ area: 'panes', data, id, render: () => null, title: id })) + } +}) + +afterEach(() => { + disposers.splice(0).forEach(dispose => dispose()) +}) + +/** The bottom tool zone, as the tree currently holds it. */ +const toolZone = () => { + const tree = $layoutTree.get()! + + const found = + tree.type === 'split' + ? tree.children.find(c => c.type === 'group' && (c.panes.includes('terminal') || c.panes.includes('logs'))) + : null + + return found as { active?: string; headerHidden?: boolean; minimized?: boolean; panes: string[] } | null +} + +/** Terminal dragged to the bottom; logs adopted into the same zone. + * + * Set via `$layoutTree.set`, NOT `declareDefaultTree` — that only adopts into + * an existing tree, and `$layoutTree` is module state that survives between + * tests, so the second case would silently assert against the first's shape. */ +const stackTree = (options?: { active?: string; headerHidden?: boolean }) => { + $layoutTree.set( + split('column', [ + group(['workspace'], { active: 'workspace', id: 'grp-main' }), + group(['terminal', 'logs'], { + active: options?.active ?? 'terminal', + headerHidden: options?.headerHidden, + id: 'g-tools' + }) + ]) + ) +} + +describe('binding a tool panel on boot', () => { + it('does not front itself over the tab the persisted tree had active', () => { + stackTree({ active: 'terminal' }) + + const $terminal = atom(true) + const $logs = atom(true) + bindPaneCollapse('terminal', $terminal) + bindPaneCollapse('logs', $logs) + + // Binding logs LAST must not steal the active slot. It used to, because + // boot called setPaneCollapsed(id, false), whose shared-zone branch + // reveals. + expect(toolZone()?.active).toBe('terminal') + }) + + it('leaves ⌃` able to collapse the zone straight after boot', () => { + stackTree({ active: 'terminal' }) + + const $terminal = atom(true) + bindPaneCollapse('terminal', $terminal) + bindPaneCollapse('logs', atom(true)) + + // THE reported symptom, driven the way the keybind drives it. When boot + // fronted logs, this asked setPaneCollapsed to fold a terminal that wasn't + // the active tab; the shared-zone branch declines that, so the first press + // did nothing at all and ⌃` read as a dead key. + $terminal.set(false) + + expect(toolZone()?.minimized).toBe(true) + expect(isToolPaneVisible('terminal')).toBe(false) + }) + + it('still collapses a tool panel whose store says it is off', () => { + stackTree() + + bindPaneCollapse('terminal', atom(false)) + + expect(toolZone()?.minimized).toBe(true) + }) +}) + +describe('toggling the terminal while it is stacked with logs', () => { + it('closes and reopens on every press', () => { + stackTree({ active: 'terminal' }) + bindPaneCollapse('terminal', atom(true)) + bindPaneCollapse('logs', atom(true)) + + toggleToolPane('terminal') + expect(isToolPaneVisible('terminal')).toBe(false) + + toggleToolPane('terminal') + expect(isToolPaneVisible('terminal')).toBe(true) + }) + + it('brings the terminal forward when logs holds the active tab', () => { + stackTree({ active: 'logs' }) + bindPaneCollapse('terminal', atom(true)) + bindPaneCollapse('logs', atom(true)) + + // The zone is open but showing LOGS, so the terminal is not on screen — + // the first press must reveal it rather than collapse the whole zone. + expect(isToolPaneVisible('terminal')).toBe(false) + + toggleToolPane('terminal') + + expect(isToolPaneVisible('terminal')).toBe(true) + expect(toolZone()?.minimized).toBeFalsy() + }) + + it('reopens after the terminal tab was closed outright', () => { + stackTree() + const $terminal = atom(true) + bindPaneCollapse('terminal', $terminal) + bindPaneCollapse('logs', atom(true)) + + closeToolPane('terminal') + expect(allPaneIds($layoutTree.get()!)).not.toContain('terminal') + + toggleToolPane('terminal') + + expect(allPaneIds($layoutTree.get()!)).toContain('terminal') + expect(isToolPaneVisible('terminal')).toBe(true) + }) +}) + +describe('a zone whose header the user hid', () => { + it('keeps it hidden after a stacked sibling is closed and toggled back', () => { + stackTree({ headerHidden: true }) + bindPaneCollapse('terminal', atom(true)) + const $logs = atom(true) + bindPaneCollapse('logs', $logs) + + setTreeGroupHeaderHidden('g-tools', true) + + // Close logs: the zone drops to one pane. normalize used to DISCARD the + // hidden flag here ("a lone zone is headerless anyway"), so the bar + // reappeared the moment logs was toggled back in. + closeToolPane('logs') + expect(toolZone()?.headerHidden).toBe(true) + + $logs.set(true) + + expect(toolZone()?.panes).toContain('logs') + expect(toolZone()?.headerHidden).toBe(true) + }) + + it('keeps it hidden when a closed pane is re-adopted into it', () => { + stackTree() + const $terminal = atom(true) + bindPaneCollapse('terminal', $terminal) + bindPaneCollapse('logs', atom(true)) + + setTreeGroupHeaderHidden('g-tools', true) + + // Re-adoption pins headerHidden:false so a surprise pane always has a + // handle — correct for a new pane, wrong for a zone the user hid. + closeToolPane('terminal') + $terminal.set(true) + + expect(toolZone()?.panes).toContain('terminal') + expect(toolZone()?.headerHidden).toBe(true) + }) +}) From 73b8847d7bab62aa7aaaaeefbbcd65e11235bbd6 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Sat, 1 Aug 2026 01:17:45 -0500 Subject: [PATCH 5/5] fix(desktop): toggle every pane off the tree, not off its own boolean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The terminal fix was only one instance. An audit of the other pane toggles found ⌘G and ⌘J diverging the same way, proven with a probe: with review stacked behind files in the right column, or either pane inside a minimized zone, the store reads open while nothing is on screen, so the press re-asserts a value it already held and the key does nothing. isPaneVisible / togglePaneVisible replace the tool-panel-only pair and now back every toggle. Close still routes through closeTreePane, so each pane keeps its own semantics: a tool panel collapses to its rail, files and review close through their store, anything else is dismissed. files and review were bound with a closer and no opener, so the boolean went stale as soon as anything but the toggle revealed them. bindPaneVisibility moves into the tree store beside bindToolPaneCollapse, documents the two as a pair, and both panes now pass both halves. Keeping the binding in the store also means the tests drive the real function — the earlier copy in the test file passed with the fix reverted, which is how the missing opener survived the first pass. setTreePaneHidden keeps its quiet path: a reactive unhide (a cwd arriving) must not front or un-minimize over what the user is looking at. Only user intent goes through the reveal path. --- apps/desktop/src/app/contrib/controller.tsx | 51 +++---- apps/desktop/src/app/hooks/use-keybinds.ts | 14 +- .../app/shell/hooks/use-statusbar-items.tsx | 6 +- .../tree/pane-toggle-visibility.test.ts | 126 ++++++++++++++++++ .../src/components/pane-shell/tree/store.ts | 72 ++++++++-- .../pane-shell/tree/tool-pane-toggle.test.ts | 24 ++-- apps/desktop/src/store/layout.ts | 22 ++- apps/desktop/src/store/review.ts | 9 +- 8 files changed, 254 insertions(+), 70 deletions(-) create mode 100644 apps/desktop/src/components/pane-shell/tree/pane-toggle-visibility.test.ts diff --git a/apps/desktop/src/app/contrib/controller.tsx b/apps/desktop/src/app/contrib/controller.tsx index 7243f7dc90..b9dc972c15 100644 --- a/apps/desktop/src/app/contrib/controller.tsx +++ b/apps/desktop/src/app/contrib/controller.tsx @@ -13,21 +13,20 @@ import { LayoutTreeRoot } from '@/components/pane-shell/tree/renderer' import type { DoubleTapContext } from '@/components/pane-shell/tree/renderer/drag-session' import { $layoutTree, + bindPaneVisibility, bindToolPaneCollapse, bindTreeSideVisibility, declareDefaultTree, dismissTreePane, dockPaneBeside, - isToolPaneVisible, + isPaneVisible, mirrorLayoutTree, paneRootSide, registerLayoutResetHandler, registerPaneCloser, - registerPaneOpener, resetLayoutTree, revealTreePane, - setTreePaneHidden, - toggleToolPane, + togglePaneVisible, watchContributedPanes } from '@/components/pane-shell/tree/store' import { SidebarProvider } from '@/components/ui/sidebar' @@ -55,7 +54,7 @@ import { SIDEBAR_MAX_WIDTH } from '@/store/layout' import { $previewOpenRequest, $previewTabs, closeRightRail } from '@/store/preview' -import { $reviewOpen, closeReview, REVIEW_PANE_ID } from '@/store/review' +import { $reviewOpen, closeReview, openReview, REVIEW_PANE_ID } from '@/store/review' import { $currentCwd, $selectedStoredSessionId, $sessions, $yoloActive, sessionMatchesStoredId } from '@/store/session' import { watchSessionPins } from '@/store/session-pin-sync' import { $statusbarVisible } from '@/store/statusbar-prefs' @@ -472,27 +471,9 @@ registerLayoutResetHandler(stackSessionTilesIntoMain) // toggle mirrors the root row. // --------------------------------------------------------------------------- -function bindPaneVisibility( - paneId: string, - $open: { get(): boolean; listen(fn: (open: boolean) => void): void }, - close?: () => void, - open?: () => void -) { - setTreePaneHidden(paneId, !$open.get()) - $open.listen(isOpen => setTreePaneHidden(paneId, !isOpen)) - - // The tab menu's Close routes through the owning store (never dismissal), - // so the pane's toggle buttons stay truthful. - if (close) { - registerPaneCloser(paneId, close) - } - - // The opener is the mirror: preset application (revealOnPreset) shows the - // pane through the same store, so the toggle stays truthful. - if (open) { - registerPaneOpener(paneId, open) - } -} +// HIDE-STYLE PANES (files, review, preview): the binding lives in the tree +// store — bindPaneVisibility — alongside bindToolPaneCollapse, so both are +// testable against the real function instead of a copy. // TOOL PANELS (terminal, logs): the binding lives in the tree store — // bindToolPaneCollapse — so the boot rule it encodes is testable against the @@ -549,15 +530,25 @@ const $hasWorkspace = computed($currentCwd, cwd => Boolean(cwd.trim())) // The tree pane's own presence tracks ⌘J directly, not just the column's // collapse — otherwise revealing a preview (which opens that shared column) // would drag the tree along with it. See revealPreview. +// +// Both get a CLOSER and an OPENER. The closer keeps ⌘J/⌘G truthful when the +// pane is closed from the tab menu; the opener is its mirror, so bringing the +// pane back through the tree (the toggle's reveal path, the rail, a preset) +// writes the store too. Without the opener the boolean went stale the moment +// anything but the toggle showed the pane — the divergence this whole change +// is about. bindPaneVisibility( 'files', - computed([$hasWorkspace, $fileBrowserOpen], (workspace, open) => workspace && open) + computed([$hasWorkspace, $fileBrowserOpen], (workspace, open) => workspace && open), + () => setFileBrowserOpen(false), + () => setFileBrowserOpen(true) ) // ⌘G — the review sidebar appears/disappears (and comes to the front). bindPaneVisibility( 'review', computed([$reviewOpen, $hasWorkspace], (open, workspace) => open && workspace), - closeReview + closeReview, + openReview ) // ⌃` / statusbar toggle — the terminal COLLAPSES to a rail (tab stays), not // hides; PTYs stay alive while collapsed (see PersistentTerminal). @@ -593,8 +584,8 @@ registry.register( keywords: ['logs', 'agent log', 'tail', 'debug'], // On-screen, not the store's boolean: logs stacks with the terminal, and // behind its sibling's tab `$logsOpen` stays true while nothing is visible. - get: () => isToolPaneVisible('logs'), - set: () => toggleToolPane('logs') + get: () => isPaneVisible('logs'), + set: () => togglePaneVisible('logs') }) ) diff --git a/apps/desktop/src/app/hooks/use-keybinds.ts b/apps/desktop/src/app/hooks/use-keybinds.ts index 3b3677815c..32d9caaba2 100644 --- a/apps/desktop/src/app/hooks/use-keybinds.ts +++ b/apps/desktop/src/app/hooks/use-keybinds.ts @@ -7,9 +7,9 @@ import { closeActiveTerminal, createTerminal, cycleTerminal } from '@/app/right- import { activateTreeTabSlot, cycleTreeTabInFocusedZone, - isToolPaneVisible, + isPaneVisible, layoutHasRootSide, - toggleToolPane + togglePaneVisible } from '@/components/pane-shell/tree/store' import { onReleaseTypingFocus } from '@/components/ui/keyboard-first' import { findBarClaimsCombo } from '@/lib/find-in-page' @@ -190,11 +190,11 @@ export function useKeybinds(deps: KeybindRuntimeDeps): void { // terminal-on-bottom) would leave it a dead key, so it falls back to the // terminal there. The single "secondary panel" toggle. 'view.toggleRightSidebar': () => - layoutHasRootSide('right') ? toggleFileBrowserOpen() : toggleToolPane('terminal'), + layoutHasRootSide('right') ? toggleFileBrowserOpen() : togglePaneVisible('terminal'), 'view.toggleReview': toggleReview, 'view.toggleStatusbar': toggleStatusbarVisible, 'view.showFiles': showFiles, - 'view.showTerminal': () => toggleToolPane('terminal'), + 'view.showTerminal': () => togglePaneVisible('terminal'), // Create first so the pane's open-effect ensure sees a non-empty set and // doesn't also spawn one — net effect is exactly one fresh terminal. 'view.newTerminal': () => { @@ -204,9 +204,9 @@ export function useKeybinds(deps: KeybindRuntimeDeps): void { // Switch / close only act while the terminal is actually ON SCREEN — ask // the tree, not the toggle store (which stays true behind a stacked // sibling tab or a minimized zone). - 'view.nextTerminal': () => isToolPaneVisible('terminal') && cycleTerminal(1), - 'view.prevTerminal': () => isToolPaneVisible('terminal') && cycleTerminal(-1), - 'view.closeTerminal': () => isToolPaneVisible('terminal') && closeActiveTerminal(), + 'view.nextTerminal': () => isPaneVisible('terminal') && cycleTerminal(1), + 'view.prevTerminal': () => isPaneVisible('terminal') && cycleTerminal(-1), + 'view.closeTerminal': () => isPaneVisible('terminal') && closeActiveTerminal(), 'view.flipPanes': togglePanesFlipped, // ⌘W: close the focused tab (terminal / preview target / zone tree tab). // On the main tab with session tabs stacked, it shifts the next one in — diff --git a/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx b/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx index fa81c227ad..adedab3fa5 100644 --- a/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx +++ b/apps/desktop/src/app/shell/hooks/use-statusbar-items.tsx @@ -5,7 +5,7 @@ import type { CommandCenterSection } from '@/app/command-center' import { useApprovalModeStatusbarItem } from '@/app/shell/approval-mode-menu' import { ContextUsagePanel } from '@/app/shell/context-usage-panel' import { GatewayMenuPanel } from '@/app/shell/gateway-menu-panel' -import { $toolPaneVisible, toggleToolPane } from '@/components/pane-shell/tree/store' +import { $paneVisible, togglePaneVisible } from '@/components/pane-shell/tree/store' import { Codicon } from '@/components/ui/codicon' import { GlyphSpinner } from '@/components/ui/glyph-spinner' import { useI18n } from '@/i18n' @@ -95,7 +95,7 @@ export function useStatusbarItems({ // What the button paints and flips is whether the terminal is ON SCREEN — // the takeover store alone stays true behind a stacked sibling tab or a // minimized zone, which lit the button for a pane the user couldn't see. - const terminalShowing = useStore($toolPaneVisible('terminal')) + const terminalShowing = useStore($paneVisible('terminal')) const primaryBusy = useStore($busy) const currentCwd = useStore($currentCwd) // Derive the workspace's project name from the already-cached project tree @@ -513,7 +513,7 @@ export function useStatusbarItems({ hidden: !chatOpen, icon: , id: 'terminal', - onSelect: () => toggleToolPane('terminal'), + onSelect: () => togglePaneVisible('terminal'), title: terminalShowing ? copy.hideTerminal : copy.showTerminal, toggleLabel: copy.toggleTerminal, variant: 'action' diff --git a/apps/desktop/src/components/pane-shell/tree/pane-toggle-visibility.test.ts b/apps/desktop/src/components/pane-shell/tree/pane-toggle-visibility.test.ts new file mode 100644 index 0000000000..1f57ed0158 --- /dev/null +++ b/apps/desktop/src/components/pane-shell/tree/pane-toggle-visibility.test.ts @@ -0,0 +1,126 @@ +import { atom } from 'nanostores' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { registry } from '@/contrib/registry' + +import { group, split } from './model' +import { + $dismissedPanes, + $hiddenTreePanes, + $layoutTree, + bindPaneVisibility, + isPaneVisible, + setTreeGroupMinimized, + togglePaneVisible +} from './store' + +// The bug class, across EVERY pane kind — not just the terminal she reported. +// +// A toggle that flips its own boolean diverges from the tree the moment +// anything else moves the pane: stacked behind a sibling tab, folded into a +// minimized zone, closed with ⌘W. The store then says "open" while nothing is +// on screen, the press re-asserts a value it already held, and the key reads +// as dead. Same shape for the COLLAPSE-style tool panels (terminal, logs) and +// the HIDE-style panes (files, review) — proven here with one table. + +const disposers: (() => void)[] = [] + +/** Bind through the REAL production function, with the closer/opener pair the + * controller gives it. */ +function bindVisibility(paneId: string, $open: ReturnType>) { + bindPaneVisibility( + paneId, + $open, + () => $open.set(false), + () => $open.set(true) + ) +} + +beforeEach(() => { + window.localStorage.clear() + $dismissedPanes.set(new Set()) + $hiddenTreePanes.set(new Set()) + + for (const [id, data] of [ + ['workspace', { placement: 'main', uncloseable: true }], + ['files', { placement: 'right' }], + ['review', { placement: 'right' }] + ] as const) { + disposers.push(registry.register({ area: 'panes', data, id, render: () => null, title: id })) + } +}) + +afterEach(() => { + disposers.splice(0).forEach(dispose => dispose()) +}) + +describe('a hide-style pane stacked behind a sibling tab', () => { + it('comes forward on the first press instead of swallowing it', () => { + $layoutTree.set( + split('row', [ + group(['workspace'], { active: 'workspace', id: 'g-main' }), + group(['files', 'review'], { active: 'files', id: 'g-right' }) + ]) + ) + + const $review = atom(true) + bindVisibility('review', $review) + + // The column is open but showing FILES, so the diff is not on screen — + // and $reviewOpen is true, which is what made ⌘G a no-op. + expect(isPaneVisible('review')).toBe(false) + + togglePaneVisible('review') + + expect(isPaneVisible('review')).toBe(true) + }) +}) + +describe('a hide-style pane inside a minimized zone', () => { + it('un-minimizes on the first press', () => { + $layoutTree.set( + split('row', [ + group(['workspace'], { active: 'workspace', id: 'g-main' }), + group(['files'], { active: 'files', id: 'g-files' }) + ]) + ) + + const $files = atom(true) + bindVisibility('files', $files) + setTreeGroupMinimized('g-files', true) + + expect(isPaneVisible('files')).toBe(false) + + togglePaneVisible('files') + + expect(isPaneVisible('files')).toBe(true) + }) +}) + +describe('the toggle round-trip', () => { + it('closes a visible hide-style pane through its own store', () => { + $layoutTree.set( + split('row', [ + group(['workspace'], { active: 'workspace', id: 'g-main' }), + group(['files'], { active: 'files', id: 'g-files' }) + ]) + ) + + const $files = atom(true) + bindVisibility('files', $files) + + expect(isPaneVisible('files')).toBe(true) + + togglePaneVisible('files') + + // Close routes through the registered closer, so the pane's own toggle + // store stays truthful rather than being bypassed by a tree edit. + expect($files.get()).toBe(false) + expect(isPaneVisible('files')).toBe(false) + + togglePaneVisible('files') + + expect($files.get()).toBe(true) + expect(isPaneVisible('files')).toBe(true) + }) +}) diff --git a/apps/desktop/src/components/pane-shell/tree/store.ts b/apps/desktop/src/components/pane-shell/tree/store.ts index 9c9be382ba..bf5be6e17b 100644 --- a/apps/desktop/src/components/pane-shell/tree/store.ts +++ b/apps/desktop/src/components/pane-shell/tree/store.ts @@ -1340,9 +1340,10 @@ export function restoreTreePane(paneId: string) { revealTreePane(paneId) } -/** Is a tool pane actually ON SCREEN? In the tree, not dismissed or chrome - * hidden, its zone un-minimized, and holding its stack's active slot. */ -export function isToolPaneVisible(paneId: string): boolean { +/** Is a pane actually ON SCREEN? In the tree, not dismissed, not chrome + * hidden, its zone un-minimized, and holding its stack's active slot. + * True for every pane class — tool panels and hide-style panes alike. */ +export function isPaneVisible(paneId: string): boolean { if ($dismissedPanes.get().has(paneId) || $hiddenTreePanes.get().has(paneId)) { return false } @@ -1352,22 +1353,54 @@ export function isToolPaneVisible(paneId: string): boolean { return Boolean(group && !group.minimized && group.active === paneId) } -const toolPaneVisibleCache = new Map>() +const paneVisibleCache = new Map>() -/** Reactive `isToolPaneVisible` for chrome that renders an on/off affordance +/** Reactive `isPaneVisible` for chrome that renders an on/off affordance * (the statusbar's terminal button). Memoized per pane id so `useStore` * subscriptions stay referentially stable across renders. */ -export function $toolPaneVisible(paneId: string): ReadableAtom { - let cached = toolPaneVisibleCache.get(paneId) +export function $paneVisible(paneId: string): ReadableAtom { + let cached = paneVisibleCache.get(paneId) if (!cached) { - cached = computed([$layoutTree, $dismissedPanes, $hiddenTreePanes], () => isToolPaneVisible(paneId)) - toolPaneVisibleCache.set(paneId, cached) + cached = computed([$layoutTree, $dismissedPanes, $hiddenTreePanes], () => isPaneVisible(paneId)) + paneVisibleCache.set(paneId, cached) } return cached } +/** + * HIDE-STYLE PANES (files, review, preview): bind a pane's visibility STORE to + * the tree so its toggle HIDES the pane — its zone collapses while the content + * stays mounted — as opposed to the tool panels, which collapse to a rail and + * keep their tab. + * + * `close` and `open` are a PAIR, and passing only one is the bug this exists to + * prevent. The closer keeps the toggle truthful when the pane is closed from + * the tab menu; the opener is its mirror, so anything that shows the pane + * through the tree — a reveal, a preset, the toggle's own un-hide path — writes + * the store too. With a closer and no opener the boolean goes stale the moment + * something other than the toggle reveals the pane, and the next press spends + * itself re-asserting a value it already held. + */ +export function bindPaneVisibility( + paneId: string, + $open: { get(): boolean; listen(fn: (open: boolean) => void): void }, + close?: () => void, + open?: () => void +) { + setTreePaneHidden(paneId, !$open.get()) + $open.listen(isOpen => setTreePaneHidden(paneId, !isOpen)) + + if (close) { + registerPaneCloser(paneId, close) + } + + if (open) { + registerPaneOpener(paneId, open) + } +} + /** * TOOL PANELS (terminal, logs): bind a pane's visibility STORE to the tree so * its toggle COLLAPSES the zone to a persistent rail (the tab stays) instead of @@ -1407,8 +1440,8 @@ export function bindToolPaneCollapse( } /** - * ⌃` / the statusbar button / ⌘J's terminal fallback: ONE resolver for "flip - * this tool panel", derived from what is on screen rather than from the + * EVERY pane toggle: ⌃`, ⌘G, the statusbar button, the ⌘K rows. ONE resolver + * for "flip this pane", derived from what is on screen rather than from the * toggle's own boolean. * * A free-floating `!$open.get()` diverges from the tree the moment anything @@ -1416,10 +1449,21 @@ export function bindToolPaneCollapse( * menu, closed with ⌘W — and then the toggle spends its press re-asserting a * value the store already held, which reads as a dead key. Asking the tree * instead means the first press always does the visible thing. + * + * This is not a tool-panel quirk. The hide-style panes (files, review) had it + * too: `setTreePaneHidden(id, false)` deliberately does NOT front or + * un-minimize, because reactive unhides (a cwd arriving) must not clobber what + * the user is looking at. Correct for a reactive change, useless for a + * keypress — so user intent routes here and reactive bindings keep the quiet + * path. + * + * Close goes through `closeTreePane` so each pane keeps its own semantics: a + * tool panel collapses to its rail, files/review close through their store, + * anything else is dismissed. */ -export function toggleToolPane(paneId: string) { - if (isToolPaneVisible(paneId)) { - collapseTreePane(paneId) +export function togglePaneVisible(paneId: string) { + if (isPaneVisible(paneId)) { + closeTreePane(paneId) } else { restoreTreePane(paneId) } diff --git a/apps/desktop/src/components/pane-shell/tree/tool-pane-toggle.test.ts b/apps/desktop/src/components/pane-shell/tree/tool-pane-toggle.test.ts index 8031fd6bab..6e24300d6d 100644 --- a/apps/desktop/src/components/pane-shell/tree/tool-pane-toggle.test.ts +++ b/apps/desktop/src/components/pane-shell/tree/tool-pane-toggle.test.ts @@ -10,9 +10,9 @@ import { $layoutTree, bindToolPaneCollapse, closeToolPane, - isToolPaneVisible, + isPaneVisible, setTreeGroupHeaderHidden, - toggleToolPane + togglePaneVisible } from './store' // Ground truth for "toggle terminal broke — ⌘J/⌘B work fine, but once I move @@ -114,7 +114,7 @@ describe('binding a tool panel on boot', () => { $terminal.set(false) expect(toolZone()?.minimized).toBe(true) - expect(isToolPaneVisible('terminal')).toBe(false) + expect(isPaneVisible('terminal')).toBe(false) }) it('still collapses a tool panel whose store says it is off', () => { @@ -132,11 +132,11 @@ describe('toggling the terminal while it is stacked with logs', () => { bindPaneCollapse('terminal', atom(true)) bindPaneCollapse('logs', atom(true)) - toggleToolPane('terminal') - expect(isToolPaneVisible('terminal')).toBe(false) + togglePaneVisible('terminal') + expect(isPaneVisible('terminal')).toBe(false) - toggleToolPane('terminal') - expect(isToolPaneVisible('terminal')).toBe(true) + togglePaneVisible('terminal') + expect(isPaneVisible('terminal')).toBe(true) }) it('brings the terminal forward when logs holds the active tab', () => { @@ -146,11 +146,11 @@ describe('toggling the terminal while it is stacked with logs', () => { // The zone is open but showing LOGS, so the terminal is not on screen — // the first press must reveal it rather than collapse the whole zone. - expect(isToolPaneVisible('terminal')).toBe(false) + expect(isPaneVisible('terminal')).toBe(false) - toggleToolPane('terminal') + togglePaneVisible('terminal') - expect(isToolPaneVisible('terminal')).toBe(true) + expect(isPaneVisible('terminal')).toBe(true) expect(toolZone()?.minimized).toBeFalsy() }) @@ -163,10 +163,10 @@ describe('toggling the terminal while it is stacked with logs', () => { closeToolPane('terminal') expect(allPaneIds($layoutTree.get()!)).not.toContain('terminal') - toggleToolPane('terminal') + togglePaneVisible('terminal') expect(allPaneIds($layoutTree.get()!)).toContain('terminal') - expect(isToolPaneVisible('terminal')).toBe(true) + expect(isPaneVisible('terminal')).toBe(true) }) }) diff --git a/apps/desktop/src/store/layout.ts b/apps/desktop/src/store/layout.ts index ba9a078920..c2debd8a72 100644 --- a/apps/desktop/src/store/layout.ts +++ b/apps/desktop/src/store/layout.ts @@ -2,6 +2,7 @@ import { atom, computed, type ReadableAtom, type WritableAtom } from 'nanostores import { SIDEBAR_COLLAPSE_MEDIA_QUERY } from '@/app/layout-constants' import { PANE_TOGGLE_REVEAL_EVENT } from '@/components/pane-shell' +import { isPaneVisible, revealTreePane } from '@/components/pane-shell/tree/store' import { matchesQuery } from '@/hooks/use-media-query' import { Codecs, persistentAtom } from '@/lib/persisted' import { arraysEqual, insertUniqueId, readKey } from '@/lib/storage' @@ -37,6 +38,9 @@ const RIGHT_RAIL_ACTIVE_TAB_STORAGE_KEY = 'hermes.desktop.rightRailActiveTab' export const CHAT_SIDEBAR_PANE_ID = 'chat-sidebar' export const FILE_BROWSER_PANE_ID = 'file-browser' +/** The file tree's id in the LAYOUT TREE — distinct from the pane-state id + * above, which keys its open/width record. Toggles need both. */ +export const FILES_PANE_ID = 'files' export const PREVIEW_PANE_ID = 'preview' /** Every rail tab is a preview of something, namespaced by what backs it: a @@ -287,9 +291,23 @@ export function toggleSidebarOpen() { } export function toggleFileBrowserOpen() { - if (!revealNarrowPane(FILE_BROWSER_PANE_ID, 'toggle')) { - togglePane(FILE_BROWSER_PANE_ID) + if (revealNarrowPane(FILE_BROWSER_PANE_ID, 'toggle')) { + return } + + // Ask the TREE, not the pane's boolean. `$fileBrowserOpen` stays true while + // the tree pane sits behind a sibling tab in the shared right column (the + // preview rail, the diff) or inside a minimized zone, so ⌘J spent its press + // re-asserting a value it already held and read as a dead key. Only fold the + // side when the tree is genuinely the thing on screen; otherwise bring it + // forward through the reveal path, which fronts and un-minimizes. + if (!isPaneVisible(FILES_PANE_ID) && $fileBrowserOpen.get()) { + revealTreePane(FILES_PANE_ID) + + return + } + + togglePane(FILE_BROWSER_PANE_ID) } export function setFileBrowserOpen(open: boolean) { diff --git a/apps/desktop/src/store/review.ts b/apps/desktop/src/store/review.ts index 91474682f8..c25c044b17 100644 --- a/apps/desktop/src/store/review.ts +++ b/apps/desktop/src/store/review.ts @@ -2,7 +2,7 @@ import { atom, computed } from 'nanostores' import { SIDEBAR_COLLAPSE_MEDIA_QUERY } from '@/app/layout-constants' import { PANE_TOGGLE_REVEAL_EVENT } from '@/components/pane-shell' -import { revealTreePane } from '@/components/pane-shell/tree/store' +import { isPaneVisible, revealTreePane } from '@/components/pane-shell/tree/store' import type { HermesReviewFile, HermesReviewShipInfo } from '@/global' import { matchesQuery } from '@/hooks/use-media-query' import { desktopGit } from '@/lib/desktop-git' @@ -285,7 +285,12 @@ export function toggleReview(scopeCwd: null | string = null): void { return } - if ($reviewOpen.get()) { + // Ask the TREE, not `$reviewOpen`. The store stays true while the pane sits + // behind a sibling tab in the right column or inside a minimized zone, so a + // boolean flip spent the press re-asserting a value it already held and ⌘G + // read as a dead key. `revealReview` fronts and un-minimizes; only close when + // the diff is genuinely the thing on screen. + if (isPaneVisible(REVIEW_PANE_ID)) { closeReview() } else { revealReview(scopeCwd)