From a4f74fe724f5f1c5fbc429d8217cd626dbe8b9f1 Mon Sep 17 00:00:00 2001 From: Mark Sagi-Kazar Date: Wed, 2 Sep 2026 06:57:53 +0000 Subject: [PATCH] Mark background content inert while a modal dialog is open MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A modal `DialogContent` installs a focus trap that only intercepts Tab, so everything behind the dialog stayed reachable: by a pointer, by a programmatic `focus()` from application code, by a background element that autofocuses on mount, and — depending on the screen reader — by a browse mode cursor. The trap also binds to its own container, so once focus leaves by any non-Tab route no handler fires and it is never brought back. `aria-modal="true"` is set, but support for it has been inconsistent enough that the WAI-ARIA Authoring Practices and MDN both recommend pairing it with `inert` on background content. While the trap is installed, walk from the dialog up to `` and mark each ancestor's other children `inert`. Each dialog attributes its marks to an owner id in a `data-inert-by` marker, so `inert` the application had already set — which carries no marker — is left alone. The whole state is recomputed from the set of open dialogs on every open and close, which is what makes stacking compose: the dialog installed last is on top, so nothing on its path to `` is marked, and every dialog underneath still marks its own background and so ends up inert itself. The unwind is addressed by owner id rather than by a recorded element list, so it also works when the dialog is unmounted while open. The owner id comes from Rust rather than being derived from the element id, so that marking and unwinding cannot disagree about it, and so that a caller supplied id containing a space cannot corrupt the space separated marker. Also fix focus restoration: `FocusTrap.remove()` focused the element that was active when the trap was built without checking it was still in the document. If the opener was unmounted while the dialog was open — routine when the dialog's action re-renders the view behind it — focus fell to `` and the keyboard position was lost. It now falls back to the main landmark, made focusable only for as long as it holds focus. `DialogRoot` and `AlertDialogRoot` take an `inert_background` prop (default true) for callers who manage `inert` themselves. The focus trap effect moves to a shared `use_focus_trap` hook so dialog and alert dialog compose by construction. Claude-Session: https://claude.ai/code/session_018qMUbzVT7pawv31DChxknW --- playwright/alert-dialog.spec.ts | 28 +++ playwright/dialog.spec.ts | 118 +++++++++++++ .../src/components/alert_dialog/component.rs | 1 + preview/src/components/dialog/component.rs | 1 + preview/src/components/sheet/component.rs | 1 + primitives/src/alert_dialog.rs | 36 ++-- primitives/src/dialog.rs | 49 +++--- primitives/src/js/focus-trap.js | 2 +- primitives/src/js/hash.txt | 2 +- primitives/src/lib.rs | 51 ++++++ primitives/src/ts/focus-trap.ts | 159 +++++++++++++++++- 11 files changed, 400 insertions(+), 48 deletions(-) diff --git a/playwright/alert-dialog.spec.ts b/playwright/alert-dialog.spec.ts index 7e464be74..92b713c3c 100644 --- a/playwright/alert-dialog.spec.ts +++ b/playwright/alert-dialog.spec.ts @@ -29,3 +29,31 @@ test('test', async ({ page }) => { // Assert the dialog is closed after confirming await expect(dialog).toHaveCount(0); }); + +test('alert dialog marks background content inert', async ({ page }) => { + await page.goto('http://127.0.0.1:8080/component/?name=alert_dialog&', { timeout: 20 * 60 * 1000 }); // Increase timeout to 20 minutes + + // Is the trigger (which sits behind the dialog) inside an inert subtree? + const triggerIsInert = () => page.evaluate(() => { + const trigger = Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Show Alert Dialog' + ); + return trigger ? trigger.closest('[inert]') !== null : null; + }); + + const trigger = page.getByRole('button', { name: 'Show Alert Dialog' }); + await expect(trigger).toBeVisible(); + await expect.poll(triggerIsInert).toBe(false); + await trigger.click(); + await expect(page.getByRole('alertdialog')).toBeVisible(); + + await expect.poll(triggerIsInert).toBe(true); + await expect(page.locator('[data-inert-by]').first()).toBeAttached(); + + await page.keyboard.press('Escape'); + await expect(page.getByRole('alertdialog')).toHaveCount(0); + + await expect.poll(triggerIsInert).toBe(false); + await expect(page.locator('[data-inert-by]')).toHaveCount(0); + await expect(trigger).toBeFocused(); +}); diff --git a/playwright/dialog.spec.ts b/playwright/dialog.spec.ts index 77d288f75..808f8b96a 100644 --- a/playwright/dialog.spec.ts +++ b/playwright/dialog.spec.ts @@ -33,3 +33,121 @@ test('test', async ({ page }) => { await page.mouse.click(2, 2); await expect(dialog).toHaveCount(0); }); + +test('modal dialog marks background content inert', async ({ page }) => { + await page.goto('http://127.0.0.1:8080/component/?name=dialog&', { timeout: 20 * 60 * 1000 }); // Increase timeout to 20 minutes + + // Is the trigger (which sits behind the dialog) inside an inert subtree? + const triggerIsInert = () => page.evaluate(() => { + const trigger = Array.from(document.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Show Dialog' + ); + return trigger ? trigger.closest('[inert]') !== null : null; + }); + + const trigger = page.getByRole('button', { name: 'Show Dialog' }); + await expect(trigger).toBeVisible(); + await expect.poll(triggerIsInert).toBe(false); + await trigger.click(); + await expect(page.getByRole('dialog')).toBeVisible(); + + // Background content is inert while the modal is open, and every element that was made + // inert records the dialog that did it. + await expect.poll(triggerIsInert).toBe(true); + await expect(page.locator('[data-inert-by]').first()).toBeAttached(); + // The dialog itself stays interactive. + await expect(page.getByRole('dialog')).not.toHaveAttribute('inert', ''); + + await page.keyboard.press('Escape'); + await expect(page.getByRole('dialog')).toHaveCount(0); + + // Closing unwinds everything it marked and returns focus to the opener. + await expect.poll(triggerIsInert).toBe(false); + await expect(page.locator('[data-inert-by]')).toHaveCount(0); + await expect(trigger).toBeFocused(); +}); + +// The stacking rules live in the focus trap module rather than in any one component, and two +// dialogs open at once has no demo to drive. Exercise the shipped bundle directly: the dialog +// page loads it, and `createFocusTrap` is the same entry point the primitive calls. +test('stacked focus traps compose and unwind independently', async ({ page }) => { + await page.goto('http://127.0.0.1:8080/component/?name=dialog&', { timeout: 20 * 60 * 1000 }); // Increase timeout to 20 minutes + await page.waitForFunction(() => typeof (window as any).createFocusTrap === 'function'); + + const result = await page.evaluate(() => { + // + //