fix(desktop): keep ⌘1-9 working when the pointer is off the panes

Hover-first targeting (#74447) made the tab verbs read the hovered zone
else the focused one. But ⌘1-9 and ⌃Tab then took that single answer as
final: with the pointer over the sidebar, the titlebar, or any non-zone
chrome, the resolver returned null and the keys did nothing at all.

Turn the resolver into an eligibility ladder — hovered, then focused,
then the workspace's zone — where each rung must actually satisfy the
verb (a real tab strip for the number keys, a chat strip for ⌃Tab and
the ⌘W / ⌘T family) to claim the keystroke. Pointing at nothing now
falls through to focus, and a fresh window with no interaction yet falls
through to main, so the keys always land somewhere sensible.

This also fixes the narrower case the old code shared with ⌘W: hovering
a zone that is NOT a tab strip (a lone file tree) used to swallow the
key rather than handing it to the next rung.

⌘W / ⌘T already laddered to the workspace, so they keep their behavior
and simply share the one resolver again.
This commit is contained in:
Brooklyn Nicholson 2026-07-29 23:58:41 -05:00
parent 8eb06e75b9
commit 0ad06ae9da
2 changed files with 115 additions and 45 deletions

View file

@ -87,4 +87,61 @@ describe('hovered zone retargets the tab verbs', () => {
expect(model.allPaneIds(tree.$layoutTree.get()!)).not.toContain('session-tile:c')
expect(model.allPaneIds(tree.$layoutTree.get()!)).toContain('workspace')
})
// Hovering chrome that is not a zone at all (the sidebar, the titlebar, the
// statusbar) reports no group. The keys must fall through to focus rather
// than dead-ending — pointing away from the panes is not a reason to stop
// switching tabs.
it('hovering non-pane chrome falls through to the focused zone', async () => {
const { activeOf, tree } = await setup()
tree.noteActiveTreeGroup('grp-side')
tree.noteHoveredTreeGroup(null)
expect(tree.activateTreeTabSlot(2)).toBe(true)
expect(activeOf('grp-side')).toBe('session-tile:c')
expect(tree.cycleTreeTabInFocusedZone(1)).toBe(true)
expect(activeOf('grp-side')).toBe('session-tile:b')
})
// Nothing interacted with yet (fresh boot) or focus parked somewhere that
// can't serve the verb: main is the last rung, so ⌘2 still works.
it('falls through to the workspace zone with neither hover nor focus', async () => {
const { activeOf, tree } = await setup()
tree.noteActiveTreeGroup(null)
tree.noteHoveredTreeGroup(null)
expect(tree.activateTreeTabSlot(2)).toBe(true)
expect(activeOf('grp-main')).toBe('session-tile:a')
expect(activeOf('grp-side')).toBe('session-tile:b')
})
// The ladder is per-rung eligible, not "first rung wins": a hovered zone that
// ISN'T a tab strip must hand the keys to the next rung, not swallow them.
it('an ineligible hovered zone hands off instead of swallowing the key', async () => {
const { activeOf, model, tree } = await setup()
const { registry } = await import('@/contrib/registry')
registry.register({ area: 'panes', data: { placement: 'right' }, id: 'files', render: () => null, title: 'files' })
// Replace the tree outright — declareDefaultTree only ADOPTS into an
// existing one, so the lone-files zone has to be set directly.
tree.$layoutTree.set(
model.split('row', [
model.group(['workspace', 'session-tile:a'], { active: 'workspace', id: 'grp-main' }),
model.group(['files'], { active: 'files', id: 'grp-files' })
])
)
// Pointer resting on the lone file tree, focus nowhere.
tree.noteActiveTreeGroup(null)
tree.noteHoveredTreeGroup('grp-files')
expect(tree.activateTreeTabSlot(2)).toBe(true)
expect(activeOf('grp-main')).toBe('session-tile:a')
// ⌘W must not close the file tree from a rung that can't serve it.
expect(activeOf('grp-files')).toBe('files')
expect(model.allPaneIds(tree.$layoutTree.get()!)).toContain('files')
})
})

View file

@ -269,13 +269,33 @@ export function noteHoveredTreeGroup(groupId: null | string) {
}
}
/** The zone every keyboard tab verb acts on: the HOVERED zone, else the focused
* one. Hover-first is what makes 19 land in the pane you're pointing at
* without clicking into it first; with the pointer outside the panes it's the
* plain focused-zone behavior. One resolver so the number keys, Tab, and the
* W / T family can never disagree about which zone is "the" zone. */
function tabTargetGroupId(): null | string {
return $hoveredTreeGroup.get() ?? $activeTreeGroup.get()
/** The zone every keyboard tab verb acts on, as an ELIGIBILITY LADDER: the
* hovered zone, else the focused one, else the workspace's. Each rung must
* satisfy `eligible` to claim the keys, so a pointer parked somewhere that
* can't serve the verb the sidebar, the titlebar, a single-pane rail
* hands off to the next rung instead of swallowing the keystroke. Hover-first
* is what makes 19 land in the pane you're pointing at without clicking
* into it; the rungs below are why the keys still work when you're pointing
* at nothing. One resolver so the number keys, Tab, and the W / T family
* can never disagree about which zone is "the" zone. */
function tabTargetGroup(eligible: (group: GroupNode) => boolean): GroupNode | null {
const tree = $layoutTree.get()
if (!tree) {
return null
}
for (const groupId of [$hoveredTreeGroup.get(), $activeTreeGroup.get()]) {
const group = groupId ? findGroup(tree, groupId) : null
if (group && eligible(group)) {
return group
}
}
const main = findGroupOfPane(tree, 'workspace')
return main && eligible(main) ? main : null
}
const treeGroupOfEvent = (event: Event): null | string => {
@ -328,23 +348,13 @@ 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 TARGET zone (hovered, else focused) when it hosts a chat strip, else the
* workspace's zone. Same source 19 indexes (`tabTargetGroupId`), so the
* number keys and the tab verbs can't disagree about which strip is "the"
* strip. A target parked in the sidebar / terminal / files must NOT retarget
* them those zones fall back to main rather than letting W close the file
* tree. */
* the first of hovered / focused / workspace that hosts a chat strip. Same
* ladder 19 indexes, so the number keys and the tab verbs can't disagree
* about which strip is "the" strip. A target parked in the sidebar / terminal
* / files must NOT retarget them those zones fall through to main rather
* than letting W close the file tree. */
function focusedSessionGroup(): GroupNode | null {
const tree = $layoutTree.get()
if (!tree) {
return null
}
const groupId = tabTargetGroupId()
const focused = groupId ? findGroup(tree, groupId) : null
return focused?.panes.some(isSessionStripPane) ? focused : findGroupOfPane(tree, 'workspace')
return tabTargetGroup(group => group.panes.some(isSessionStripPane))
}
/** The pane a NEW session tab should dock beside (T): the focused chat zone's
@ -467,50 +477,53 @@ function shownPanesInGroup(group: { panes: readonly string[] }): string[] {
})
}
/** 19: activate the Nth *visible* tab of the target zone (hovered, else
* focused), but only when it's a real tab strip (2 shown panes).
* Returns false so the caller falls back to its default (profile switch)
* the number keys mean "switch tab" only while a multi-tab zone is targeted. */
/** 19: activate the Nth *visible* tab of the target zone the first of
* hovered / focused / workspace that is a real tab strip (2 shown panes).
* Pointing at the sidebar (or nothing) therefore still switches main's tabs
* instead of dead-ending. Returns false so the caller falls back to its
* default (profile switch) when no zone qualifies. */
export function activateTreeTabSlot(slot: number): boolean {
const groupId = tabTargetGroupId()
const tree = $layoutTree.get()
const group = groupId && tree ? findGroup(tree, groupId) : null
const group = tabTargetGroup(candidate => shownPanesInGroup(candidate).length >= 2)
const panes = group ? shownPanesInGroup(group) : []
if (panes.length < 2 || slot < 1 || slot > panes.length) {
if (!group || slot < 1 || slot > panes.length) {
return false
}
activateTreePane(groupId!, panes[slot - 1])
activateTreePane(group.id, panes[slot - 1])
return true
}
/** ⌃Tab / Tab: cycle the target zone's *visible* tabs (wrapping) but only a
* session/main strip with 2 shown tabs. Returns false so the caller falls
* back to the recent-session switcher when the target isn't a chat tab strip. */
/** ⌃Tab / Tab: cycle the target zone's *visible* tabs (wrapping) the first
* of hovered / focused / workspace that is a chat strip with 2 shown tabs.
* Returns false so the caller falls back to the recent-session switcher when
* no zone qualifies. */
export function cycleTreeTabInFocusedZone(direction: 1 | -1): boolean {
const groupId = tabTargetGroupId()
const tree = $layoutTree.get()
const group = groupId && tree ? findGroup(tree, groupId) : null
const panes = group ? shownPanesInGroup(group) : []
const group = tabTargetGroup(candidate => {
const shown = shownPanesInGroup(candidate)
if (panes.length < 2 || !panes.some(isSessionStripPane)) {
return shown.length >= 2 && shown.some(isSessionStripPane)
})
if (!group) {
return false
}
const panes = shownPanesInGroup(group)
// Active may itself be hidden (Files collapsed mid-cycle) — treat it as
// missing so the step starts from a real chip rather than landing on a ghost.
const current = Math.max(0, panes.indexOf(group!.active ?? ''))
const idx = panes.includes(group!.active ?? '') ? current : 0
const current = Math.max(0, panes.indexOf(group.active ?? ''))
const idx = panes.includes(group.active ?? '') ? current : 0
const nextId = panes[(idx + direction + panes.length) % panes.length]
activateTreePane(group!.id, nextId)
activateTreePane(group.id, nextId)
// Cycling onto a session/main tab must surface the name card — a zone that
// was double-tap-hidden stays headerless otherwise ("the one that cycles
// never gets it").
if (nextId === 'workspace' || nextId.startsWith('session-tile:')) {
setTreeGroupHeaderHidden(group!.id, false)
if (isSessionStripPane(nextId)) {
setTreeGroupHeaderHidden(group.id, false)
}
return true