mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-24 16:54:43 +00:00
feat(desktop): add keyboard navigation to clarify choices (#69799)
Co-authored-by: Mapurite <272619650+mapu-og@users.noreply.github.com>
This commit is contained in:
brooklyn!
GitHub
parent
328e4f5a1e
commit
6096f73ce8
2 changed files with 227 additions and 15 deletions
|
|
@ -1,15 +1,28 @@
|
|||
import type { ToolCallMessagePartProps } from '@assistant-ui/react'
|
||||
import { cleanup, fireEvent, render, screen } from '@testing-library/react'
|
||||
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
|
||||
import type { ReactNode } from 'react'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { onComposerInsertRequest } from '@/app/chat/composer/focus'
|
||||
import { I18nProvider } from '@/i18n'
|
||||
import { clearClarifyRequest, setClarifyRequest } from '@/store/clarify'
|
||||
import { $gateway } from '@/store/gateway'
|
||||
import { $activeSessionId } from '@/store/session'
|
||||
|
||||
import { ClarifyTool, readClarifyResult } from './clarify-tool'
|
||||
|
||||
// The live pending card only renders while its message is running. Force that so
|
||||
// keyboard-navigation tests can exercise ClarifyToolPending directly.
|
||||
vi.mock('@assistant-ui/react', () => ({
|
||||
useAuiState: () => true
|
||||
}))
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
clearClarifyRequest()
|
||||
$activeSessionId.set(null)
|
||||
$gateway.set(null)
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
function renderClarify(ui: ReactNode) {
|
||||
|
|
@ -40,6 +53,40 @@ function settledClarifyProps(
|
|||
}
|
||||
}
|
||||
|
||||
function liveClarifyProps(choices = ['staging', 'production']): ToolCallMessagePartProps {
|
||||
const args = { choices, question: 'Which deployment target?' }
|
||||
|
||||
return {
|
||||
addResult: vi.fn(),
|
||||
args,
|
||||
argsText: JSON.stringify(args),
|
||||
isError: false,
|
||||
respondToApproval: vi.fn(),
|
||||
result: undefined,
|
||||
resume: vi.fn(),
|
||||
status: { type: 'running' },
|
||||
toolCallId: 'clarify-live',
|
||||
toolName: 'clarify',
|
||||
type: 'tool-call'
|
||||
}
|
||||
}
|
||||
|
||||
function renderLiveClarify() {
|
||||
const request = vi.fn().mockResolvedValue({ ok: true })
|
||||
|
||||
$activeSessionId.set('session-1')
|
||||
$gateway.set({ request } as never)
|
||||
setClarifyRequest({
|
||||
choices: ['staging', 'production'],
|
||||
question: 'Which deployment target?',
|
||||
requestId: 'request-1',
|
||||
sessionId: 'session-1'
|
||||
})
|
||||
renderClarify(<ClarifyTool {...liveClarifyProps()} />)
|
||||
|
||||
return request
|
||||
}
|
||||
|
||||
describe('readClarifyResult', () => {
|
||||
it('reads question + user_response from the tool JSON payload', () => {
|
||||
expect(
|
||||
|
|
@ -182,3 +229,70 @@ describe('ClarifyTool settled view', () => {
|
|||
expect(document.querySelector('[data-clarify-late-choices]')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
describe('ClarifyTool keyboard navigation', () => {
|
||||
it('cycles through choices and Other with the arrow keys', () => {
|
||||
renderLiveClarify()
|
||||
|
||||
const staging = screen.getByRole('button', { name: /staging/ })
|
||||
const production = screen.getByRole('button', { name: /production/ })
|
||||
const other = screen.getByPlaceholderText(/Other/)
|
||||
|
||||
expect(staging.getAttribute('data-highlighted')).toBe('true')
|
||||
expect(staging.getAttribute('aria-current')).toBe('true')
|
||||
expect(staging.getAttribute('aria-keyshortcuts')).toBe('A 1')
|
||||
|
||||
fireEvent.keyDown(window, { key: 'ArrowDown' })
|
||||
expect(production.getAttribute('data-highlighted')).toBe('true')
|
||||
|
||||
fireEvent.keyDown(window, { key: 'ArrowDown' })
|
||||
expect(other.closest('label')?.getAttribute('data-highlighted')).toBe('true')
|
||||
expect(other.getAttribute('aria-current')).toBe('true')
|
||||
expect(other.getAttribute('aria-keyshortcuts')).toBe('C 3')
|
||||
|
||||
fireEvent.keyDown(window, { key: 'ArrowDown' })
|
||||
expect(staging.getAttribute('data-highlighted')).toBe('true')
|
||||
|
||||
fireEvent.keyDown(window, { key: 'ArrowUp' })
|
||||
expect(other.closest('label')?.getAttribute('data-highlighted')).toBe('true')
|
||||
})
|
||||
|
||||
it('selects by number and confirms the answer with Enter', async () => {
|
||||
const request = renderLiveClarify()
|
||||
|
||||
fireEvent.keyDown(window, { key: '2' })
|
||||
fireEvent.keyDown(window, { key: 'Enter' })
|
||||
|
||||
await waitFor(() => {
|
||||
expect(request).toHaveBeenCalledWith('clarify.respond', {
|
||||
answer: 'production',
|
||||
request_id: 'request-1'
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
it('focuses Other when its number is pressed and leaves typing keys alone', () => {
|
||||
renderLiveClarify()
|
||||
|
||||
const other = screen.getByPlaceholderText(/Other/)
|
||||
|
||||
fireEvent.keyDown(window, { key: '3' })
|
||||
expect(document.activeElement).toBe(other)
|
||||
|
||||
fireEvent.change(other, { target: { value: 'canary' } })
|
||||
fireEvent.keyDown(window, { key: 'ArrowUp' })
|
||||
expect(document.activeElement).toBe(other)
|
||||
expect((other as HTMLTextAreaElement).value).toBe('canary')
|
||||
})
|
||||
|
||||
it('does not intercept keyboard events while an action button has focus', () => {
|
||||
const request = renderLiveClarify()
|
||||
const skip = screen.getByRole('button', { name: 'Skip' })
|
||||
|
||||
skip.focus()
|
||||
|
||||
expect(fireEvent.keyDown(window, { key: 'Enter' })).toBe(true)
|
||||
expect(fireEvent.keyDown(window, { key: 'ArrowDown' })).toBe(true)
|
||||
expect(request).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -138,16 +138,20 @@ function KeyBadge({ char, preview, selected }: { char: string; preview?: boolean
|
|||
* selects an answer) and the settled skip card (where a click drafts a
|
||||
* follow-up), so both stay visually identical. */
|
||||
function ChoiceButton({
|
||||
active = false,
|
||||
char,
|
||||
choice,
|
||||
disabled,
|
||||
keyShortcuts,
|
||||
onClick,
|
||||
selected = false,
|
||||
title
|
||||
}: {
|
||||
active?: boolean
|
||||
char: string
|
||||
choice: string
|
||||
disabled?: boolean
|
||||
keyShortcuts?: string
|
||||
onClick: () => void
|
||||
selected?: boolean
|
||||
title?: string
|
||||
|
|
@ -156,20 +160,28 @@ function ChoiceButton({
|
|||
// tooltip on a <button> is banned by the no-native-title guard). It renders
|
||||
// the child untouched when `label` is falsy, so the live card (no tip) is
|
||||
// unaffected and only the settled skip card gets the hover hint.
|
||||
//
|
||||
// `active` is the keyboard cursor on the live card (arrow-key navigation);
|
||||
// it highlights the row and previews its key badge. The settled skip card
|
||||
// never passes it, so its rows stay plain.
|
||||
return (
|
||||
<Tip label={title}>
|
||||
<button
|
||||
aria-current={active || undefined}
|
||||
aria-keyshortcuts={keyShortcuts}
|
||||
className={cn(
|
||||
OPTION_ROW_CLASS,
|
||||
'text-(--ui-text-secondary) hover:bg-(--chrome-action-hover) hover:text-(--ui-text-primary)',
|
||||
active && 'bg-(--chrome-action-hover) text-(--ui-text-primary)',
|
||||
selected && 'text-(--ui-text-primary)'
|
||||
)}
|
||||
data-choice
|
||||
data-highlighted={active || undefined}
|
||||
disabled={disabled}
|
||||
onClick={onClick}
|
||||
type="button"
|
||||
>
|
||||
<KeyBadge char={char} selected={selected} />
|
||||
<KeyBadge char={char} preview={active} selected={selected} />
|
||||
<span className="flex-1 wrap-anywhere">{choice}</span>
|
||||
</button>
|
||||
</Tip>
|
||||
|
|
@ -298,6 +310,9 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
|
|||
const [draft, setDraft] = useState('')
|
||||
const [submitting, setSubmitting] = useState(false)
|
||||
const [selectedChoice, setSelectedChoice] = useState<string | null>(null)
|
||||
// The keyboard cursor. Indices 0..choices.length-1 are the options; the
|
||||
// trailing index (=== choices.length) is the "Other" free-text row.
|
||||
const [activeIndex, setActiveIndex] = useState(0)
|
||||
const [otherFocused, setOtherFocused] = useState(false)
|
||||
const textareaRef = useRef<HTMLTextAreaElement | null>(null)
|
||||
|
||||
|
|
@ -346,12 +361,31 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
|
|||
// confirms with Continue (or Enter from the field).
|
||||
const pendingAnswer = selectedChoice ?? (trimmedDraft || null)
|
||||
|
||||
const selectChoice = useCallback((choice: string) => {
|
||||
const selectChoice = useCallback((choice: string, index: number) => {
|
||||
// Picking a choice and typing are mutually exclusive answers.
|
||||
setDraft('')
|
||||
setSelectedChoice(choice)
|
||||
setActiveIndex(index)
|
||||
}, [])
|
||||
|
||||
// Keep the cursor in range when the choice set changes (never past "Other").
|
||||
useEffect(() => {
|
||||
setActiveIndex(index => Math.min(index, choices.length))
|
||||
}, [choices.length])
|
||||
|
||||
const moveActive = useCallback(
|
||||
(delta: number) => {
|
||||
const itemCount = choices.length + 1
|
||||
|
||||
// Arrow navigation is a move, not a pick — clear any staged answer so the
|
||||
// cursor and the selection can't disagree.
|
||||
setDraft('')
|
||||
setSelectedChoice(null)
|
||||
setActiveIndex(index => (index + delta + itemCount) % itemCount)
|
||||
},
|
||||
[choices.length]
|
||||
)
|
||||
|
||||
const submitAnswer = useCallback(() => {
|
||||
if (selectedChoice !== null) {
|
||||
void respond(selectedChoice)
|
||||
|
|
@ -364,6 +398,27 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
|
|||
}
|
||||
}, [respond, selectedChoice, trimmedDraft])
|
||||
|
||||
const activateActive = useCallback(() => {
|
||||
// A staged answer (picked choice or typed text) wins — confirm it.
|
||||
if (pendingAnswer) {
|
||||
submitAnswer()
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// Otherwise act on the highlighted row: a choice responds immediately, and
|
||||
// the trailing "Other" row focuses the free-text field.
|
||||
const choice = choices[activeIndex]
|
||||
|
||||
if (choice) {
|
||||
void respond(choice)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
textareaRef.current?.focus()
|
||||
}, [activeIndex, choices, pendingAnswer, respond, submitAnswer])
|
||||
|
||||
const handleTextareaKey = useCallback(
|
||||
(event: KeyboardEvent<HTMLTextAreaElement>) => {
|
||||
if (event.nativeEvent.isComposing) {
|
||||
|
|
@ -386,10 +441,11 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
|
|||
[submitAnswer]
|
||||
)
|
||||
|
||||
// Letter shortcuts: A/B/C… pick the matching option, the trailing letter jumps
|
||||
// into "Other", and Enter confirms the current pick. Stands down whenever a
|
||||
// field is focused (you're typing, not navigating) so it never eats keystrokes
|
||||
// meant for the composer or the Other box.
|
||||
// Arrow keys move a visual cursor, 1-9 and A/B/C… pick directly, and Enter
|
||||
// confirms the current answer (or acts on the highlighted row). Stands down
|
||||
// whenever a focusable control (a field, a choice button, the action bar) is
|
||||
// focused, so it never eats keystrokes meant for the composer, the Other box,
|
||||
// or a button the user tabbed to.
|
||||
useEffect(() => {
|
||||
if (!ready || !hasChoices || submitting) {
|
||||
return
|
||||
|
|
@ -402,7 +458,32 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
|
|||
|
||||
const active = document.activeElement as HTMLElement | null
|
||||
|
||||
if (active && (active.tagName === 'INPUT' || active.tagName === 'TEXTAREA' || active.isContentEditable)) {
|
||||
if (
|
||||
active &&
|
||||
(active.isContentEditable || active.matches('a[href], button, input, select, textarea, [role="button"]'))
|
||||
) {
|
||||
return
|
||||
}
|
||||
|
||||
if (event.key === 'ArrowDown' || event.key === 'ArrowUp') {
|
||||
event.preventDefault()
|
||||
moveActive(event.key === 'ArrowDown' ? 1 : -1)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if (/^[1-9]$/.test(event.key)) {
|
||||
const index = Number(event.key) - 1
|
||||
|
||||
if (index < choices.length) {
|
||||
event.preventDefault()
|
||||
selectChoice(choices[index], index)
|
||||
} else if (index === choices.length) {
|
||||
event.preventDefault()
|
||||
setActiveIndex(index)
|
||||
textareaRef.current?.focus()
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -413,25 +494,26 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
|
|||
|
||||
if (index < choices.length) {
|
||||
event.preventDefault()
|
||||
selectChoice(choices[index])
|
||||
selectChoice(choices[index], index)
|
||||
} else if (index === choices.length) {
|
||||
event.preventDefault()
|
||||
setActiveIndex(index)
|
||||
textareaRef.current?.focus()
|
||||
}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
if (event.key === 'Enter' && pendingAnswer) {
|
||||
if (event.key === 'Enter') {
|
||||
event.preventDefault()
|
||||
submitAnswer()
|
||||
activateActive()
|
||||
}
|
||||
}
|
||||
|
||||
window.addEventListener('keydown', onKeyDown)
|
||||
|
||||
return () => window.removeEventListener('keydown', onKeyDown)
|
||||
}, [choices, hasChoices, pendingAnswer, ready, selectChoice, submitAnswer, submitting])
|
||||
}, [activateActive, choices, hasChoices, moveActive, ready, selectChoice, submitting])
|
||||
|
||||
if (loading) {
|
||||
return (
|
||||
|
|
@ -467,23 +549,39 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
|
|||
<div className="grid gap-px" role="group">
|
||||
{choices.map((choice, index) => (
|
||||
<ChoiceButton
|
||||
active={activeIndex === index}
|
||||
char={letterFor(index)}
|
||||
choice={choice}
|
||||
disabled={submitting}
|
||||
key={`${index}-${choice}`}
|
||||
onClick={() => selectChoice(choice)}
|
||||
keyShortcuts={`${letterFor(index)} ${index + 1}`}
|
||||
onClick={() => selectChoice(choice, index)}
|
||||
selected={selectedChoice === choice}
|
||||
/>
|
||||
))}
|
||||
<label className={cn(OPTION_ROW_CLASS, 'items-center')}>
|
||||
<KeyBadge char={letterFor(choices.length)} preview={otherFocused} selected={Boolean(trimmedDraft)} />
|
||||
<label
|
||||
className={cn(
|
||||
OPTION_ROW_CLASS,
|
||||
'items-center',
|
||||
activeIndex === choices.length && 'bg-(--chrome-action-hover)'
|
||||
)}
|
||||
data-highlighted={activeIndex === choices.length || undefined}
|
||||
>
|
||||
<KeyBadge
|
||||
char={letterFor(choices.length)}
|
||||
preview={otherFocused || activeIndex === choices.length}
|
||||
selected={Boolean(trimmedDraft)}
|
||||
/>
|
||||
<Textarea
|
||||
aria-current={activeIndex === choices.length || undefined}
|
||||
aria-keyshortcuts={`${letterFor(choices.length)} ${choices.length + 1}`}
|
||||
className={CLARIFY_TEXTAREA_CLASS}
|
||||
disabled={submitting}
|
||||
onBlur={() => setOtherFocused(false)}
|
||||
onChange={event => onDraftChange(event.target.value)}
|
||||
onFocus={() => {
|
||||
setSelectedChoice(null)
|
||||
setActiveIndex(choices.length)
|
||||
setOtherFocused(true)
|
||||
}}
|
||||
onKeyDown={handleTextareaKey}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue