diff --git a/src/renderer/src/components/sidebar/worktree-keyboard-cycle.test.ts b/src/renderer/src/components/sidebar/worktree-keyboard-cycle.test.ts index 9f5562d91cd..f3b00037106 100644 --- a/src/renderer/src/components/sidebar/worktree-keyboard-cycle.test.ts +++ b/src/renderer/src/components/sidebar/worktree-keyboard-cycle.test.ts @@ -1,11 +1,49 @@ import { describe, expect, it } from 'vitest' import type { HostSectionRow } from './host-section-rows' +import type { FolderWorkspaceRow } from './worktree-list/grouping/row-types' import { + getCyclableRowIdentity, + getCyclableWorktreeRows, getCyclableWorktreeIds, getCyclableWorktrees, + resolveActiveCycleIdentity, resolveCycledWorktreeId } from './worktree-keyboard-cycle' +const folderRow: FolderWorkspaceRow = { + type: 'folder-workspace', + key: 'folder-workspace:folder-1', + folderWorkspace: { + id: 'folder-1', + projectGroupId: 'group-1', + name: 'Folder 1', + folderPath: '/group-1/folder-1', + linkedTask: null, + comment: '', + isArchived: false, + isUnread: false, + isPinned: false, + sortOrder: 1, + lastActivityAt: 1, + createdAt: 1, + updatedAt: 1 + }, + projectGroup: { + id: 'group-1', + name: 'Group 1', + parentPath: '/group-1', + parentGroupId: null, + createdFrom: 'folder-scan', + tabOrder: 0, + isCollapsed: false, + color: null, + createdAt: 1, + updatedAt: 1 + }, + depth: 0, + groupDepth: 0 +} + describe('resolveCycledWorktreeId', () => { const worktreeIds = ['a', 'b', 'c'] @@ -110,22 +148,62 @@ describe('getCyclableWorktreeIds', () => { ]) }) - it('leaves folder workspaces out of the rotation', () => { - // Why: their synthetic `folder:` id is not activatable through - // activateAndRevealWorktree, so arrowing onto one would be a dead keypress. - const rows: HostSectionRow[] = [ - { - type: 'folder-workspace', - key: 'folder-workspace:folder-1', - folderWorkspace: { id: 'folder-1', projectGroupId: 'group-1' } as never, - projectGroup: { id: 'group-1' } as never, - depth: 0, - groupDepth: 0 - }, - worktree('plain-b') + it('includes folder workspaces between git worktrees in visible order', () => { + const rows = [worktree('a'), folderRow, worktree('b')] + + expect(getCyclableWorktreeIds(rows, 'single-location')).toEqual(['a', 'folder:folder-1', 'b']) + }) + + it('anchors both directions on the active folder workspace', () => { + const rows = getCyclableWorktreeRows( + [worktree('a'), folderRow, worktree('b')], + 'single-location' + ) + const activeWorktreeId = resolveActiveCycleIdentity({ + rows, + activeWorktreeId: 'folder:folder-1', + activeWorkspaceExecutionHostId: 'local' + }) + const worktreeIds = rows.map(getCyclableRowIdentity) + + expect(resolveCycledWorktreeId({ worktreeIds, activeWorktreeId, direction: 'up' })).toBe( + getCyclableRowIdentity(rows[0]) + ) + expect(resolveCycledWorktreeId({ worktreeIds, activeWorktreeId, direction: 'down' })).toBe( + getCyclableRowIdentity(rows[2]) + ) + }) + + it('keeps folder placement while preferring a pinned worktree natural row', () => { + const rows = [ + worktree('dup', true), + folderRow, + { ...worktree('dup'), rowKey: 'row:dup-natural' }, + worktree('b') ] - expect(getCyclableWorktreeIds(rows, 'single-location')).toEqual(['plain-b']) + expect(getCyclableWorktreeIds(rows, 'duplicate-in-groups')).toEqual([ + 'folder:folder-1', + 'dup', + 'b' + ]) + }) + + it('keeps folder keys distinct from git ids and preserves same-id host ownership', () => { + const otherHostFolder = { + ...folderRow, + folderWorkspace: { ...folderRow.folderWorkspace, executionHostId: 'ssh:host-b' as const } + } + const rows = getCyclableWorktreeRows( + [worktree('folder-1'), folderRow, otherHostFolder], + 'single-location' + ) + + expect(rows.map(getCyclableRowIdentity)).toEqual([ + 'local|folder-1', + 'local|folder:folder-1', + 'ssh:host-b|folder:folder-1' + ]) }) it('drops worktrees the sidebar elided inside a collapsed host section', () => { diff --git a/src/renderer/src/components/sidebar/worktree-keyboard-cycle.ts b/src/renderer/src/components/sidebar/worktree-keyboard-cycle.ts index 4c50c78b5f5..022c241c453 100644 --- a/src/renderer/src/components/sidebar/worktree-keyboard-cycle.ts +++ b/src/renderer/src/components/sidebar/worktree-keyboard-cycle.ts @@ -2,8 +2,11 @@ import type { HostSectionRow } from './host-section-rows' import type { Worktree } from '../../../../shared/worktree/types' import { composeWorktreeHostIdentity } from '../../../../shared/worktree/host-qualified-identity' import { getWorktreeExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' -import type { PinnedWorktreeDisplayPolicy, WorktreeRow } from './worktree-list/grouping/row-types' -import { getPreferredWorktreeRows } from './worktree-sidebar-row-preference' +import type { PinnedWorktreeDisplayPolicy } from './worktree-list/grouping/row-types' +import { + getRenderedWorkspaceRowsInSidebarOrder, + type RenderedWorkspaceRow +} from './worktree-sidebar-row-preference' /** Host-resolved identity for a cyclable row. * @@ -11,7 +14,7 @@ import { getPreferredWorktreeRows } from './worktree-sidebar-row-preference' * `hostId` (`withRepoHostOwnership` leaves it unqualified), but every activation * path stores the host it resolved to, so raw and resolved identities never match. */ -export function getCyclableRowIdentity(row: Pick): string { +export function getCyclableRowIdentity(row: RenderedWorkspaceRow): string { return composeWorktreeHostIdentity( getWorktreeExecutionHostId(row.worktree, row.repo), row.worktree.id @@ -21,14 +24,13 @@ export function getCyclableRowIdentity(row: Pick row.type === 'item') - return getPreferredWorktreeRows(itemRows, pinnedDisplayPolicy) +): RenderedWorkspaceRow[] { + return getRenderedWorkspaceRowsInSidebarOrder(rows, pinnedDisplayPolicy) } /** Identity that locates the active workspace among the cyclable rows. */ export function resolveActiveCycleIdentity(args: { - rows: readonly WorktreeRow[] + rows: readonly RenderedWorkspaceRow[] activeWorktreeId: string | null activeWorkspaceExecutionHostId: ExecutionHostId | null }): string | null { @@ -44,14 +46,12 @@ export function resolveActiveCycleIdentity(args: { return row ? getCyclableRowIdentity(row) : null } -/** Worktree ids in sidebar order, taken from the rows the sidebar actually +/** Workspace ids in sidebar order, taken from the rows the sidebar actually * rendered, so collapsed groups and collapsed host sections drop out on their own. */ export function getCyclableWorktreeIds( rows: readonly HostSectionRow[], pinnedDisplayPolicy: PinnedWorktreeDisplayPolicy ): string[] { - // Why item-only: folder workspaces render as their own row type and are not - // activatable through activateAndRevealWorktree, so cycling has never included them. const ids: string[] = [] const seen = new Set() for (const row of getCyclableWorktreeRows(rows, pinnedDisplayPolicy)) { diff --git a/src/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.folder-workspace.test.ts b/src/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.folder-workspace.test.ts index d56020c488b..7534f9adde8 100644 --- a/src/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.folder-workspace.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.folder-workspace.test.ts @@ -47,7 +47,7 @@ describe('host-qualified reveal lookup finds folder workspaces', () => { projectGroup: PROJECT_GROUP, depth: 0, groupDepth: 0 - } as RenderRow + } ] const index = findPreferredRenderRowIndexForWorktreeIdentity( @@ -68,7 +68,7 @@ describe('host-qualified reveal lookup finds folder workspaces', () => { projectGroup: PROJECT_GROUP, depth: 0, groupDepth: 0 - } as RenderRow + } ] expect( @@ -79,4 +79,28 @@ describe('host-qualified reveal lookup finds folder workspaces', () => { ) ).toBe(-1) }) + + it.each([ + ['local', 0], + ['runtime:env', 1], + [undefined, 0], + ['runtime:missing', -1] + ] as const)('matches the folder row for host %s', (hostId, expectedIndex) => { + const rows: RenderRow[] = (['local', 'runtime:env'] as const).map((executionHostId) => ({ + type: 'folder-workspace', + key: `folder-workspace:${executionHostId}:${FOLDER_WORKSPACE.id}`, + folderWorkspace: { ...FOLDER_WORKSPACE, executionHostId }, + projectGroup: PROJECT_GROUP, + depth: 0, + groupDepth: 0 + })) + + expect( + findPreferredRenderRowIndexForWorktreeIdentity( + rows, + { id: folderWorkspaceKey(FOLDER_WORKSPACE.id), hostId }, + 'single-location' + ) + ).toBe(expectedIndex) + }) }) diff --git a/src/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.ts b/src/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.ts index 9728ff4634e..250441854ad 100644 --- a/src/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.ts +++ b/src/renderer/src/components/sidebar/worktree-list/navigation/render-row-lookup.ts @@ -1,4 +1,5 @@ import { folderWorkspaceKey } from '../../../../../../shared/workspace-scope' +import { folderWorkspaceToWorktree } from '../../../../../../shared/folder-workspace-worktree' import { getWorktreeExecutionHostId } from '../../../../../../shared/execution-host' import type { ExecutionHostId } from '../../../../../../shared/execution-host' import type { Worktree } from '../../../../../../shared/worktree/types' @@ -112,7 +113,11 @@ export function findPreferredRenderRowIndexForWorktreeIdentity( // Why: host-qualified reveals are emitted for folder workspaces too, and a // walker that only knows item rows returns -1 so the reveal never lands. if (row.type === 'folder-workspace') { - if (folderWorkspaceKey(row.folderWorkspace.id) === worktree.id) { + if ( + folderWorkspaceKey(row.folderWorkspace.id) === worktree.id && + (!worktree.hostId || + getWorktreeHostIdentity(folderWorkspaceToWorktree(row.folderWorkspace)) === identity) + ) { return index } continue diff --git a/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.focus.test.tsx b/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.focus.test.tsx index c09637354f2..1939f6f00f0 100644 --- a/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.focus.test.tsx +++ b/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.focus.test.tsx @@ -148,6 +148,7 @@ describe('workspace list focus ownership', () => { expect(activate.mock.calls.map(([id]) => id)).toEqual(['b', 'c', 'b']) expect(activate).toHaveBeenLastCalledWith('b', { navigationIntent: 'user-open', + revealInSidebar: false, executionHostId: 'ssh:fixture' }) expect(terminal.focus).not.toHaveBeenCalled() diff --git a/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.host-identity.test.tsx b/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.host-identity.test.tsx index 4944d69340a..cc1fd19eb13 100644 --- a/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.host-identity.test.tsx +++ b/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.host-identity.test.tsx @@ -101,7 +101,10 @@ describe('worktree keyboard cycling with a resolved active host', () => { press('down') - expect(activateAndRevealWorktree).toHaveBeenCalledWith('c', { navigationIntent: 'user-open' }) + expect(activateAndRevealWorktree).toHaveBeenCalledWith('c', { + navigationIntent: 'user-open', + revealInSidebar: false + }) }) it('steps to the previous row when the active host resolved to local', () => { @@ -109,7 +112,10 @@ describe('worktree keyboard cycling with a resolved active host', () => { press('up') - expect(activateAndRevealWorktree).toHaveBeenCalledWith('a', { navigationIntent: 'user-open' }) + expect(activateAndRevealWorktree).toHaveBeenCalledWith('a', { + navigationIntent: 'user-open', + revealInSidebar: false + }) }) it('still steps normally when the active host is unqualified', () => { @@ -117,6 +123,9 @@ describe('worktree keyboard cycling with a resolved active host', () => { press('down') - expect(activateAndRevealWorktree).toHaveBeenCalledWith('c', { navigationIntent: 'user-open' }) + expect(activateAndRevealWorktree).toHaveBeenCalledWith('c', { + navigationIntent: 'user-open', + revealInSidebar: false + }) }) }) diff --git a/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.ts b/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.ts index 612bd82b505..7eb2ed4a43d 100644 --- a/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.ts +++ b/src/renderer/src/components/sidebar/worktree-list/navigation/use-keyboard.ts @@ -2,7 +2,7 @@ import { useCallback, useEffect } from 'react' import type React from 'react' import type { Virtualizer } from '@tanstack/react-virtual' import { useAppStore } from '@/store' -import { activateAndRevealWorktree } from '@/lib/worktree-activation' +import { activateWorktreeFromSidebar } from '@/lib/sidebar-worktree-activation' import { focusRuntimeTerminalSurface } from '@/runtime/sync-runtime-graph' import { hasVisibleOverlay } from '@/lib/visible-overlay' import type { ExecutionHostId } from '../../../../../../shared/execution-host' @@ -88,11 +88,7 @@ export function useWorktreeListKeyboardNavigation(args: { return } - // Why: keyboard cycling is real navigation; route through the activation helper that records history. - activateAndRevealWorktree(nextWorktree.id, { - navigationIntent: 'user-open', - ...(nextWorktree.hostId ? { executionHostId: nextWorktree.hostId } : {}) - }) + void activateWorktreeFromSidebar(nextWorktree.id, nextWorktree.hostId) const rowIndex = findPreferredRenderRowIndexForWorktreeIdentity( renderRows, diff --git a/src/renderer/src/components/sidebar/worktree-sidebar-row-preference.ts b/src/renderer/src/components/sidebar/worktree-sidebar-row-preference.ts index 8b7f05e7e9b..a389fa5978f 100644 --- a/src/renderer/src/components/sidebar/worktree-sidebar-row-preference.ts +++ b/src/renderer/src/components/sidebar/worktree-sidebar-row-preference.ts @@ -44,22 +44,36 @@ export function getPreferredWorktreeRows( return preferredRows } -export function getRenderedWorktreesInSidebarOrder( +export type RenderedWorkspaceRow = Pick + +export function getRenderedWorkspaceRowsInSidebarOrder( rows: readonly HostSectionRow[], pinnedDisplayPolicy: PinnedWorktreeDisplayPolicy -): Worktree[] { +): RenderedWorkspaceRow[] { const itemRows = rows.filter((row): row is WorktreeRow => row.type === 'item') const preferredRowKeys = new Set( getPreferredWorktreeRows(itemRows, pinnedDisplayPolicy).map((row) => row.rowKey) ) - const renderedWorktrees: Worktree[] = [] + const renderedRows: RenderedWorkspaceRow[] = [] for (const row of rows) { if (row.type === 'item' && preferredRowKeys.has(row.rowKey)) { - renderedWorktrees.push(row.worktree) + renderedRows.push({ worktree: row.worktree, repo: row.repo }) } else if (row.type === 'folder-workspace') { - renderedWorktrees.push(folderWorkspaceToWorktree(row.folderWorkspace)) + renderedRows.push({ + worktree: folderWorkspaceToWorktree(row.folderWorkspace), + repo: undefined + }) } } - return renderedWorktrees + return renderedRows +} + +export function getRenderedWorktreesInSidebarOrder( + rows: readonly HostSectionRow[], + pinnedDisplayPolicy: PinnedWorktreeDisplayPolicy +): Worktree[] { + return getRenderedWorkspaceRowsInSidebarOrder(rows, pinnedDisplayPolicy).map( + (row) => row.worktree + ) } diff --git a/tests/e2e/sidebar-folder-keyboard-navigation.spec.ts b/tests/e2e/sidebar-folder-keyboard-navigation.spec.ts new file mode 100644 index 00000000000..9c036e994df --- /dev/null +++ b/tests/e2e/sidebar-folder-keyboard-navigation.spec.ts @@ -0,0 +1,138 @@ +import { mkdtempSync, realpathSync, rmSync } from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { test, expect } from './helpers/orca-app' +import { waitForSessionReady } from './helpers/store' + +test('cycles visible folder workspaces with shortcuts and focused list arrows', async ({ + orcaPage, + electronApp, + registerPostElectronShutdownCleanup +}, testInfo) => { + await waitForSessionReady(orcaPage) + await orcaPage.emulateMedia({ reducedMotion: 'reduce' }) + const folderPath = realpathSync(mkdtempSync(path.join(os.tmpdir(), 'orca-folder-navigation-'))) + registerPostElectronShutdownCleanup(async () => + rmSync(folderPath, { recursive: true, force: true }) + ) + + const ids = await orcaPage.evaluate(async (parentPath) => { + const store = window.__store! + const group = await window.api.projectGroups.create({ + name: 'Folder project', + parentPath, + createdFrom: 'folder-scan' + }) + await store.getState().fetchProjectGroups() + const folder = await store.getState().createFolderWorkspace({ + projectGroupId: group.id, + name: 'Folder workspace', + folderPath: parentPath + }) + const repo = store.getState().repos[0] + const [first, last] = repo ? (store.getState().worktreesByRepo[repo.id] ?? []) : [] + if (!folder || !repo || !first || !last) { + throw new Error('Expected two git worktrees and one folder workspace') + } + await store.getState().updateFolderWorkspace(folder.id, { workspaceStatus: 'in-progress' }) + store.getState().setGroupBy('workspace-status') + store.setState({ + collapsedGroups: new Set(), + worktreesByRepo: { + [repo.id]: [ + { ...first, hostId: 'local', displayName: 'Git workspace A', workspaceStatus: 'todo' }, + { ...last, hostId: 'local', displayName: 'Git workspace B', workspaceStatus: 'completed' } + ] + } + }) + return [first.id, `folder:${folder.id}`, last.id] + }, folderPath) + + expect( + await electronApp.evaluate(({ BrowserWindow }) => + BrowserWindow.getAllWindows().every((window) => !window.isVisible()) + ) + ).toBe(true) + const sidebar = orcaPage.locator('[data-worktree-sidebar]') + const row = (id: string) => + sidebar.locator(`[role="option"][data-worktree-id=${JSON.stringify(id)}]`) + await expect(sidebar.locator('[role="option"]')).toHaveCount(3) + await expect + .poll(() => + sidebar + .locator('[role="option"]') + .evaluateAll((rows) => rows.map((row) => row.getAttribute('data-worktree-id'))) + ) + .toEqual(ids) + const [first, folder, last] = ids + if (!first || !folder || !last) { + throw new Error('Missing navigation targets') + } + const mod = await orcaPage.evaluate(() => + navigator.userAgent.includes('Mac') ? 'Meta' : 'Control' + ) + const transitions = [ + { from: first, key: 'ArrowDown', to: folder }, + { from: last, key: 'ArrowUp', to: folder }, + { from: folder, key: 'ArrowDown', to: last }, + { from: folder, key: 'ArrowUp', to: first }, + { from: last, key: 'ArrowDown', to: first }, + { from: first, key: 'ArrowUp', to: last } + ] + const observed: { mode: string; from: string; key: string; to: string; current: string[] }[] = [] + + // Clicking each starting row proves the folder already activates before keyboard cycling. + for (const mode of ['shortcut', 'focused-list']) { + for (const { from, key, to } of transitions) { + await row(from).click() + await expect(row(from)).toHaveAttribute('aria-current', 'page') + if (mode === 'focused-list') { + await orcaPage.keyboard.press(`${mod}+Shift+0`) + await expect(sidebar).toBeFocused() + } + await orcaPage.keyboard.press(mode === 'shortcut' ? `${mod}+Shift+${key}` : key) + await expect.soft(row(to)).toHaveAttribute('aria-current', 'page', { timeout: 3000 }) + observed.push({ + mode, + from, + key, + to, + current: await sidebar + .locator('[aria-current="page"]') + .evaluateAll((rows) => rows.map((row) => row.getAttribute('data-worktree-id') ?? '')) + }) + if (mode === 'focused-list') { + await expect.soft(sidebar).toBeFocused() + } + if (to === folder) { + const proofPath = testInfo.outputPath(`${mode}-${key}.png`) + await sidebar.screenshot({ path: proofPath }) + await testInfo.attach(`${mode}-${key}`, { path: proofPath, contentType: 'image/png' }) + } + } + } + await testInfo.attach('navigation-transitions', { + body: JSON.stringify(observed, null, 2), + contentType: 'application/json' + }) + + const folderSection = sidebar.getByRole('button', { name: /^In progress/ }) + await folderSection.click() + await expect(row(folder)).toHaveCount(0) + await row(first).click() + await orcaPage.keyboard.press(`${mod}+Shift+ArrowDown`) + await expect(row(last)).toHaveAttribute('aria-current', 'page') + await folderSection.click() + await expect(row(folder)).toHaveCount(1) + + await row(first).click() + await orcaPage.setViewportSize({ width: 1000, height: 400 }) + await expect.poll(() => sidebar.evaluate((element) => element.clientHeight)).toBeGreaterThan(50) + await sidebar.evaluate((element) => { + element.scrollTop = 0 + }) + await orcaPage.keyboard.press(`${mod}+Shift+ArrowDown`) + await expect(row(folder)).toHaveAttribute('aria-current', 'page') + await expect(row(folder)).toBeInViewport({ ratio: 1 }) + await expect.poll(() => sidebar.evaluate((element) => element.scrollTop)).toBeGreaterThan(0) +})