mirror of
https://github.com/stablyai/orca.git
synced 2026-10-18 08:03:22 +00:00
fix(shortcuts): jump to the only terminal from non-terminal focus
Address codex review (round 2). The 'no-op when terminalTabs.length <= 1' guard skipped the legitimate case of 'one terminal exists, but an editor or browser is currently focused' - the chord did nothing instead of jumping to that single terminal, defeating the whole point of the shortcut. Narrow the no-op to the only true case: zero terminals, or the only terminal already focused. Add tests for both edges.
This commit is contained in:
@@ -109,4 +109,33 @@ describe('handleSwitchTerminalTab', () => {
|
||||
expect(store.setActiveTab).toHaveBeenCalledWith('term-1')
|
||||
expect(store.setActiveTabType).toHaveBeenCalledWith('terminal')
|
||||
})
|
||||
|
||||
it('jumps from an editor to the only terminal when one terminal exists', () => {
|
||||
const store = makeStore('editor')
|
||||
store.activeFileId = 'editor-1'
|
||||
getStateMock.mockReturnValue(store)
|
||||
getActiveTabNavOrderMock.mockReturnValue([
|
||||
{ type: 'editor', id: 'editor-1' },
|
||||
{ type: 'terminal', id: 'term-1' },
|
||||
{ type: 'browser', id: 'browser-1' }
|
||||
])
|
||||
|
||||
expect(handleSwitchTerminalTab(1)).toBe(true)
|
||||
expect(store.setActiveTab).toHaveBeenCalledWith('term-1')
|
||||
expect(store.setActiveTabType).toHaveBeenCalledWith('terminal')
|
||||
})
|
||||
|
||||
it('returns false when the only terminal is already active', () => {
|
||||
const store = makeStore('terminal')
|
||||
store.activeTabId = 'term-1'
|
||||
getStateMock.mockReturnValue(store)
|
||||
getActiveTabNavOrderMock.mockReturnValue([
|
||||
{ type: 'terminal', id: 'term-1' },
|
||||
{ type: 'editor', id: 'editor-1' }
|
||||
])
|
||||
|
||||
expect(handleSwitchTerminalTab(1)).toBe(false)
|
||||
expect(store.setActiveTab).not.toHaveBeenCalled()
|
||||
expect(store.setActiveTabType).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -55,7 +55,7 @@ export function handleSwitchTerminalTab(direction: number): boolean {
|
||||
const terminalTabs = getActiveTabNavOrder(store, worktreeId).filter(
|
||||
(entry) => entry.type === 'terminal'
|
||||
)
|
||||
if (terminalTabs.length <= 1) {
|
||||
if (terminalTabs.length === 0) {
|
||||
return false
|
||||
}
|
||||
const currentId =
|
||||
@@ -67,6 +67,13 @@ export function handleSwitchTerminalTab(direction: number): boolean {
|
||||
// Why: when an editor/browser tab is active, jump to the first terminal on
|
||||
// forward navigation instead of skipping to index 1.
|
||||
const idx = terminalTabs.findIndex((t) => t.id === currentId)
|
||||
// Why: only no-op when the sole terminal is already focused. With one terminal
|
||||
// and an editor/browser active, the chord must still jump to that terminal -
|
||||
// that is the whole point of the shortcut. The single-terminal-already-active
|
||||
// case is the only true no-op.
|
||||
if (terminalTabs.length === 1 && idx === 0) {
|
||||
return false
|
||||
}
|
||||
const currentIndex = idx === -1 && direction > 0 ? -1 : idx === -1 ? 0 : idx
|
||||
const next = terminalTabs[(currentIndex + direction + terminalTabs.length) % terminalTabs.length]
|
||||
store.setActiveTab(next.id)
|
||||
|
||||
Reference in New Issue
Block a user