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
This commit is contained in:
Jinjing
2026-08-08 12:43:45 -07:00
parent 7acad444ed
commit 19e4caa919
7 changed files with 61 additions and 79 deletions
@@ -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' })
@@ -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
@@ -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'
@@ -82,11 +82,8 @@ export default function TabBarCreateEntry({
const [pending, setPending] = useState(false)
const [error, setError] = useState<string | null>(null)
const [switchError, setSwitchError] = useState<string | null>(null)
const [selectedOptionId, setSelectedOptionId] = useState<string | null>(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<string | null>(null)
const [lastMenuOpen, setLastMenuOpen] = useState(menuOpen)
const inputRef = useRef<HTMLInputElement>(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)
}}
@@ -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,
@@ -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', () => {
@@ -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 })
}
}