diff --git a/design-system/packages/ui/src/components/Listbox/Listbox.module.css b/design-system/packages/ui/src/components/Listbox/Listbox.module.css index aada1b1a27..2975fbeab8 100644 --- a/design-system/packages/ui/src/components/Listbox/Listbox.module.css +++ b/design-system/packages/ui/src/components/Listbox/Listbox.module.css @@ -87,16 +87,17 @@ background-color var(--openbitfun-motion-duration-fast) var(--openbitfun-motion-easing-standard); } - .option:not(:disabled):is(:hover, [data-active="true"]) { + /* Transient interaction must not override the persistent selected state. */ + .option:where(:not(:disabled):is(:hover, [data-active="true"])) { background: var(--openbitfun-color-action-neutral-surface); } - .option[data-selected="true"] { - background: var(--openbitfun-color-selection-surface); + .option:where(:not(:disabled):active) { + background: var(--openbitfun-color-action-neutral-surface-pressed); } - .option:not(:disabled):active { - background: var(--openbitfun-color-action-neutral-surface-pressed); + .option[data-selected="true"] { + background: var(--openbitfun-color-selection-surface); } .option:focus-visible { diff --git a/src/web-ui/src/locales/en-US/panels/git.json b/src/web-ui/src/locales/en-US/panels/git.json index bc44aa80dc..0f89a8532e 100644 --- a/src/web-ui/src/locales/en-US/panels/git.json +++ b/src/web-ui/src/locales/en-US/panels/git.json @@ -197,6 +197,7 @@ "current": "Current" }, "quickSwitch": { + "refresh": "Refresh branches", "menuLabel": "Switch branch", "searchLabel": "Search branches", "searchPlaceholder": "Search branches...", @@ -225,6 +226,7 @@ }, "errors": { "title": "Branch switch failed", + "refreshFailed": "Could not refresh branches. Showing the previous list.", "loadFailed": "Could not load branches", "switchFailed": "Failed to switch branch", "switchFailedWithMessage": "Failed to switch branch: {{error}}", diff --git a/src/web-ui/src/locales/zh-CN/panels/git.json b/src/web-ui/src/locales/zh-CN/panels/git.json index 63e9946768..414766b667 100644 --- a/src/web-ui/src/locales/zh-CN/panels/git.json +++ b/src/web-ui/src/locales/zh-CN/panels/git.json @@ -197,6 +197,7 @@ "current": "当前" }, "quickSwitch": { + "refresh": "刷新分支", "menuLabel": "切换分支", "searchLabel": "搜索分支", "searchPlaceholder": "搜索分支...", @@ -225,6 +226,7 @@ }, "errors": { "title": "分支切换失败", + "refreshFailed": "刷新分支失败，当前显示上次的列表。", "loadFailed": "无法加载分支列表", "switchFailed": "无法切换分支", "switchFailedWithMessage": "无法切换分支：{{error}}", diff --git a/src/web-ui/src/locales/zh-TW/panels/git.json b/src/web-ui/src/locales/zh-TW/panels/git.json index b4ed1eafad..abf6956862 100644 --- a/src/web-ui/src/locales/zh-TW/panels/git.json +++ b/src/web-ui/src/locales/zh-TW/panels/git.json @@ -197,6 +197,7 @@ "current": "目前" }, "quickSwitch": { + "refresh": "重新整理分支", "menuLabel": "切換分支", "searchLabel": "搜尋分支", "searchPlaceholder": "搜尋分支...", @@ -225,6 +226,7 @@ }, "errors": { "title": "分支切換失敗", + "refreshFailed": "重新整理分支失敗，目前顯示上次的清單。", "loadFailed": "無法載入分支清單", "switchFailed": "無法切換分支", "switchFailedWithMessage": "無法切換分支：{{error}}", diff --git a/src/web-ui/src/tools/git/components/BranchQuickSwitch.scss b/src/web-ui/src/tools/git/components/BranchQuickSwitch.scss index 18f7cfba1b..9c164079b2 100644 --- a/src/web-ui/src/tools/git/components/BranchQuickSwitch.scss +++ b/src/web-ui/src/tools/git/components/BranchQuickSwitch.scss @@ -48,12 +48,21 @@ } .branch-quick-switch__search { + display: flex; + align-items: center; + gap: 4px; padding: 8px; border-bottom: 1px solid var(--openbitfun-color-border-subtle); } .branch-quick-switch__input-field { - width: 100%; + flex: 1; + min-width: 0; +} + +.branch-quick-switch__error { + padding: 8px; + color: var(--openbitfun-color-status-danger-content); } .branch-quick-switch__list { diff --git a/src/web-ui/src/tools/git/components/BranchQuickSwitch.test.tsx b/src/web-ui/src/tools/git/components/BranchQuickSwitch.test.tsx index 1e919039cc..2c2abca338 100644 --- a/src/web-ui/src/tools/git/components/BranchQuickSwitch.test.tsx +++ b/src/web-ui/src/tools/git/components/BranchQuickSwitch.test.tsx @@ -41,6 +41,7 @@ vi.mock('@/infrastructure/i18n', () => ({ 'quickSwitch.conflict.retrySwitchAction': 'Retry switch', 'quickSwitch.conflict.title': 'Commit changes to switch branch', 'quickSwitch.menuLabel': 'Switch branch', + 'quickSwitch.refresh': 'Refresh branches', 'quickSwitch.searchLabel': 'Search branches', }; return labels[key] ?? key; @@ -219,17 +220,102 @@ describe('BranchQuickSwitch', () => { expect(document.activeElement).toBe(trigger); }); - it('closes on Tab without cancelling the page tab sequence', async () => { + it('tabs from search to refresh before resuming the page tab sequence', async () => { await act(async () => root.render()); const search = document.querySelector('input[type="search"]')!; const trigger = container.querySelector('[data-testid="branch-trigger"]')!; const event = new KeyboardEvent('keydown', { key: 'Tab', bubbles: true, cancelable: true }); act(() => search.dispatchEvent(event)); expect(event.defaultPrevented).toBe(false); + expect(trigger.getAttribute('aria-expanded')).toBe('true'); + const refresh = document.querySelector('button[aria-label="Refresh branches"]')!; + act(() => { + refresh.focus(); + refresh.dispatchEvent(new KeyboardEvent('keydown', { key: 'Tab', bubbles: true })); + }); expect(trigger.getAttribute('aria-expanded')).toBe('false'); expect(document.activeElement).toBe(trigger); }); + it('refreshes the scoped branch list without closing or clearing search', async () => { + await act(async () => root.render()); + const search = document.querySelector('input[type="search"]')!; + act(() => { + Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value')!.set!.call(search, 'feature'); + search.dispatchEvent(new Event('input', { bubbles: true })); + }); + let resolve!: (value: typeof branches) => void; + mocks.getBranches.mockReturnValueOnce(new Promise(done => { resolve = done; })); + const refresh = document.querySelector('button[aria-label="Refresh branches"]')!; + act(() => refresh.click()); + expect(refresh.disabled).toBe(true); + expect(refresh.getAttribute('aria-busy')).toBe('true'); + expect(mocks.getBranches).toHaveBeenLastCalledWith( + { workspaceId: 'workspace-1' }, true, { throwOnError: true }, + ); + act(() => refresh.click()); + expect(mocks.getBranches).toHaveBeenCalledTimes(2); + await act(async () => resolve([...branches, { ...branches[1], name: 'feature-new' }])); + expect(search.value).toBe('feature'); + expect(document.querySelector('[data-testid="branch-quick-switch-option-feature-new"]')).not.toBeNull(); + expect(document.querySelector('[data-testid="branch-quick-switch-option-main"]')).toBeNull(); + expect(refresh.disabled).toBe(false); + expect(container.querySelector('[data-testid="branch-trigger"]')?.getAttribute('aria-expanded')).toBe('true'); + }); + + it('keeps the previous list on refresh failure and allows retry', async () => { + await act(async () => root.render()); + mocks.getBranches.mockRejectedValueOnce(new Error('Host offline')); + const refresh = document.querySelector('button[aria-label="Refresh branches"]')!; + await act(async () => refresh.click()); + expect(document.querySelector('[data-testid="branch-quick-switch-option-feature"]')).not.toBeNull(); + expect(document.querySelector('[role="status"]')?.textContent).toBe('quickSwitch.errors.refreshFailed'); + expect(refresh.disabled).toBe(false); + await act(async () => refresh.click()); + expect(document.querySelector('[role="status"]')).toBeNull(); + }); + + it('reports initial load failure instead of presenting an empty repository', async () => { + mocks.getState.mockReturnValue(undefined); + mocks.getBranches.mockRejectedValueOnce(new Error('Host offline')); + await act(async () => root.render()); + expect(document.querySelector('[role="status"]')?.textContent).toBe('quickSwitch.errors.loadFailed'); + const refresh = document.querySelector('button[aria-label="Refresh branches"]')!; + await act(async () => refresh.click()); + expect(document.querySelector('[data-testid="branch-quick-switch-option-main"]')).not.toBeNull(); + }); + + it('does not check out the keyboard-highlighted branch when Enter activates refresh', async () => { + await act(async () => root.render()); + const search = document.querySelector('input[type="search"]')!; + const refresh = document.querySelector('button[aria-label="Refresh branches"]')!; + act(() => search.dispatchEvent(new KeyboardEvent('keydown', { key: 'ArrowDown', bubbles: true }))); + await act(async () => { + refresh.focus(); + refresh.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + refresh.click(); + }); + expect(mocks.checkoutBranch).not.toHaveBeenCalled(); + expect(mocks.getBranches).toHaveBeenCalledTimes(2); + }); + + it('ignores a previous workspace request after switching workspaces', async () => { + const anchorRef = { current: document.createElement('button') }; + let resolveOld!: (value: typeof branches) => void; + mocks.getBranches.mockReturnValueOnce(new Promise(done => { resolveOld = done; })); + await act(async () => root.render()); + mocks.getBranches.mockResolvedValueOnce([{ ...branches[0], name: 'new-main' }]); + await act(async () => root.render()); + await act(async () => resolveOld(branches)); + expect(document.querySelector('[data-testid="branch-quick-switch-option-new-main"]')).not.toBeNull(); + expect(document.querySelector('[data-testid="branch-quick-switch-option-main"]')).toBeNull(); + expect(mocks.getBranches).toHaveBeenLastCalledWith( + { workspaceId: 'new', repositoryPath: '/remote/new' }, true, { throwOnError: true }, + ); + }); + describe('motion lifecycle', () => { beforeEach(() => { vi.useFakeTimers(); diff --git a/src/web-ui/src/tools/git/components/BranchQuickSwitch.tsx b/src/web-ui/src/tools/git/components/BranchQuickSwitch.tsx index 653727e168..ac469f85fd 100644 --- a/src/web-ui/src/tools/git/components/BranchQuickSwitch.tsx +++ b/src/web-ui/src/tools/git/components/BranchQuickSwitch.tsx @@ -12,6 +12,7 @@ import React, { import { createOverlayPortal, OverflowText, Button, Icon, + IconButton, Input, Listbox, ListboxEmpty, @@ -104,6 +105,8 @@ export const BranchQuickSwitch: React.FC = ({ const panelRef = useRef(null); const listRef = useRef(null); const switchInFlightRef = useRef(false); + const branchRequestRef = useRef(0); + const refreshButtonRef = useRef(null); // The "current" marker comes from the shared branch (passed in by the // composer strip) rather than from the fetched list's own flag, so a switch @@ -164,33 +167,20 @@ export const BranchQuickSwitch: React.FC = ({ }, [focusReady]); const loadBranches = useCallback(async () => { + const request = ++branchRequestRef.current; setIsLoading(true); setLoadFailed(false); try { - const cachedBranches = branchListFromCache(repositoryPath); - if (cachedBranches?.length) setBranches(cachedBranches); - - await gitStateManager.refresh(repositoryPath, { - layers: ['detailed'], - force: true, - silent: true, - reason: 'manual', - source: 'branch_quick_switch', - }); - const refreshedBranches = branchListFromCache(repositoryPath); - if (refreshedBranches) { - setBranches(refreshedBranches); - return; - } - - const serviceBranches = await gitService.getBranches(repositoryPath, false); - setBranches(serviceBranches); + // Read only branches through the workspace-aware service. The passive + // detailed cache also loads commits and turns branch failures into []. + const serviceBranches = await gitService.getBranches(repositoryPath, true, { throwOnError: true }); + if (request === branchRequestRef.current) setBranches(serviceBranches); } catch (error) { + if (request !== branchRequestRef.current) return; log.error('Failed to load branches', { repositoryPath, error }); setLoadFailed(true); - if (!branchListFromCache(repositoryPath)?.length) setBranches([]); } finally { - setIsLoading(false); + if (request === branchRequestRef.current) setIsLoading(false); } }, [repositoryPath]); @@ -204,8 +194,10 @@ export const BranchQuickSwitch: React.FC = ({ useEffect(() => { if (!isOpen) return; + setBranches(branchListFromCache(repositoryPath) ?? []); void loadBranches(); - }, [isOpen, loadBranches]); + return () => { branchRequestRef.current += 1; }; + }, [isOpen, loadBranches, repositoryPath]); useEffect(() => { setBlocker(null); @@ -343,10 +335,15 @@ export const BranchQuickSwitch: React.FC = ({ return; } if (event.key === 'Tab') { - // Resume the page's tab order at the trigger, outside the portal. - closePicker(); + // Search and refresh are the two tab stops in this virtual-focus picker. + const leaving = event.shiftKey + ? event.target === inputRef.current + : event.target !== inputRef.current || refreshButtonRef.current?.disabled; + if (leaving) closePicker(); return; } + // Enter on the refresh button must not also check out the active option. + if (event.target !== inputRef.current) return; if (filteredBranches.length === 0) return; if (event.key === 'ArrowDown') { @@ -476,12 +473,27 @@ export const BranchQuickSwitch: React.FC = ({ data-openbitfun-product-component="branch-quick-switch" data-openbitfun-product-part="input" /> + } + loading={isLoading} + disabled={isSwitching} + onClick={() => void loadBranches()} + /> + {loadFailed && branches.length > 0 && ( +
+ {t('quickSwitch.errors.refreshFailed')} +
+ )} {isLoading && branches.length === 0 ? ( ({ isGitRepository: vi.fn(), getRepository: vi.fn(), getStatus: vi.fn(), + getBranches: vi.fn(), })); const gitStateManagerMock = vi.hoisted(() => ({ @@ -30,6 +31,24 @@ vi.mock('../state/GitStateManager', () => ({ const repositoryPath = { workspaceId: 'workspace-1', repositoryPath: 'D:/workspace/OpenBitFun' }; +describe('GitService branch loading', () => { + it('preserves the legacy empty-list fallback but lets interactive callers receive errors', async () => { + const error = new Error('Remote host unavailable'); + gitApiMocks.getBranches.mockRejectedValue(error); + await expect(gitService.getBranches(repositoryPath)).resolves.toEqual([]); + await expect(gitService.getBranches(repositoryPath, true, { throwOnError: true })).rejects.toBe(error); + }); + + it('forwards the workspace scope and adapts a successful strict read', async () => { + const scope = { workspaceId: 'ssh-workspace', repositoryPath: '/srv/project' }; + gitApiMocks.getBranches.mockResolvedValue([{ name: 'main', current: true, remote: false }]); + await expect(gitService.getBranches(scope, true, { throwOnError: true })).resolves.toEqual([ + expect.objectContaining({ name: 'main', current: true, ahead: 0, behind: 0 }), + ]); + expect(gitApiMocks.getBranches).toHaveBeenLastCalledWith(scope, true); + }); +}); + describe('GitService dangerous operation refresh guard', () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/src/web-ui/src/tools/git/services/GitService.ts b/src/web-ui/src/tools/git/services/GitService.ts index ca86824d14..6328f1af5e 100644 --- a/src/web-ui/src/tools/git/services/GitService.ts +++ b/src/web-ui/src/tools/git/services/GitService.ts @@ -288,12 +288,18 @@ export class GitService { } } - async getBranches(repositoryPath: GitWorkspaceScope, includeRemote: boolean = false): Promise { + async getBranches( + repositoryPath: GitWorkspaceScope, + includeRemote: boolean = false, + options: { throwOnError?: boolean } = {}, + ): Promise { try { const result = await gitAPI.getBranches(repositoryPath, includeRemote); return this.adaptBranches(result); } catch (error) { log.error('Failed to get branches', error); + // Interactive callers must distinguish a failed read from an empty repository. + if (options.throwOnError) throw error; return []; } }