mirror of
https://github.com/stablyai/orca.git
synced 2026-11-03 18:47:40 +00:00
fix(native-chat): return focus to the composer after a pick (#26574)
Radix restores focus to the trigger on close, so the model and effort pills kept it. Escape and Tab still land there; only a choice moves on.
This commit is contained in:
@@ -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 ? <NativeChatContextUsageRing usage={contextUsage} /> : null}
|
||||
<Tooltip>
|
||||
|
||||
@@ -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}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
<NativeChatQueueSendConfirmDialog
|
||||
confirm={queueSendConfirm}
|
||||
focusComposer={() => textareaRef.current?.focus()}
|
||||
/>
|
||||
<NativeChatQueueSendConfirmDialog confirm={queueSendConfirm} focusComposer={focusComposer} />
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -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<HTMLButtonElement>) => 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 (
|
||||
<Popover open={open} onOpenChange={setOpen}>
|
||||
@@ -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)
|
||||
}}
|
||||
|
||||
+167
@@ -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<string, string | number>) =>
|
||||
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<void> =>
|
||||
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<HTMLElement> {
|
||||
const messageBox = document.createElement('textarea')
|
||||
render(
|
||||
<TooltipProvider>
|
||||
<NativeChatSessionOptionPickers
|
||||
surface={surface}
|
||||
snapshot={snapshot}
|
||||
isWorking={false}
|
||||
focusComposer={() => messageBox.focus()}
|
||||
/>
|
||||
</TooltipProvider>
|
||||
)
|
||||
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<HTMLElement> => {
|
||||
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)
|
||||
})
|
||||
})
|
||||
@@ -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 (
|
||||
<DropdownMenuItem disabled={!descriptor.settable || pending} onSelect={() => invokeAction()}>
|
||||
<DropdownMenuItem
|
||||
disabled={!descriptor.settable || pending}
|
||||
onSelect={() => {
|
||||
notePick()
|
||||
invokeAction()
|
||||
}}
|
||||
>
|
||||
{translate('components.native-chat.composer.toggleOption', 'Toggle {{value0}}', {
|
||||
value0: nativeChatSessionOptionLabel(descriptor).toLowerCase()
|
||||
})}
|
||||
</DropdownMenuItem>
|
||||
)
|
||||
}
|
||||
// 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 (
|
||||
<DropdownMenuItem disabled={!descriptor.settable || pending} onSelect={() => invokeAction()}>
|
||||
@@ -167,7 +178,10 @@ function DescriptorMenuRows(props: {
|
||||
<DropdownMenuRadioGroup
|
||||
aria-label={nativeChatSessionOptionLabel(descriptor)}
|
||||
value={descriptor.kind.currentValue}
|
||||
onValueChange={(value) => setValue(value)}
|
||||
onValueChange={(value) => {
|
||||
notePick()
|
||||
setValue(value)
|
||||
}}
|
||||
>
|
||||
{descriptor.kind.choices.map((choice) => (
|
||||
<DropdownMenuRadioItem
|
||||
@@ -214,9 +228,12 @@ function NativeChatSessionOptionPickersInner({
|
||||
surface,
|
||||
snapshot,
|
||||
isWorking,
|
||||
pickerRequest
|
||||
pickerRequest,
|
||||
focusComposer
|
||||
}: NativeChatSessionOptionPickersProps): React.JSX.Element | null {
|
||||
const [pendingId, setPendingId] = useState<string | null>(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) => (
|
||||
<PickerTrigger Trigger={PopoverTrigger} {...modelTrigger} onKeyDown={onKeyDown} />
|
||||
)}
|
||||
@@ -271,7 +289,13 @@ function NativeChatSessionOptionPickersInner({
|
||||
) : (
|
||||
<DropdownMenu key={modelPickerKey} defaultOpen={modelPickerDefaultOpen}>
|
||||
<PickerTrigger Trigger={DropdownMenuTrigger} {...modelTrigger} />
|
||||
<DropdownMenuContent align="start" side="top" collisionPadding={8} className="w-64">
|
||||
<DropdownMenuContent
|
||||
align="start"
|
||||
side="top"
|
||||
collisionPadding={8}
|
||||
className="w-64"
|
||||
onCloseAutoFocus={modelMenuFocus.onCloseAutoFocus}
|
||||
>
|
||||
{modelReason && !model.settable ? (
|
||||
<DropdownMenuLabel className="font-normal">{modelReason}</DropdownMenuLabel>
|
||||
) : null}
|
||||
@@ -280,6 +304,7 @@ function NativeChatSessionOptionPickersInner({
|
||||
pending={pendingId !== null}
|
||||
setValue={(value) => setOption(model, value)}
|
||||
invokeAction={() => invokeAction(model)}
|
||||
notePick={modelMenuFocus.notePick}
|
||||
/>
|
||||
</DropdownMenuContent>
|
||||
</DropdownMenu>
|
||||
@@ -297,7 +322,13 @@ function NativeChatSessionOptionPickersInner({
|
||||
disabledReason={optionsReason}
|
||||
dispatched={options.some(sessionOptionDispatchUnconfirmed)}
|
||||
/>
|
||||
<DropdownMenuContent align="start" side="top" collisionPadding={8} className="w-60">
|
||||
<DropdownMenuContent
|
||||
align="start"
|
||||
side="top"
|
||||
collisionPadding={8}
|
||||
className="w-60"
|
||||
onCloseAutoFocus={optionsMenuFocus.onCloseAutoFocus}
|
||||
>
|
||||
{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}
|
||||
/>
|
||||
</div>
|
||||
)
|
||||
|
||||
@@ -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()
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user