From cbbee09edaf3b6cc5290b9d626e2a02359e1bd28 Mon Sep 17 00:00:00 2001 From: Sergey Kozyrenko Date: Sun, 26 Jul 2026 18:11:00 +0700 Subject: [PATCH] fix(a11y): hand focus back to whatever opened a dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing any dialog left focus on , so a keyboard user restarted from the top of the page. Radix restores focus to its own DialogTrigger, and nearly every dialog here is a controlled `` opened from state — and the page unmounts the content before Radix's close sequence runs, so its `onCloseAutoFocus` never fires at all (verified by instrumenting the handler: it never ran). The shared DialogContent now remembers the element that had focus when it rendered and restores it when it unmounts, if that element is still on the page. Captured during the first render because by the time effects run focus is already inside the dialog. Co-Authored-By: Claude Opus 4.8 --- frontend/e2e/specs/cross/a11y.spec.ts | 25 +++++++++++++++++++++++++ frontend/src/components/ui/dialog.tsx | 16 ++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/frontend/e2e/specs/cross/a11y.spec.ts b/frontend/e2e/specs/cross/a11y.spec.ts index dbb7b6ae..1a3fbdad 100644 --- a/frontend/e2e/specs/cross/a11y.spec.ts +++ b/frontend/e2e/specs/cross/a11y.spec.ts @@ -1,5 +1,7 @@ import { expect, test } from '../../fixtures/test.ts'; import { scanA11y, waiversForScan } from '../../helpers/a11y.ts'; +import { expectCleanPage } from '../../helpers/errors.ts'; +import { resourcesCassette } from '../../mocks/cassettes/resources.ts'; import { populatedSettingsProvidersCassette } from '../../mocks/cassettes/settings-providers.ts'; import { loginJourneyCassette } from '../../mocks/cassettes/smoke.ts'; import { ROUTE_MANIFEST } from '../../routes.ts'; @@ -69,3 +71,26 @@ for (const theme of THEMES) { } }); } + +test.describe('dialog keyboard contract', { tag: '@cross' }, () => { + test.use({ cassette: resourcesCassette() }); + + // Closing a dialog must hand focus back to whatever opened it. Radix only does that for its own + // DialogTrigger, and this app opens dialogs from controlled state, so without help focus lands on + // and a keyboard user starts again from the top of the page. + test('Escape returns focus to the control that opened the dialog', async ({ page, pageErrorLog }) => { + await page.goto('/resources'); + + const opener = page.getByRole('button', { name: 'New folder' }); + + await opener.focus(); + await page.keyboard.press('Enter'); + await expect(page.getByRole('dialog')).toBeVisible(); + + await page.keyboard.press('Escape'); + await expect(page.getByRole('dialog')).toBeHidden(); + + await expect(opener).toBeFocused(); + expectCleanPage(pageErrorLog); + }); +}); diff --git a/frontend/src/components/ui/dialog.tsx b/frontend/src/components/ui/dialog.tsx index b0915fe5..ef6dd1d3 100644 --- a/frontend/src/components/ui/dialog.tsx +++ b/frontend/src/components/ui/dialog.tsx @@ -23,6 +23,22 @@ function DialogClose({ ...props }: React.ComponentProps) { + // Radix hands focus back to its own DialogTrigger, and almost every dialog here is a controlled + // `` with no trigger — and its content is unmounted by the page before Radix's + // close sequence runs at all, so `onCloseAutoFocus` never fires. Without this, closing a dialog + // drops focus on and a keyboard user restarts from the top of the page. Captured during + // the first render, because by the time effects run focus is already inside the dialog. + const [opener] = React.useState(() => document.activeElement as HTMLElement | null); + + React.useEffect( + () => () => { + if (opener?.isConnected) { + opener.focus(); + } + }, + [opener], + ); + return (