Merge pull request #73218 from NousResearch/bb/focused-zone-tabs

fix(desktop): make the tab verbs follow the focused zone like ⌘1-9
This commit is contained in:
brooklyn! 2026-07-28 03:14:03 -05:00 committed by GitHub
commit e4564586bc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 170 additions and 26 deletions

View file

@ -1,5 +1,5 @@
import { closeActiveTerminal } from '@/app/right-sidebar/terminal/terminals'
import { closeWorkspaceTab } from '@/components/pane-shell/tree/store'
import { closeFocusedSessionTab } from '@/components/pane-shell/tree/store'
import { isFocusWithin } from '@/lib/keybinds/combo'
import { $previewTabs, closeActiveRightRailTab } from '@/store/preview'
import { closeSessionTile, nextSessionTileForWorkspace } from '@/store/session-states'
@ -8,14 +8,17 @@ import { closeSessionTile, nextSessionTileForWorkspace } from '@/store/session-s
* W close the tab of the context you're in, by precedence:
* 1. a focused terminal its active terminal tab,
* 2. right-rail tabs (live preview and/or file peeks),
* 3. the MAIN zone its active tab (a session tile stacked into the workspace).
* 4. the MAIN (workspace) tab itself, when session tabs are stacked with it:
* 3. the FOCUSED chat zone its active tab (a session tile stacked into it).
* 4. the workspace tab itself, when session tabs are stacked with it:
* the workspace can't close, so W shifts the NEXT session tab into main
* (loads it as the primary + drops its now-redundant tile).
* Returns false when nothing closes, so W is a no-op it never closes the
* window (a bare workspace stays put). Shared by the keyboard path (Win/Linux)
* and the macOS menu-accelerator IPC.
*
* Steps 3-4 follow the same focused zone 19 indexes, so a second chat zone
* with its own tab strip closes ITS tab instead of main's.
*
* `loadSessionIntoWorkspace` carries the app's route-based "load this session
* into main" (the two call sites have router access); omitting it disables the
* step-4 promotion (W stays the pre-existing no-op on the main tab).
@ -28,15 +31,15 @@ export function closeActiveTab(loadSessionIntoWorkspace?: (storedSessionId: stri
}
// Gate on tab *presence*, not on the selection: a stale `$rightRailActiveTabId`
// would otherwise make ⌘W fall through to closeWorkspaceTab() and look broken
// with a tab still on screen. The store resolves which tab that is.
// would otherwise make ⌘W fall through to closeFocusedSessionTab() and look
// broken with a tab still on screen. The store resolves which tab that is.
if ($previewTabs.get().length > 0) {
return closeActiveRightRailTab()
}
// A closeable main-zone tab (a session tile that's the active tab) closes
// outright; the uncloseable workspace tab returns false and falls through.
if (closeWorkspaceTab()) {
// A closeable tab in the focused chat zone (a session tile that's the active
// tab) closes outright; the uncloseable workspace tab falls through.
if (closeFocusedSessionTab()) {
return true
}

View file

@ -43,6 +43,7 @@ import {
$layoutTree,
$treeDragging,
type DropHint,
isSessionStripPane,
revealTreePane,
SESSION_TILE_DRAG
} from '@/components/pane-shell/tree/store'
@ -84,7 +85,7 @@ function chatZonePane(groupId: string): null | string {
const tree = $layoutTree.get()
const panes = tree ? (findGroup(tree, groupId)?.panes ?? []) : []
return panes.find(p => p === 'workspace' || p.startsWith('session-tile:')) ?? null
return panes.find(isSessionStripPane) ?? null
}
/**

View file

@ -0,0 +1,83 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
// ⌘W / ⌘T / ⌘⇧T / the strip "+" used to hardcode the workspace's zone while
// ⌘1…⌘9 followed the interacted zone — so every tab verb missed a SECOND chat
// zone the user was working in. They now resolve the same focused zone.
describe('focused chat zone drives the tab verbs', () => {
beforeEach(() => {
window.localStorage.clear()
vi.resetModules()
})
afterEach(() => {
vi.resetModules()
})
async function setup() {
const tree = await import('@/components/pane-shell/tree/store')
const model = await import('@/components/pane-shell/tree/model')
const { registry } = await import('@/contrib/registry')
for (const id of ['workspace', 'files', 'session-tile:a', 'session-tile:b']) {
registry.register({
area: 'panes',
data: id === 'workspace' ? { placement: 'main', uncloseable: true } : { placement: 'main' },
id,
render: () => null,
title: id
})
}
// Two chat zones side by side: main holds the workspace, the second holds
// its own session tabs (a split-off stack).
tree.declareDefaultTree(
model.split('row', [
model.group(['workspace'], { active: 'workspace', id: 'grp-main' }),
model.group(['session-tile:a', 'session-tile:b'], { active: 'session-tile:b', id: 'grp-side' })
])
)
return { model, tree }
}
it('⌘T anchors the new tab to the focused zone, not always the workspace', async () => {
const { tree } = await setup()
tree.noteActiveTreeGroup('grp-side')
expect(tree.focusedSessionTabAnchor()).toBe('session-tile:b')
tree.noteActiveTreeGroup('grp-main')
expect(tree.focusedSessionTabAnchor()).toBe('workspace')
})
it('a non-chat zone (files/terminal focus) falls back to the workspace', async () => {
const { model, tree } = await setup()
tree.declareDefaultTree(
model.split('row', [
model.group(['workspace'], { active: 'workspace', id: 'grp-main' }),
model.group(['files'], { active: 'files', id: 'grp-files' })
])
)
tree.noteActiveTreeGroup('grp-files')
expect(tree.focusedSessionTabAnchor()).toBe('workspace')
// ⌘W must not close the file tree just because it holds focus.
expect(tree.closeFocusedSessionTab()).toBe(false)
})
it('⌘W closes the focused zone active tab and leaves the workspace alone', async () => {
const { model, tree } = await setup()
tree.noteActiveTreeGroup('grp-side')
expect(tree.closeFocusedSessionTab()).toBe(true)
expect(model.allPaneIds(tree.$layoutTree.get()!)).not.toContain('session-tile:b')
// Back on main: the uncloseable workspace is a no-op (the caller promotes
// a stacked session instead of closing the window).
tree.noteActiveTreeGroup('grp-main')
expect(tree.closeFocusedSessionTab()).toBe(false)
expect(model.allPaneIds(tree.$layoutTree.get()!)).toContain('workspace')
})
})

View file

@ -39,7 +39,9 @@ import {
collapseTreePane,
dismissTreePane,
isCollapsePane,
isSessionStripPane,
moveTreePane,
noteActiveTreeGroup,
restoreTreePane,
SESSION_TILE_DRAG,
setTreeGroupHeaderHidden,
@ -507,16 +509,26 @@ export function TreeGroup({
return <Fragment key={paneId}>{chrome.tabWrap ? chrome.tabWrap(tab) : tab}</Fragment>
})}
{/* Plain "+" after the last tab of the MAIN strip (the workspace
zone) always shown, no tab/button chrome, just the glyph.
Creates a new session tab (mirrors T) via the app-registered
action; hidden when unwired or the zone is minimized. */}
{node.panes.includes('workspace') && newSessionTabAction && !node.minimized && (
{/* Plain "+" after the last tab of a CHAT strip (the workspace
zone, or any zone holding session tabs) always shown, no
tab/button chrome, just the glyph. Creates a new session tab
(mirrors T) via the app-registered action; the pointerdown
focuses this zone first, so the tab lands in THIS strip.
Hidden when unwired or the zone is minimized. */}
{shown.some(isSessionStripPane) && newSessionTabAction && !node.minimized && (
<button
aria-label={t.zones.newSessionTab}
className="grid size-7 shrink-0 place-items-center self-center bg-transparent text-(--ui-text-quaternary) transition-colors hover:text-foreground [-webkit-app-region:no-drag]"
onClick={() => newSessionTabAction()}
onPointerDown={e => e.stopPropagation()}
onPointerDown={e => {
e.stopPropagation()
// The action docks into the FOCUSED chat zone; clicking a
// background strip's "+" must make THAT zone the focused
// one first, or the tab opens in whichever zone was last
// clicked. (pointerdown's own focus tracking would land
// after the click handler reads the anchor.)
noteActiveTreeGroup(node.id)
}}
title={t.zones.newSessionTab}
type="button"
>
@ -716,7 +728,7 @@ function ZoneDropOverlay({ node }: { node: GroupNode }) {
// now (stack into its tabs / split its edges); only a CHAT zone's center is
// a link-to-chat (the composer overlay owns that visual).
const sessionDrag = dragging === SESSION_TILE_DRAG
const chatZone = node.panes.some(p => p === 'workspace' || p.startsWith('session-tile:'))
const chatZone = node.panes.some(isSessionStripPane)
const isDragSource = node.panes.includes(dragging)

View file

@ -21,6 +21,7 @@ import {
findGroup,
findGroupOfPane,
groupLeafIds,
type GroupNode,
insertAtGroup,
isLayoutNode,
type LayoutNode,
@ -282,12 +283,49 @@ const isUncloseablePane = (paneId: string): boolean =>
(registry.getArea('panes').find(c => c.id === paneId)?.data as { uncloseable?: boolean } | undefined)?.uncloseable
)
/** W "main tabs always": close the MAIN (workspace) zone's active tab, unless
* it's the uncloseable workspace itself. Returns false when there's nothing to
* close, so W stays a no-op it never closes the window. */
export function closeWorkspaceTab(): boolean {
/** A pane that belongs to a CHAT tab strip — the workspace or a session tile. */
export const isSessionStripPane = (paneId: string): boolean =>
paneId === 'workspace' || paneId.startsWith('session-tile:')
/** The zone the session-tab verbs (⌘W / T / T / the strip's "+") act on:
* the FOCUSED zone when it hosts a chat strip, else the workspace's zone.
* Same source 19 indexes ($activeTreeGroup), so the number keys and the
* tab verbs can't disagree about which strip is "the" strip. Focus parked in
* the sidebar / terminal / files must NOT retarget them those zones fall
* back to main rather than letting W close the file tree. */
function focusedSessionGroup(): GroupNode | null {
const tree = $layoutTree.get()
const active = tree ? findGroupOfPane(tree, 'workspace')?.active : null
if (!tree) {
return null
}
const groupId = $activeTreeGroup.get()
const focused = groupId ? findGroup(tree, groupId) : null
return focused?.panes.some(isSessionStripPane) ? focused : findGroupOfPane(tree, 'workspace')
}
/** The pane a NEW session tab should dock beside (T): the focused chat zone's
* active session pane, else its first. Null when no zone hosts a chat strip
* the caller falls back to the workspace. */
export function focusedSessionTabAnchor(): null | string {
const group = focusedSessionGroup()
if (!group) {
return null
}
const active = group.active
return active && isSessionStripPane(active) ? active : (group.panes.find(isSessionStripPane) ?? null)
}
/** W: close the FOCUSED chat zone's active tab, unless it's the uncloseable
* workspace itself. Returns false when there's nothing to close, so W stays a
* no-op it never closes the window. */
export function closeFocusedSessionTab(): boolean {
const active = focusedSessionGroup()?.active
if (!active || isUncloseablePane(active)) {
return false
@ -416,7 +454,7 @@ export function cycleTreeTabInFocusedZone(direction: 1 | -1): boolean {
const group = groupId && tree ? findGroup(tree, groupId) : null
const panes = group ? shownPanesInGroup(group) : []
if (panes.length < 2 || !panes.some(id => id === 'workspace' || id.startsWith('session-tile:'))) {
if (panes.length < 2 || !panes.some(isSessionStripPane)) {
return false
}

View file

@ -23,6 +23,7 @@ import { findGroup, findGroupOfPane, type LayoutNode } from '@/components/pane-s
import {
$activeTreeGroup,
$layoutTree,
focusedSessionTabAnchor,
moveTreePane,
noteActiveTreeGroup,
revealTreePane
@ -522,7 +523,11 @@ function syncTileStripOrder() {
* move path is what lets a tile's own TAB be dragged like a sidebar row drop
* it on a zone/edge/strip and the tile goes there (drop-on-a-composer links
* instead, handled by the drag resolver). The session LOADED IN MAIN never
* opens as a tile (same transcript twice, fighting one runtime silly). */
* opens as a tile (same transcript twice, fighting one runtime silly).
*
* An unanchored open (T, T on a tile that predates anchors) docks into the
* FOCUSED chat zone the same zone 19 and W act on so a new tab lands
* in the strip the user is looking at, not always main's. */
export function openSessionTile(
storedSessionId: string,
dir: TileDock = 'right',
@ -535,8 +540,10 @@ export function openSessionTile(
return
}
const dock = anchor ?? focusedSessionTabAnchor() ?? undefined
if (!tiles.some(t => t.storedSessionId === storedSessionId)) {
saveTiles([...tiles, { anchor, before, dir, storedSessionId }])
saveTiles([...tiles, { anchor: dock, before, dir, storedSessionId }])
// Adoption is async via the registry — order sync runs after the move path
// below; a brand-new tile's strip slot is already in `before`.
@ -546,11 +553,11 @@ export function openSessionTile(
// Already open: relocate the existing pane to the drop target (pane-mirror
// only docks on first adoption, so a re-drag must move the tree pane itself).
const tree = $layoutTree.get()
const target = tree ? findGroupOfPane(tree, anchor ?? 'workspace')?.id : null
const target = tree ? findGroupOfPane(tree, dock ?? 'workspace')?.id : null
if (target) {
moveTreePane(`${TILE_PANE_PREFIX}${storedSessionId}`, { before: before ?? null, groupId: target, pos: dir })
patchSessionTile(storedSessionId, { anchor, before: before ?? undefined, dir })
patchSessionTile(storedSessionId, { anchor: dock, before: before ?? undefined, dir })
syncTileStripOrder()
}
}