From 19e4caa9190aa8b15decb34aec7fdf441a2df659 Mon Sep 17 00:00:00 2001 From: Jinjing <6427696+AmethystLiang@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:43:37 -0700 Subject: [PATCH] Consolidate browser focus requests and simplify selection state - Extract requestBrowserFocus to handle queueing + event dispatch atomically - Simplify omnibox selection tracking with single pinnedOptionId state - Optimize host resolution in tab search to compute once per query --- .../browser-pane/browser-focus.test.ts | 23 ++++++++++- .../components/browser-pane/browser-focus.ts | 6 +++ .../TabBarCreateEntry.tab-results.test.tsx | 5 +-- .../components/tab-bar/TabBarCreateEntry.tsx | 40 ++++++------------- .../src/components/tab-bar/open-tab-search.ts | 35 +++++----------- .../open-tab-selection-routing.test.ts | 18 +++------ .../tab-bar/open-tab-selection-routing.ts | 13 +----- 7 files changed, 61 insertions(+), 79 deletions(-) diff --git a/src/renderer/src/components/browser-pane/browser-focus.test.ts b/src/renderer/src/components/browser-pane/browser-focus.test.ts index 722aec3fa9f..3e58d4ed637 100644 --- a/src/renderer/src/components/browser-pane/browser-focus.test.ts +++ b/src/renderer/src/components/browser-pane/browser-focus.test.ts @@ -1,5 +1,12 @@ +// @vitest-environment happy-dom + import { afterEach, describe, expect, it, vi } from 'vitest' -import { consumeBrowserFocusRequest, queueBrowserFocusRequest } from './browser-focus' +import { + consumeBrowserFocusRequest, + ORCA_BROWSER_FOCUS_REQUEST_EVENT, + queueBrowserFocusRequest, + requestBrowserFocus +} from './browser-focus' describe('browser-focus', () => { afterEach(() => { @@ -13,6 +20,20 @@ describe('browser-focus', () => { expect(consumeBrowserFocusRequest('page-1')).toBeNull() }) + it('requestBrowserFocus queues and dispatches the focus event', () => { + const detail = { pageId: 'page-req', target: 'address-bar' as const } + const events: CustomEvent[] = [] + const onFocusRequest = (event: Event): void => { + events.push(event as CustomEvent) + } + window.addEventListener(ORCA_BROWSER_FOCUS_REQUEST_EVENT, onFocusRequest) + requestBrowserFocus(detail) + window.removeEventListener(ORCA_BROWSER_FOCUS_REQUEST_EVENT, onFocusRequest) + + expect(consumeBrowserFocusRequest('page-req')).toBe('address-bar') + expect(events[0]?.detail).toEqual(detail) + }) + it('overwrites older requests for the same page id', () => { queueBrowserFocusRequest({ pageId: 'page-2', target: 'webview' }) queueBrowserFocusRequest({ pageId: 'page-2', target: 'address-bar' }) diff --git a/src/renderer/src/components/browser-pane/browser-focus.ts b/src/renderer/src/components/browser-pane/browser-focus.ts index fb2324ebb17..e8b204c8139 100644 --- a/src/renderer/src/components/browser-pane/browser-focus.ts +++ b/src/renderer/src/components/browser-pane/browser-focus.ts @@ -64,6 +64,12 @@ export function queueBrowserFocusRequest(detail: BrowserFocusRequestDetail): voi scheduleExpiredRequestCleanup() } +/** Queue + announce so a mounting browser pane and live listeners both see the request. */ +export function requestBrowserFocus(detail: BrowserFocusRequestDetail): void { + queueBrowserFocusRequest(detail) + window.dispatchEvent(new CustomEvent(ORCA_BROWSER_FOCUS_REQUEST_EVENT, { detail })) +} + export function consumeBrowserFocusRequest(pageId: string): BrowserFocusTarget | null { purgeExpiredFocusRequests() const pending = pendingBrowserFocusByPageId.get(pageId) ?? null diff --git a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tab-results.test.tsx b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tab-results.test.tsx index b793566182d..d96d28b89d2 100644 --- a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tab-results.test.tsx +++ b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tab-results.test.tsx @@ -48,7 +48,7 @@ const activationMocks = vi.hoisted(() => ({ browser: vi.fn(), simulator: vi.fn(), focusTerminalTabSurface: vi.fn(), - queueBrowserFocusRequest: vi.fn() + requestBrowserFocus: vi.fn() })) vi.mock('@/lib/workspace-tab-palette-activation', () => ({ activateWorkspaceTabPaletteResult: activationMocks.workspace @@ -63,8 +63,7 @@ vi.mock('@/lib/focus-terminal-tab-surface', () => ({ focusTerminalTabSurface: activationMocks.focusTerminalTabSurface })) vi.mock('@/components/browser-pane/browser-focus', () => ({ - ORCA_BROWSER_FOCUS_REQUEST_EVENT: 'orca:browser-focus-request', - queueBrowserFocusRequest: activationMocks.queueBrowserFocusRequest + requestBrowserFocus: activationMocks.requestBrowserFocus })) import TabBarCreateEntry from './TabBarCreateEntry' diff --git a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx index bab26355034..98d56ac259c 100644 --- a/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx +++ b/src/renderer/src/components/tab-bar/TabBarCreateEntry.tsx @@ -82,11 +82,8 @@ export default function TabBarCreateEntry({ const [pending, setPending] = useState(false) const [error, setError] = useState(null) const [switchError, setSwitchError] = useState(null) - const [selectedOptionId, setSelectedOptionId] = useState(null) - const [selectedOptionQuery, setSelectedOptionQuery] = useState(query) - // True while selection still tracks ranking (no arrow/click yet). Deferred tab - // rows can prepend a better match; re-pin only while this stays true. - const [selectionIsAutoDefault, setSelectionIsAutoDefault] = useState(true) + // null = follow ranking (deferred tabs can prepend); set on arrow keys only. + const [pinnedOptionId, setPinnedOptionId] = useState(null) const [lastMenuOpen, setLastMenuOpen] = useState(menuOpen) const inputRef = useRef(null) const imeEnter = useImeEnterGestureOwnership() @@ -173,8 +170,7 @@ export default function TabBarCreateEntry({ setPending(false) setError(null) setSwitchError(null) - setSelectedOptionId(null) - setSelectionIsAutoDefault(true) + setPinnedOptionId(null) } } @@ -198,31 +194,19 @@ export default function TabBarCreateEntry({ option })) ] - const topOptionId = activeOptions.length > 0 ? getActiveOptionId(activeOptions[0]) : null - if (selectedOptionQuery !== query) { - setSelectedOptionQuery(query) - setSelectedOptionId(topOptionId) - setSelectionIsAutoDefault(true) - } else if (selectedOptionId === null && topOptionId !== null) { - // Why pin the top row by id: the tab search defers the query, so tab rows - // arrive a render later and would otherwise slide under an index-kept highlight. - setSelectedOptionId(topOptionId) - setSelectionIsAutoDefault(true) - } else if (selectionIsAutoDefault && topOptionId !== null && selectedOptionId !== topOptionId) { - // Files paint before deferred tab search; follow the new top while the user - // has not moved selection themselves. - setSelectedOptionId(topOptionId) - } - const selectedOptionIndex = selectedOptionId - ? activeOptions.findIndex((option) => getActiveOptionId(option) === selectedOptionId) + // Why pin by id (not index): deferred tab rows prepend and would steal a + // user-moved highlight if we kept a raw index. Null pin follows top rank. + const pinnedOptionIndex = pinnedOptionId + ? activeOptions.findIndex((option) => getActiveOptionId(option) === pinnedOptionId) : -1 - const activeSelectedIndex = Math.max(selectedOptionIndex, 0) + const activeSelectedIndex = Math.max(pinnedOptionIndex, 0) const selectedActiveOption = activeOptions[activeSelectedIndex] const statusOption = options.find( (option) => option.classification.kind === 'empty' || option.classification.kind === 'blocked' ) const statusMessage = - statusOption?.classification.kind === 'empty' || statusOption?.classification.kind === 'blocked' + statusOption != null && + (statusOption.classification.kind === 'empty' || statusOption.classification.kind === 'blocked') ? statusOption.classification.message : omniboxPlaceholder() @@ -304,8 +288,7 @@ export default function TabBarCreateEntry({ const delta = event.key === 'ArrowDown' ? 1 : -1 const nextIndex = (activeSelectedIndex + delta + activeOptions.length) % activeOptions.length - setSelectionIsAutoDefault(false) - setSelectedOptionId(getActiveOptionId(activeOptions[nextIndex])) + setPinnedOptionId(getActiveOptionId(activeOptions[nextIndex])) return } // Why: with no result rows the static create/agent items render below; @@ -337,6 +320,7 @@ export default function TabBarCreateEntry({ // it in this event rather than a later effect after the render commits. setQuery(nextQuery) onQueryChange?.(nextQuery) + setPinnedOptionId(null) setError(null) setSwitchError(null) }} diff --git a/src/renderer/src/components/tab-bar/open-tab-search.ts b/src/renderer/src/components/tab-bar/open-tab-search.ts index 7fc414daf3b..29be24f2834 100644 --- a/src/renderer/src/components/tab-bar/open-tab-search.ts +++ b/src/renderer/src/components/tab-bar/open-tab-search.ts @@ -86,7 +86,7 @@ const TITLE_SUBSTRING_TIER = 1 // weights. See the plan's tiering decision. const SECONDARY_TIER = 2 -export function isOpenTabSearchQueryTooLarge( +function isOpenTabSearchQueryTooLarge( query: string, maxBytes = OPEN_TAB_SEARCH_QUERY_MAX_BYTES ): boolean { @@ -168,23 +168,20 @@ export function searchOpenTabs({ return [] } + // Single-worktree builders stamp one host on every entry; resolve once. + const executionHostId = + workspaceTabs[0]?.worktree.hostId ?? + browserPages[0]?.worktree.hostId ?? + simulatorTabs[0]?.worktree.hostId ?? + LOCAL_EXECUTION_HOST_ID + // Why map workspace only: editor relativePath is read from the searchable entry. const workspaceEntriesByTabId = new Map(workspaceTabs.map((entry) => [entry.tab.id, entry])) - const browserEntriesByPageId = new Map(browserPages.map((entry) => [entry.page.id, entry])) - const simulatorEntriesByTabId = new Map(simulatorTabs.map((entry) => [entry.tab.id, entry])) - const entryHostId = ( - entry: SearchableWorkspaceTab | SearchableBrowserPage | SearchableSimulatorTab | undefined - ): ExecutionHostId => entry?.worktree.hostId ?? LOCAL_EXECUTION_HOST_ID return [ // Why no isCurrentTab filter: Cmd+J lists the tab you are on, and hiding it // made the omnibox look broken when you searched for the tab on screen. ...rank('workspace', searchWorkspaceTabs([...workspaceTabs], trimmed), (result) => ({ - ...baseResult( - 'workspace', - result.tabId, - result, - entryHostId(workspaceEntriesByTabId.get(result.tabId)) - ), + ...baseResult('workspace', result.tabId, result, executionHostId), source: 'workspace', contentType: result.contentType, tabId: result.tabId, @@ -193,24 +190,14 @@ export function searchOpenTabs({ relativePath: getEditorRelativePath(workspaceEntriesByTabId.get(result.tabId)) })), ...rank('browser', searchBrowserPages([...browserPages], trimmed), (result) => ({ - ...baseResult( - 'browser', - result.pageId, - result, - entryHostId(browserEntriesByPageId.get(result.pageId)) - ), + ...baseResult('browser', result.pageId, result, executionHostId), source: 'browser', contentType: 'browser', pageId: result.pageId, workspaceId: result.workspaceId })), ...rank('simulator', searchSimulatorTabs([...simulatorTabs], trimmed), (result) => ({ - ...baseResult( - 'simulator', - result.tabId, - result, - entryHostId(simulatorEntriesByTabId.get(result.tabId)) - ), + ...baseResult('simulator', result.tabId, result, executionHostId), source: 'simulator', contentType: 'simulator', tabId: result.tabId, diff --git a/src/renderer/src/components/tab-bar/open-tab-selection-routing.test.ts b/src/renderer/src/components/tab-bar/open-tab-selection-routing.test.ts index 3e0973dfe1c..f9ec2cd76e5 100644 --- a/src/renderer/src/components/tab-bar/open-tab-selection-routing.test.ts +++ b/src/renderer/src/components/tab-bar/open-tab-selection-routing.test.ts @@ -8,7 +8,7 @@ const mocks = vi.hoisted(() => ({ activateBrowserPage: vi.fn(), activateSimulatorTab: vi.fn(), focusTerminalTabSurface: vi.fn(), - queueBrowserFocusRequest: vi.fn() + requestBrowserFocus: vi.fn() })) vi.mock('@/lib/workspace-tab-palette-activation', () => ({ @@ -24,8 +24,7 @@ vi.mock('@/lib/focus-terminal-tab-surface', () => ({ focusTerminalTabSurface: mocks.focusTerminalTabSurface })) vi.mock('@/components/browser-pane/browser-focus', () => ({ - ORCA_BROWSER_FOCUS_REQUEST_EVENT: 'orca:browser-focus-request', - queueBrowserFocusRequest: mocks.queueBrowserFocusRequest + requestBrowserFocus: mocks.requestBrowserFocus })) import { activateOpenTabSearchResult } from './open-tab-selection-routing' @@ -122,20 +121,15 @@ describe('activateOpenTabSearchResult', () => { worktreeId: 'wt-1' }) - const events: CustomEvent[] = [] - const onFocusRequest = (event: Event): void => { - events.push(event as CustomEvent) - } - window.addEventListener('orca:browser-focus-request', onFocusRequest) if (outcome.status !== 'activated') { throw new Error('expected activation') } outcome.focus?.() - window.removeEventListener('orca:browser-focus-request', onFocusRequest) - const detail = { pageId: 'page-1', target: 'address-bar' } - expect(mocks.queueBrowserFocusRequest).toHaveBeenCalledWith(detail) - expect(events[0]?.detail).toEqual(detail) + expect(mocks.requestBrowserFocus).toHaveBeenCalledWith({ + pageId: 'page-1', + target: 'address-bar' + }) }) it('leaves focus unchanged after activating a simulator tab', () => { diff --git a/src/renderer/src/components/tab-bar/open-tab-selection-routing.ts b/src/renderer/src/components/tab-bar/open-tab-selection-routing.ts index 35feb992eff..ceeabe7d8a7 100644 --- a/src/renderer/src/components/tab-bar/open-tab-selection-routing.ts +++ b/src/renderer/src/components/tab-bar/open-tab-selection-routing.ts @@ -1,11 +1,7 @@ // Routes an omnibox switch row to the matching palette activation and reports // how the destination should take keyboard focus once the menu closes. -import { - ORCA_BROWSER_FOCUS_REQUEST_EVENT, - queueBrowserFocusRequest, - type BrowserFocusRequestDetail -} from '@/components/browser-pane/browser-focus' +import { requestBrowserFocus } from '@/components/browser-pane/browser-focus' import { translate } from '@/i18n/i18n' import { activateBrowserPagePaletteResult } from '@/lib/browser-page-palette-activation' import { focusTerminalTabSurface } from '@/lib/focus-terminal-tab-surface' @@ -32,11 +28,6 @@ function failed(reason: string, staleMessage: string): OpenTabSelectionOutcome { } } -function requestBrowserPageFocus(detail: BrowserFocusRequestDetail): void { - queueBrowserFocusRequest(detail) - window.dispatchEvent(new CustomEvent(ORCA_BROWSER_FOCUS_REQUEST_EVENT, { detail })) -} - export function activateOpenTabSearchResult(result: OpenTabSearchResult): OpenTabSelectionOutcome { if (result.source === 'browser') { const activation = activateBrowserPagePaletteResult({ @@ -57,7 +48,7 @@ export function activateOpenTabSearchResult(result: OpenTabSearchResult): OpenTa return { status: 'activated', focus: () => - requestBrowserPageFocus({ pageId: activation.pageId, target: activation.focusTarget }) + requestBrowserFocus({ pageId: activation.pageId, target: activation.focusTarget }) } }