diff --git a/addons/talk/ui/src/components/talk/SessionSheet.jsx b/addons/talk/ui/src/components/talk/SessionSheet.jsx index 97f31f7b1..b714b428e 100644 --- a/addons/talk/ui/src/components/talk/SessionSheet.jsx +++ b/addons/talk/ui/src/components/talk/SessionSheet.jsx @@ -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 ? [ diff --git a/core/http/react-ui/e2e/agent-jobs-task.spec.js b/core/http/react-ui/e2e/agent-jobs-task.spec.js index 6fb9793f1..dba88c189 100644 --- a/core/http/react-ui/e2e/agent-jobs-task.spec.js +++ b/core/http/react-ui/e2e/agent-jobs-task.spec.js @@ -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 * * *') diff --git a/core/http/react-ui/e2e/agents-edit.spec.js b/core/http/react-ui/e2e/agents-edit.spec.js index 60192d454..47c1058d7 100644 --- a/core/http/react-ui/e2e/agents-edit.spec.js +++ b/core/http/react-ui/e2e/agents-edit.spec.js @@ -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') diff --git a/core/http/react-ui/e2e/chat-settings.spec.js b/core/http/react-ui/e2e/chat-settings.spec.js index fd9a78a46..900c0f583 100644 --- a/core/http/react-ui/e2e/chat-settings.spec.js +++ b/core/http/react-ui/e2e/chat-settings.spec.js @@ -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 } })]) diff --git a/core/http/react-ui/e2e/talk.spec.js b/core/http/react-ui/e2e/talk.spec.js index 540bbb743..073eaddc3 100644 --- a/core/http/react-ui/e2e/talk.spec.js +++ b/core/http/react-ui/e2e/talk.spec.js @@ -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) diff --git a/core/http/react-ui/src/components/chat/ChatSettingsSheet.jsx b/core/http/react-ui/src/components/chat/ChatSettingsSheet.jsx index e39b2099f..fc76600e2 100644 --- a/core/http/react-ui/src/components/chat/ChatSettingsSheet.jsx +++ b/core/http/react-ui/src/components/chat/ChatSettingsSheet.jsx @@ -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 diff --git a/core/http/react-ui/src/pages/AgentCreate.jsx b/core/http/react-ui/src/pages/AgentCreate.jsx index 617b1d6fc..f962f1bcb 100644 --- a/core/http/react-ui/src/pages/AgentCreate.jsx +++ b/core/http/react-ui/src/pages/AgentCreate.jsx @@ -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]) diff --git a/core/http/react-ui/src/pages/AgentTaskDetails.jsx b/core/http/react-ui/src/pages/AgentTaskDetails.jsx index 01545d815..9053f633c 100644 --- a/core/http/react-ui/src/pages/AgentTaskDetails.jsx +++ b/core/http/react-ui/src/pages/AgentTaskDetails.jsx @@ -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(