From 464bd5a54b3364021c5cc0319859616e5ec41bbf Mon Sep 17 00:00:00 2001 From: Kelvin Amoaba <97001695+AmoabaKelvin@users.noreply.github.com> Date: Wed, 14 Oct 2026 02:27:05 +0000 Subject: [PATCH] feat(native-chat): combine model and options into one picker (#27972) * feat(native-chat): combine model and options into one picker Setting a model and its effort took two menus that each closed on a pick. * feat(native-chat): close the model picker on an effort pick Effort is the last thing set before typing. * fix(native-chat): write the picker's kept rows after commit React Doctor rejects a ref written in render, and a discarded render could leave a row behind. * fix(native-chat): open the picker once per /model or /effort, tooltip only when informative The session kept a slash command's picker request, and the picker remounted open from it whenever a question or approval card gave the composer back. The request is now consumed when the picker sees it, and open state lives in the popover, which unmounts with the pill, so it comes back closed. The pill's hover tooltip only named the categories the pill already shows, and the picker covered it on click. It now shows only the disabled reason or the not-confirmed note. Also drops the removed Options key from the locale test and covers the new short effort labels. --------- Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> --- .../native-chat/NativeChatComposerActions.tsx | 4 +- .../NativeChatModelCombobox.test.tsx | 70 +-- .../native-chat/NativeChatModelCombobox.tsx | 110 ++-- .../NativeChatSessionOptionChoiceBody.tsx | 4 +- ...SessionOptionPickers.focus-return.test.tsx | 267 +++++++-- .../NativeChatSessionOptionPickers.test.tsx | 562 ++++++++++-------- .../NativeChatSessionOptionPickers.tsx | 513 ++++++++-------- .../NativeChatSessionOptionRows.tsx | 223 +++++++ .../NativeChatSessionOptionWriteGroup.tsx | 24 + ...tiveChatStructuredSession.test-harness.tsx | 1 + .../NativeChatStructuredSession.test.tsx | 9 +- .../native-chat/native-chat-composer-types.ts | 4 +- .../native-chat-picker-focus-return.ts | 29 +- .../native-chat-session-option-labels.test.ts | 8 +- .../native-chat-session-option-labels.ts | 42 +- ...native-chat-session-option-surface-call.ts | 12 +- ...tive-chat-structured-composer-transport.ts | 2 +- .../src/components/ui/toggle-group.tsx | 2 +- src/renderer/src/components/ui/toggle.tsx | 6 +- src/renderer/src/i18n/locales/en.json | 8 +- src/renderer/src/i18n/locales/es.json | 8 +- src/renderer/src/i18n/locales/fr.json | 8 +- src/renderer/src/i18n/locales/ja.json | 8 +- src/renderer/src/i18n/locales/ko.json | 8 +- src/renderer/src/i18n/locales/zh.json | 8 +- .../src/i18n/native-chat-locales.test.ts | 5 +- tests/e2e/omp-model-picker.spec.ts | 6 +- 27 files changed, 1231 insertions(+), 720 deletions(-) create mode 100644 src/renderer/src/components/native-chat/NativeChatSessionOptionRows.tsx create mode 100644 src/renderer/src/components/native-chat/NativeChatSessionOptionWriteGroup.tsx diff --git a/src/renderer/src/components/native-chat/NativeChatComposerActions.tsx b/src/renderer/src/components/native-chat/NativeChatComposerActions.tsx index 23bcd942689..334db3e2b4c 100644 --- a/src/renderer/src/components/native-chat/NativeChatComposerActions.tsx +++ b/src/renderer/src/components/native-chat/NativeChatComposerActions.tsx @@ -153,8 +153,8 @@ export function NativeChatComposerActions({ {onExitGoalMode ? : null}
- {/* Why: keep session controls beside the actions they affect; the - model trigger is ordered last so only the context ring separates it from dictation. */} + {/* Why: keep the session picker beside the actions it affects; only the + context ring separates it from dictation. */} ({ translate: (_key: string, fallback: string) => fallback })) -import { Button } from '@/components/ui/button' -import { PopoverTrigger } from '@/components/ui/popover' import { NativeChatModelCombobox } from './NativeChatModelCombobox' const CHOICES = [ @@ -18,34 +16,24 @@ const CHOICES = [ const settle = (): Promise => act(async () => new Promise((resolve) => setTimeout(resolve, 30))) -type ComboboxOptions = { defaultOpen?: boolean; readOnly?: boolean; readOnlyReason?: string | null } +type ComboboxOptions = { readOnly?: boolean; readOnlyReason?: string | null } type OnSelect = ReturnType void>> -function renderCombobox(props: ComboboxOptions = {}): OnSelect { +/** Renders the model list and returns its search field. */ +async function renderCombobox( + props: ComboboxOptions = {} +): Promise<{ search: HTMLElement; onSelect: OnSelect }> { const onSelect = vi.fn<(value: string) => void>() render( ( - - - - )} /> ) - return onSelect -} - -/** Renders the picker already open and returns its search field. */ -async function openCombobox( - props: ComboboxOptions = {} -): Promise<{ search: HTMLElement; onSelect: OnSelect }> { - const onSelect = renderCombobox(props) await settle() return { search: screen.getByRole('combobox'), onSelect } } @@ -59,33 +47,16 @@ const highlightedModel = (): string | null => afterEach(() => cleanup()) describe('NativeChatModelCombobox', () => { - it('opens with the search focused and the current model highlighted and marked', async () => { - const { search } = await openCombobox() - expect(document.activeElement).toBe(search) + it('lists the models with the current one highlighted and marked', async () => { + const { search } = await renderCombobox() expect(screen.getByRole('combobox', { name: 'Search models…' })).toBe(search) expect(listedModels()).toEqual(['gpt-5.5', 'gpt-5.2-codex', 'opus']) expect(highlightedModel()).toBe('opus') expect(screen.getByRole('option', { current: true }).getAttribute('data-value')).toBe('opus') }) - it('focuses the search when the pill is clicked', async () => { - renderCombobox({ defaultOpen: false }) - fireEvent.click(screen.getByRole('button', { name: 'Model' })) - await settle() - expect(document.activeElement).toBe(screen.getByRole('combobox')) - }) - - it('opens with the search focused when ArrowDown is pressed on the pill', async () => { - renderCombobox({ defaultOpen: false }) - const pill = screen.getByRole('button', { name: 'Model' }) - pill.focus() - fireEvent.keyDown(pill, { key: 'ArrowDown' }) - await settle() - expect(document.activeElement).toBe(screen.getByRole('combobox')) - }) - it('filters by every typed term across label, id and description, in list order', async () => { - const { search } = await openCombobox() + const { search } = await renderCombobox() fireEvent.change(search, { target: { value: 'gpt' } }) expect(listedModels()).toEqual(['gpt-5.5', 'gpt-5.2-codex']) @@ -102,36 +73,27 @@ describe('NativeChatModelCombobox', () => { expect(screen.getByText('No models match your search.')).not.toBeNull() }) - it('moves the highlight with the arrows while focus stays in the search', async () => { - const { search, onSelect } = await openCombobox() + it('moves the highlight with the arrows and picks it with Enter', async () => { + const { search, onSelect } = await renderCombobox() fireEvent.change(search, { target: { value: 'gpt' } }) fireEvent.keyDown(search, { key: 'ArrowDown' }) expect(highlightedModel()).toBe('gpt-5.2-codex') - expect(document.activeElement).toBe(search) fireEvent.keyDown(search, { key: 'Enter' }) - await settle() expect(onSelect).toHaveBeenCalledExactlyOnceWith('gpt-5.2-codex') - expect(screen.queryByRole('combobox')).toBeNull() }) - it('closes on Tab instead of trapping focus in the search', async () => { - const { search } = await openCombobox() - fireEvent.keyDown(search, { key: 'Tab' }) - await settle() - expect(screen.queryByRole('combobox')).toBeNull() - }) + it('picks a clicked row, and sends nothing for the model already in use', async () => { + const { onSelect } = await renderCombobox() + fireEvent.click(screen.getByRole('option', { name: /Opus 4.8/ })) + expect(onSelect).not.toHaveBeenCalled() - it('picks a clicked row and closes', async () => { - const { onSelect } = await openCombobox() fireEvent.click(screen.getByRole('option', { name: /GPT-5.2 Codex/ })) - await settle() expect(onSelect).toHaveBeenCalledExactlyOnceWith('gpt-5.2-codex') - expect(screen.queryByRole('combobox')).toBeNull() }) it('lists but does not pick models it cannot set, and says why', async () => { - const { search, onSelect } = await openCombobox({ + const { search, onSelect } = await renderCombobox({ readOnly: true, readOnlyReason: 'Set when the session starts.' }) diff --git a/src/renderer/src/components/native-chat/NativeChatModelCombobox.tsx b/src/renderer/src/components/native-chat/NativeChatModelCombobox.tsx index 4dcad60d56d..9d52c5d5bf7 100644 --- a/src/renderer/src/components/native-chat/NativeChatModelCombobox.tsx +++ b/src/renderer/src/components/native-chat/NativeChatModelCombobox.tsx @@ -1,31 +1,29 @@ -import { useState } from 'react' +import { useRef } from 'react' import { Check } from 'lucide-react' import { Command, CommandEmpty, + CommandGroup, CommandInput, CommandItem, CommandList } from '@/components/ui/command' -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' +import { NativeChatSessionOptionWriteGroup } from './NativeChatSessionOptionWriteGroup' type NativeChatModelComboboxProps = { choices: readonly SessionOptionSelectChoice[] currentValue: string | undefined - /** Read on mount only: the owner remounts to open on request. */ - defaultOpen: boolean /** Choices stay listed and searchable, but cannot be picked. */ readOnly: boolean readOnlyReason: string | null + /** A write is in flight; the rows take no pick until it settles, and the search stays usable. */ + busy: boolean + /** Called for a model other than the current one; the list stays as it is. */ onSelect: (value: string) => void - focusComposer?: () => void - /** Renders the `PopoverTrigger` pill, which must take the key handler. */ - renderTrigger: (onKeyDown: React.KeyboardEventHandler) => React.ReactNode } // Why not cmdk's scorer: it matches scattered letters and reorders the host's list. @@ -35,58 +33,49 @@ function modelChoiceFilter(value: string, search: string, keywords?: string[]): return terms.every((term) => haystack.includes(term)) ? 1 : 0 } +/** The searchable model list of the session picker. */ export function NativeChatModelCombobox({ choices, currentValue, - defaultOpen, readOnly, readOnlyReason, - onSelect, - focusComposer, - renderTrigger + busy, + onSelect }: NativeChatModelComboboxProps): React.JSX.Element { - const [open, setOpen] = useState(defaultOpen) - const focusReturn = useNativeChatPickerFocusReturn(focusComposer) const searchLabel = translate('components.native-chat.composer.searchModels', 'Search models…') + const search = useRef(null) return ( - - {/* Why: a popover trigger opens on Enter and Space only; the menu it replaced also took ArrowDown. */} - {renderTrigger((event) => { - if (event.key === 'ArrowDown') { + + + {readOnly && readOnlyReason ? ( +
{readOnlyReason}
+ ) : null} + { event.preventDefault() - setOpen(true) - } - })} - { - if (event.key === 'Tab') { - setOpen(false) - } + search.current?.focus() }} > - - - {readOnly && readOnlyReason ? ( -
{readOnlyReason}
- ) : null} - - - {translate( - 'components.native-chat.composer.noModelsMatch', - 'No models match your search.' - )} - + + {translate( + 'components.native-chat.composer.noModelsMatch', + 'No models match your search.' + )} + + + {choices.map((choice) => { const label = nativeChatSessionChoiceLabel(choice) const current = choice.value === currentValue @@ -95,27 +84,30 @@ export function NativeChatModelCombobox({ key={choice.value} value={choice.value} keywords={[label, choice.description ?? '']} + selection="palette" disabled={readOnly} aria-current={current || undefined} onSelect={() => { - focusReturn.notePick() - setOpen(false) - onSelect(choice.value) + if (!current) { + onSelect(choice.value) + } }} > - - {current ? : null} - + {/* Why always there: a row that gains the check must not re-wrap its text while + the picker is open. */} + + {current ? : null} + ) })} - -
-
-
+ + + + ) } diff --git a/src/renderer/src/components/native-chat/NativeChatSessionOptionChoiceBody.tsx b/src/renderer/src/components/native-chat/NativeChatSessionOptionChoiceBody.tsx index f5b100ef5a8..a4e8db2fa97 100644 --- a/src/renderer/src/components/native-chat/NativeChatSessionOptionChoiceBody.tsx +++ b/src/renderer/src/components/native-chat/NativeChatSessionOptionChoiceBody.tsx @@ -6,7 +6,9 @@ export function NativeChatSessionOptionChoiceBody(props: {
{props.label}
{props.description ? ( -
{props.description}
+
+ {props.description} +
) : null}
) 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 index bbba4d02d6d..2d83cc37f80 100644 --- a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.focus-return.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.focus-return.test.tsx @@ -1,6 +1,7 @@ // @vitest-environment happy-dom import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' +import { useState } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' vi.mock('@/i18n/i18n', () => ({ @@ -15,6 +16,7 @@ vi.mock('@/i18n/i18n', () => ({ import { TooltipProvider } from '@/components/ui/tooltip' import type { SessionOptionDescriptor } from '../../../../shared/native-chat-session-options' +import type { NativeChatOptionPickerRequest } from './native-chat-composer-types' import { NativeChatSessionOptionPickers } from './NativeChatSessionOptionPickers' const settle = (): Promise => @@ -61,6 +63,23 @@ const effort: SessionOptionDescriptor = { settable: true } +const speed: SessionOptionDescriptor = { + id: 'serviceTier', + label: 'Speed', + category: 'mode', + kind: { + type: 'select', + currentValue: 'default', + choices: [ + { value: 'default', label: 'Standard' }, + { value: 'fast', label: 'Fast' } + ] + }, + valueSource: 'applied', + transport: 'catalog', + settable: true +} + const fast: SessionOptionDescriptor = { id: 'fastMode', label: 'Fast mode', @@ -71,8 +90,11 @@ const fast: SessionOptionDescriptor = { settable: true } -/** The pickers beside a real message box, so focus has somewhere to come back to. */ -async function renderComposer(snapshot: SessionOptionDescriptor[]): Promise { +/** The picker beside a real message box, so focus has somewhere to come back to. */ +async function renderComposer( + snapshot: SessionOptionDescriptor[], + pickerRequest: NativeChatOptionPickerRequest | null = null +): Promise { const messageBox = document.createElement('textarea') render( @@ -80,6 +102,7 @@ async function renderComposer(snapshot: SessionOptionDescriptor[]): Promise messageBox.focus()} /> @@ -89,79 +112,237 @@ async function renderComposer(snapshot: SessionOptionDescriptor[]): Promise { - const found = screen - .getAllByRole('button') - .find((button) => button.getAttribute('aria-haspopup') === surfaceKind) - if (!found) { - throw new Error(`no pill opening a ${surfaceKind}`) - } - return found +/** A slash command's request held by the session, which drops it when the picker consumes it. */ +function Session(props: { + requestId: string + composerShown?: boolean + surfaceShown?: boolean +}): React.JSX.Element { + const [request, setRequest] = useState({ + id: props.requestId, + consume: () => setRequest(null) + }) + return ( + + {props.composerShown === false ? null : ( + + )} + + ) } -/** 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) - } +const pill = (): HTMLElement => screen.getByRole('button', { name: /^Model / }) + +const openPicker = async (): Promise => { + const trigger = pill() + fireEvent.click(trigger) await settle() return trigger } +const pressEscape = async (): Promise => { + fireEvent.keyDown(document.activeElement ?? document.body, { key: 'Escape' }) + await settle() +} + 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/ })) +describe('session option picker focus', () => { + it.each(['click', 'ArrowDown'] as const)('opens on %s with the search focused', async (how) => { + await renderComposer([model, effort]) + const trigger = pill() + if (how === 'click') { + fireEvent.click(trigger) + } else { + trigger.focus() + fireEvent.keyDown(trigger, { key: 'ArrowDown' }) + } await settle() - expect(surface.setOption).toHaveBeenCalledWith('effort', 'high') - expect(document.activeElement).toBe(messageBox) + expect(document.activeElement).toBe(screen.getByRole('combobox')) }) - it('lands focus in the message box after a model choice', async () => { - const messageBox = await renderComposer([model, effort]) - await openPill('dialog') + it('opens on the option a slash command asked for, once', async () => { + await renderComposer([model, effort], { id: 'effort', consume: () => {} }) + expect(document.activeElement).toBe(screen.getByRole('radio', { name: 'Low' })) + + // The request is spent: opening by hand afterwards focuses the search as usual. + await pressEscape() + await openPicker() + expect(document.activeElement).toBe(screen.getByRole('combobox')) + }) + + // Effort is the last thing set before typing; the model and the other options are not. + it('stays open across a model, a speed and a switch pick, and closes into the message box on an effort pick', async () => { + const messageBox = await renderComposer([model, effort, speed, fast]) + await openPicker() fireEvent.click(screen.getByRole('option', { name: /Sonnet 5/ })) await settle() + fireEvent.click(screen.getByRole('radio', { name: 'Fast' })) + await settle() + fireEvent.click(screen.getByRole('switch', { name: 'Fast mode' })) + await settle() + expect(screen.getByRole('dialog').contains(document.activeElement)).toBe(true) - expect(surface.setOption).toHaveBeenCalledWith('model', 'sonnet') + fireEvent.click(screen.getByRole('radio', { name: 'High' })) + await settle() + + expect(surface.setOption.mock.calls).toEqual([ + ['model', 'sonnet'], + ['serviceTier', 'fast'], + ['fastMode', true], + ['effort', 'high'] + ]) + expect(screen.queryByRole('dialog')).toBeNull() expect(document.activeElement).toBe(messageBox) }) + it('closes on a press of the effort level already in use, without a write', async () => { + const messageBox = await renderComposer([model, effort]) + await openPicker() + + fireEvent.click(screen.getByRole('radio', { name: 'Low' })) + await settle() + + expect(surface.setOption).not.toHaveBeenCalled() + expect(screen.queryByRole('dialog')).toBeNull() + expect(document.activeElement).toBe(messageBox) + }) + + // A level the picked model lacks is removed, and a removed element takes focus with it. + it('moves focus from an option to the search when a model row is pressed', async () => { + await renderComposer([model, effort]) + await openPicker() + screen.getByRole('radio', { name: 'High' }).focus() + + fireEvent.mouseDown(screen.getByRole('option', { name: /Sonnet 5/ })) + + expect(document.activeElement).toBe(screen.getByRole('combobox')) + }) + + // The tooltip is the top layer, so Radix gives it the first Escape; one press still closes both. + it('closes on one Escape from a level whose tooltip is showing', async () => { + const levels: SessionOptionDescriptor = { + ...effort, + kind: { + type: 'select', + currentValue: 'xhigh', + choices: [ + { value: 'medium', label: 'Medium' }, + { value: 'xhigh', label: 'Extra high' } + ] + } + } + await renderComposer([model, levels]) + const trigger = await openPicker() + const level = screen.getByRole('radio', { name: 'XHigh' }) + // The tooltip's own open/closed state must not replace the one the level is styled by. + expect(level.getAttribute('data-state')).toBe('on') + level.focus() + await settle() + expect(screen.getByRole('tooltip')).not.toBeNull() + + await pressEscape() + + expect(screen.queryByRole('dialog')).toBeNull() + expect(document.activeElement).toBe(trigger) + }) + it('leaves focus on the pill when the picker is dismissed instead of used', async () => { await renderComposer([model, effort]) - const trigger = await openPill('menu') + const trigger = await openPicker() - fireEvent.keyDown(document.activeElement ?? document.body, { key: 'Escape' }) - await settle() + await pressEscape() 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' })) + // The pill is how the mouse closes the picker, so a press on it is not focus going elsewhere. + it('lands focus in the message box when the pill closes the picker after a pick', async () => { + const messageBox = await renderComposer([model, effort]) + const trigger = await openPicker() + fireEvent.click(screen.getByRole('option', { name: /Sonnet 5/ })) await settle() - expect(surface.setOption).toHaveBeenCalledWith('fastMode', true) - const menu = screen.getByRole('menu') - expect(menu).toBeTruthy() - expect(menu.contains(document.activeElement)).toBe(true) + // What a press is to the browser: pointerdown, focus on the button, then the click. + fireEvent.pointerDown(trigger) + trigger.focus() + fireEvent.click(trigger) + await settle() + + expect(screen.queryByRole('dialog')).toBeNull() + expect(document.activeElement).toBe(messageBox) + }) + + it('leaves focus where a click outside put it, even after a pick', async () => { + await renderComposer([model, effort]) + const elsewhere = document.createElement('input') + document.body.append(elsewhere) + await openPicker() + fireEvent.click(screen.getByRole('option', { name: /Sonnet 5/ })) + await settle() + + // Radix waits for the click that follows an outside pointerdown before it dismisses. + fireEvent.pointerDown(elsewhere) + elsewhere.focus() + fireEvent.click(elsewhere) + await settle() + + expect(screen.queryByRole('dialog')).toBeNull() + expect(document.activeElement).toBe(elsewhere) + }) + + it('opens once for a request, and comes back closed after the pill went away', async () => { + const { rerender } = render() + await settle() + expect(screen.queryByRole('dialog')).not.toBeNull() + + rerender() + await settle() + expect(screen.queryByRole('button', { name: /^Model / })).toBeNull() + rerender() + await settle() + expect(screen.queryByRole('dialog')).toBeNull() + }) + + it('focuses the effort for /effort once, not again after a prompt card', async () => { + const { rerender } = render() + await settle() + expect(document.activeElement).toBe(screen.getByRole('radio', { name: 'Low' })) + + rerender() + await settle() + rerender() + await settle() + expect(screen.queryByRole('dialog')).toBeNull() + + await openPicker() + expect(document.activeElement).toBe(screen.getByRole('combobox')) + }) + + // The pill already shows its values; a tooltip there only covers the picker it opens. + it('shows a tooltip on the pill only for what the pill cannot show', async () => { + await renderComposer([model, effort]) + fireEvent.pointerMove(pill(), { pointerType: 'mouse' }) + pill().focus() + await settle() + expect(screen.queryByRole('tooltip')).toBeNull() + + cleanup() + await renderComposer([{ ...model, settable: false, disabledReason: 'set-when-session-starts' }]) + pill().focus() + await settle() + expect(screen.getByRole('tooltip').textContent).toBe('Set when the session starts.') }) }) diff --git a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.test.tsx b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.test.tsx index db6b463b506..a94a21c0a02 100644 --- a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.test.tsx @@ -1,7 +1,7 @@ // @vitest-environment happy-dom -import { cleanup, render, screen, waitFor } from '@testing-library/react' -import type * as ReactModule from 'react' +import { act, cleanup, fireEvent, render, screen, waitFor, within } from '@testing-library/react' +import { useState } from 'react' import { afterEach, describe, expect, it, vi } from 'vitest' import { OMP_SESSION_OPTION_CATALOG } from '../../../../shared/agent-session-option-catalog-omp' import { @@ -39,135 +39,11 @@ vi.mock('@/components/ui/tooltip', () => ({ TooltipContent: ({ children }: { children: React.ReactNode }) =>
{children}
})) -vi.mock('@/components/ui/dropdown-menu', () => { - const React = require('react') as typeof ReactModule - return { - DropdownMenu: ({ - children, - defaultOpen - }: { - children: React.ReactNode - defaultOpen?: boolean - }) => ( -
- {children} -
- ), - DropdownMenuTrigger: ({ - children, - disabled - }: { - children: React.ReactNode - disabled?: boolean - }) =>
{children}
, - DropdownMenuContent: ({ - children, - side, - collisionPadding - }: { - children: React.ReactNode - side?: string - collisionPadding?: number - }) => ( -
- {children} -
- ), - DropdownMenuLabel: ({ children }: { children: React.ReactNode }) =>
{children}
, - DropdownMenuSeparator: () =>
, - // Forwards role/aria-* and hands onSelect an event: the switch rows set both, - // and preventDefault is how a toggle keeps the menu open. - DropdownMenuItem: ({ - children, - disabled, - onSelect, - ...rest - }: React.ButtonHTMLAttributes & { - onSelect?: (event: { preventDefault: () => void }) => void - }) => ( - - ), - // Why: exercises value binding + onValueChange contract the real Radix - // group provides; selected value is exposed via data-radio-value. - DropdownMenuRadioGroup: ({ - children, - value, - onValueChange, - 'aria-label': ariaLabel - }: { - children: React.ReactNode - value?: string - onValueChange?: (value: string) => void - 'aria-label'?: string - }) => ( -
- {React.Children.map(children, (child) => { - if (!React.isValidElement(child)) { - return child - } - const props = child.props as { - value?: string - disabled?: boolean - children?: React.ReactNode - } - const selected = props.value !== undefined && props.value === value - return ( - - ) - })} -
- ), - DropdownMenuRadioItem: ({ - children, - disabled, - value - }: React.ButtonHTMLAttributes & { value: string }) => ( - // Why: parent RadioGroup mock reads `value` via Children.map — keep it on - // props even though native span has no value attribute. - )} - > - {children} - - ) - } -}) - -// Same test ids as the dropdown mock: the model pill is a popover, the options pill a menu. +// The popover is always mounted here, so a test reads its rows without opening it; real +// focus and dismissal are covered in the focus-return file. vi.mock('@/components/ui/popover', () => ({ Popover: ({ children, open }: { children: React.ReactNode; open?: boolean }) => ( -
+
{children}
), @@ -177,15 +53,18 @@ vi.mock('@/components/ui/popover', () => ({ PopoverContent: ({ children, side, + align, collisionPadding }: { children: React.ReactNode side?: string + align?: string collisionPadding?: number }) => (
{children} @@ -194,6 +73,7 @@ vi.mock('@/components/ui/popover', () => ({ })) import { NativeChatSessionOptionPickers } from './NativeChatSessionOptionPickers' +import type { NativeChatOptionPickerRequest } from './native-chat-composer-types' const surface = { getSnapshot: vi.fn(() => []), @@ -255,52 +135,99 @@ const fast: SessionOptionDescriptor = { settable: true } +const settle = (): Promise => act(async () => {}) + +const pill = (name: string): HTMLElement => screen.getByRole('button', { name }) + +const isOpen = (trigger: HTMLElement): boolean => + trigger.closest('[data-testid="picker-root"]')?.getAttribute('data-open') === 'true' + +const effortLevels = (): HTMLElement[] => + within(screen.getByRole('radiogroup', { name: 'Effort' })).getAllByRole('radio') + afterEach(() => cleanup()) describe('NativeChatSessionOptionPickers', () => { - it('opens the native picker requested by a structured slash command', async () => { + it.each(['model', 'effort'])( + 'opens the one picker when a slash command asks for the %s', + async (id) => { + const { rerender } = render( + + ) + expect(isOpen(pill('Model Opus 4.8, Effort High'))).toBe(false) + + rerender( + {} }} + /> + ) + await waitFor(() => expect(isOpen(pill('Model Opus 4.8, Effort High'))).toBe(true)) + } + ) + + it('keeps the picker a slash command opened when the next model lacks that option', () => { + const request = { id: 'effort', consume: () => {} } const { rerender } = render( ) + expect(isOpen(pill('Model Opus 4.8, Effort High'))).toBe(true) rerender( ) - await waitFor(() => - expect( - screen - .getByRole('button', { name: 'Model Opus 4.8' }) - .closest('[data-testid="dropdown-root"]') - ?.getAttribute('data-open') - ).toBe('true') - ) + // Still the same visit: open, with the effort row it opened on kept in place. + expect(isOpen(pill('Model Opus 4.8'))).toBe(true) + expect(effortLevels().every((level) => level.hasAttribute('disabled'))).toBe(true) + }) - rerender( - - ) - await waitFor(() => - expect( - screen - .getByRole('button', { name: 'Effort High' }) - .closest('[data-testid="dropdown-root"]') - ?.getAttribute('data-open') - ).toBe('true') - ) + // Why: the session holds the request across the pill's unmount; a request left standing + // reopened the picker each time a prompt card handed the composer back. + it.each<[string, { composerShown?: boolean; snapshot?: SessionOptionDescriptor[] }]>([ + ['the composer comes back after a prompt card', { composerShown: false }], + ['the session reports its model again', { snapshot: [] }] + ])('opens once for a request, not again when %s', async (_case, gone) => { + function Session(props: { + composerShown?: boolean + snapshot?: SessionOptionDescriptor[] + }): React.JSX.Element | null { + const [request, setRequest] = useState({ + id: 'model', + consume: () => setRequest(null) + }) + return props.composerShown === false ? null : ( + + ) + } + const { rerender } = render() + await waitFor(() => expect(isOpen(pill('Model Opus 4.8, Effort High'))).toBe(true)) + + rerender() + expect(screen.queryByRole('button', { name: /^Model/ })).toBeNull() + rerender() + expect(isOpen(pill('Model Opus 4.8, Effort High'))).toBe(false) }) it('sends a picked model to the session surface', async () => { @@ -316,7 +243,8 @@ describe('NativeChatSessionOptionPickers', () => { await waitFor(() => expect(setOption).toHaveBeenCalledExactlyOnceWith('model', 'sonnet')) }) - it('prefers collision-aware upward placement for model and option menus', () => { + // End-aligned: the pill grows leftwards as its label changes, and the picker stays open. + it('opens one picker upwards from the pill, anchored at its right edge', () => { render( { /> ) - const menus = screen.getAllByTestId('session-option-menu') - expect(menus).toHaveLength(2) - for (const menu of menus) { - expect(menu.getAttribute('data-side')).toBe('top') - expect(menu.getAttribute('data-collision-padding')).toBe('8') - } + const content = screen.getByTestId('picker-content') + expect(content.getAttribute('data-side')).toBe('top') + expect(content.getAttribute('data-align')).toBe('end') + expect(content.getAttribute('data-collision-padding')).toBe('8') }) - it('renders model and joined option labels, and hides an empty options pill', () => { + it('shows the model and its option values in one pill', () => { const { rerender } = render( { isWorking={false} /> ) - expect(screen.getByRole('button', { name: 'Model Opus 4.8' }).textContent).toContain('Opus 4.8') - expect(screen.getByRole('button', { name: 'Model Opus 4.8' }).textContent).not.toContain( - 'Model:' - ) - expect(screen.getByRole('button', { name: 'Effort High · Fast' }).textContent).toContain( - 'High · Fast' - ) - expect( - screen - .getByRole('button', { name: 'Model Opus 4.8' }) - .compareDocumentPosition(screen.getByRole('button', { name: 'Effort High · Fast' })) & - Node.DOCUMENT_POSITION_FOLLOWING - ).not.toBe(0) + expect(pill('Model Opus 4.8, Effort High · Fast').textContent).toBe('Opus 4.8High · Fast') + // An option nobody has reported a value for adds nothing to the pill. rerender( - - ) - expect(screen.queryByRole('button', { name: /Effort/ })).toBeNull() - }) - - it('names a lone unknown effort control explicitly', () => { - render( ) + expect(pill('Model Opus 4.8').textContent).toBe('Opus 4.8') - expect(screen.getByRole('button', { name: 'Effort' }).textContent).toContain('Effort') + rerender( + + ) + expect(pill('Model Opus 4.8, Session options Fast').textContent).toBe('Opus 4.8Fast') }) - it('disables both picker triggers while the agent is working', () => { + it('disables the pill while the agent is working', () => { render( ) - expect( - screen - .getByRole('button', { name: 'Model Opus 4.8' }) - .parentElement?.getAttribute('data-disabled') - ).toBe('true') - expect( - screen - .getByRole('button', { name: 'Effort High' }) - .parentElement?.getAttribute('data-disabled') - ).toBe('true') + expect(pill('Model Opus 4.8, Effort High').parentElement?.getAttribute('data-disabled')).toBe( + 'true' + ) }) it('renders a session with no reported model without an empty model control', () => { @@ -406,37 +315,15 @@ describe('NativeChatSessionOptionPickers', () => { expect(screen.queryAllByRole('button', { name: '' })).toEqual([]) }) - it('does not duplicate titles for unknown values or misname generic controls', () => { - const { rerender } = render( + it('names the control once when the model is unknown', () => { + render( ) - expect(screen.getByRole('button', { name: 'Model' }).textContent).toContain('Model') - expect(screen.getByRole('button', { name: 'Model' }).textContent).not.toContain('Model: Model') - expect(screen.getByRole('button', { name: 'Effort' }).textContent).not.toContain( - 'Effort: Effort' - ) - - rerender( - - ) - expect(screen.getByRole('button', { name: 'Session options Fast' }).textContent).toContain( - 'Fast' - ) - expect(screen.queryByRole('button', { name: /^Effort/ })).toBeNull() + expect(pill('Model').textContent).toBe('Model') }) // The terminal transport typed the value at the agent and has not read it back, @@ -450,7 +337,6 @@ describe('NativeChatSessionOptionPickers', () => { isWorking={false} /> ) - expect(screen.getByText('Model')).not.toBeNull() expect(screen.getAllByText('Sent to the agent — not confirmed').length).toBeGreaterThan(0) }) @@ -462,7 +348,6 @@ describe('NativeChatSessionOptionPickers', () => { isWorking={false} /> ) - expect(screen.getByText('Model')).not.toBeNull() expect(screen.queryByText(/not confirmed/)).toBeNull() }) @@ -476,12 +361,33 @@ describe('NativeChatSessionOptionPickers', () => { isWorking={false} /> ) - expect(screen.getByText('Model')).not.toBeNull() expect(screen.queryByText(/not confirmed/)).toBeNull() } ) - it('renders agent-picker routes as one action instead of radio choices', async () => { + it('hedges the pill for a dispatched option as it does for the model', () => { + render( + + ) + expect(screen.getAllByText('Sent to the agent — not confirmed').length).toBeGreaterThan(0) + }) + + it('renders agent-picker routes as one action instead of model rows', async () => { const invokeAction = vi.fn().mockResolvedValue({ snapshot: [] }) const liveSurface = { ...surface, invokeAction } render( @@ -578,10 +484,9 @@ describe('NativeChatSessionOptionPickers', () => { isWorking={false} /> ) - expect(screen.getByText('Toggle fast mode')).not.toBeNull() - expect(screen.queryByText('On')).toBeNull() - expect(screen.queryByText('Off')).toBeNull() - screen.getByText('Toggle fast mode').click() + // Flip-only without a baseline: an action, never a switch claiming On or Off. + expect(screen.queryByRole('switch')).toBeNull() + screen.getByRole('button', { name: 'Toggle fast mode' }).click() await waitFor(() => expect(invokeAction).toHaveBeenCalledWith('fastMode')) expect(setOption).not.toHaveBeenCalled() }) @@ -604,19 +509,14 @@ describe('NativeChatSessionOptionPickers', () => { isWorking={false} /> ) - expect(screen.queryByText('Toggle fast mode')).toBeNull() // One control, not an On/Off pair, and the row carries the label itself. - expect(screen.queryByRole('radio', { name: 'On' })).toBeNull() - expect(screen.queryByRole('radio', { name: 'Off' })).toBeNull() + expect(screen.queryByRole('radio')).toBeNull() const fastSwitch = screen.getByRole('switch', { name: 'Fast mode' }) expect(fastSwitch.getAttribute('aria-checked')).toBe('true') - expect( - fastSwitch.querySelector('[data-slot="switch-indicator"]')?.getAttribute('data-state') - ).toBe('checked') - // The label is not duplicated by a separate group header. expect(screen.getAllByText('Fast mode')).toHaveLength(1) fastSwitch.click() await waitFor(() => expect(setOption).toHaveBeenCalledWith('fastMode', false)) + await settle() setOption.mockClear() rerender( @@ -665,32 +565,194 @@ describe('NativeChatSessionOptionPickers', () => { /> ) const control = screen.getByRole('switch', { name: 'Fast mode' }) - expect(control.textContent).toBe('Fast mode') expect(control.hasAttribute('aria-describedby')).toBe(false) expect(screen.queryByText('Not reported')).toBeNull() expect(screen.queryByText('Default')).toBeNull() }) - it('tooltips a dispatched option pill with the category alone', () => { + it('makes the effort levels one tab stop, moves between them with the arrow keys, and sends the one picked', async () => { + const setOption = vi.fn().mockResolvedValue({ snapshot: [] }) + render( + + ) + const [low, high] = effortLevels() + expect([low?.textContent, high?.textContent]).toEqual(['Low', 'High']) + expect([low?.getAttribute('aria-checked'), high?.getAttribute('aria-checked')]).toEqual([ + 'false', + 'true' + ]) + // Tab stops at the group, not at each level, and lands on the level in use. + expect(effortLevels().map((level) => level.tabIndex)).toEqual([-1, -1]) + screen.getByRole('radiogroup', { name: 'Effort' }).focus() + await waitFor(() => expect(document.activeElement).toBe(high)) + + // An arrow key moves focus without writing: each write is a round trip to the agent. + fireEvent.keyDown(document.activeElement ?? document.body, { key: 'ArrowLeft' }) + await waitFor(() => expect(document.activeElement).toBe(low)) + // The level already in use is not a write either. + high?.click() + expect(setOption).not.toHaveBeenCalled() + + low?.click() + await waitFor(() => expect(setOption).toHaveBeenCalledExactlyOnceWith('effort', 'low')) + }) + + // Voice control targets what is on screen (WCAG 2.5.3), so the name is the visible text. + it('names a level by what it shows and describes it with the rest', () => { render( ) - expect(screen.getAllByText('Thinking').length).toBeGreaterThan(0) - expect(screen.getAllByText('Sent to the agent — not confirmed').length).toBeGreaterThan(0) + const described = (name: string): string | null | undefined => { + const id = screen.getByRole('radio', { name }).getAttribute('aria-describedby') + return id ? document.getElementById(id)?.textContent : null + } + expect(described('Low')).toBe('Quick answers') + expect(described('Med')).toBe('Medium') + expect(described('XHigh')).toBe('Extra high Deepest reasoning') + expect(described('Max')).toBeNull() + // The pill has the room for the whole name. + expect(pill('Model Opus 4.8, Effort Medium')).not.toBeNull() + }) + + it('shows a pick at once and holds every write control until it settles', async () => { + let settleWrite: () => void = () => {} + const setOption = vi.fn( + () => + new Promise<{ snapshot: SessionOptionDescriptor[] }>((resolve) => { + settleWrite = () => resolve({ snapshot: [] }) + }) + ) + render( + + ) + const held = (): (string | null | undefined)[] => [ + screen.getByRole('listbox').getAttribute('aria-busy'), + screen + .getByRole('radiogroup', { name: 'Effort' }) + .closest('[data-busy]') + ?.getAttribute('aria-disabled') + ] + expect(held()).toEqual(['false', 'false']) + + screen.getByRole('radio', { name: 'Low' }).click() + await waitFor(() => + expect(screen.getByRole('radio', { name: 'Low' }).getAttribute('aria-checked')).toBe('true') + ) + expect(pill('Model Opus 4.8, Effort Low')).not.toBeNull() + // The list reports busy and the options unavailable; the surface would drop a second write. + expect(held()).toEqual(['true', 'true']) + screen.getByRole('option', { name: 'Sonnet 5' }).click() + expect(setOption).toHaveBeenCalledExactlyOnceWith('effort', 'low') + + // The snapshot is the truth once the write settles; here it still says High. + await act(async () => settleWrite()) + await waitFor(() => expect(held()).toEqual(['false', 'false'])) + expect(screen.getByRole('radio', { name: 'High' }).getAttribute('aria-checked')).toBe('true') + + screen.getByRole('option', { name: 'Sonnet 5' }).click() + expect(setOption).toHaveBeenLastCalledWith('model', 'sonnet') + }) + + it("holds the model's own action while a write is in flight, without closing on it", async () => { + let settleWrite: () => void = () => {} + const setOption = vi.fn( + () => + new Promise<{ snapshot: SessionOptionDescriptor[] }>((resolve) => { + settleWrite = () => resolve({ snapshot: [] }) + }) + ) + const invokeAction = vi.fn().mockResolvedValue({ snapshot: [] }) + render( + {} }} + /> + ) + const trigger = pill('Model') + const action = screen.getByRole('button', { name: 'Choose in agent picker…' }) + const unavailable = (): string | null | undefined => + action.closest('[data-busy]')?.getAttribute('aria-disabled') + expect(unavailable()).toBe('false') + + screen.getByRole('switch', { name: 'Fast mode' }).click() + await waitFor(() => expect(unavailable()).toBe('true')) + // fireEvent flushes the render, so a picker that closed here would read as closed. + fireEvent.click(action) + expect(invokeAction).not.toHaveBeenCalled() + expect(isOpen(trigger)).toBe(true) + + await act(async () => settleWrite()) + fireEvent.click(action) + await waitFor(() => expect(invokeAction).toHaveBeenCalledExactlyOnceWith('model')) + expect(isOpen(trigger)).toBe(false) + }) + + // The popover hangs from its bottom edge: a row that vanished would slide the list. + it('keeps a row the next model lacks, switched off, while the picker is mounted', () => { + const { rerender } = render( + + ) + rerender( + + ) + const fastSwitch = screen.getByRole('switch', { name: 'Fast mode' }) + expect(fastSwitch).toHaveProperty('disabled', true) + expect(fastSwitch.getAttribute('aria-checked')).toBe('false') + expect(effortLevels().map((level) => level.getAttribute('aria-checked'))).toEqual([ + 'false', + 'false' + ]) + expect(effortLevels().every((level) => level.hasAttribute('disabled'))).toBe(true) }) }) diff --git a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.tsx b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.tsx index 18d99fc98b9..0c1b17ed5d0 100644 --- a/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.tsx +++ b/src/renderer/src/components/native-chat/NativeChatSessionOptionPickers.tsx @@ -1,18 +1,7 @@ -import { memo, useState } from 'react' +import { memo, useEffect, useState } from 'react' import { ChevronDown } from 'lucide-react' import { Button } from '@/components/ui/button' -import { SwitchIndicator } from '@/components/ui/switch' -import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuLabel, - DropdownMenuRadioGroup, - DropdownMenuRadioItem, - DropdownMenuSeparator, - DropdownMenuTrigger -} from '@/components/ui/dropdown-menu' -import { PopoverTrigger } from '@/components/ui/popover' +import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover' import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' import { translate } from '@/i18n/i18n' import { sortNativeChatSessionOptions } from '../../../../shared/native-chat-session-option-snapshot' @@ -24,175 +13,272 @@ import { } from '../../../../shared/native-chat-session-options' import { nativeChatModelPillLabel, - nativeChatOptionsPillLabel, + nativeChatOptionValuesLabel, nativeChatOptionsPillTitle, - nativeChatSessionChoiceLabel, - nativeChatSessionOptionDisabledReason, - nativeChatSessionOptionLabel + nativeChatSessionOptionActionLabel, + nativeChatSessionOptionDisabledReason } from './native-chat-session-option-labels' import type { NativeChatOptionPickerRequest } from './native-chat-composer-types' import { runSessionOptionSurfaceCall } from './native-chat-session-option-surface-call' import { NativeChatModelCombobox } from './NativeChatModelCombobox' import { useNativeChatPickerFocusReturn } from './native-chat-picker-focus-return' -import { NativeChatSessionOptionChoiceBody } from './NativeChatSessionOptionChoiceBody' +import { NativeChatSessionOptionRows } from './NativeChatSessionOptionRows' +import { NativeChatSessionOptionWriteGroup } from './NativeChatSessionOptionWriteGroup' export type NativeChatSessionOptionPickersProps = { surface: SessionOptionsSurface | null snapshot: SessionOptionDescriptor[] isWorking: boolean pickerRequest?: NativeChatOptionPickerRequest | null - /** Where focus lands after a pick; see `useNativeChatPickerFocusReturn`. */ + /** Where focus lands after a visit that changed something; see `useNativeChatPickerFocusReturn`. */ focusComposer?: () => void } -function PickerTooltipContent(props: { - label: string - disabledReason?: string | null - dispatched: boolean -}): React.JSX.Element { - return ( -
-
{props.disabledReason ?? props.label}
- {props.dispatched ? ( -
- {translate( - 'components.native-chat.composer.sentNotConfirmed', - 'Sent to the agent — not confirmed' - )} -
- ) : null} -
- ) +/** A write in flight. Its value shows at once, and nothing else is sent until it settles. */ +type PendingPick = { id: string; value?: SessionOptionValue } + +// Why: the picker stays open across a pick, so the choice has to land where it was made before +// the host answers; a refused write falls back to the snapshot. +function withPendingValue( + descriptor: SessionOptionDescriptor, + pending: PendingPick | null +): SessionOptionDescriptor { + if (pending?.id !== descriptor.id) { + return descriptor + } + const { kind } = descriptor + if (kind.type === 'select' && typeof pending.value === 'string') { + return { + ...descriptor, + valueSource: 'dispatched', + kind: { ...kind, currentValue: pending.value } + } + } + if (kind.type === 'boolean' && typeof pending.value === 'boolean') { + return { + ...descriptor, + valueSource: 'dispatched', + kind: { ...kind, currentValue: pending.value } + } + } + return descriptor +} + +// Why: value-only visible text must still include the category in the +// accessible name (WCAG 2.5.3 Label in Name / voice control). +function pillAccessibleName(title: string, value: string): string { + return value === title + ? title + : translate('components.native-chat.composer.pillAccessibleName', '{{value0}} {{value1}}', { + value0: title, + value1: value + }) } function PickerTrigger(props: { - /** The trigger of whichever surface the pill opens. */ - Trigger: typeof DropdownMenuTrigger | typeof PopoverTrigger - label: string - tooltipLabel: string + modelLabel: string + /** The options' values, shown after the model; empty when there are none to show. */ + optionsLabel: string + accessibleName: string disabled: boolean - disabledReason?: string | null + disabledReason: string | null dispatched: boolean - onKeyDown?: React.KeyboardEventHandler + triggerRef: React.Ref + onKeyDown: React.KeyboardEventHandler }): React.JSX.Element { - // Why: value-only visible text must still include the category in the - // accessible name (WCAG 2.5.3 Label in Name / voice control). - const accessibleName = - props.label === props.tooltipLabel - ? props.tooltipLabel - : translate('components.native-chat.composer.pillAccessibleName', '{{value0}} {{value1}}', { - value0: props.tooltipLabel, - value1: props.label - }) + const [tooltipOpen, setTooltipOpen] = useState(false) + // Why: the pill already shows its values, so it only gets a tooltip for what it cannot show. + // Held closed rather than unmounted: a pick can flip this, and the open picker hangs off the pill. + const informative = props.disabledReason !== null || props.dispatched return ( - + - + - + - +
+ {props.disabledReason ?
{props.disabledReason}
: null} + {props.dispatched ? ( +
+ {translate( + 'components.native-chat.composer.sentNotConfirmed', + 'Sent to the agent — not confirmed' + )} +
+ ) : null} +
) } -function DescriptorMenuRows(props: { - descriptor: SessionOptionDescriptor - pending: boolean - setValue: (value: SessionOptionValue) => void - invokeAction: () => void - /** Called by the rows whose choice closes the menu. */ - notePick: () => void +function SessionOptionPopover(props: { + model: SessionOptionDescriptor + options: readonly SessionOptionDescriptor[] + isWorking: boolean + /** A write is in flight: nothing else is taken, and the write controls say so. */ + busy: boolean + request: NativeChatOptionPickerRequest | null + onSet: (descriptor: SessionOptionDescriptor, value: SessionOptionValue) => void + onAction: (descriptor: SessionOptionDescriptor) => void + focusComposer?: () => void }): React.JSX.Element { - 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 ( - { - notePick() - invokeAction() - }} - > - {translate('components.native-chat.composer.toggleOption', 'Toggle {{value0}}', { - value0: nativeChatSessionOptionLabel(descriptor).toLowerCase() - })} - - ) + const { model, options, busy } = props + // Why here: this unmounts with the pill, so a picker that went away comes back closed. + const [open, setOpen] = useState(false) + const [seenRequest, setSeenRequest] = useState(null) + // The option a slash command asked for, focused in place of the search by that opening only. + const [requestedOptionId, setRequestedOptionId] = useState(null) + if (props.request !== seenRequest) { + setSeenRequest(props.request) + if (props.request) { + setOpen(true) + setRequestedOptionId(props.request.id === model.id ? null : props.request.id) + } } - // 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()}> - {translate( - 'components.native-chat.composer.chooseInAgentPicker', - 'Choose in agent picker…' - )} - - ) + const focusReturn = useNativeChatPickerFocusReturn(props.focusComposer) + const modelTitle = translate('components.native-chat.composer.model', 'Model') + const modelLabel = nativeChatModelPillLabel(model) + const modelName = pillAccessibleName(modelTitle, modelLabel) + const optionsTitle = nativeChatOptionsPillTitle(options) + const optionsLabel = nativeChatOptionValuesLabel(options) + const modelReason = nativeChatSessionOptionDisabledReason(model.disabledReason) + const optionsReason = + options.length > 0 && options.every((descriptor) => !descriptor.settable) + ? nativeChatSessionOptionDisabledReason(options[0]?.disabledReason) + : null + + // Why not sent while busy: the surface drops a write made while another is in flight. + const set = (descriptor: SessionOptionDescriptor, value: SessionOptionValue): void => { + if (busy) { + return + } + focusReturn.notePick() + // Why: effort is the last thing set before typing, so its level closes the picker. + if (descriptor.id === 'effort') { + setOpen(false) + } + if (descriptor.kind.currentValue !== value) { + props.onSet(descriptor, value) + } } - // Why one switch row and not On/Off: the option is binary, so a single control - // carries it. The row owns the label, which is why the caller drops its header. - if (descriptor.kind.type === 'boolean') { - const checked = descriptor.kind.currentValue - const label = nativeChatSessionOptionLabel(descriptor) - return ( - { - event.preventDefault() - setValue(!checked) - }} - className="justify-between gap-2" - > - {label} - - - ) + const act = (descriptor: SessionOptionDescriptor): void => { + if (busy) { + return + } + // Why: agent-picker opens the TUI, and that picker owns focus. + if (descriptor.action?.type !== 'agent-picker') { + focusReturn.notePick() + } + setOpen(false) + props.onAction(descriptor) } + return ( - { - notePick() - setValue(value) - }} - > - {descriptor.kind.choices.map((choice) => ( - - + { + if (event.key === 'ArrowDown') { + event.preventDefault() + setOpen(true) + } + }} + /> + { + const content = event.currentTarget + const optionId = requestedOptionId + setRequestedOptionId(null) + if (!optionId || !(content instanceof HTMLElement)) { + return + } + const row = content.querySelector(`[data-option-id="${CSS.escape(optionId)}"]`) + const control = + row?.querySelector('[aria-checked="true"]:not(:disabled)') ?? + row?.querySelector('button:not(:disabled)') + if (control) { + event.preventDefault() + control.focus() + } + }} + onInteractOutside={focusReturn.onInteractOutside} + onCloseAutoFocus={focusReturn.onCloseAutoFocus} + // Why: a focused level's tooltip is the top layer, so Radix hands it the first Escape and + // the picker would take a second. The search is left to Radix, which waits out an IME. + onKeyDown={(event) => { + if (event.key === 'Escape' && !(event.target instanceof HTMLInputElement)) { + setOpen(false) + } + }} + > + {model.kind.type === 'select' && !model.action ? ( + set(model, value)} /> - - ))} - + ) : ( + + {modelReason && !model.settable ? ( +
{modelReason}
+ ) : null} + +
+ )} + + + ) } @@ -203,132 +289,39 @@ function NativeChatSessionOptionPickersInner({ pickerRequest, focusComposer }: NativeChatSessionOptionPickersProps): React.JSX.Element | null { - const [pendingId, setPendingId] = useState(null) - const modelMenuFocus = useNativeChatPickerFocusReturn(focusComposer) - const optionsMenuFocus = useNativeChatPickerFocusReturn(focusComposer) - const model = snapshot.find((descriptor) => descriptor.category === 'model') - const options = sortNativeChatSessionOptions(snapshot) - if (!surface || !model) { + const [pending, setPending] = useState(null) + // Why: the session holds a request across composer remounts; one left standing reopens the + // picker each time a prompt card gives the composer back. Dropped even when nothing could open. + useEffect(() => pickerRequest?.consume(), [pickerRequest]) + const reportedModel = snapshot.find((descriptor) => descriptor.category === 'model') + if (!surface || !reportedModel) { return null } - const requestedModelSequence = pickerRequest?.id === model.id ? pickerRequest.sequence : null - const requestedOptionsSequence = options.some((descriptor) => descriptor.id === pickerRequest?.id) - ? (pickerRequest?.sequence ?? null) - : null - - const setOption = (descriptor: SessionOptionDescriptor, value: SessionOptionValue): void => { - runSessionOptionSurfaceCall(descriptor.id, setPendingId, () => - surface.setOption(descriptor.id, value) - ) - } - const invokeAction = (descriptor: SessionOptionDescriptor): void => { - runSessionOptionSurfaceCall(descriptor.id, setPendingId, () => - surface.invokeAction(descriptor.id) - ) - } - - const modelReason = nativeChatSessionOptionDisabledReason(model.disabledReason) - const modelPickerKey = `model:${requestedModelSequence ?? 'idle'}` - // Read only when a request remounts the picker. - const modelPickerDefaultOpen = requestedModelSequence !== null - const modelTrigger = { - label: nativeChatModelPillLabel(model), - tooltipLabel: translate('components.native-chat.composer.model', 'Model'), - disabled: isWorking || pendingId !== null, - disabledReason: modelReason, - dispatched: sessionOptionDispatchUnconfirmed(model) - } - const optionsTooltip = nativeChatOptionsPillTitle(options) - const optionsReason = - options.length > 0 && options.every((descriptor) => !descriptor.settable) - ? nativeChatSessionOptionDisabledReason(options[0]?.disabledReason) - : null + const model = withPendingValue(reportedModel, pending) + const options = sortNativeChatSessionOptions(snapshot).map((descriptor) => + withPendingValue(descriptor, pending) + ) return ( -
- {model.kind.type === 'select' && !model.action ? ( - setOption(model, value)} - focusComposer={focusComposer} - renderTrigger={(onKeyDown) => ( - - )} - /> - ) : ( - - - - {modelReason && !model.settable ? ( - {modelReason} - ) : null} - setOption(model, value)} - invokeAction={() => invokeAction(model)} - notePick={modelMenuFocus.notePick} - /> - - - )} - {options.length > 0 ? ( - - - - {options.map((descriptor, index) => { - const reason = nativeChatSessionOptionDisabledReason(descriptor.disabledReason) - return ( -
- {index > 0 ? : null} - {descriptor.kind.type === 'boolean' && !descriptor.action ? null : ( - - {nativeChatSessionOptionLabel(descriptor)} - - )} - {reason && !descriptor.settable ? ( - {reason} - ) : null} - setOption(descriptor, value)} - invokeAction={() => invokeAction(descriptor)} - notePick={optionsMenuFocus.notePick} - /> -
- ) - })} -
-
- ) : null} +
+ + runSessionOptionSurfaceCall({ id: descriptor.id, value }, setPending, () => + surface.setOption(descriptor.id, value) + ) + } + onAction={(descriptor) => + runSessionOptionSurfaceCall({ id: descriptor.id }, setPending, () => + surface.invokeAction(descriptor.id) + ) + } + focusComposer={focusComposer} + />
) } diff --git a/src/renderer/src/components/native-chat/NativeChatSessionOptionRows.tsx b/src/renderer/src/components/native-chat/NativeChatSessionOptionRows.tsx new file mode 100644 index 00000000000..4f708a47a74 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatSessionOptionRows.tsx @@ -0,0 +1,223 @@ +import { useId, useLayoutEffect, useRef } from 'react' +import { Button } from '@/components/ui/button' +import { Switch } from '@/components/ui/switch' +import { ToggleGroup, ToggleGroupItem } from '@/components/ui/toggle-group' +import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip' +import { sortNativeChatSessionOptions } from '../../../../shared/native-chat-session-option-snapshot' +import type { + SessionOptionDescriptor, + SessionOptionSelectChoice, + SessionOptionValue +} from '../../../../shared/native-chat-session-options' +import { + nativeChatSessionChoiceLabel, + nativeChatSessionChoiceShortLabel, + nativeChatSessionOptionActionLabel, + nativeChatSessionOptionDisabledReason, + nativeChatSessionOptionLabel +} from './native-chat-session-option-labels' +import { NativeChatSessionOptionWriteGroup } from './NativeChatSessionOptionWriteGroup' + +type NativeChatSessionOptionRowsProps = { + /** The current model's options, in display order. */ + options: readonly SessionOptionDescriptor[] + /** A reason already shown above the list, so a row does not repeat it. */ + shownReason: string | null + /** A write is in flight; the controls take nothing until it settles. */ + busy: boolean + onSet: (descriptor: SessionOptionDescriptor, value: SessionOptionValue) => void + /** A value-less action; it has nothing to show in place, so it closes the picker. */ + onAction: (descriptor: SessionOptionDescriptor) => void +} + +// Why: the popover hangs from its bottom edge, so a row vanishing under the next model would +// slide the list beneath the pointer. A row shown during this visit stays, switched off. +function withEarlierRows( + earlier: readonly SessionOptionDescriptor[], + current: readonly SessionOptionDescriptor[] +): readonly SessionOptionDescriptor[] { + const gone = earlier.filter((row) => !current.some((option) => option.id === row.id)) + return gone.length === 0 ? current : sortNativeChatSessionOptions([...current, ...gone]) +} + +function LevelSegment(props: { + optionId: string + choice: SessionOptionSelectChoice +}): React.JSX.Element { + const { optionId, choice } = props + const detailId = useId() + const label = nativeChatSessionChoiceLabel(choice, optionId) + const shortLabel = nativeChatSessionChoiceShortLabel(choice, optionId) + // What the segment's text leaves out: the full name behind an abbreviation, and the description. + const detail = [label === shortLabel ? null : label, choice.description].filter(Boolean).join(' ') + if (!detail) { + return {shortLabel} + } + return ( + + {/* Why the span: the trigger writes its own data-state, which on the segment itself would + replace the on/off the segment is styled by. */} + + + {/* Named by what it shows (WCAG 2.5.3); the rest is its description, linked here + because the tooltip's own link lands on the span, and only while it is open. */} + + {shortLabel} + + + {detail} + + + + +
{label}
+ {choice.description ?
{choice.description}
: null} +
+
+ ) +} + +function OptionRow(props: { + descriptor: SessionOptionDescriptor + /** Kept from an earlier model in this visit; the current one does not offer it. */ + gone: boolean + onSet: (value: SessionOptionValue) => void + onAction: () => void +}): React.JSX.Element { + const { descriptor, gone, onSet, onAction } = props + const labelId = useId() + const label = nativeChatSessionOptionLabel(descriptor) + const off = gone || !descriptor.settable + if (descriptor.action?.type === 'toggle-command') { + return ( + + ) + } + if (descriptor.action) { + return ( +
+ {label} + +
+ ) + } + if (descriptor.kind.type === 'boolean') { + return ( + + ) + } + const current = descriptor.kind.currentValue + return ( +
+ + {label} + + { + // Radix reports a press on the level in use as an empty value; it is still a press. + const pressed = value || current + if (pressed) { + onSet(pressed) + } + }} + // Why -ml-2: a wrapped group lines its first label up under the row's. + className="-ml-2 flex-wrap" + > + {descriptor.kind.choices.map((choice) => ( + + ))} + +
+ ) +} + +/** The current model's options, pinned under the model list. */ +export function NativeChatSessionOptionRows({ + options, + shownReason, + busy, + onSet, + onAction +}: NativeChatSessionOptionRowsProps): React.JSX.Element { + const shown = useRef(options) + const rows = withEarlierRows(shown.current, options) + // Why after commit: a render React throws away must not leave a row behind. + useLayoutEffect(() => { + shown.current = rows + }) + const frame = useRef(null) + const content = useRef(null) + // Why measured: a row arriving with the next model would otherwise jump the popover; the list + // above glides to its height the same way, from the size cmdk publishes. + useLayoutEffect(() => { + const frameElement = frame.current + const contentElement = content.current + if (!frameElement || !contentElement) { + return + } + const publish = (): void => { + frameElement.style.setProperty('--option-rows-height', `${contentElement.offsetHeight}px`) + } + publish() + const observer = new ResizeObserver(publish) + observer.observe(contentElement) + return () => observer.disconnect() + }, []) + return ( +
+
+ {rows.length > 0 ? ( +
+ + {rows.map((descriptor) => { + const reason = nativeChatSessionOptionDisabledReason(descriptor.disabledReason) + return ( +
+ option.id === descriptor.id)} + onSet={(value) => onSet(descriptor, value)} + onAction={() => onAction(descriptor)} + /> + {reason && !descriptor.settable && reason !== shownReason ? ( +
{reason}
+ ) : null} +
+ ) + })} +
+
+ ) : null} +
+
+ ) +} diff --git a/src/renderer/src/components/native-chat/NativeChatSessionOptionWriteGroup.tsx b/src/renderer/src/components/native-chat/NativeChatSessionOptionWriteGroup.tsx new file mode 100644 index 00000000000..4355a43bf45 --- /dev/null +++ b/src/renderer/src/components/native-chat/NativeChatSessionOptionWriteGroup.tsx @@ -0,0 +1,24 @@ +import { cn } from '@/lib/utils' + +/** Controls that write to the session. While another write is in flight they take nothing and say + * so, without `disabled`: that would drop keyboard focus from the control just used. The dimming + * waits out a local write, so only a slow one is seen waiting (STYLEGUIDE, in-flight feedback). */ +export function NativeChatSessionOptionWriteGroup(props: { + busy: boolean + className?: string + children: React.ReactNode +}): React.JSX.Element { + return ( +
+ {props.children} +
+ ) +} diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx index c183a7a2751..29fda770a1f 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test-harness.tsx @@ -134,6 +134,7 @@ export function createStructuredSessionMocks() { structuredTransport?: Record & { queueResume?: QueueResumeMock onError?: NativeChatStructuredComposerTransport['onError'] + optionPickerRequest?: NativeChatStructuredComposerTransport['optionPickerRequest'] } isWorking?: boolean isStopping?: boolean diff --git a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx index 41741aea8b2..2b8c9c193ab 100644 --- a/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx +++ b/src/renderer/src/components/native-chat/NativeChatStructuredSession.test.tsx @@ -681,10 +681,11 @@ describe('NativeChatStructuredSession', () => { await expect(dispatchCommand?.('/model')).resolves.toMatchObject({ accepted: true }) }) - expect(mocks.composerProps?.structuredTransport?.optionPickerRequest).toEqual({ - id: 'model', - sequence: 1 - }) + const request = mocks.composerProps?.structuredTransport?.optionPickerRequest + expect(request).toMatchObject({ id: 'model' }) + + act(() => request?.consume()) + expect(mocks.composerProps?.structuredTransport?.optionPickerRequest).toBeNull() }) it('passes Claude grouped questions and one shared answer through the card', () => { diff --git a/src/renderer/src/components/native-chat/native-chat-composer-types.ts b/src/renderer/src/components/native-chat/native-chat-composer-types.ts index fae1c3cabce..0b635fc3137 100644 --- a/src/renderer/src/components/native-chat/native-chat-composer-types.ts +++ b/src/renderer/src/components/native-chat/native-chat-composer-types.ts @@ -25,9 +25,11 @@ export type NativeChatComposerErrorDetail = { refusedWhile?: StructuredAgentSessionCommandRefusalCause } +/** One opening of the picker, asked for by a slash command. */ export type NativeChatOptionPickerRequest = { id: string - sequence: number + /** Drops the request once the picker has seen it, whether or not it could open. */ + consume: () => void } /** A queue the host holds, with cards waiting. */ 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 index 9486b8d3f4c..33a86f12336 100644 --- 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 @@ -1,22 +1,39 @@ 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`. */ +/** Radix restores focus to the trigger on close, which is right for a visit that changed nothing + * and wrong after one that did: the message is what comes next. `notePick` marks the visit as + * used; a click that took focus somewhere else keeps it. */ export function useNativeChatPickerFocusReturn(focusComposer: (() => void) | undefined): { + /** Goes on the trigger, whose own press closes the surface. */ + triggerRef: React.RefObject notePick: () => void + onInteractOutside: (event: Event) => void onCloseAutoFocus: (event: Event) => void } { + const triggerRef = useRef(null) const picked = useRef(false) + const interactedOutside = useRef(false) return { + triggerRef, notePick: () => { picked.current = true }, - onCloseAutoFocus: (event: Event) => { - if (!picked.current) { - return + onInteractOutside: (event: Event) => { + // Why: Radix reports a press on the trigger too, and that press only closes the surface. + if (!(event.target instanceof Node && triggerRef.current?.contains(event.target))) { + interactedOutside.current = true } + }, + onCloseAutoFocus: (event: Event) => { + const used = picked.current + // Read after the click that closed it has settled its own focus. + const focusedElsewhere = + interactedOutside.current && + document.activeElement !== null && + document.activeElement !== document.body picked.current = false - if (!focusComposer) { + interactedOutside.current = false + if (!used || !focusComposer || focusedElsewhere) { return } event.preventDefault() diff --git a/src/renderer/src/components/native-chat/native-chat-session-option-labels.test.ts b/src/renderer/src/components/native-chat/native-chat-session-option-labels.test.ts index 8f2005aab1b..1d3e25f4791 100644 --- a/src/renderer/src/components/native-chat/native-chat-session-option-labels.test.ts +++ b/src/renderer/src/components/native-chat/native-chat-session-option-labels.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import { translate } from '@/i18n/i18n' import { nativeChatModelPillLabel, - nativeChatOptionsPillLabel, + nativeChatOptionValuesLabel, nativeChatSessionChoiceLabel } from './native-chat-session-option-labels' import type { SessionOptionDescriptor } from '../../../../shared/native-chat-session-options' @@ -76,7 +76,7 @@ describe('nativeChatSessionChoiceLabel for speed', () => { }) }) -describe('nativeChatOptionsPillLabel', () => { +describe('nativeChatOptionValuesLabel', () => { function speedDescriptor(currentValue: string): SessionOptionDescriptor { return { id: 'serviceTier', @@ -97,7 +97,7 @@ describe('nativeChatOptionsPillLabel', () => { } it('names a faster tier and leaves Standard out, as Fast off was', () => { - expect(nativeChatOptionsPillLabel([speedDescriptor('ultrafast')])).toBe('Ultrafast') - expect(nativeChatOptionsPillLabel([speedDescriptor('default')])).toBe('Options') + expect(nativeChatOptionValuesLabel([speedDescriptor('ultrafast')])).toBe('Ultrafast') + expect(nativeChatOptionValuesLabel([speedDescriptor('default')])).toBe('') }) }) diff --git a/src/renderer/src/components/native-chat/native-chat-session-option-labels.ts b/src/renderer/src/components/native-chat/native-chat-session-option-labels.ts index 1ecb2901eb5..a4d5f9a7f7b 100644 --- a/src/renderer/src/components/native-chat/native-chat-session-option-labels.ts +++ b/src/renderer/src/components/native-chat/native-chat-session-option-labels.ts @@ -51,6 +51,33 @@ export function nativeChatSessionChoiceLabel( } } +/** What a level's segment shows: the long effort names shortened, anything else as it is. */ +export function nativeChatSessionChoiceShortLabel( + choice: SessionOptionSelectChoice, + optionId?: string +): string { + switch (choice.value) { + case 'minimal': + return translate('components.native-chat.composer.optionValueShort.minimal', 'Min') + case 'medium': + return translate('components.native-chat.composer.optionValueShort.medium', 'Med') + case 'xhigh': + return translate('components.native-chat.composer.optionValueShort.xhigh', 'XHigh') + default: + return nativeChatSessionChoiceLabel(choice, optionId) + } +} + +/** The label of a value-less action row; see `SessionOptionDescriptor.action`. */ +export function nativeChatSessionOptionActionLabel(descriptor: SessionOptionDescriptor): string { + // Why: flip-only without a baseline is an action — never claim On/Off. + return descriptor.action?.type === 'toggle-command' + ? translate('components.native-chat.composer.toggleOption', 'Toggle {{value0}}', { + value0: nativeChatSessionOptionLabel(descriptor).toLowerCase() + }) + : translate('components.native-chat.composer.chooseInAgentPicker', 'Choose in agent picker…') +} + export function nativeChatSessionOptionDisabledReason( reason: SessionOptionDisabledReason | undefined ): string | null { @@ -102,10 +129,11 @@ export function nativeChatOptionsPillTitle( : translate('components.native-chat.composer.sessionOptions', 'Session options') } -export function nativeChatOptionsPillLabel( +/** The options' values as the pill shows them after the model, e.g. `High · Fast`; empty when + * none has a value worth showing. */ +export function nativeChatOptionValuesLabel( descriptors: readonly SessionOptionDescriptor[] ): string { - const effort = descriptors.find((descriptor) => descriptor.id === 'effort') const labels: string[] = [] for (const descriptor of descriptors) { if (descriptor.valueSource === 'unknown' || isDefaultServiceTier(descriptor)) { @@ -131,12 +159,6 @@ export function nativeChatOptionsPillLabel( ) } } - // Why: value-only pill (no "Effort:" prefix) — category lives on the tooltip. - if (labels.length > 0) { - return labels.join(' · ') - } - if (effort) { - return nativeChatSessionOptionLabel(effort) - } - return translate('components.native-chat.composer.options', 'Options') + // Why: values only (no "Effort:" prefix) — the category lives on the tooltip. + return labels.join(' · ') } diff --git a/src/renderer/src/components/native-chat/native-chat-session-option-surface-call.ts b/src/renderer/src/components/native-chat/native-chat-session-option-surface-call.ts index a314a7e8ac4..b226ac9c585 100644 --- a/src/renderer/src/components/native-chat/native-chat-session-option-surface-call.ts +++ b/src/renderer/src/components/native-chat/native-chat-session-option-surface-call.ts @@ -13,13 +13,13 @@ function optionUpdateFailureDescription(error: unknown): string { return error instanceof Error ? error.message : String(error) } -/** One picker write: marks `pendingKey` until it settles and reports a failure as a toast. */ -export function runSessionOptionSurfaceCall( - pendingKey: string, - setPendingId: (id: string | null) => void, +/** One picker write: marks `pending` until it settles and reports a failure as a toast. */ +export function runSessionOptionSurfaceCall( + pending: T, + setPending: (pending: T | null) => void, call: () => Promise ): void { - setPendingId(pendingKey) + setPending(pending) void call() .catch((error) => { toast.error( @@ -27,5 +27,5 @@ export function runSessionOptionSurfaceCall( { description: optionUpdateFailureDescription(error) } ) }) - .finally(() => setPendingId(null)) + .finally(() => setPending(null)) } diff --git a/src/renderer/src/components/native-chat/use-native-chat-structured-composer-transport.ts b/src/renderer/src/components/native-chat/use-native-chat-structured-composer-transport.ts index d2a3980f06b..189fabfaa12 100644 --- a/src/renderer/src/components/native-chat/use-native-chat-structured-composer-transport.ts +++ b/src/renderer/src/components/native-chat/use-native-chat-structured-composer-transport.ts @@ -57,7 +57,7 @@ export function useNativeChatStructuredComposerTransport(args: { agent: props.agent, snapshot: controller.optionSnapshot, invokeAction: async (id) => { - setOptionPickerRequest((current) => ({ id, sequence: (current?.sequence ?? 0) + 1 })) + setOptionPickerRequest({ id, consume: () => setOptionPickerRequest(null) }) return true }, setOption: controller.setStructuredOption, diff --git a/src/renderer/src/components/ui/toggle-group.tsx b/src/renderer/src/components/ui/toggle-group.tsx index 41144a6ca90..bc45aed4c70 100644 --- a/src/renderer/src/components/ui/toggle-group.tsx +++ b/src/renderer/src/components/ui/toggle-group.tsx @@ -68,7 +68,7 @@ function ToggleGroupItem({ variant: context.variant || variant, size: context.size || size }), - 'w-auto min-w-0 shrink-0 px-3 focus:z-10 focus-visible:z-10', + 'w-auto min-w-0 shrink-0 px-3 focus:z-10 focus-visible:z-10 data-[size=xs]:px-2', 'data-[spacing=0]:rounded-none data-[spacing=0]:shadow-none data-[spacing=0]:first:rounded-l-md data-[spacing=0]:last:rounded-r-md data-[spacing=0]:data-[variant=outline]:border-l-0 data-[spacing=0]:data-[variant=outline]:first:border-l', className )} diff --git a/src/renderer/src/components/ui/toggle.tsx b/src/renderer/src/components/ui/toggle.tsx index 591668f735d..663042ad68a 100644 --- a/src/renderer/src/components/ui/toggle.tsx +++ b/src/renderer/src/components/ui/toggle.tsx @@ -11,10 +11,14 @@ const toggleVariants = cva( variant: { default: 'bg-transparent', outline: - 'border border-input bg-transparent shadow-xs hover:bg-accent hover:text-accent-foreground' + 'border border-input bg-transparent shadow-xs hover:bg-accent hover:text-accent-foreground', + // Why not bg-accent: on a light popover it is nearly the surface colour (STYLEGUIDE, list rows). + segmented: + 'bg-transparent text-muted-foreground transition-[color,background-color,box-shadow] hover:bg-foreground/6 hover:text-foreground data-[state=on]:bg-foreground/12 data-[state=on]:text-foreground dark:hover:bg-accent/50 dark:data-[state=on]:bg-accent' }, size: { default: 'h-9 min-w-9 px-2', + xs: 'h-6 min-w-6 px-2 text-xs', sm: 'h-8 min-w-8 px-1.5', lg: 'h-10 min-w-10 px-2.5' } diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index 6cbff2b3e17..1a1bc56e5f9 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -18417,7 +18417,6 @@ "fastMode": "Fast mode", "speed": "Speed", "thinking": "Thinking", - "options": "Options", "sessionOptions": "Session options", "chooseInAgentPicker": "Choose in agent picker…", "searchModels": "Search models…", @@ -18465,7 +18464,12 @@ "permissionFullAccessCodexDescription": "Never asks; can edit any file and use the network", "permissionFullAccessSharedDescription": "Never asks before edits or commands", "pasteFailed": "Paste failed.", - "commandAttachmentsUnsupported": "Remove attachments before using a chat-session command." + "commandAttachmentsUnsupported": "Remove attachments before using a chat-session command.", + "optionValueShort": { + "minimal": "Min", + "medium": "Med", + "xhigh": "XHigh" + } }, "tool": { "exitCode": "exit {{value0}}", diff --git a/src/renderer/src/i18n/locales/es.json b/src/renderer/src/i18n/locales/es.json index b6adb38b0da..32849788bc2 100644 --- a/src/renderer/src/i18n/locales/es.json +++ b/src/renderer/src/i18n/locales/es.json @@ -17945,7 +17945,6 @@ "fastMode": "Modo rápido", "speed": "Velocidad", "thinking": "Razonamiento", - "options": "Opciones", "sessionOptions": "Opciones de sesión", "chooseInAgentPicker": "Elegir en el selector del agente…", "searchModels": "Buscar modelos…", @@ -18006,7 +18005,12 @@ "permissionFullAccessCodexDescription": "Nunca pregunta; puede editar cualquier archivo y usar la red", "permissionFullAccessSharedDescription": "Nunca pregunta antes de ediciones o comandos", "pasteFailed": "Error al pegar.", - "commandAttachmentsUnsupported": "Quita los archivos adjuntos antes de usar un comando de la sesión de chat." + "commandAttachmentsUnsupported": "Quita los archivos adjuntos antes de usar un comando de la sesión de chat.", + "optionValueShort": { + "minimal": "Mín.", + "medium": "Medio", + "xhigh": "Muy alto" + } }, "tool": { "running": "Ejecutando…", diff --git a/src/renderer/src/i18n/locales/fr.json b/src/renderer/src/i18n/locales/fr.json index 7426bcf3c40..6ef9490c28b 100644 --- a/src/renderer/src/i18n/locales/fr.json +++ b/src/renderer/src/i18n/locales/fr.json @@ -18059,7 +18059,6 @@ "fastMode": "Mode rapide", "speed": "Vitesse", "thinking": "Réflexion", - "options": "Options", "sessionOptions": "Options de session", "chooseInAgentPicker": "Choisir dans le sélecteur d'agents…", "searchModels": "Rechercher des modèles…", @@ -18108,7 +18107,12 @@ "permissionFullAccessCodexDescription": "Ne demande jamais ; peut modifier n’importe quel fichier et utiliser le réseau", "permissionFullAccessSharedDescription": "Ne demande jamais avant les modifications ou les commandes", "pasteFailed": "Échec du collage.", - "commandAttachmentsUnsupported": "Retirez les pièces jointes avant d'utiliser une commande de session de chat." + "commandAttachmentsUnsupported": "Retirez les pièces jointes avant d'utiliser une commande de session de chat.", + "optionValueShort": { + "minimal": "Min.", + "medium": "Moy.", + "xhigh": "Extra" + } }, "tool": { "running": "En cours…", diff --git a/src/renderer/src/i18n/locales/ja.json b/src/renderer/src/i18n/locales/ja.json index adf7fbbcf87..285c2161d74 100644 --- a/src/renderer/src/i18n/locales/ja.json +++ b/src/renderer/src/i18n/locales/ja.json @@ -17976,7 +17976,6 @@ "fastMode": "高速モード", "speed": "速度", "thinking": "思考", - "options": "オプション", "sessionOptions": "セッションオプション", "chooseInAgentPicker": "Agent の選択画面で選ぶ…", "searchModels": "モデルを検索…", @@ -18041,7 +18040,12 @@ "permissionFullAccessCodexDescription": "確認しません。任意のファイルを編集し、ネットワークを使用できます", "permissionFullAccessSharedDescription": "編集やコマンドの前に確認しません", "pasteFailed": "貼り付けに失敗しました。", - "commandAttachmentsUnsupported": "チャットセッションのコマンドを使う前に添付ファイルを削除してください。" + "commandAttachmentsUnsupported": "チャットセッションのコマンドを使う前に添付ファイルを削除してください。", + "optionValueShort": { + "minimal": "最小", + "medium": "中", + "xhigh": "最高" + } }, "tool": { "running": "実行中…", diff --git a/src/renderer/src/i18n/locales/ko.json b/src/renderer/src/i18n/locales/ko.json index 8e00cafbf8f..3739271a7d2 100644 --- a/src/renderer/src/i18n/locales/ko.json +++ b/src/renderer/src/i18n/locales/ko.json @@ -18006,7 +18006,6 @@ "fastMode": "빠른 모드", "speed": "속도", "thinking": "사고", - "options": "옵션", "sessionOptions": "세션 옵션", "chooseInAgentPicker": "에이전트 선택기에서 선택…", "searchModels": "모델 검색…", @@ -18071,7 +18070,12 @@ "permissionFullAccessCodexDescription": "묻지 않습니다. 모든 파일을 편집하고 네트워크를 사용할 수 있습니다", "permissionFullAccessSharedDescription": "편집이나 명령 전에 묻지 않습니다", "pasteFailed": "붙여넣기 실패.", - "commandAttachmentsUnsupported": "채팅 세션 명령을 사용하기 전에 첨부 파일을 제거하세요." + "commandAttachmentsUnsupported": "채팅 세션 명령을 사용하기 전에 첨부 파일을 제거하세요.", + "optionValueShort": { + "minimal": "최소", + "medium": "중간", + "xhigh": "매우 높음" + } }, "tool": { "running": "실행 중…", diff --git a/src/renderer/src/i18n/locales/zh.json b/src/renderer/src/i18n/locales/zh.json index 840b4a44b34..ea66a6b6899 100644 --- a/src/renderer/src/i18n/locales/zh.json +++ b/src/renderer/src/i18n/locales/zh.json @@ -17955,7 +17955,6 @@ "fastMode": "快速模式", "speed": "速度", "thinking": "思考", - "options": "选项", "sessionOptions": "会话选项", "chooseInAgentPicker": "在智能体选择器中选择…", "searchModels": "搜索模型…", @@ -18020,7 +18019,12 @@ "permissionFullAccessCodexDescription": "从不询问;可以编辑任何文件并使用网络", "permissionFullAccessSharedDescription": "编辑或运行命令前从不询问", "pasteFailed": "粘贴失败。", - "commandAttachmentsUnsupported": "使用聊天会话命令前,请先移除附件。" + "commandAttachmentsUnsupported": "使用聊天会话命令前,请先移除附件。", + "optionValueShort": { + "minimal": "最小", + "medium": "中", + "xhigh": "极高" + } }, "tool": { "running": "正在运行…", diff --git a/src/renderer/src/i18n/native-chat-locales.test.ts b/src/renderer/src/i18n/native-chat-locales.test.ts index a10f7e8fd97..e8652e66a5f 100644 --- a/src/renderer/src/i18n/native-chat-locales.test.ts +++ b/src/renderer/src/i18n/native-chat-locales.test.ts @@ -47,7 +47,6 @@ describe('native chat locale copy', () => { 'effort', 'fastMode', 'thinking', - 'options', 'sessionOptions', 'chooseInAgentPicker', 'toggleOption', @@ -60,6 +59,10 @@ describe('native chat locale copy', () => { expect(composer.optionValue[key].trim()).not.toBe('') expect(composer.optionValue[key]).not.toBe(englishComposer.optionValue[key]) } + for (const key of ['minimal', 'medium', 'xhigh'] as const) { + expect(composer.optionValueShort[key].trim()).not.toBe('') + expect(composer.optionValueShort[key]).not.toBe(englishComposer.optionValueShort[key]) + } } ) }) diff --git a/tests/e2e/omp-model-picker.spec.ts b/tests/e2e/omp-model-picker.spec.ts index f0255e4ff8f..0302d007299 100644 --- a/tests/e2e/omp-model-picker.spec.ts +++ b/tests/e2e/omp-model-picker.spec.ts @@ -75,14 +75,12 @@ test('OMP model picker dispatches its advertised command and adopts the host rep const picker = orcaPage.getByRole('button', { name: 'Model Sonnet 4.5', exact: true }) await expect(picker).toBeVisible() await picker.click() - await expect( - orcaPage.getByRole('menuitemradio', { name: 'Sonnet 4.6', exact: true }) - ).toBeVisible() + await expect(orcaPage.getByRole('option', { name: 'Sonnet 4.6', exact: true })).toBeVisible() await orcaPage.screenshot({ animations: 'disabled', path: testInfo.outputPath('omp-model-choices.png') }) - await orcaPage.getByRole('menuitemradio', { name: 'Sonnet 4.6', exact: true }).click() + await orcaPage.getByRole('option', { name: 'Sonnet 4.6', exact: true }).click() await expect .poll(async () => readFile(output, 'utf8').catch(() => '')) .toContain('/orca-model anthropic/claude-sonnet-4-6')