diff --git a/src/renderer/src/components/native-chat/NativeChatComposerActions.tsx b/src/renderer/src/components/native-chat/NativeChatComposerActions.tsx index 2bcea04f877..7344937fec2 100644 --- a/src/renderer/src/components/native-chat/NativeChatComposerActions.tsx +++ b/src/renderer/src/components/native-chat/NativeChatComposerActions.tsx @@ -39,6 +39,7 @@ export type NativeChatComposerActionsProps = { sessionOptionsSurface: SessionOptionsSurface | null sessionOptionsSnapshot: SessionOptionDescriptor[] sessionOptionsPickerRequest?: NativeChatOptionPickerRequest | null + focusComposer?: () => void /** Present while the composer is in goal mode; the chip calls it to leave. */ onExitGoalMode?: () => void /** Absent until the session has reported or the transcript can estimate. */ @@ -67,7 +68,8 @@ export function NativeChatComposerActions({ sessionOptionsSnapshot, sessionOptionsPickerRequest, onExitGoalMode, - contextUsage + contextUsage, + focusComposer }: NativeChatComposerActionsProps): React.JSX.Element { const stops = primaryAction === 'stop' const resumes = primaryAction === 'resume' @@ -149,6 +151,7 @@ export function NativeChatComposerActions({ snapshot={sessionOptionsSnapshot} isWorking={isWorking} pickerRequest={sessionOptionsPickerRequest} + focusComposer={focusComposer} /> {contextUsage ? : null} diff --git a/src/renderer/src/components/native-chat/NativeChatComposerField.tsx b/src/renderer/src/components/native-chat/NativeChatComposerField.tsx index b39d6548170..d895611466f 100644 --- a/src/renderer/src/components/native-chat/NativeChatComposerField.tsx +++ b/src/renderer/src/components/native-chat/NativeChatComposerField.tsx @@ -1,7 +1,7 @@ import { NativeChatPromptEditor } from './NativeChatPromptEditor' import type { NativeChatComposerInput } from './native-chat-composer-input' import type { ClipboardEventHandler, KeyboardEventHandler, RefObject } from 'react' -import { useLayoutEffect, useRef } from 'react' +import { useCallback, useLayoutEffect, useRef } from 'react' import { flushSync } from 'react-dom' import type { useImeEnterGestureOwnership } from '@/lib/ime-composition-keyboard-event' import { cn } from '@/lib/utils' @@ -219,6 +219,8 @@ export function NativeChatComposerField({ sendDisabled: sendButtonDisabled }) const { resume } = primary + // Stable so the memoized option pickers keep their identity. + const focusComposer = useCallback(() => textareaRef.current?.focus(), [textareaRef]) // The button disables while resuming, which drops its focus; typing is what comes next. const resumeQueue = (): void => { resume?.() @@ -378,16 +380,14 @@ export function NativeChatComposerField({ sessionOptionsSnapshot={sessionOptionsSnapshot} contextUsage={contextUsage} sessionOptionsPickerRequest={sessionOptionsPickerRequest} + focusComposer={focusComposer} onExitGoalMode={goalMode?.active ? goalMode.exit : undefined} /> - textareaRef.current?.focus()} - /> + ) } diff --git a/src/renderer/src/components/native-chat/NativeChatModelCombobox.tsx b/src/renderer/src/components/native-chat/NativeChatModelCombobox.tsx index a6d1842cbf4..03c768e2851 100644 --- a/src/renderer/src/components/native-chat/NativeChatModelCombobox.tsx +++ b/src/renderer/src/components/native-chat/NativeChatModelCombobox.tsx @@ -10,6 +10,7 @@ import { import { Popover, PopoverContent } from '@/components/ui/popover' import { translate } from '@/i18n/i18n' import type { SessionOptionSelectChoice } from '../../../../shared/native-chat-session-options' +import { useNativeChatPickerFocusReturn } from './native-chat-picker-focus-return' import { nativeChatSessionChoiceLabel } from './native-chat-session-option-labels' import { NativeChatSessionOptionChoiceBody } from './NativeChatSessionOptionChoiceBody' @@ -22,6 +23,7 @@ type NativeChatModelComboboxProps = { readOnly: boolean readOnlyReason: string | null onSelect: (value: string) => void + focusComposer?: () => void /** Renders the `PopoverTrigger` pill, which must take the key handler. */ renderTrigger: (onKeyDown: React.KeyboardEventHandler) => React.ReactNode } @@ -40,9 +42,11 @@ export function NativeChatModelCombobox({ readOnly, readOnlyReason, onSelect, + focusComposer, renderTrigger }: NativeChatModelComboboxProps): React.JSX.Element { const [open, setOpen] = useState(defaultOpen) + const focusReturn = useNativeChatPickerFocusReturn(focusComposer) const searchLabel = translate('components.native-chat.composer.searchModels', 'Search models…') return ( @@ -58,6 +62,7 @@ export function NativeChatModelCombobox({ side="top" collisionPadding={8} className="w-64" + onCloseAutoFocus={focusReturn.onCloseAutoFocus} // Why: the popover loops Tab inside itself, which would trap focus in the search field. onKeyDown={(event) => { if (event.key === 'Tab') { @@ -88,6 +93,7 @@ export function NativeChatModelCombobox({ disabled={readOnly} aria-current={current || undefined} onSelect={() => { + focusReturn.notePick() setOpen(false) onSelect(choice.value) }} diff --git a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.focus-return.test.tsx b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.focus-return.test.tsx new file mode 100644 index 00000000000..bbba4d02d6d --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.focus-return.test.tsx @@ -0,0 +1,167 @@ +// @vitest-environment happy-dom + +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' +import { afterEach, describe, expect, it, vi } from 'vitest' + +vi.mock('@/i18n/i18n', () => ({ + translate: (_key: string, fallback: string, values?: Record) => + values + ? Object.entries(values).reduce( + (text, [name, value]) => text.replaceAll(`{{${name}}}`, String(value)), + fallback + ) + : fallback +})) + +import { TooltipProvider } from '@/components/ui/tooltip' +import type { SessionOptionDescriptor } from '../../../../shared/native-chat-session-options' +import { NativeChatSessionOptionPickers } from './NativeChatSessionOptionPickers' + +const settle = (): Promise => + act(async () => new Promise((resolve) => setTimeout(resolve, 30))) + +const surface = { + getSnapshot: vi.fn(() => []), + setOption: vi.fn(() => Promise.resolve({ snapshot: [] })), + invokeAction: vi.fn(() => Promise.resolve({ snapshot: [] })), + subscribe: vi.fn(() => vi.fn()) +} + +const model: SessionOptionDescriptor = { + id: 'model', + label: 'Model', + category: 'model', + kind: { + type: 'select', + currentValue: 'opus', + choices: [ + { value: 'opus', label: 'Opus 4.8' }, + { value: 'sonnet', label: 'Sonnet 5' } + ] + }, + valueSource: 'applied', + transport: 'catalog', + settable: true +} + +const effort: SessionOptionDescriptor = { + id: 'effort', + label: 'Effort', + category: 'thought_level', + kind: { + type: 'select', + currentValue: 'low', + choices: [ + { value: 'low', label: 'Low' }, + { value: 'high', label: 'High' } + ] + }, + valueSource: 'applied', + transport: 'catalog', + settable: true +} + +const fast: SessionOptionDescriptor = { + id: 'fastMode', + label: 'Fast mode', + category: 'mode', + kind: { type: 'boolean', currentValue: false }, + valueSource: 'applied', + transport: 'catalog', + settable: true +} + +/** The pickers beside a real message box, so focus has somewhere to come back to. */ +async function renderComposer(snapshot: SessionOptionDescriptor[]): Promise { + const messageBox = document.createElement('textarea') + render( + + messageBox.focus()} + /> + + ) + document.body.append(messageBox) + await settle() + return messageBox +} + +/** The options pill opens a menu; the model pill opens the search popover. */ +const pill = (surfaceKind: 'menu' | 'dialog'): HTMLElement => { + const found = screen + .getAllByRole('button') + .find((button) => button.getAttribute('aria-haspopup') === surfaceKind) + if (!found) { + throw new Error(`no pill opening a ${surfaceKind}`) + } + return found +} + +/** The menu opens on pointerdown, which happy-dom does not synthesize, so press Enter instead. */ +const openPill = async (surfaceKind: 'menu' | 'dialog'): Promise => { + const trigger = pill(surfaceKind) + if (surfaceKind === 'menu') { + fireEvent.keyDown(trigger, { key: 'Enter' }) + } else { + fireEvent.click(trigger) + } + await settle() + return trigger +} + +afterEach(() => { + cleanup() + document.body.replaceChildren() + vi.clearAllMocks() +}) + +describe('session option picker focus return', () => { + it('lands focus in the message box after an effort choice', async () => { + const messageBox = await renderComposer([model, effort]) + await openPill('menu') + + fireEvent.click(screen.getByRole('menuitemradio', { name: /High/ })) + await settle() + + expect(surface.setOption).toHaveBeenCalledWith('effort', 'high') + expect(document.activeElement).toBe(messageBox) + }) + + it('lands focus in the message box after a model choice', async () => { + const messageBox = await renderComposer([model, effort]) + await openPill('dialog') + + fireEvent.click(screen.getByRole('option', { name: /Sonnet 5/ })) + await settle() + + expect(surface.setOption).toHaveBeenCalledWith('model', 'sonnet') + expect(document.activeElement).toBe(messageBox) + }) + + it('leaves focus on the pill when the picker is dismissed instead of used', async () => { + await renderComposer([model, effort]) + const trigger = await openPill('menu') + + fireEvent.keyDown(document.activeElement ?? document.body, { key: 'Escape' }) + await settle() + + expect(surface.setOption).not.toHaveBeenCalled() + expect(document.activeElement).toBe(trigger) + }) + + it('keeps focus in the open menu when a toggle leaves it open', async () => { + await renderComposer([model, effort, fast]) + await openPill('menu') + + fireEvent.click(screen.getByRole('switch', { name: 'Fast mode' })) + await settle() + + expect(surface.setOption).toHaveBeenCalledWith('fastMode', true) + const menu = screen.getByRole('menu') + expect(menu).toBeTruthy() + expect(menu.contains(document.activeElement)).toBe(true) + }) +}) diff --git a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.tsx b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.tsx index 8accb867631..36c4fa683ca 100644 --- a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.tsx +++ b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.tsx @@ -36,6 +36,7 @@ import { agentSessionThrownFailure } from '../../../../shared/agent-session-writ import { RuntimeRpcCallError } from '@/runtime/runtime-rpc-result' import { agentSessionWriteFailureText } from './agent-session-write-notice-text' import { NativeChatModelCombobox } from './NativeChatModelCombobox' +import { useNativeChatPickerFocusReturn } from './native-chat-picker-focus-return' import { NativeChatSessionOptionChoiceBody } from './NativeChatSessionOptionChoiceBody' export type NativeChatSessionOptionPickersProps = { @@ -43,6 +44,8 @@ export type NativeChatSessionOptionPickersProps = { snapshot: SessionOptionDescriptor[] isWorking: boolean pickerRequest?: NativeChatOptionPickerRequest | null + /** Where focus lands after a pick; see `useNativeChatPickerFocusReturn`. */ + focusComposer?: () => void } function PickerTooltipContent(props: { @@ -117,19 +120,27 @@ function DescriptorMenuRows(props: { pending: boolean setValue: (value: SessionOptionValue) => void invokeAction: () => void + /** Called by the rows whose choice closes the menu. */ + notePick: () => void }): React.JSX.Element { - const { descriptor, pending, setValue, invokeAction } = props + const { descriptor, pending, setValue, invokeAction, notePick } = props // Why: flip-only without a baseline is an action — never claim On/Off. if (descriptor.action?.type === 'toggle-command') { return ( - invokeAction()}> + { + notePick() + invokeAction() + }} + > {translate('components.native-chat.composer.toggleOption', 'Toggle {{value0}}', { value0: nativeChatSessionOptionLabel(descriptor).toLowerCase() })} ) } - // Why: agent-picker opens the TUI; it is not a set of radio choices. + // Why: agent-picker opens the TUI; it is not a set of radio choices, and that picker owns focus. if (descriptor.action?.type === 'agent-picker') { return ( invokeAction()}> @@ -167,7 +178,10 @@ function DescriptorMenuRows(props: { setValue(value)} + onValueChange={(value) => { + notePick() + setValue(value) + }} > {descriptor.kind.choices.map((choice) => ( (null) + const modelMenuFocus = useNativeChatPickerFocusReturn(focusComposer) + const optionsMenuFocus = useNativeChatPickerFocusReturn(focusComposer) const model = snapshot.find((descriptor) => descriptor.category === 'model') const options = sortNativeChatSessionOptions(snapshot) if (!surface || !model) { @@ -264,6 +281,7 @@ function NativeChatSessionOptionPickersInner({ readOnly={!model.settable || pendingId !== null} readOnlyReason={modelReason} onSelect={(value) => setOption(model, value)} + focusComposer={focusComposer} renderTrigger={(onKeyDown) => ( )} @@ -271,7 +289,13 @@ function NativeChatSessionOptionPickersInner({ ) : ( - + {modelReason && !model.settable ? ( {modelReason} ) : null} @@ -280,6 +304,7 @@ function NativeChatSessionOptionPickersInner({ pending={pendingId !== null} setValue={(value) => setOption(model, value)} invokeAction={() => invokeAction(model)} + notePick={modelMenuFocus.notePick} /> @@ -297,7 +322,13 @@ function NativeChatSessionOptionPickersInner({ disabledReason={optionsReason} dispatched={options.some(sessionOptionDispatchUnconfirmed)} /> - + {options.map((descriptor, index) => { const reason = nativeChatSessionOptionDisabledReason(descriptor.disabledReason) return ( @@ -316,6 +347,7 @@ function NativeChatSessionOptionPickersInner({ pending={pendingId !== null} setValue={(value) => setOption(descriptor, value)} invokeAction={() => invokeAction(descriptor)} + notePick={optionsMenuFocus.notePick} /> ) diff --git a/src/renderer/src/components/native-chat/native-chat-picker-focus-return.ts b/src/renderer/src/components/native-chat/native-chat-picker-focus-return.ts new file mode 100644 index 00000000000..9486b8d3f4c --- /dev/null +++ b/src/renderer/src/components/native-chat/native-chat-picker-focus-return.ts @@ -0,0 +1,26 @@ +import { useRef } from 'react' + +/** Radix restores focus to the trigger on close, which is right for a dismissal and wrong after a + * pick. `notePick` marks the choices that close the surface, so only those reach `focusComposer`. */ +export function useNativeChatPickerFocusReturn(focusComposer: (() => void) | undefined): { + notePick: () => void + onCloseAutoFocus: (event: Event) => void +} { + const picked = useRef(false) + return { + notePick: () => { + picked.current = true + }, + onCloseAutoFocus: (event: Event) => { + if (!picked.current) { + return + } + picked.current = false + if (!focusComposer) { + return + } + event.preventDefault() + focusComposer() + } + } +}