fix(desktop): close the terminal and logs tabs like any other tab
Two things made a tool panel tab feel unclosable. Cmd-W was a dead key over the terminal and the logs pane. The keyboard close ladder resolved its target with focusedSessionGroup, which only matches zones hosting a CHAT strip, so a focused tool panel fell through every rung and Cmd-W emptied the main tab instead. Add a tool rung that resolves through the same hover/focus ladder the number keys use. Right-click Close was missing or inert. The zone menu's target was only resolved by the tab strip's own onContextMenu, so a right-click anywhere else in the zone (pane body, collapsed rail, edit veil) reused the PREVIOUS target -- landing on the uncloseable workspace dropped Close from the menu entirely. Resolve the target on the zone instead, so every surface that opens the menu names the chip under the pointer. Close on a tool panel now takes the tab out of the strip and syncs its owning store, so the ctrl-backtick toggle and the Cmd-K row stay truthful and bring the pane back; the toggle's open path reveals (un-dismiss + re-adopt) rather than un-collapsing a pane that has left the tree.
This commit is contained in:
parent
cfae306ab6
commit
c25e4fe687
|
|
@ -1,12 +1,14 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const closeFocusedSessionTab = vi.fn(() => false)
|
||||
const closeFocusedToolTab = vi.fn(() => false)
|
||||
const nextSessionTileForWorkspace = vi.fn<() => null | string>(() => null)
|
||||
const closeSessionTile = vi.fn()
|
||||
const requestFreshSession = vi.fn()
|
||||
|
||||
vi.mock('@/components/pane-shell/tree/store', () => ({
|
||||
closeFocusedSessionTab: () => closeFocusedSessionTab()
|
||||
closeFocusedSessionTab: () => closeFocusedSessionTab(),
|
||||
closeFocusedToolTab: () => closeFocusedToolTab()
|
||||
}))
|
||||
|
||||
vi.mock('@/store/session-states', () => ({
|
||||
|
|
@ -51,6 +53,7 @@ beforeEach(() => {
|
|||
$activeSessionId.set(null)
|
||||
$workspaceIsPage.set(false)
|
||||
closeFocusedSessionTab.mockReturnValue(false)
|
||||
closeFocusedToolTab.mockReturnValue(false)
|
||||
nextSessionTileForWorkspace.mockReturnValue(null)
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
|
@ -135,4 +138,13 @@ describe('closeWorkspaceTab', () => {
|
|||
expect(closeActiveTab(vi.fn())).toBe(true)
|
||||
expect(requestFreshSession).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('a focused tool panel (terminal / logs) claims ⌘W before main empties', () => {
|
||||
loadedMainOnly()
|
||||
closeFocusedToolTab.mockReturnValue(true)
|
||||
|
||||
expect(closeActiveTab(vi.fn())).toBe(true)
|
||||
// The logs/terminal tab closed — main keeps its loaded chat.
|
||||
expect(requestFreshSession).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { mainChatOccupied } from '@/app/open-session'
|
||||
import { closeActiveTerminal } from '@/app/right-sidebar/terminal/terminals'
|
||||
import { $workspaceIsPage } from '@/app/routes'
|
||||
import { closeFocusedSessionTab } from '@/components/pane-shell/tree/store'
|
||||
import { closeFocusedSessionTab, closeFocusedToolTab } from '@/components/pane-shell/tree/store'
|
||||
import { isFocusWithin } from '@/lib/keybinds/combo'
|
||||
import { $previewTabs, closeActiveRightRailTab } from '@/store/preview'
|
||||
import { requestFreshSession } from '@/store/profile'
|
||||
|
|
@ -56,12 +56,13 @@ export function closeWorkspaceTab(loadSessionIntoWorkspace?: (storedSessionId: s
|
|||
* 1. a focused terminal → its active terminal tab,
|
||||
* 2. right-rail tabs (live preview and/or file peeks),
|
||||
* 3. the FOCUSED chat zone → its active tab (a session tile stacked into it).
|
||||
* 4. the workspace tab itself — see `closeWorkspaceTab`.
|
||||
* 4. a focused TOOL PANEL zone (terminal / logs) → its active tab.
|
||||
* 5. the workspace tab itself — see `closeWorkspaceTab`.
|
||||
* Returns false when nothing closes, so ⌘W is a no-op — it never closes the
|
||||
* window. Shared by the keyboard path (Win/Linux) and the macOS
|
||||
* menu-accelerator IPC.
|
||||
*
|
||||
* Steps 3-4 follow the same focused zone ⌘1…⌘9 indexes, so a second chat zone
|
||||
* Steps 3-5 follow the same focused zone ⌘1…⌘9 indexes, so a second chat zone
|
||||
* with its own tab strip closes ITS tab instead of main's.
|
||||
*/
|
||||
export function closeActiveTab(loadSessionIntoWorkspace?: (storedSessionId: string) => void): boolean {
|
||||
|
|
@ -84,5 +85,11 @@ export function closeActiveTab(loadSessionIntoWorkspace?: (storedSessionId: stri
|
|||
return true
|
||||
}
|
||||
|
||||
// A tool panel zone hosts no chat strip, so the chat rung skips it — but its
|
||||
// tabs close like any other. Without this ⌘W was dead over terminal / logs.
|
||||
if (closeFocusedToolTab()) {
|
||||
return true
|
||||
}
|
||||
|
||||
return closeWorkspaceTab(loadSessionIntoWorkspace)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -491,7 +491,12 @@ 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; the tab's ✕ removes it.
|
||||
// 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 },
|
||||
|
|
@ -500,7 +505,7 @@ function bindPaneCollapse(
|
|||
) {
|
||||
markCollapsePane(paneId)
|
||||
setPaneCollapsed(paneId, !$open.get())
|
||||
$open.listen(isOpen => setPaneCollapsed(paneId, !isOpen))
|
||||
$open.listen(isOpen => (isOpen ? revealTreePane(paneId) : setPaneCollapsed(paneId, true)))
|
||||
registerPaneCloser(paneId, close)
|
||||
registerPaneOpener(paneId, open)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,185 @@
|
|||
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { registry } from '@/contrib/registry'
|
||||
|
||||
import { group, split } from '../model'
|
||||
import {
|
||||
$layoutTree,
|
||||
declareDefaultTree,
|
||||
markCollapsePane,
|
||||
noteActiveTreeGroup,
|
||||
registerPaneCloser,
|
||||
setTreeGroupMinimized
|
||||
} from '../store'
|
||||
|
||||
import { TreeGroup } from './tree-group'
|
||||
|
||||
// Ground truth for "right-clicking logs doesn't even show Close, and ⌘W
|
||||
// doesn't close it". Renders the REAL zone renderer and opens the REAL
|
||||
// context menu.
|
||||
|
||||
class TestResizeObserver {
|
||||
observe() {}
|
||||
unobserve() {}
|
||||
disconnect() {}
|
||||
}
|
||||
|
||||
beforeAll(() => {
|
||||
vi.stubGlobal('ResizeObserver', TestResizeObserver)
|
||||
// jsdom lacks CSS.escape, which tab-strip-scroll uses in a layout effect.
|
||||
vi.stubGlobal('CSS', { ...globalThis.CSS, escape: (value: string) => value })
|
||||
Element.prototype.hasPointerCapture ??= () => false
|
||||
Element.prototype.setPointerCapture ??= () => undefined
|
||||
Element.prototype.releasePointerCapture ??= () => undefined
|
||||
HTMLElement.prototype.scrollIntoView ??= () => undefined
|
||||
})
|
||||
|
||||
const disposers: (() => void)[] = []
|
||||
|
||||
beforeEach(async () => {
|
||||
window.localStorage.clear()
|
||||
|
||||
// Per-test isolation: earlier cases dismiss / hide panes, and both records
|
||||
// live in module state that survives into the next test.
|
||||
const { $dismissedPanes, $hiddenTreePanes } = await import('../store')
|
||||
$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 }))
|
||||
}
|
||||
|
||||
markCollapsePane('terminal')
|
||||
markCollapsePane('logs')
|
||||
registerPaneCloser('terminal', () => undefined)
|
||||
registerPaneCloser('logs', () => undefined)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
disposers.splice(0).forEach(dispose => dispose())
|
||||
})
|
||||
|
||||
/** Radix opens a ContextMenu on contextmenu after a pointerdown positions it. */
|
||||
function openContextMenu(target: HTMLElement) {
|
||||
fireEvent.pointerDown(target, { button: 2, pointerType: 'mouse' })
|
||||
fireEvent.contextMenu(target, { button: 2 })
|
||||
}
|
||||
|
||||
const zoneAt = (index: number) => {
|
||||
const node = $layoutTree.get()!
|
||||
|
||||
return (node.type === 'split' ? node.children[index] : node) as never
|
||||
}
|
||||
|
||||
const tabEl = (paneId: string) => document.querySelector<HTMLElement>(`[data-tree-tab="${paneId}"]`)
|
||||
|
||||
describe('right-clicking a tool panel tab', () => {
|
||||
it('offers Close when logs is STACKED with the terminal', async () => {
|
||||
declareDefaultTree(
|
||||
split('column', [
|
||||
group(['workspace'], { active: 'workspace', id: 'grp-main' }),
|
||||
group(['terminal', 'logs'], { active: 'logs', id: 'grp-tools' })
|
||||
])
|
||||
)
|
||||
render(<TreeGroup node={zoneAt(1)} parentAxis="column" />)
|
||||
|
||||
openContextMenu(tabEl('logs')!)
|
||||
|
||||
expect(await screen.findByRole('menuitem', { name: /^close$/i })).toBeTruthy()
|
||||
})
|
||||
|
||||
it('offers Close when logs is ALONE in its zone', async () => {
|
||||
declareDefaultTree(
|
||||
split('column', [
|
||||
group(['workspace'], { active: 'workspace', id: 'grp-main' }),
|
||||
group(['logs'], { active: 'logs', id: 'grp-logs' })
|
||||
])
|
||||
)
|
||||
render(<TreeGroup node={zoneAt(1)} parentAxis="column" />)
|
||||
|
||||
const tab = tabEl('logs')
|
||||
expect(tab).toBeTruthy()
|
||||
|
||||
openContextMenu(tab!)
|
||||
|
||||
expect(await screen.findByRole('menuitem', { name: /^close$/i })).toBeTruthy()
|
||||
})
|
||||
|
||||
it('offers Close while the zone is MINIMIZED to its rail', async () => {
|
||||
declareDefaultTree(
|
||||
split('column', [
|
||||
group(['workspace'], { active: 'workspace', id: 'grp-main' }),
|
||||
group(['terminal', 'logs'], { active: 'logs', id: 'grp-tools' })
|
||||
])
|
||||
)
|
||||
setTreeGroupMinimized('grp-tools', true)
|
||||
render(<TreeGroup node={zoneAt(1)} parentAxis="column" />)
|
||||
|
||||
openContextMenu(tabEl('logs')!)
|
||||
|
||||
expect(await screen.findByRole('menuitem', { name: /^close$/i })).toBeTruthy()
|
||||
})
|
||||
})
|
||||
|
||||
describe('⌘W over a focused tool panel', () => {
|
||||
it('closes the logs tab and the toggle brings it back', async () => {
|
||||
const { closeActiveTab } = await import('@/app/chat/close-tab')
|
||||
const { allPaneIds } = await import('../model')
|
||||
const { revealTreePane, setPaneCollapsed } = await import('../store')
|
||||
|
||||
declareDefaultTree(
|
||||
split('column', [
|
||||
group(['workspace'], { active: 'workspace', id: 'grp-main' }),
|
||||
group(['terminal', 'logs'], { active: 'logs', id: 'grp-tools' })
|
||||
])
|
||||
)
|
||||
|
||||
// Production wiring (bindPaneCollapse): a store drives visibility, its
|
||||
// closer collapses, its opener reveals.
|
||||
let open = true
|
||||
registerPaneCloser('logs', () => {
|
||||
open = false
|
||||
setPaneCollapsed('logs', true)
|
||||
})
|
||||
|
||||
// The user clicked into the tools zone — ⌘W must act on ITS active tab.
|
||||
noteActiveTreeGroup('grp-tools')
|
||||
|
||||
closeActiveTab()
|
||||
|
||||
// The tab left the strip, and the toggle knows the pane is off.
|
||||
expect(allPaneIds($layoutTree.get()!)).not.toContain('logs')
|
||||
expect(open).toBe(false)
|
||||
|
||||
// Toggling it back on re-adopts the pane (revealTreePane un-dismisses).
|
||||
open = true
|
||||
revealTreePane('logs')
|
||||
|
||||
expect(allPaneIds($layoutTree.get()!)).toContain('logs')
|
||||
})
|
||||
|
||||
it('leaves the uncloseable workspace zone to the chat rung', async () => {
|
||||
const { allPaneIds } = await import('../model')
|
||||
const { closeFocusedToolTab } = await import('../store')
|
||||
|
||||
declareDefaultTree(
|
||||
split('column', [
|
||||
group(['workspace'], { active: 'workspace', id: 'grp-main' }),
|
||||
group(['logs'], { active: 'logs', id: 'grp-logs' })
|
||||
])
|
||||
)
|
||||
|
||||
noteActiveTreeGroup('grp-main')
|
||||
|
||||
// Pointer/focus on main: the tool rung must decline, not reach across and
|
||||
// close the logs pane the user isn't looking at.
|
||||
expect(closeFocusedToolTab()).toBe(false)
|
||||
expect(allPaneIds($layoutTree.get()!)).toContain('logs')
|
||||
})
|
||||
})
|
||||
|
|
@ -36,6 +36,7 @@ import {
|
|||
activateTreePane,
|
||||
closeAllTreeTabs,
|
||||
closeOtherTreeTabs,
|
||||
closeTabPane,
|
||||
closeTreePane,
|
||||
closeTreeTabsToRight,
|
||||
collapseTreePane,
|
||||
|
|
@ -101,7 +102,7 @@ function ZoneMenu({
|
|||
renderActionItem(kit, {
|
||||
icon: 'close',
|
||||
label: t.common.close,
|
||||
onSelect: () => closeTreePane(paneId)
|
||||
onSelect: () => closeTabPane(paneId)
|
||||
})}
|
||||
{renderActionItem(kit, {
|
||||
disabled: !targets.others,
|
||||
|
|
@ -287,10 +288,9 @@ export function TreeGroup({
|
|||
// MAIN strands the whole app behind a strip.
|
||||
const minimizable = !shown.some(id => paneChrome(paneFor(id)).uncloseable)
|
||||
|
||||
// Tab ✕: a tool panel (terminal/logs) is REMOVED from the layout (comes back
|
||||
// via its toggle); everything else routes through its Close (a session tile
|
||||
// closes the session, a store-bound pane collapses).
|
||||
const closeTab = (paneId: string) => (isCollapsePane(paneId) ? dismissTreePane(paneId) : closeTreePane(paneId))
|
||||
// Middle-click / ⌘-click on a tab: one routing for every tab kind, the same
|
||||
// one the zone menu's Close and ⌘W use.
|
||||
const closeTab = (paneId: string) => closeTabPane(paneId)
|
||||
|
||||
// A pane whose store owns Close keeps the gesture even when the pane itself
|
||||
// is uncloseable — the workspace tab empties to a fresh draft rather than
|
||||
|
|
@ -324,6 +324,16 @@ export function TreeGroup({
|
|||
// Advertises the visible tab strip so panes can drop their own
|
||||
// self-naming labels (see [data-pane-self-label] in styles.css).
|
||||
data-zone-header={headerVisible || undefined}
|
||||
// The zone menu opens from the strip, the rail, the edit veil and the
|
||||
// body. Only the strip can name a chip, so resolve the target HERE for
|
||||
// every one of them — otherwise a right-click off the strip reused the
|
||||
// PREVIOUS target, and landing on the uncloseable workspace dropped
|
||||
// Close from the menu for a pane that closes fine.
|
||||
onContextMenu={e => {
|
||||
setMenuPane(
|
||||
(e.target as HTMLElement).closest('[data-tree-tab]')?.getAttribute('data-tree-tab') ?? undefined
|
||||
)
|
||||
}}
|
||||
ref={ref}
|
||||
style={wcOverlap ? { paddingTop: wcOverlap.y + wcOverlap.height } : undefined}
|
||||
>
|
||||
|
|
@ -392,11 +402,6 @@ export function TreeGroup({
|
|||
// data-zone-tabstrip: a drop over here STACKS (drag-session reads it).
|
||||
className="group/pane-header relative flex h-7 shrink-0 select-none bg-(--ui-sidebar-surface-background) [-webkit-app-region:no-drag] [--pane-tab-active-bg:var(--ui-sidebar-surface-background)]"
|
||||
data-zone-tabstrip={node.id}
|
||||
onContextMenu={e => {
|
||||
setMenuPane(
|
||||
(e.target as HTMLElement).closest('[data-tree-tab]')?.getAttribute('data-tree-tab') ?? undefined
|
||||
)
|
||||
}}
|
||||
onPointerDown={e =>
|
||||
// Tap the header to collapse to it / expand back — the DetailPane
|
||||
// / sidebar-section gesture (never for the main zone). Double-tap
|
||||
|
|
|
|||
|
|
@ -400,6 +400,36 @@ export function closeFocusedSessionTab(): boolean {
|
|||
return true
|
||||
}
|
||||
|
||||
/** ⌘W / zone-menu Close over a TOOL PANEL (terminal / logs): take the tab OUT
|
||||
* of the strip like any other tab, and sync the owning store so its toggle
|
||||
* (⌃` / the ⌘K row) stays truthful and can bring the pane back.
|
||||
*
|
||||
* A tool panel's closer is its visibility STORE, so routing Close through
|
||||
* `closeTreePane` only collapsed the zone to a rail — the tab stayed put and
|
||||
* Close read as a no-op. Dismiss first so the store listener's collapse lands
|
||||
* on an absent pane instead of minimizing a shared zone's surviving sibling. */
|
||||
export function closeToolPane(paneId: string) {
|
||||
dismissTreePane(paneId)
|
||||
paneClosers[paneId]?.()
|
||||
}
|
||||
|
||||
/** ⌘W over a TOOL PANEL zone (terminal / logs): close its active tab, the same
|
||||
* as any other tab. These zones host no chat strip, so `focusedSessionGroup`
|
||||
* skips them — without this rung ⌘W was a dead key over the terminal and the
|
||||
* logs pane, the only tabs in the app you couldn't close from the keyboard. */
|
||||
export function closeFocusedToolTab(): boolean {
|
||||
const group = tabTargetGroup(g => g.panes.some(isCollapsePane))
|
||||
const active = group?.active
|
||||
|
||||
if (!active || !isCollapsePane(active)) {
|
||||
return false
|
||||
}
|
||||
|
||||
closeToolPane(active)
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
/** Closeable siblings of `paneId` within its group, split by position — powers
|
||||
* the tab menu's Close-others / Close-to-the-right verbs (and their enablement). */
|
||||
function closeableTreeSiblings(paneId: string): { others: string[]; right: string[] } {
|
||||
|
|
@ -420,12 +450,22 @@ export function treeTabCloseTargets(paneId: string): { all: number; others: numb
|
|||
return { all: others.length + (isUncloseablePane(paneId) ? 0 : 1), others: others.length, right: right.length }
|
||||
}
|
||||
|
||||
/** Close a tab the way its kind expects: a tool panel leaves the strip (and
|
||||
* syncs its toggle), everything else routes through its owning Close. */
|
||||
export function closeTabPane(paneId: string) {
|
||||
if (isCollapsePane(paneId)) {
|
||||
closeToolPane(paneId)
|
||||
} else {
|
||||
closeTreePane(paneId)
|
||||
}
|
||||
}
|
||||
|
||||
export function closeOtherTreeTabs(paneId: string): void {
|
||||
closeableTreeSiblings(paneId).others.forEach(closeTreePane)
|
||||
closeableTreeSiblings(paneId).others.forEach(closeTabPane)
|
||||
}
|
||||
|
||||
export function closeTreeTabsToRight(paneId: string): void {
|
||||
closeableTreeSiblings(paneId).right.forEach(closeTreePane)
|
||||
closeableTreeSiblings(paneId).right.forEach(closeTabPane)
|
||||
}
|
||||
|
||||
/** Close every closeable tab in `paneId`'s group (the uncloseable workspace stays). */
|
||||
|
|
@ -433,7 +473,7 @@ export function closeAllTreeTabs(paneId: string): void {
|
|||
const tree = $layoutTree.get()
|
||||
const panes = (tree ? findGroupOfPane(tree, paneId) : null)?.panes ?? []
|
||||
|
||||
panes.filter(id => !isUncloseablePane(id)).forEach(closeTreePane)
|
||||
panes.filter(id => !isUncloseablePane(id)).forEach(closeTabPane)
|
||||
}
|
||||
|
||||
/** Pane ids in the tree under a `${prefix}:` namespace — lets a mirror prune
|
||||
|
|
|
|||
Loading…
Reference in New Issue