fix(ui): keep focus in settings sheets when the page re-renders

The chat settings sheet, the Talk session sheet and the agent and task
preview sheets set up their focus handling in an effect that depended on
onClose. Every parent passes onClose as an inline arrow, so each re-render
of the page gave it a new identity: the effect cleanup sent focus back to
the opener and the effect ran again and focused the Close button.

Any edit in a sheet re-renders the page, so focus left the control being
used. In the chat sheet, typing in the system prompt moved focus to Close
after the first key, and a following space activated Close and dismissed
the sheet; an open native dropdown in the sheet closed by itself.

Keep the latest onClose in a ref and run the focus effect once per open.
Escape still calls the current onClose; initial focus on Close, the Tab
trap and restoring focus to the opener are unchanged.

The e2e specs type key by key (or switch preview tabs) and assert that
focus stays put; they fail without this change.

Docs-Not-Needed: bug fix, no user-facing behaviour change beyond the fix
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Stefan Walcz <stefan.walcz@walcz.de>
This commit is contained in:
Stefan Walcz committed 2026-10-21 15:40:43 +02:00
1 parent 7f7b10fba9
commit 773cae7259
8 files changed
+79 -8

No files matched your search

@@ -161,7 +161,9 @@ test.describe('Edit a task', () => {
await page.getByTestId('task-preview-open').click()
const sheet = page.getByTestId('task-preview')
await expect(sheet.getByTestId('task-preview-prompt')).toContainText('Summarise overnight news about local inference in bullet points format. Keep it to 8 items.')
// Switching tabs re-renders the page; focus stays on the tab.
await sheet.getByRole('tab', { name: /Changes/ }).click()
await expect(sheet.getByRole('tab', { name: /Changes/ })).toBeFocused()
const changes = sheet.getByTestId('task-preview-changes')
await expect(changes.locator('[data-key="cron"]')).toContainText('0 7 * * *')
await expect(changes.locator('[data-key="cron"]')).toContainText('0 8 * * *')
@@ -60,7 +60,9 @@ test.describe('Create and edit an agent', () => {
expect(cfg).not.toContain('sk-secret-value')
expect(cfg).not.toContain('tok-123')
// Switching tabs re-renders the page; focus stays on the tab.
await sheet.getByRole('tab', { name: /Changes/ }).click()
await expect(sheet.getByRole('tab', { name: /Changes/ })).toBeFocused()
const changes = sheet.getByTestId('agent-preview-changes')
await expect(changes.locator('[data-key]')).toHaveCount(1)
await expect(changes.locator('[data-key="system_prompt"]')).toContainText('You are a careful research assistant')
@@ -80,6 +80,36 @@ test.describe('Chat settings sheet', () => {
expect(answer.request().messages[0]).toEqual({ role: 'system', content: 'Answer in one line.' })
})
// Every edit in the sheet re-renders the chat page, which hands the sheet a
// new onClose. Focus must stay where the user put it: before the fix each
// re-render sent focus to the opener and then to the Close button, so a
// second keystroke went nowhere and an open native dropdown closed itself.
test('keeps focus on the control being edited while the page re-renders', async ({ page }) => {
await openChat(page, [chatOf('c1', 'Settings', 'qwen3-8b', pair('hi', 'hello'))])
await open(page)
const prompt = sheet(page).getByTestId('chat-system-prompt')
await prompt.click()
await page.keyboard.type('Be brief.', { delay: 20 })
await expect(prompt).toBeFocused()
await expect(prompt).toHaveValue('Be brief.')
await expect.poll(async () => (await storedChats(page)).chats[0].systemPrompt).toBe('Be brief.')
const topK = sheet(page).locator('.cx-field', { hasText: 'Top K' }).getByRole('slider')
await topK.focus()
await page.keyboard.press('ArrowRight')
await page.keyboard.press('ArrowRight')
await expect(topK).toBeFocused()
await expect.poll(async () => (await storedChats(page)).chats[0].topK).toBe(42)
// Escape still reaches the current onClose, and focus goes back to the
// button that opened the sheet.
await expect(sheet(page)).toBeVisible()
await page.keyboard.press('Escape')
await expect(sheet(page)).toHaveCount(0)
await expect(page.getByTestId('chat-settings-button')).toBeFocused()
})
test('the context size has quick sizes, drives the header meter and is not sent', async ({ page }) => {
const answer = await answerWith(page, 'ok')
await openChat(page, [chatOf('c1', 'Settings', 'qwen3-8b', pair('hi', 'hello'), { tokenUsage: { prompt: 1000, completion: 96, total: 1096 } })])
+17
View File
@@ -263,6 +263,23 @@ test.describe('Talk session settings', () => {
expect(update.session.audio.input.transcription.language).toBe('it')
})
// Each keystroke re-renders the Talk page, which hands the sheet a new
// onClose. Focus has to stay in the field rather than jump to Close.
test('typing in the sheet keeps focus in the field while the page re-renders', async ({ page }) => {
await fakeRealtime(page)
await page.goto('/app/talk')
await page.getByTestId('talk-settings-button').click()
const sheet = page.getByTestId('talk-settings')
const instructions = sheet.getByRole('textbox', { name: 'Instructions' })
await instructions.fill('')
await page.keyboard.type('Be brief.', { delay: 20 })
await expect(instructions).toBeFocused()
await expect(instructions).toHaveValue('Be brief.')
await page.keyboard.press('Escape')
await expect(sheet).toHaveCount(0)
await expect(page.getByTestId('talk-settings-button')).toBeFocused()
})
test('Manage mode is fixed while a session is open', async ({ page }) => {
await fakeRealtime(page)
await startSession(page)
@@ -64,12 +64,17 @@ export default function ChatSettingsSheet({
const sheetRef = useRef(null)
const closeRef = useRef(null)
const openerRef = useRef(null)
// The page passes a fresh onClose on every render. Read it through a ref so
// the effect below runs once per open: re-running it would move focus to
// the opener and back to Close, pulling it out of the control being edited.
const onCloseRef = useRef(onClose)
useEffect(() => { onCloseRef.current = onClose })
useEffect(() => {
openerRef.current = document.activeElement
closeRef.current?.focus()
const onKey = (e) => {
if (e.key === 'Escape') { e.preventDefault(); e.stopPropagation(); onClose(); return }
if (e.key === 'Escape') { e.preventDefault(); e.stopPropagation(); onCloseRef.current(); return }
if (e.key !== 'Tab' || !sheetRef.current) return
const focusable = Array.from(sheetRef.current.querySelectorAll('button:not([disabled]), input:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])'))
if (focusable.length === 0) return
@@ -84,7 +89,7 @@ export default function ChatSettingsSheet({
const el = openerRef.current
if (el && document.contains(el)) el.focus?.()
}
}, [onClose])
}, [])
const set = (patch) => onUpdate(patch)
const used = chat.tokenUsage?.total || 0
@@ -19,12 +19,17 @@ export default function SessionSheet({
const sheetRef = useRef(null)
const closeRef = useRef(null)
const openerRef = useRef(null)
// The parent passes a fresh onClose on every render. Read it through a ref
// so the effect below runs once per open: re-running it would move focus to
// the opener and back to Close, pulling it out of the control in use.
const onCloseRef = useRef(onClose)
useEffect(() => { onCloseRef.current = onClose })
useEffect(() => {
openerRef.current = document.activeElement
closeRef.current?.focus()
const onKey = (e) => {
if (e.key === 'Escape') { e.preventDefault(); e.stopPropagation(); onClose(); return }
if (e.key === 'Escape') { e.preventDefault(); e.stopPropagation(); onCloseRef.current(); return }
if (e.key !== 'Tab' || !sheetRef.current) return
const focusable = Array.from(sheetRef.current.querySelectorAll('button:not([disabled]), input:not([disabled]), textarea:not([disabled]), [tabindex]:not([tabindex="-1"])'))
if (focusable.length === 0) return
@@ -39,7 +44,7 @@ export default function SessionSheet({
const el = openerRef.current
if (el && document.contains(el)) el.focus?.()
}
}, [onClose])
}, [])
const slots = pipeline && !pipeline.self_contained
? [
+7 -2
View File
@@ -1128,12 +1128,17 @@ function PreviewSheet({ tab, onTab, payload, diff, isEdit, saving, onSave, onClo
const sheetRef = useRef(null)
const closeRef = useRef(null)
const openerRef = useRef(null)
// The parent passes a fresh onClose on every render. Read it through a ref
// so the effect below runs once per open: re-running it would move focus to
// the opener and back to Close, pulling it out of the control in use.
const onCloseRef = useRef(onClose)
useEffect(() => { onCloseRef.current = onClose })
useEffect(() => {
openerRef.current = document.activeElement
closeRef.current?.focus()
const onKey = (e) => {
if (e.key === 'Escape') { e.preventDefault(); e.stopPropagation(); onClose(); return }
if (e.key === 'Escape') { e.preventDefault(); e.stopPropagation(); onCloseRef.current(); return }
if (e.key !== 'Tab' || !sheetRef.current) return
const focusable = Array.from(sheetRef.current.querySelectorAll('button:not([disabled]), [tabindex]:not([tabindex="-1"])'))
if (focusable.length === 0) return
@@ -1148,7 +1153,7 @@ function PreviewSheet({ tab, onTab, payload, diff, isEdit, saving, onSave, onClo
const el = openerRef.current
if (el && document.contains(el)) el.focus?.()
}
}, [onClose])
}, [])
const shown = useCallback((v) => displayValue(maskSecrets(v)), [])
const json = useMemo(() => JSON.stringify(maskSecrets(payload), null, 2), [payload])
@@ -723,12 +723,17 @@ function PreviewSheet({ tab, onTab, task, params, diff, isNew, saving, onSave, o
const { t } = useTranslation('agents')
const sheetRef = useRef(null)
const closeRef = useRef(null)
// The parent passes a fresh onClose on every render. Read it through a ref
// so the effect below runs once per open: re-running it would move focus to
// the opener and back to Close, pulling it out of the control in use.
const onCloseRef = useRef(onClose)
useEffect(() => { onCloseRef.current = onClose })
useEffect(() => {
const opener = document.activeElement
closeRef.current?.focus()
const onKey = (e) => {
if (e.key === 'Escape') { e.preventDefault(); e.stopPropagation(); onClose(); return }
if (e.key === 'Escape') { e.preventDefault(); e.stopPropagation(); onCloseRef.current(); return }
if (e.key !== 'Tab' || !sheetRef.current) return
const focusable = Array.from(sheetRef.current.querySelectorAll('button:not([disabled]), [tabindex]:not([tabindex="-1"])'))
if (focusable.length === 0) return
@@ -742,7 +747,7 @@ function PreviewSheet({ tab, onTab, task, params, diff, isNew, saving, onSave, o
window.removeEventListener('keydown', onKey, true)
if (opener && document.contains(opener)) opener.focus?.()
}
}, [onClose])
}, [])
return createPortal(
<div className="dk-sheet-veil" data-state="open" onMouseDown={onClose}>