diff --git a/apps/desktop/src/components/pane-shell/tree/store.ts b/apps/desktop/src/components/pane-shell/tree/store.ts index f40427156c6..e8a6bc4a28b 100644 --- a/apps/desktop/src/components/pane-shell/tree/store.ts +++ b/apps/desktop/src/components/pane-shell/tree/store.ts @@ -351,14 +351,52 @@ export function treePanesWithPrefix(prefix: string): string[] { * An atom so the strip re-renders when the action becomes available. */ export const $newSessionTabAction = atom<(() => void) | null>(null) -/** ⌘1…⌘9: activate the Nth tab of the FOCUSED zone (the interaction tracker's - * group), but only when it's a real tab strip (≥2 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 holds focus. */ +/** + * Keyboard slots (⌘1…⌘9, ⌃Tab) must index the SAME tabs the strip paints — + * chrome-hidden panes (files in Focus layout), unregistered ones, and + * narrow-collapsed collapsibles stay in `group.panes` but aren't chips. Walking + * the raw array made ⌘2 land on what the strip called tab 1 after a hidden + * pane sat earlier in the list (classic after-⌘W-shift offset). + */ +function shownPanesInGroup(group: { panes: readonly string[] }): string[] { + const hidden = $hiddenTreePanes.get() + const registered = registry.getArea('panes') + const paneFor = (id: string) => registered.find(c => c.id === id) + + return group.panes.filter(id => { + const pane = paneFor(id) + + if (!pane) { + return false + } + + if (hidden.has(id)) { + return false + } + + // Match TreeGroup's paneShown for the narrow breakpoint — collapsible + // panes drop out of the strip when the viewport collapses them. + if ( + typeof window !== 'undefined' && + window.matchMedia?.(SIDEBAR_COLLAPSE_MEDIA_QUERY).matches && + Boolean((pane.data as { collapsible?: boolean } | undefined)?.collapsible) + ) { + return false + } + + return true + }) +} + +/** ⌘1…⌘9: activate the Nth *visible* tab of the FOCUSED zone (the interaction + * tracker's group), 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 holds focus. */ export function activateTreeTabSlot(slot: number): boolean { const groupId = $activeTreeGroup.get() const tree = $layoutTree.get() - const panes = (groupId && tree ? findGroup(tree, groupId)?.panes : null) ?? [] + const group = groupId && tree ? findGroup(tree, groupId) : null + const panes = group ? shownPanesInGroup(group) : [] if (panes.length < 2 || slot < 1 || slot > panes.length) { return false @@ -369,20 +407,23 @@ export function activateTreeTabSlot(slot: number): boolean { return true } -/** ⌃Tab / ⌃⇧Tab: cycle the FOCUSED zone's tabs (wrapping) — but only a - * session/main strip with ≥2 tabs. Returns false so the caller falls back to - * the recent-session switcher when the focus isn't a chat tab strip. */ +/** ⌃Tab / ⌃⇧Tab: cycle the FOCUSED 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 focus isn't a chat tab strip. */ export function cycleTreeTabInFocusedZone(direction: 1 | -1): boolean { const groupId = $activeTreeGroup.get() const tree = $layoutTree.get() const group = groupId && tree ? findGroup(tree, groupId) : null - const panes = group?.panes ?? [] + const panes = group ? shownPanesInGroup(group) : [] if (panes.length < 2 || !panes.some(id => id === 'workspace' || id.startsWith('session-tile:'))) { return false } - const idx = Math.max(0, panes.indexOf(group!.active ?? '')) + // 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 nextId = panes[(idx + direction + panes.length) % panes.length] activateTreePane(group!.id, nextId) diff --git a/apps/desktop/src/components/pane-shell/tree/tab-slot-shown.test.ts b/apps/desktop/src/components/pane-shell/tree/tab-slot-shown.test.ts new file mode 100644 index 00000000000..5d8921a9192 --- /dev/null +++ b/apps/desktop/src/components/pane-shell/tree/tab-slot-shown.test.ts @@ -0,0 +1,81 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +// ⌘1…⌘9 / ⌃Tab must index the same tabs the strip paints. A chrome-hidden +// pane (Focus layout's `files`) stays in `group.panes` but isn't a chip — +// indexing the raw array made ⌘2 land on the strip's first session tab. + +describe('activateTreeTabSlot indexes shown panes only', () => { + 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 === 'files' ? { collapsible: true, placement: 'side' } : { placement: 'main' }, + id, + render: () => null, + title: id + }) + } + + // Focus-ish packing: files rides with workspace; two session tiles stacking + // of top of that — pane order in the tree: workspace, files, A, B. + tree.declareDefaultTree( + model.group(['workspace', 'files', 'session-tile:a', 'session-tile:b'], { + active: 'workspace', + id: 'grp-main' + }) + ) + tree.noteActiveTreeGroup('grp-main') + tree.setTreePaneHidden('files', true) + + const activeOf = () => model.findGroup(tree.$layoutTree.get()!, 'grp-main')?.active ?? null + + return { activeOf, tree } + } + + it('⌘1 is workspace and ⌘2 is the first SESSION tab when files is hidden', async () => { + const { activeOf, tree } = await setup() + + expect(tree.activateTreeTabSlot(1)).toBe(true) + expect(activeOf()).toBe('workspace') + + expect(tree.activateTreeTabSlot(2)).toBe(true) + expect(activeOf()).toBe('session-tile:a') + + expect(tree.activateTreeTabSlot(3)).toBe(true) + expect(activeOf()).toBe('session-tile:b') + }) + + it('does not claim a slot past the visible count (falls through to profile switch)', async () => { + const { tree } = await setup() + + // Shown: workspace + A + B → 3. Slot 4 would have been `B` on the raw array + // (workspace, files, A, B) before this fix — now it correctly refuses. + expect(tree.activateTreeTabSlot(4)).toBe(false) + }) + + it('⌃Tab cycles only visible chips', async () => { + const { activeOf, tree } = await setup() + + expect(tree.cycleTreeTabInFocusedZone(1)).toBe(true) + expect(activeOf()).toBe('session-tile:a') + + expect(tree.cycleTreeTabInFocusedZone(1)).toBe(true) + expect(activeOf()).toBe('session-tile:b') + + expect(tree.cycleTreeTabInFocusedZone(1)).toBe(true) + expect(activeOf()).toBe('workspace') + }) +})