diff --git a/.changes/menu-keyboard.json b/.changes/menu-keyboard.json new file mode 100644 index 00000000..07f15898 --- /dev/null +++ b/.changes/menu-keyboard.json @@ -0,0 +1 @@ +{ "type": "user-facing", "releaseNoteId": "release:0.18.1" } diff --git a/CHANGELOG.md b/CHANGELOG.md index 5179fb4f..b5ee3921 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ All notable Loop Studio releases, newest first. Behavioral changes are pinned in versioned spec documents (see the [README](README.md#technical-reference)); this file is the narrative history, not the contract. +## v0.18.1 — 2026-10-06 + +A fix release (issue #307): the menus answer the keyboard the same way, and the phone's sheets take focus and step back one level at a time. + +- **Every desktop menu button opens, moves and closes alike**: Templates, Insert module, File, Data, Help, the temporary-session chip, Theme and the Distribution export. A keyboard open puts focus on the first item, and Arrow Down or Arrow Up on a closed button opens it at the first or last item. Inside, the arrows wrap, Home and End jump, and disabled items are skipped. Escape returns to the button; Tab and Shift+Tab close the menu and move on. After a keyboard choice that opens no dialog, focus returns to the button; a dialog returns it there when it closes. Pointer use is unchanged. +- **Settings and the `⋯` button are disclosures**, not menus: Tab moves through them, and Escape closes them and returns to their button. Theme's three choices are announced as a choice of one. Language keeps its search field, and Arrow Down or Arrow Up on its closed button opens it at the first or last language. +- **On a phone, each sheet takes focus when it opens.** Escape in a sheet opened from More goes back to More, on the row that opened it; Close still closes everything. A sheet stays non-modal: the run bar and the update bar are still usable while it is open, by touch, keyboard and screen reader, and only what the sheet covers leaves the keyboard order. A dialog opened from a sheet returns focus to the row that opened it. +- **A dialog is modal for real, on every screen**: while one is open (the Monte Carlo dialog, a confirmation, About, the guided tour), nothing outside it can be reached by pointer, keyboard or screen reader. The update notice waits behind it and is back, unchanged, the moment it closes; while only a sheet is open it stays usable. + +**No migration.** Three release-note lines in 18 languages, 16 of them without native review. The informational `meta.tool` string is now `loop-studio/0.18.1`. + ## v0.18.0 — 2026-10-05 The third-party open-source licenses, inside the app (issue #301). diff --git a/README.md b/README.md index 0dc8eac6..94ca1e99 100644 --- a/README.md +++ b/README.md @@ -140,7 +140,17 @@ Additional feature-specific design documents (localization, mobile, module system, large-graph readability, simulation playback, edge routing, data import, …) live under [`docs/`](docs/). -## Latest — v0.18.0 +## Latest — v0.18.1 + +A fix release: the menus answer the keyboard the same way. + +- **Every menu button** opens at its first item from the keyboard, moves with the arrows, + Home and End, and closes with Escape or Tab, back to its button +- **Settings and `⋯`** are disclosures you Tab through; Theme is a choice of one +- **On a phone**, a sheet takes focus when it opens, and Escape in a sheet opened from + More goes back to More; the run bar and the update bar stay usable + +## v0.18.0 The third-party open-source licenses, inside the app. @@ -167,15 +177,9 @@ A fix release: share links use the browser's own compression. unchanged - **A browser without them** makes no link and says so, and the open diagram is kept -## v0.17.0 - -- **Password-protected share links** — an optional password encrypts the diagram inside - the link, in the browser; the password is asked for before anything from the diagram is - shown, and a lost password cannot be recovered. A plain link is still the default - -See [`CHANGELOG.md`](CHANGELOG.md) for the full notes of these releases, v0.16.0 (the -storage gate, temporary sessions and the Storage and privacy area), the v0.15 releases and -every earlier one. +See [`CHANGELOG.md`](CHANGELOG.md) for the full notes of these releases, v0.17.0 +(password-protected share links), v0.16.0 (the storage gate, temporary sessions and the +Storage and privacy area), the v0.15 releases and every earlier one. ## Credits diff --git a/docs/guided-tour.md b/docs/guided-tour.md index 9a49e24f..b5f182a7 100644 --- a/docs/guided-tour.md +++ b/docs/guided-tour.md @@ -240,9 +240,11 @@ is resolved once: page session (`offeredThisSession`), so a re-render, route change, or a surface opening/closing after the check changes nothing. 4. **Z-order** (§GT4): the tour / Welcome layer is above the Canvas / Toolbar / - Timeline but **below** `ConfirmDialog` — a confirm can always appear over the - tour and take focus. (They should not coexist, but the ordering is fixed - regardless.) + Timeline and the PWA update bar but **below** `ConfirmDialog` — a confirm can + always appear over the tour and take focus. (They should not coexist, but the + ordering is fixed regardless.) The tour's card is a real modal dialog: while + it is open everything outside it is `inert`, the update bar included + (issue #307, docs/mobile.md §MV8a). Manual entry via `Help → Take a tour` (§GT7) has **no** timing gate — the user asked for it — beyond the normal focus handoff. diff --git a/docs/mobile.md b/docs/mobile.md index 045c5894..038b6b6b 100644 --- a/docs/mobile.md +++ b/docs/mobile.md @@ -252,6 +252,22 @@ Both are bottom sheets. **Shared sheet contract:** `role="dialog"` + `aria-label`; - a visible **Close** button (44 px), **Escape** closes, and focus **returns to the trigger** on close; focus moves into the sheet on open; +- **a sheet is not modal** (issue #307): no `aria-modal` and no Tab trap, + because the run bar and the PWA update bar are used while a sheet is open + (MV-D11, MV-D18, MV-D19). They stay reachable by pointer, keyboard and screen + reader, and Tab follows the page's own order out of the sheet. Only what the + sheet's scrim covers for the pointer (the canvas, the top bar's `More` + button, the open-file card) is `inert` while it is open, so the keyboard and + a screen reader reach the same controls as a finger. A real dialog opened + over a sheet is modal like every real dialog: it traps Tab and everything + else, the sheet, the run bar and the PWA update bar included, is `inert` + until it closes (MV8a); focus then returns to the sheet row that opened it, + and the sheet is non-modal again. Only the top dialog or sheet answers + Escape (`src/ui/overlayStack.ts`); +- **Escape in a sheet opened from `More`** (Templates, Export, Filters, Help, + Share) goes back one level: it closes that sheet, reopens `More` and puts + focus on the row that opened it. Close and the scrim still close + everything; - `env(safe-area-inset-bottom)` padding; max-height leaves the top bar visible; the sheet body scrolls internally (`overflow-y: auto`; `overscroll-behavior: contain`); @@ -397,15 +413,24 @@ opening. Its mobile placement rules: full-width minus the left/right safe-area insets. This keeps it away from the crowded bottom edge (run bar + rising sheets) entirely, so it can collide with neither. -- **Z-index — above everything**, so its two 44 px buttons (**Update** / - **Dismiss**) stay reachable even while an exclusive sheet or the MC dialog is - open: `--z-canvas < --z-runbar < --z-sheet <= --z-mc-dialog < --z-pwa-update`. - Bottom sheets open to at most ~55 vh and the MC dialog is centred with a - safe-area top margin, so the top-anchored bar and a sheet **do not overlap** - in practice; the z-order is the guarantee if they ever do. +- **Usable while a sheet is open; behind a real dialog (issue #307).** The + update notice can be used while a sheet is open. While a real modal dialog + is open (the MC dialog, a confirmation, About, the guided tour, any + `aria-modal` dialog) it is covered and inactive, and the moment that dialog + closes it is back in the state it was in. So its two 44 px buttons + (**Update** / **Dismiss**) sit above every sheet and the run bar but below + the tour and the dialogs: + `--z-sheet < --z-runbar < --z-pwa-update < --z-tour < --z-mc-dialog`, and + while a dialog is open everything outside it, the bar included, is `inert` + (`src/ui/overlayStack.ts`), so the pointer, Tab and a screen reader cannot + reach it either. Nothing about the waiting update is dropped: the bar is + still mounted behind the dialog's scrim. Bottom sheets open to at most + ~55 vh, so the top-anchored bar and a sheet **do not overlap** in practice; + the z-order is the guarantee if they ever do. - **The dialog layer (2026-10-02, issue #296).** The order in the stylesheet is - `canvas < open-file card < sheet < run bar < dialogs < PWA update bar`. Two - things did not follow it and were measured before being fixed: + `canvas < open-file card < sheet < run bar < PWA update bar < tour < dialogs` + (the update bar was above the dialogs until issue #307). Two things did not + follow it and were measured before being fixed: - A dialog declared INSIDE a sheet (About, the contextual-tips dialog, the export-author dialog) was drawn in the sheet's layer, so its own z-index only competed with the sheet's other children. The run bar and the @@ -460,7 +485,7 @@ opening. Its mobile placement rules: | MV-D16a | opening files | no account / cloud sync (MV6a). `More` → `Import file` accepts Graph **and** Workspace JSON; a `#g1=` Share link is the other path. An **"Open a file"** card with the "No account sync" copy sits on the pristine first screen and clears once a document loads | | MV-D17 | viewport height | `100dvh` with `100vh` fallback everywhere (MV4a); a `visualViewport` listener nudges the bottom bar if a browser lags; the fixed bottom bar stays on-screen through iOS address-bar / keyboard height changes | | MV-D18 | PWA update bar | **not** in the exclusive set (MV8a) — a pending update never closes a sheet and a sheet never blocks it | -| MV-D19 | PWA update bar placement | fixed at the **top**, below the top bar (`top: calc(topbar + safe-area)`); **highest z-index** (`canvas < runbar < sheet <= mc-dialog < pwa-update`) so Update/Dismiss stay clickable with a sheet open; Canvas top padding grows by its height; can only ever occlude canvas | +| MV-D19 | PWA update bar placement | fixed at the **top**, below the top bar (`top: calc(topbar + safe-area)`); z-index above every sheet and the run bar, below the tour and the dialogs (`sheet < runbar < pwa-update < tour < mc-dialog`), so Update/Dismiss stay clickable with a sheet open and are covered and inert while a real dialog is open (issue #307); Canvas top padding grows by its height; can only ever occlude canvas | ## MV10. Required E2E @@ -497,7 +522,7 @@ A dedicated **`mobile` Playwright project** — `devices['iPhone 13']`, run at - with the update bar **and** a sheet open at once (open the Inspector while the bar shows): the sheet's **Close**, the bar's **Update**, and the run bar's **Play** are each fully visible and independently clickable — none is occluded - by another (the bar carries the highest z-index, MV8a); + by another (the bar is above every sheet and the run bar, MV8a); - the Share result URL field: open `More` → Share, the selectable URL field's rect is fully within the viewport and the text is selectable. diff --git a/docs/toolbar-responsive.md b/docs/toolbar-responsive.md index fd23898b..e609492d 100644 --- a/docs/toolbar-responsive.md +++ b/docs/toolbar-responsive.md @@ -233,3 +233,57 @@ browser: the attribute at the first animation frame for every stored value, desktop and mobile; no light or blank frame painted before a dark start, on the browser's own screencast, including a throttled network; the choice survives a reload; the boot script is in the page once. + +## The menu keyboard contract (issue #307) + +One contract for every desktop menu button, in `src/ui/useMenuKeyboard.ts` +(`useMenuTrigger` + `useMenuKeyboard`): Templates, Insert module, File, Data, +Help, the temporary-session chip, Settings → Theme and the Distribution +panel's export. It follows the WAI-ARIA menu-button pattern, with typeahead +left out. + +- **Opening.** Enter, Space or a screen reader's activation opens the menu + at its first item; Arrow Down on the closed trigger opens it at the first + item, Arrow Up at the last. A pointer open leaves focus on the trigger. +- **Inside.** Arrow Down / Arrow Up move between items and wrap; Home and End + go to the first and last. Disabled items, hidden items and separators are + skipped (`usableItems`). +- **Leaving.** Escape closes the menu and returns focus to its trigger. Tab + and Shift+Tab close it and move to the element after or before the + trigger; Tab never moves between items. The popup is gone before Tab's + default action runs (`flushSync`), so focus cannot land inside it. +- **After a choice.** A keyboard choice that opens no dialog returns focus to + the trigger; one that opens a dialog leaves focus to the dialog, which + returns it to the trigger when it closes. A pointer choice never pulls + focus (`useReturnFocusAfterKeyboardChoice`). +- **Theme** offers one of three, so its items are `menuitemradio` with + `aria-checked`. +- **`aria-controls` only while the panel exists.** A popup mounts on open, + so Settings, `⋯`, File, the temporary-session chip and Theme name it in + `aria-controls` only while it is open. MEASURED with Narrator and Edge on a + comparison page: a button whose `aria-controls` named a panel absent while + closed was never read as "expanded" after opening, even on a re-read; with + the attribute only while the panel exists, "expanded" and "collapsed" were + both read at once. + +Three controls are deliberately not menu buttons: + +- **Settings and `⋯` are disclosures**: no `role="menu"`, the trigger + carries `aria-expanded` and `aria-controls`, the panel is a labelled + `role="group"`, Tab moves through it, opening leaves focus on the trigger, + and Escape inside closes it and returns to the trigger. A keyboard choice + in a group nested inside `⋯` still returns focus to `⋯`. +- **Language keeps its combobox**: real focus stays in the search field + (`aria-activedescendant`); Arrow Down on the closed trigger opens it at the + first language, Arrow Up at the last, Enter or Space at the current one. +- **Help** keeps the behaviour it had since issue #306 and now shares the + hook. + +The phone's sheets are not modal; the open dialogs and sheets are ordered for +Escape in `src/ui/overlayStack.ts`: see docs/mobile.md §MV5. Every real +dialog (`aria-modal="true"`, at any width) makes everything outside it +`inert` while it is the top one, the PWA update bar included, which sits +behind the dialog layer meanwhile (docs/mobile.md §MV8a). + +`e2e/menu-keyboard.spec.ts` runs the same contract on every menu above, the +disclosures, the Language combobox and the phone's sheets. diff --git a/e2e/data-import-guide.spec.ts b/e2e/data-import-guide.spec.ts index 3b0c745c..2d631381 100644 --- a/e2e/data-import-guide.spec.ts +++ b/e2e/data-import-guide.spec.ts @@ -414,8 +414,8 @@ test('a 2-table import with a lookup-only table reports "0 (lookup only)" and th test('keyboard only: the quick start, fields, role selects and buttons are reachable in order; Escape returns focus to the Data trigger', async ({ page }) => { await dataButton(page).focus() - await page.keyboard.press('Enter') // opens the Data menu - await page.keyboard.press('Tab') // the first menu item follows the trigger in DOM order + await page.keyboard.press('Enter') // opens the Data menu at its first item (issue #307) + await expect(page.getByRole('menuitem').first()).toBeFocused() await page.keyboard.press('Enter') await expect(dialog(page)).toBeVisible() const active = () => page.evaluate(() => { diff --git a/e2e/menu-keyboard.spec.ts b/e2e/menu-keyboard.spec.ts new file mode 100644 index 00000000..791ac406 --- /dev/null +++ b/e2e/menu-keyboard.spec.ts @@ -0,0 +1,569 @@ +import type { Locator, Page } from '@playwright/test' +import { expect, importGraph, openApp, readRiskyFactory, runMc, test } from './support/loop' + +// Issue #307 — one keyboard contract for every menu, measured on every menu +// (src/ui/useMenuKeyboard.ts states it): +// +// - Enter / Space open with focus on the first item; a pointer open leaves +// focus on the button +// - Arrow Down / Up on the CLOSED button open at the first / last item +// - inside: Arrow Down / Up step and wrap, Home / End jump, separators and +// disabled items are never landed on +// - Escape closes the menu once and returns focus to its button +// - Tab / Shift+Tab close the menu and move on from its button +// - a keyboard choice that opens no dialog returns focus to the button; one +// that opens a dialog gives the dialog focus, and closing it returns here +// +// Settings and the toolbar overflow are disclosures, not menus; Language is a +// combobox; the phone's sheets are dialogs, one active layer at a time. + +const ITEM = '[role="menuitem"], [role="menuitemradio"], [role="menuitemcheckbox"]' + +/** the popup that belongs to a trigger: inside the trigger's own wrapper */ +const popOf = (trigger: Locator): Locator => trigger.locator('xpath=..').locator('[role="menu"]').first() + +/** which usable item has focus (-1 = none), and how many there are */ +const activeItem = (pop: Locator) => + pop.evaluate((p, sel) => { + const items = [...p.querySelectorAll(sel)].filter((e) => e.offsetParent !== null && !(e as HTMLButtonElement).disabled && e.getAttribute('aria-disabled') !== 'true') + return { index: items.indexOf(document.activeElement as HTMLElement), count: items.length, role: document.activeElement?.getAttribute('role') ?? null } + }, ITEM) + +/** mark the element Tab (or Shift+Tab) reaches from the CLOSED trigger */ +async function markNeighbour(page: Page, trigger: Locator, key: 'Tab' | 'Shift+Tab', mark: string): Promise { + await trigger.focus() + await page.keyboard.press(key) + await page.evaluate((m) => document.activeElement?.setAttribute(m, ''), mark) +} +const markedIsFocused = (page: Page, mark: string) => page.evaluate((m) => document.activeElement?.hasAttribute(m) ?? false, mark) + +async function expectMenuContract(page: Page, trigger: Locator, prepare: () => Promise = async () => {}): Promise { + await prepare() + const pop = popOf(trigger) + await markNeighbour(page, trigger, 'Tab', 'data-e2e-next') + await prepare() + await markNeighbour(page, trigger, 'Shift+Tab', 'data-e2e-prev') + await prepare() + + // Enter: open at the first item; the keys inside + await trigger.focus() + await page.keyboard.press('Enter') + await expect(pop).toBeVisible() + await expect(trigger).toHaveAttribute('aria-expanded', 'true') + await expect.poll(() => activeItem(pop).then((a) => a.index)).toBe(0) + const { count } = await activeItem(pop) + expect(count).toBeGreaterThan(1) + await page.keyboard.press('ArrowDown') + expect((await activeItem(pop)).index).toBe(1) + await page.keyboard.press('ArrowUp') + expect((await activeItem(pop)).index).toBe(0) + await page.keyboard.press('ArrowUp') // wraps to the last + expect((await activeItem(pop)).index).toBe(count - 1) + await page.keyboard.press('ArrowDown') // wraps to the first + expect((await activeItem(pop)).index).toBe(0) + await page.keyboard.press('End') + expect((await activeItem(pop)).index).toBe(count - 1) + await page.keyboard.press('Home') + expect((await activeItem(pop)).index).toBe(0) + // Escape: closed, focus on the button + await page.keyboard.press('Escape') + await expect(pop).toHaveCount(0) + await expect(trigger).toBeFocused() + + // Space: the same entry + await page.keyboard.press(' ') + await expect.poll(() => activeItem(pop).then((a) => a.index)).toBe(0) + await page.keyboard.press('Escape') + await expect(trigger).toBeFocused() + + // the closed button: Arrow Down at the first item, Arrow Up at the last + await page.keyboard.press('ArrowDown') + await expect.poll(() => activeItem(pop).then((a) => a.index)).toBe(0) + await page.keyboard.press('Escape') + await page.keyboard.press('ArrowUp') + await expect.poll(() => activeItem(pop).then((a) => a.index)).toBe(count - 1) + await page.keyboard.press('Escape') + await expect(trigger).toBeFocused() + + // Tab / Shift+Tab: the menu closes and focus moves on from the BUTTON + await page.keyboard.press('Enter') + await expect.poll(() => activeItem(pop).then((a) => a.index)).toBe(0) + await page.keyboard.press('ArrowDown') + await page.keyboard.press('Tab') + await expect(pop).toHaveCount(0) + expect(await markedIsFocused(page, 'data-e2e-next')).toBe(true) + await prepare() + await trigger.focus() + await page.keyboard.press('Enter') + await expect.poll(() => activeItem(pop).then((a) => a.index)).toBe(0) + await page.keyboard.press('Shift+Tab') + await expect(pop).toHaveCount(0) + expect(await markedIsFocused(page, 'data-e2e-prev')).toBe(true) + + // a pointer open leaves focus on the button + await prepare() + await trigger.click() + await expect(pop).toBeVisible() + await expect(trigger).toBeFocused() + await page.keyboard.press('Escape') + await expect(pop).toHaveCount(0) + await page.evaluate(() => + document.querySelectorAll('[data-e2e-next], [data-e2e-prev]').forEach((e) => { + e.removeAttribute('data-e2e-next') + e.removeAttribute('data-e2e-prev') + }), + ) +} + +const toolbarButton = (page: Page, name: string) => page.locator('.toolbar__actions .menu > button', { hasText: name }).first() + +test.describe('every menu keeps the one keyboard contract', () => { + test('Templates', async ({ page }) => { + await openApp(page) + await expectMenuContract(page, page.locator('.toolbar__actions-core > .menu > .btn').first()) + }) + test('Insert module', async ({ page }) => { + await openApp(page) + await expectMenuContract(page, toolbarButton(page, 'Insert module')) + }) + test('File, with its separator never landed on', async ({ page }) => { + await openApp(page) + await expectMenuContract(page, toolbarButton(page, 'File')) + }) + test('Data', async ({ page }) => { + await openApp(page) + await expectMenuContract(page, toolbarButton(page, 'Data')) + }) + test('Help, as #306 made it, with its separators never landed on', async ({ page }) => { + await openApp(page) + await expectMenuContract(page, page.locator('[data-tour="help-trigger"]')) + }) + test('Theme, inside Settings: radio items, and Escape closes it alone', async ({ page }) => { + await openApp(page) + const settings = page.locator('.toolbar__settingsmenu > button') + const openSettings = async () => { + if ((await settings.getAttribute('aria-expanded')) !== 'true') await settings.click() + await expect(page.locator('.toolbar__settingsmenu-pop')).toBeVisible() + } + const theme = page.locator('.theme-menu > button') + await expectMenuContract(page, theme, openSettings) + await openSettings() + await theme.focus() + await page.keyboard.press('Enter') + const radios = page.locator('.theme-menu__pop [role="menuitemradio"]') + await expect(radios).toHaveCount(3) + await expect(page.locator('.theme-menu__pop [role="menuitemradio"][aria-checked="true"]')).toHaveCount(1) + await page.keyboard.press('Escape') + await expect(page.locator('.theme-menu__pop')).toHaveCount(0) + await expect(page.locator('.toolbar__settingsmenu-pop')).toBeVisible() + await expect(theme).toBeFocused() + }) + test('the Distribution panel’s export menu', async ({ page }) => { + await openApp(page) + await importGraph(page, readRiskyFactory()) + await runMc(page, { runs: 20, steps: 10 }) + const trigger = page.locator('.dist .timeline__csv[aria-haspopup="true"]') + await expect(trigger).toBeEnabled() + await expectMenuContract(page, trigger) + }) +}) + +test.describe('the temporary session’s menu', () => { + // Reached the way a person reaches it, from Settings → Storage and privacy. + // A profile that STARTS temporary reads no stored key, so the first-run tour + // would open over the toolbar and take the clicks. + const toTemporary = async (page: Page) => { + await openApp(page) + await page.locator('.toolbar__settingsmenu > button').click() + await page.locator('[data-settings-row="storage-privacy"]').click() + const dlg = page.locator('.mcdlg--storage') + await dlg.locator('[data-storage-action="to-temporary"]').click() + await dlg.locator('[data-storage-confirm]').click() + await expect(dlg).toHaveCount(0) + await expect(page.locator('[data-session-chip="temporary"]')).toBeVisible() + } + test('keeps the contract too', async ({ page }) => { + await toTemporary(page) + await expectMenuContract(page, page.locator('[data-session-chip="temporary"]')) + }) + test('a download chosen from the keyboard returns focus to the button', async ({ page }) => { + await toTemporary(page) + const chip = page.locator('[data-session-chip="temporary"]') + await chip.focus() + await page.keyboard.press('Enter') + await expect(page.locator('.session-chip__pop [role="menuitem"]').first()).toBeFocused() + await page.keyboard.press('Enter') // Export the diagram as a file + await expect(chip).toBeFocused() + }) +}) + +test.describe('where focus goes after a choice', () => { + test('an insert chosen from the keyboard returns focus to the button; chosen with the pointer, focus is left as it was', async ({ page }) => { + await openApp(page) + const trigger = toolbarButton(page, 'Insert module') + await trigger.focus() + await page.keyboard.press('Enter') + await expect(popOf(trigger).locator('[role="menuitem"]').first()).toBeFocused() + await page.keyboard.press('Enter') + await expect(trigger).toBeFocused() + // the pointer: open and choose by mouse - focus is not pulled to the button + await trigger.click() + await popOf(trigger).locator('[role="menuitem"]').first().click() + await expect(popOf(trigger)).toHaveCount(0) + await expect(trigger).not.toBeFocused() + }) + test('an outside link chosen from the keyboard returns focus to Help', async ({ page, context }) => { + await openApp(page) + await context.route(/^https?:\/\/(?!localhost)/, (r) => r.abort()) + const help = page.locator('[data-tour="help-trigger"]') + await help.focus() + await page.keyboard.press('Enter') + await page.keyboard.press('End') + await page.keyboard.press('ArrowUp') // Send feedback, before About + await expect(page.locator('a[role="menuitem"]')).toBeFocused() + const popup = context.waitForEvent('page') + await page.keyboard.press('Enter') + await (await popup).close() + await expect(help).toBeFocused() + }) + test('an item that opens a dialog gives it focus, and closing it returns to the button', async ({ page }) => { + await openApp(page) + const help = page.locator('[data-tour="help-trigger"]') + await help.focus() + await page.keyboard.press('Enter') + await page.keyboard.press('ArrowDown') // Turn contextual tips back on + await page.keyboard.press('Enter') + const dlg = page.locator('.mcdlg--contextual-help') + await expect(dlg).toBeVisible() + expect(await dlg.evaluate((d) => d.contains(document.activeElement))).toBe(true) + await page.keyboard.press('Escape') + await expect(dlg).toHaveCount(0) + await expect(help).toBeFocused() + }) + test('the Templates confirmation returns to Templates', async ({ page }) => { + await openApp(page) + const trigger = page.locator('.toolbar__actions-core > .menu > .btn').first() + await trigger.focus() + await page.keyboard.press('Enter') + await page.keyboard.press('Enter') // the first Template; the sample document is not empty + const confirm = page.locator('.mcdlg--confirm') + await expect(confirm).toBeVisible() + await page.keyboard.press('Escape') + await expect(confirm).toHaveCount(0) + await expect(trigger).toBeFocused() + }) +}) + +test.describe('Settings and the toolbar overflow are disclosures', () => { + test('Settings: no menu role, focus stays on the button, Tab goes in, Escape comes back', async ({ page }) => { + await openApp(page) + const btn = page.locator('.toolbar__settingsmenu > button') + await expect(btn).not.toHaveAttribute('aria-haspopup', /.+/) + await btn.focus() + await page.keyboard.press('Enter') + const pop = page.locator('.toolbar__settingsmenu-pop') + await expect(pop).toBeVisible() + await expect(btn).toHaveAttribute('aria-expanded', 'true') + await expect(btn).toHaveAttribute('aria-controls', (await pop.getAttribute('id'))!) + await expect(pop).toHaveAttribute('role', 'group') + await expect(pop.locator('[role="menuitem"]')).toHaveCount(0) + await expect(btn).toBeFocused() + await page.keyboard.press('ArrowDown') // nothing: Tab moves inside + await expect(btn).toBeFocused() + await page.keyboard.press('Tab') + await expect(page.locator('.theme-menu > button')).toBeFocused() + await page.keyboard.press('Tab') + await page.keyboard.press('Tab') + await expect(page.locator('[data-settings-row="storage-privacy"]')).toBeFocused() + await page.keyboard.press('Escape') + await expect(pop).toHaveCount(0) + await expect(btn).toBeFocused() + }) +}) + +test.describe('the toolbar overflow, at a width where it holds groups', () => { + test.use({ viewport: { width: 760, height: 800 } }) + test('a disclosure; a keyboard choice made in a group inside it returns to its button', async ({ page, context }) => { + await openApp(page) + await context.route(/^https?:\/\/(?!localhost)/, (r) => r.abort()) + const more = page.locator('.toolbar__overflow-btn') + await expect(more).toBeVisible() + await expect(more).not.toHaveAttribute('aria-haspopup', /.+/) + await more.focus() + await page.keyboard.press('Enter') + const pop = page.locator('.toolbar__overflow-pop') + await expect(pop).toHaveAttribute('role', 'group') + await expect(more).toBeFocused() + // a menu inside it: Help sits here at 760 px (MEASURED: Data, Settings, Help) + const help = pop.locator('[data-tour="help-trigger"]') + await expect(help).toHaveCount(1) + await help.focus() + await page.keyboard.press('Enter') + await page.keyboard.press('End') + await page.keyboard.press('ArrowUp') + const popup = context.waitForEvent('page') + await page.keyboard.press('Enter') // Send feedback + await (await popup).close() + await expect(pop).toHaveCount(0) + await expect(more).toBeFocused() + }) +}) + +// Issue #307 - MEASURED with Narrator + Edge on a comparison page: a button +// whose `aria-controls` names a panel that is not in the DOM while closed is +// never read as "expanded" once it opens, even when re-read; the same button +// with `aria-controls` present only while the panel exists is. So every +// trigger whose panel mounts on open carries `aria-controls` only while open. +async function expectControlsOnlyWhileOpen(page: Page, btn: Locator, prepare: () => Promise = async () => {}) { + await prepare() + await expect(btn).toHaveAttribute('aria-expanded', 'false') + await expect(btn).not.toHaveAttribute('aria-controls') + for (const how of ['Escape', 'press again'] as const) { + await btn.click() + await expect(btn).toHaveAttribute('aria-expanded', 'true') + const id = (await btn.getAttribute('aria-controls')) ?? '' + expect(id, 'aria-controls names the open panel').not.toBe('') + await expect(page.locator(`[id="${id}"]`)).toHaveCount(1) + await expect(page.locator(`[id="${id}"]`)).toBeVisible() + if (how === 'Escape') { + await btn.focus() + await page.keyboard.press('Escape') + } else { + await btn.click() + } + await expect(btn).toHaveAttribute('aria-expanded', 'false') + await expect(btn).not.toHaveAttribute('aria-controls') + await expect(page.locator(`[id="${id}"]`)).toHaveCount(0) + } +} + +test.describe('aria-controls names a panel only while it exists', () => { + test('Settings', async ({ page }) => { + await openApp(page) + await expectControlsOnlyWhileOpen(page, page.locator('.toolbar__settingsmenu > button')) + }) + test('File', async ({ page }) => { + await openApp(page) + await expectControlsOnlyWhileOpen(page, toolbarButton(page, 'File')) + }) + test('Theme, inside Settings', async ({ page }) => { + await openApp(page) + await page.locator('.toolbar__settingsmenu > button').click() + await expectControlsOnlyWhileOpen(page, page.locator('.theme-menu > button')) + // closing Theme left Settings open + await expect(page.locator('.toolbar__settingsmenu-pop')).toBeVisible() + }) + test('Temporary session', async ({ page }) => { + await openApp(page) + await page.locator('.toolbar__settingsmenu > button').click() + await page.locator('[data-settings-row="storage-privacy"]').click() + const dlg = page.locator('.mcdlg--storage') + await dlg.locator('[data-storage-action="to-temporary"]').click() + await dlg.locator('[data-storage-confirm]').click() + await expect(dlg).toHaveCount(0) + await expectControlsOnlyWhileOpen(page, page.locator('[data-session-chip="temporary"]')) + }) +}) + +test.describe('aria-controls on the toolbar overflow, at a width where it holds groups', () => { + test.use({ viewport: { width: 760, height: 800 } }) + test('⋯', async ({ page }) => { + await openApp(page) + await expectControlsOnlyWhileOpen(page, page.locator('.toolbar__overflow-btn')) + }) +}) + +test.describe('Language is a combobox', () => { + test('Arrow Down on the closed row starts at the first language, Arrow Up at the last; focus stays in the search field', async ({ page }) => { + await openApp(page) + await page.locator('.toolbar__settingsmenu > button').click() + const row = page.locator('.toolbar__settingsmenu-pop .lang-switch') + const active = () => + page.evaluate(() => { + const a = document.activeElement as HTMLElement | null + const id = a?.getAttribute('aria-activedescendant') + const opts = [...document.querySelectorAll('[role="listbox"] [role="option"]')] + return { input: a?.tagName === 'INPUT', index: id ? opts.findIndex((o) => o.id === id) : -2, count: opts.length } + }) + await row.focus() + await page.keyboard.press('ArrowDown') + await expect.poll(() => active().then((a) => a.input && a.index === 0)).toBe(true) + await page.keyboard.press('Escape') + await expect(row).toBeFocused() + await page.keyboard.press('ArrowUp') + await expect.poll(() => active().then((a) => a.input && a.index === a.count - 1)).toBe(true) + await page.keyboard.press('Escape') + await expect(row).toBeFocused() + }) +}) + +test.describe('the hook’s items', () => { + test('separators, disabled, aria-disabled and hidden items are never landed on', async ({ page }) => { + await openApp(page) + const roles = await page.evaluate(async () => { + const { usableItems } = await import('/src/ui/useMenuKeyboard.ts') + const pop = document.createElement('div') + const add = (role: string, name: string, mut?: (b: HTMLButtonElement) => void) => { + const b = document.createElement('button') + b.setAttribute('role', role) + b.textContent = name + mut?.(b) + pop.append(b) + } + add('menuitem', 'a') + const sep = document.createElement('div') + sep.setAttribute('role', 'separator') + pop.append(sep) + add('menuitem', 'disabled', (b) => (b.disabled = true)) + add('menuitem', 'aria-disabled', (b) => b.setAttribute('aria-disabled', 'true')) + add('menuitem', 'hidden', (b) => (b.hidden = true)) + add('menuitemradio', 'b') + add('menuitemcheckbox', 'c') + document.body.append(pop) + const names = usableItems(pop).map((e) => e.textContent) + pop.remove() + return names + }) + expect(roles).toEqual(['a', 'b', 'c']) + }) +}) + +test.describe('the phone: sheets are dialogs, one active layer at a time', () => { + test.use({ viewport: { width: 390, height: 844 }, isMobile: true, hasTouch: true }) + + const more = (page: Page) => page.locator('.mob-more') + const sheet = (page: Page, name: string) => page.locator(`.sheet[role="dialog"][aria-label="${name}"]`) + const activeModals = (page: Page) => + page.evaluate(() => [...document.querySelectorAll('[aria-modal="true"]')].filter((d) => !d.closest('[inert]')).map((d) => d.getAttribute('aria-label') ?? d.querySelector('[id]')?.textContent ?? '?')) + + async function openMore(page: Page): Promise { + await more(page).focus() + await page.keyboard.press('Enter') + await expect(sheet(page, 'More')).toBeVisible() + } + + const isInert = (page: Page, selector: string) => page.evaluate((sel) => Boolean(document.querySelector(sel)?.closest('[inert]')), selector) + const showUpdateBar = async (page: Page) => { + // dev has no service worker: the store is poked so the bar renders + await page.evaluate(() => (window as unknown as { __loop: { pwa: { setState: (s: unknown) => void } } }).__loop.pwa.setState({ waitingWorker: { fake: true }, dismissedWorker: null })) + await expect(page.locator('.pwa-update')).toBeVisible() + } + + // Lumi's decision D (2026-10-05): a sheet is NOT modal, so the run bar and + // the PWA update bar stay usable while it is open (docs/mobile.md MV-D11, + // MV-D18, MV-D19). Only what its scrim covers for the pointer leaves the + // keyboard and screen-reader order too. + test('the More sheet is not modal: focus goes in, the run bar and the update bar stay reachable, Tab leaves it, Escape returns to More', async ({ page }) => { + await openApp(page) + await showUpdateBar(page) + await openMore(page) + const s = sheet(page, 'More') + await expect(s).not.toHaveAttribute('aria-modal') + expect(await s.evaluate((d) => d.contains(document.activeElement))).toBe(true) + expect(await activeModals(page)).toEqual([]) + // covered by the scrim: out of the keyboard order too + expect(await isInert(page, '.mob-more')).toBe(true) + expect(await isInert(page, '.canvas')).toBe(true) + // on top of the scrim: reachable + expect(await isInert(page, '.pstrip--mobile')).toBe(false) + expect(await isInert(page, '.pwa-update')).toBe(false) + // Tab out of the sheet's last control reaches the run bar, not the canvas + const last = s.locator('button:not([disabled])').last() + await last.focus() + await page.keyboard.press('Tab') + expect(await page.evaluate(() => Boolean(document.activeElement?.closest('.pstrip--mobile')))).toBe(true) + // Shift+Tab out of its first control reaches the update bar + await s.locator('.sheet__x').focus() + await page.keyboard.press('Shift+Tab') + expect(await page.evaluate(() => Boolean(document.activeElement?.closest('.pwa-update')))).toBe(true) + // Play runs without closing the sheet + await page.locator('.pstrip--mobile button', { hasText: 'Play' }).focus() + await page.keyboard.press('Enter') + await expect(page.locator('.pstrip--mobile button', { hasText: 'Pause' })).toBeVisible() + await expect(s).toBeVisible() + await page.locator('.pstrip--mobile button', { hasText: 'Pause' }).click() + // Escape from the run bar still closes the sheet, the one open entry + await page.keyboard.press('Escape') + await expect(s).toHaveCount(0) + await expect(more(page)).toBeFocused() + expect(await page.evaluate(() => document.querySelectorAll('[inert]').length)).toBe(0) + }) + + for (const [row, name] of [ + ['export', 'Export'], + ['templates', 'Templates'], + ['filter', 'Filters'], + ['help', 'Help'], + ] as const) { + test(`${name}: focus goes in, not modal, Escape goes back to the row in More; Close still closes everything`, async ({ page }) => { + await openApp(page) + await openMore(page) + const r = sheet(page, 'More').locator(`[data-more-row="${row}"]`) + await r.focus() + await page.keyboard.press('Enter') + const sub = sheet(page, name) + await expect(sub).toBeVisible() + expect(await sub.evaluate((d) => d.contains(document.activeElement))).toBe(true) + await expect(sub).not.toHaveAttribute('aria-modal') + expect(await activeModals(page)).toEqual([]) + expect(await isInert(page, '.pstrip--mobile')).toBe(false) + await page.keyboard.press('Escape') + await expect(sub).toHaveCount(0) + await expect(sheet(page, 'More')).toBeVisible() + await expect(sheet(page, 'More').locator(`[data-more-row="${row}"]`)).toBeFocused() + // the pointer's way out is unchanged: Close closes it all + await sheet(page, 'More').locator(`[data-more-row="${row}"]`).click() + await sheet(page, name).locator('.sheet__x').click() + await expect(page.locator('.sheet')).toHaveCount(0) + await expect(more(page)).toBeFocused() + }) + } + + test('a dialog over the More sheet is the one modal layer, the update bar included; Escape closes it alone and returns to its row', async ({ page }) => { + await openApp(page) + await showUpdateBar(page) + await openMore(page) + const row = page.locator('[data-settings-row="storage-privacy"]') + await row.focus() + await page.keyboard.press('Enter') + const dlg = page.locator('.mcdlg--storage') + await expect(dlg).toBeVisible() + expect(await isInert(page, '.sheet[aria-label="More"]')).toBe(true) + expect(await isInert(page, '.pstrip--mobile')).toBe(true) + // docs/mobile.md MV8a: a real dialog covers the update bar too + expect(await isInert(page, '.pwa-update')).toBe(true) + expect((await activeModals(page)).length).toBe(1) + await page.keyboard.press('Escape') + await expect(dlg).toHaveCount(0) + await expect(sheet(page, 'More')).toBeVisible() + await expect(row).toBeFocused() + expect(await page.evaluate(() => Boolean(document.querySelector('.sheet[aria-label="More"]')?.closest('[inert]')))).toBe(false) + }) + + // The dialogs a sub-sheet opens name the top bar's More button as their + // return target, which is outside the sub-sheet still open: focus goes back + // to the row that opened the dialog instead. MEASURED before that rule, while + // the page behind was inert: focus fell to . + for (const [row, name, item, selector] of [ + ['help', 'Help', /^About Loop Studio$/, '.mcdlg--about'], + ['export', 'Export', /^Set the export author/, '.mcdlg'], + ] as const) { + test(`a dialog opened from ${name}: Escape closes it alone and focus returns to its row in ${name}`, async ({ page }) => { + await openApp(page) + await openMore(page) + await sheet(page, 'More').locator(`[data-more-row="${row}"]`).focus() + await page.keyboard.press('Enter') + const sub = sheet(page, name) + const opener = sub.locator('.sheet__row', { hasText: item }) + await opener.focus() + await page.keyboard.press('Enter') + const dlg = page.locator(selector).last() + await expect(dlg).toBeVisible() + expect((await activeModals(page)).length).toBe(1) + await page.keyboard.press('Escape') + await expect(page.locator('.mcdlg')).toHaveCount(0) + await expect(sub).toBeVisible() + await expect(opener).toBeFocused() + }) + } +}) diff --git a/e2e/mobile.spec.ts b/e2e/mobile.spec.ts index 32f564ca..d134b0b6 100644 --- a/e2e/mobile.spec.ts +++ b/e2e/mobile.spec.ts @@ -1673,6 +1673,20 @@ test.describe('sheet row secondary label contrast (§MV5 / WCAG 1.4.3)', () => { return page.locator(`.sheet[aria-label="${which}"]`) } + /** Escape out of the sheet `openSheet` opened. Issue #307: Escape in a sheet + * opened from More goes back to More, on the row that opened it; a second + * Escape closes More. */ + async function escapeSheet(page: Page, which: 'More' | 'Templates' | 'Export') { + await page.keyboard.press('Escape') + if (which !== 'More') { + const more = page.locator('.sheet[aria-label="More"]') + await expect(more).toBeVisible() + await expect(more.locator('.sheet__row', { hasText: which })).toBeFocused() + await page.keyboard.press('Escape') + } + await expect(page.locator('.sheet')).toHaveCount(0) + } + /** every row whose sub-label is really painted in the sub colour */ async function textSubRows(page: Page, sheet: Locator) { const all = sheet.locator('.sheet__row').filter({ has: page.locator('.sheet__row-sub') }) @@ -1720,7 +1734,7 @@ test.describe('sheet row secondary label contrast (§MV5 / WCAG 1.4.3)', () => { console.log(`[sub] ${which}/${name} hover ${s.sub} on ${s.bg} = ${r2(r)}:1`) if (r < 4.5) bad.push(`${which}/${name} ${r2(r)}:1`) } - await page.keyboard.press('Escape') + await escapeSheet(page, which) } expect(checked, 'the walk must reach every text sub-label: 5 More + 5 Templates + 4 enabled Export').toBe(14) expect(bad, 'hovered secondary labels below 4.5:1').toEqual([]) @@ -1745,7 +1759,7 @@ test.describe('sheet row secondary label contrast (§MV5 / WCAG 1.4.3)', () => { if (r < 4.5) bad.push(`${which}/${name} ${r2(r)}:1`) if (which === 'More' && s.subArrow) markers.push(name) } - await page.keyboard.press('Escape') + await escapeSheet(page, which) } // the four submenu rows carry their affordance IN the sub-label, so they // are covered by the same contract rather than by the row's own text diff --git a/e2e/modal-inert.spec.ts b/e2e/modal-inert.spec.ts new file mode 100644 index 00000000..f418cf3e --- /dev/null +++ b/e2e/modal-inert.spec.ts @@ -0,0 +1,292 @@ +import type { Locator, Page } from '@playwright/test' +import { expect, openApp, test } from './support/loop' + +// Issue #307, Lumi's decision (2026-10-05): a real dialog (`aria-modal="true"`) +// is modal for real. While it is the top one, nothing outside it can be reached +// by the pointer, by Tab, or in the accessibility tree - the PWA update bar and +// the phone's run bar included. The update bar keeps its state behind the +// dialog's scrim and is back, usable, the moment the dialog closes; focus +// returns where the dialog's owner says. A phone sheet is NOT modal: with only +// a sheet open the update bar and Play stay usable (docs/mobile.md MV8a, MV5). +// +// Each case below checks the three paths while the dialog is open and again +// after it closes. + +const UPDATE = '.pwa-update button:has-text("Update")' +const DISMISS = '.pwa-update button:has-text("Dismiss")' +const PLAY = '.pstrip--mobile button:has-text("Play")' + +async function showUpdateBar(page: Page) { + // dev has no service worker: the store is poked so the bar renders + await page.evaluate(() => + (window as unknown as { __loop: { pwa: { setState: (s: unknown) => void } } }).__loop.pwa.setState({ waitingWorker: { fake: true }, dismissedWorker: null }), + ) + await expect(page.locator('.pwa-update')).toBeVisible() +} + +/** does a pointer aimed at the centre of `selector` land on it? */ +const pointerReaches = (page: Page, selector: string) => + page.locator(selector).evaluate((el) => { + const r = el.getBoundingClientRect() + const hit = document.elementFromPoint(r.left + r.width / 2, r.top + r.height / 2) + return !!hit && (hit === el || el.contains(hit)) + }) + +/** Tab and Shift+Tab from where focus is: does focus ever land in `region`? */ +async function tabReaches(page: Page, region: string): Promise { + await page.evaluate(() => { + for (const el of document.querySelectorAll('[data-tab-start]')) el.removeAttribute('data-tab-start') + document.activeElement?.setAttribute('data-tab-start', '') + }) + let reached = false + for (const key of ['Tab', 'Shift+Tab']) { + await page.evaluate(() => (document.querySelector('[data-tab-start]') as HTMLElement | null)?.focus()) + for (let i = 0; i < 40 && !reached; i++) { + await page.keyboard.press(key) + reached = await page.evaluate((sel) => Boolean(document.activeElement?.closest(sel)), region) + } + } + // focus goes back where it was, so the caller's next check starts there + await page.evaluate(() => { + const start = document.querySelector('[data-tab-start]') as HTMLElement | null + start?.removeAttribute('data-tab-start') + start?.focus() + }) + return reached +} + +/** how many buttons with this exact name the accessibility tree exposes */ +async function exposedButtons(page: Page, name: string): Promise { + const cdp = await page.context().newCDPSession(page) + const { nodes } = (await cdp.send('Accessibility.getFullAXTree')) as { nodes: { role?: { value?: string }; name?: { value?: string }; ignored?: boolean }[] } + await cdp.detach() + return nodes.filter((n) => n.role?.value === 'button' && n.name?.value === name && !n.ignored).length +} + +/** the three paths to the update bar's buttons (and, on a phone, to Play) */ +async function expectUnreachable(page: Page, phone: boolean) { + expect(await pointerReaches(page, UPDATE), 'pointer reaches Update').toBe(false) + expect(await pointerReaches(page, DISMISS), 'pointer reaches Dismiss').toBe(false) + expect(await tabReaches(page, '.pwa-update'), 'Tab reaches the update bar').toBe(false) + expect(await exposedButtons(page, 'Update'), 'Update in the accessibility tree').toBe(0) + expect(await exposedButtons(page, 'Dismiss'), 'Dismiss in the accessibility tree').toBe(0) + if (phone) { + expect(await pointerReaches(page, PLAY), 'pointer reaches Play').toBe(false) + expect(await tabReaches(page, '.pstrip--mobile'), 'Tab reaches the run bar').toBe(false) + expect(await exposedButtons(page, 'Play'), 'Play in the accessibility tree').toBe(0) + } +} + +async function expectReachable(page: Page, phone: boolean) { + await expect(page.locator('.pwa-update')).toBeVisible() + await expect(page.locator(UPDATE)).toBeEnabled() + expect(await pointerReaches(page, UPDATE), 'pointer reaches Update').toBe(true) + expect(await pointerReaches(page, DISMISS), 'pointer reaches Dismiss').toBe(true) + expect(await exposedButtons(page, 'Update'), 'Update in the accessibility tree').toBe(1) + expect(await exposedButtons(page, 'Dismiss'), 'Dismiss in the accessibility tree').toBe(1) + expect(await page.locator('.pwa-update').evaluate((el) => Boolean(el.closest('[inert]')))).toBe(false) + expect(await tabReaches(page, '.pwa-update'), 'Tab reaches the update bar').toBe(true) + if (phone) { + expect(await pointerReaches(page, PLAY), 'pointer reaches Play').toBe(true) + expect(await exposedButtons(page, 'Play'), 'Play in the accessibility tree').toBe(1) + expect(await tabReaches(page, '.pstrip--mobile'), 'Tab reaches the run bar').toBe(true) + } +} + +/** what the update bar says and offers, to compare before and after */ +const barState = (page: Page) => + page.locator('.pwa-update').evaluate((el) => ({ + text: el.textContent?.replace(/\s+/g, ' ').trim(), + buttons: [...el.querySelectorAll('button')].map((b) => `${b.textContent?.trim()}:${(b as HTMLButtonElement).disabled}`), + })) + +async function closeAndCheck(page: Page, dialog: Locator, before: Awaited>, focused: Locator, phone: boolean) { + await page.keyboard.press('Escape') + await expect(dialog).toHaveCount(0) + await expect(focused).toBeFocused() + expect(await barState(page)).toEqual(before) + await expectReachable(page, phone) +} + +test.describe('desktop: a real dialog makes the page behind it unreachable', () => { + test('About: the update bar is covered and inert while it is open, and back, unchanged, when it closes', async ({ page }) => { + await openApp(page) + await showUpdateBar(page) + await expectReachable(page, false) + const before = await barState(page) + await page.locator('[data-tour="help-trigger"]').click() + await page.getByRole('menuitem', { name: 'About Loop Studio' }).click() + const dialog = page.locator('.mcdlg--about') + await expect(dialog).toBeVisible() + await expectUnreachable(page, false) + expect(await dialog.evaluate((d) => d.contains(document.activeElement))).toBe(true) + await closeAndCheck(page, dialog, before, page.locator('[data-tour="help-trigger"]'), false) + }) + + test('an update that arrives while About is open joins the inert background', async ({ page }) => { + await openApp(page) + await page.locator('[data-tour="help-trigger"]').click() + await page.getByRole('menuitem', { name: 'About Loop Studio' }).click() + const dialog = page.locator('.mcdlg--about') + await expect(dialog).toBeVisible() + await showUpdateBar(page) + await expect.poll(() => page.locator('.pwa-update').evaluate((el) => Boolean(el.closest('[inert]')))).toBe(true) + await expectUnreachable(page, false) + const before = await barState(page) + await closeAndCheck(page, dialog, before, page.locator('[data-tour="help-trigger"]'), false) + }) + + test('the guided tour: the update bar is covered and inert while the tour is up, and back when it is dismissed', async ({ page }) => { + await openApp(page) + await showUpdateBar(page) + const before = await barState(page) + await page.locator('[data-tour="help-trigger"]').click() + await page.getByRole('menuitem', { name: 'Restart the tour' }).click() + const tour = page.locator('.tour-popover[role="dialog"]') + await expect(tour).toBeVisible() + await expectUnreachable(page, false) + // the tour's own scrim is part of its modal layer, not of the page behind: + // it is not inert, and it takes the click it exists to swallow + expect(await page.locator('.tour-scrim').evaluate((el) => Boolean(el.closest('[inert]')))).toBe(false) + await page.locator('.tour-scrim').click({ position: { x: 5, y: 5 } }) + await expect(tour).toBeVisible() + await tour.locator('button').first().focus() + await closeAndCheck(page, tour, before, page.locator('[data-tour="help-trigger"]'), false) + }) + + // The product opens no dialog over another today (the tour and a + // confirmation are kept apart, docs/guided-tour.md §GT6.1), so the stack is + // driven directly, through the same module the app uses. + test('nested modals: only the top one can be reached; a pure live region stays live; everything comes back in order', async ({ page }) => { + await openApp(page) + const r = await page.evaluate(async () => { + const m = (await import('/src/ui/overlayStack.ts')) as typeof import('../src/ui/overlayStack') + const make = (html: string) => { + const el = document.createElement('div') + el.innerHTML = html + document.body.append(el) + return el + } + const task = () => new Promise((resolve) => setTimeout(resolve, 0)) + const live = make('

announcements

').firstElementChild as HTMLElement + const notice = make('
a notice
').firstElementChild as HTMLElement + // inert before anything opened, and a direct child of : exactly + // where the stack walks and would set and clear inert itself + const preInert = make('') + preInert.inert = true + const a = make('') + const b = make('') + const inert = (el: Element) => Boolean(el.closest('[inert]')) + // a control of the app behind: #root itself stays, because the app's + // own pure live regions inside it stay live + const app = document.querySelector('[data-tour="help-trigger"]')! + const offA = m.pushOverlay(a, 'modal') + const one = { a: inert(a), app: inert(app), live: inert(live), notice: inert(notice), preInert: inert(preInert) } + const offB = m.pushOverlay(b, 'modal') + const two = { a: inert(a), b: inert(b), app: inert(app), live: inert(live) } + // the observer works while a modal is open: a late element is inert + // once a task has passed + const late = make('') + await task() + const lateWhileOpen = inert(late) + offB() + const back = { a: inert(a), app: inert(app), late: inert(late) } + offA() + const none = { + a: inert(a), + b: inert(b), + app: inert(app), + notice: inert(notice), + late: inert(late), + preInert: preInert.inert, + inertLeft: [...document.querySelectorAll('[inert]')].filter((el) => el !== preInert).length, + } + // the observer is gone once the last modal closed: an element added now, + // given the same task for a callback to run, stays active + const after = make('') + await task() + const afterClose = inert(after) + for (const el of [live.parentElement, notice.parentElement, preInert, a, b, late, after]) el?.remove() + return { one, two, lateWhileOpen, back, none, afterClose } + }) + expect(r.one).toEqual({ a: false, app: true, live: false, notice: true, preInert: true }) + expect(r.two).toEqual({ a: true, b: false, app: true, live: false }) + expect(r.lateWhileOpen).toBe(true) + expect(r.back).toEqual({ a: false, app: true, late: true }) + expect(r.none).toEqual({ a: false, b: false, app: false, notice: false, late: false, preInert: true, inertLeft: 0 }) + expect(r.afterClose).toBe(false) + }) + + test('a sheet makes only what its scrim covers inert, and never releases an element inert before it opened', async ({ page }) => { + await openApp(page) + const r = await page.evaluate(async () => { + const m = (await import('/src/ui/overlayStack.ts')) as typeof import('../src/ui/overlayStack') + const make = (attrs: string) => { + const wrap = document.createElement('div') + wrap.innerHTML = `
` + document.body.append(wrap) + return wrap.firstElementChild as HTMLElement + } + // both are marked as covered by sheets, the set the sheet entry manages + const covered = make('data-covered-by-sheets=""') + const coveredPreInert = make('data-covered-by-sheets=""') + coveredPreInert.inert = true + const sheet = make('class="fake-sheet"') + const off = m.pushOverlay(sheet, 'sheet') + const open = { covered: covered.inert, coveredPreInert: coveredPreInert.inert, sheet: sheet.inert } + off() + const closed = { covered: covered.inert, coveredPreInert: coveredPreInert.inert } + for (const el of [covered, coveredPreInert, sheet]) el.parentElement?.remove() + return { open, closed } + }) + expect(r.open).toEqual({ covered: true, coveredPreInert: true, sheet: false }) + expect(r.closed).toEqual({ covered: false, coveredPreInert: true }) + }) +}) + +test.describe('phone: a real dialog makes the page behind it unreachable; a sheet does not', () => { + test.use({ viewport: { width: 390, height: 844 }, isMobile: true, hasTouch: true }) + + test('the MC dialog: the update bar and the run bar are covered and inert, and back when it closes', async ({ page }) => { + await openApp(page) + await showUpdateBar(page) + await expectReachable(page, true) + const before = await barState(page) + const mc = page.locator('.pstrip--mobile').getByRole('button', { name: 'Monte Carlo' }) + await mc.focus() + await page.keyboard.press('Enter') + const dialog = page.locator('.mcdlg') + await expect(dialog).toBeVisible() + await expectUnreachable(page, true) + await closeAndCheck(page, dialog, before, mc, true) + }) + + test('the Storage dialog over the More sheet: only the dialog is active; closing it returns to its row, and the sheet is non-modal again', async ({ page }) => { + await openApp(page) + await showUpdateBar(page) + await page.locator('.mob-more').focus() + await page.keyboard.press('Enter') + const more = page.locator('.sheet[aria-label="More"]') + await expect(more).toBeVisible() + // a sheet alone: Update, Dismiss and Play stay usable + await expectReachable(page, true) + const before = await barState(page) + const row = more.locator('[data-settings-row="storage-privacy"]') + await row.focus() + await page.keyboard.press('Enter') + const dialog = page.locator('.mcdlg--storage') + await expect(dialog).toBeVisible() + expect(await more.evaluate((d) => Boolean(d.closest('[inert]')))).toBe(true) + await expectUnreachable(page, true) + await closeAndCheck(page, dialog, before, row, true) + await expect(more).toBeVisible() + await expect(more).not.toHaveAttribute('aria-modal') + expect(await more.evaluate((d) => Boolean(d.closest('[inert]')))).toBe(false) + // the sheet is still open, so what its scrim covers stays inert: closing + // the dialog gives back only what the dialog took + await expect(page.locator('.openhint')).toBeVisible() + for (const covered of ['.mob-more', '.openhint', '.canvas']) { + expect(await page.locator(covered).evaluate((el) => Boolean(el.closest('[inert]'))), `${covered} inert`).toBe(true) + } + }) +}) diff --git a/e2e/storage-sessions.spec.ts b/e2e/storage-sessions.spec.ts index c11b1837..98c3b2c4 100644 --- a/e2e/storage-sessions.spec.ts +++ b/e2e/storage-sessions.spec.ts @@ -205,11 +205,10 @@ test.describe('switching sessions', () => { expect(m.border).toBe(m.warning) expect(m.border).not.toBe(m.btnBorder) // the session still shows await expect(chip(page)).toHaveAccessibleName('Temporary session') - // keyboard: Enter opens, ArrowDown enters the menu, Escape closes it and focus comes back + // keyboard: Enter opens at the first item (issue #307), Escape closes it and focus comes back await chip(page).focus() await page.keyboard.press('Enter') await expect(chip(page)).toHaveAttribute('aria-expanded', 'true') - await page.keyboard.press('ArrowDown') await expect(page.locator('.session-chip__pop').getByRole('menuitem').first()).toBeFocused() await page.keyboard.press('Escape') await expect(page.locator('.session-chip__pop')).toHaveCount(0) diff --git a/e2e/theme-boot.spec.ts b/e2e/theme-boot.spec.ts index 0407229b..a10a9c9d 100644 --- a/e2e/theme-boot.spec.ts +++ b/e2e/theme-boot.spec.ts @@ -187,7 +187,7 @@ test.describe('choosing a theme still works, and the choice survives a reload', expect(await attr(page)).toBeNull() await page.locator('.toolbar__actions .menu > button', { hasText: /^Settings$/ }).click() await page.getByRole('button', { name: /^Theme/ }).click() - await page.getByRole('menuitem', { name: /Dark/ }).click() + await page.getByRole('menuitemradio', { name: /Dark/ }).click() expect(await attr(page)).toBe('dark') expect(await page.evaluate(() => localStorage.getItem('loop-studio:theme'))).toBe('dark') await page.reload() diff --git a/e2e/theme-submenu.spec.ts b/e2e/theme-submenu.spec.ts index d93947b7..12ff9184 100644 --- a/e2e/theme-submenu.spec.ts +++ b/e2e/theme-submenu.spec.ts @@ -62,9 +62,11 @@ test('opening it shows exactly Auto / Light / Dark, in English, with the active expect(texts.map((s) => s.trim())).toEqual(['Auto', 'Light', 'Dark']) expect(texts.join(' ')).not.toContain('System') // the internal mode literal must never leak into EN UI - await expect(items.nth(0)).toHaveAttribute('aria-selected', 'true') - await expect(items.nth(1)).toHaveAttribute('aria-selected', 'false') - await expect(items.nth(2)).toHaveAttribute('aria-selected', 'false') + // issue #307: one-of-three choices, so menuitemradio + aria-checked + await expect(items.nth(0)).toHaveAttribute('role', 'menuitemradio') + await expect(items.nth(0)).toHaveAttribute('aria-checked', 'true') + await expect(items.nth(1)).toHaveAttribute('aria-checked', 'false') + await expect(items.nth(2)).toHaveAttribute('aria-checked', 'false') }) test('selecting Dark applies it immediately, checks it, and closes only the Theme submenu', async ({ @@ -83,17 +85,26 @@ test('selecting Dark applies it immediately, checks it, and closes only the Them // reopening shows Dark checked await row.click() - await expect(page.locator('.theme-menu__pop .menu__item[aria-selected="true"]', { hasText: /^Dark$/ })).toBeVisible() + await expect(page.locator('.theme-menu__pop .menu__item[aria-checked="true"]', { hasText: /^Dark$/ })).toBeVisible() }) test('keyboard: ArrowDown/ArrowUp move between options, Enter selects, Escape closes only Theme', async ({ page, }) => { - await openThemeSubmenu(page) + await openSettings(page) + const row = page.locator('.toolbar__settingsmenu-pop .theme-menu .settings-row') const pop = page.locator('.theme-menu__pop') const items = pop.locator('.menu__item') - await expect(items.nth(0)).toBeFocused() // opens focused on the current (Auto) item + // issue #307: a pointer open leaves focus on the row; a keyboard open + // enters at the first item + await row.click() + await expect(pop).toBeVisible() + await expect(row).toBeFocused() + await page.keyboard.press('Escape') + await expect(pop).toBeHidden() + await row.press('Enter') + await expect(items.nth(0)).toBeFocused() await page.keyboard.press('ArrowDown') await expect(items.nth(1)).toBeFocused() await page.keyboard.press('ArrowDown') diff --git a/e2e/toolbar-responsive.spec.ts b/e2e/toolbar-responsive.spec.ts index 525b7eef..376b0bc3 100644 --- a/e2e/toolbar-responsive.spec.ts +++ b/e2e/toolbar-responsive.spec.ts @@ -482,7 +482,9 @@ test.describe('toolbar — an overflowed control is reachable with the mouse and const moreBtn = page.locator('.toolbar__overflow-btn') await expect(moreBtn).toBeVisible() - await expect(moreBtn).toHaveAttribute('aria-haspopup', 'menu') + // issue #307: a disclosure, not a menu button + await expect(moreBtn).not.toHaveAttribute('aria-haspopup') + await expect(moreBtn).toHaveAttribute('aria-expanded', 'false') await moreBtn.click() const pop = page.locator('.toolbar__overflow-pop') diff --git a/e2e/whats-new.spec.ts b/e2e/whats-new.spec.ts index 56bce9d8..2dc454c0 100644 --- a/e2e/whats-new.spec.ts +++ b/e2e/whats-new.spec.ts @@ -239,7 +239,7 @@ test.describe('closing the notice and opening the panel are different things', ( expect(text).not.toContain('text unavailable') } } - expect(RELEASE_NOTES.map((n) => n.version)).toEqual(['0.18.0', '0.17.2', '0.17.1', '0.17.0', '0.16.0', '0.15.3', '0.15.2', '0.15.1', '0.15.0', '0.14.0']) + expect(RELEASE_NOTES.map((n) => n.version)).toEqual(['0.18.1', '0.18.0', '0.17.2', '0.17.1', '0.17.0', '0.16.0', '0.15.3', '0.15.2', '0.15.1', '0.15.0', '0.14.0']) await page.keyboard.press('Escape') await expect(panel).toHaveCount(0) diff --git a/package-lock.json b/package-lock.json index d42f2439..64878c0a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "loop-studio", - "version": "0.18.0", + "version": "0.18.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "loop-studio", - "version": "0.18.0", + "version": "0.18.1", "dependencies": { "@fontsource/ibm-plex-mono": "^5.3.0", "@fontsource/ibm-plex-sans": "^5.3.0", diff --git a/package.json b/package.json index 8bf19c3c..909a6454 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "loop-studio", "private": true, - "version": "0.18.0", + "version": "0.18.1", "type": "module", "engines": { "node": ">=22.12.0" diff --git a/src/components/Canvas.tsx b/src/components/Canvas.tsx index affab417..27d4a985 100644 --- a/src/components/Canvas.tsx +++ b/src/components/Canvas.tsx @@ -782,6 +782,7 @@ export function Canvas() { ref={canvasRef} className={`canvas${canvasLocked ? ' canvas--locked' : ''}${refInsertArmed ? ' canvas--ref-insert' : ''}`} data-tour="canvas" + data-covered-by-sheets="" onDrop={noEdit ? undefined : handleDrop} onDragOver={noEdit ? undefined : handleDragOver} onContextMenu={noEdit ? (e) => e.preventDefault() : undefined} diff --git a/src/components/DistributionPanel.tsx b/src/components/DistributionPanel.tsx index b936fb2b..931d9162 100644 --- a/src/components/DistributionPanel.tsx +++ b/src/components/DistributionPanel.tsx @@ -1,5 +1,6 @@ -import { useEffect, useRef, useState } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import { useMcStore } from '../store/mcStore' +import { useMenuKeyboard, useMenuTrigger } from '../ui/useMenuKeyboard' import { useT } from '../i18n' import { BandChart } from './BandChart' import { TerminationSparkline } from './TerminationSparkline' @@ -26,19 +27,21 @@ function ExportMenu({ result, disabled }: { result: MonteCarloResult; disabled: const t = useT() const [open, setOpen] = useState(false) const ref = useRef(null) + const btnRef = useRef(null) + const popRef = useRef(null) useEffect(() => { if (!open) return const onDown = (e: MouseEvent) => { if (ref.current && !ref.current.contains(e.target as Node)) setOpen(false) } - const onKey = (e: KeyboardEvent) => e.key === 'Escape' && setOpen(false) window.addEventListener('mousedown', onDown) - window.addEventListener('keydown', onKey) - return () => { - window.removeEventListener('mousedown', onDown) - window.removeEventListener('keydown', onKey) - } + return () => window.removeEventListener('mousedown', onDown) }, [open]) + // the menu keyboard contract (issue #307): the shared hook owns Escape, the + // arrows and Tab; a download chosen from the keyboard returns focus here + const close = useCallback(() => setOpen(false), []) + const { entry, triggerProps } = useMenuTrigger(open, setOpen) + useMenuKeyboard(open, popRef, btnRef, close, entry) const saveCsv = (suffix: string, text: string) => { downloadCsv(text, `loop-studio-montecarlo-${suffix}`) @@ -58,13 +61,14 @@ function ExportMenu({ result, disabled }: { result: MonteCarloResult; disabled: aria-expanded={open} disabled={disabled} title={disabled ? t('dist.export.staleTitle') : t('dist.export.title')} - onClick={() => setOpen((v) => !v)} + ref={btnRef} + {...triggerProps} > {t('export.button')} {open ? ( -
+
diff --git a/src/components/LanguageSwitch.tsx b/src/components/LanguageSwitch.tsx index ff305613..d6cd5fde 100644 --- a/src/components/LanguageSwitch.tsx +++ b/src/components/LanguageSwitch.tsx @@ -106,9 +106,12 @@ export function LanguageSwitch({ if (open) optionRefs.current[focusIdx]?.scrollIntoView({ block: 'nearest' }) }, [open, focusIdx]) - function openMenu() { + // issue #307 - the combobox keeps its own keys (it is not a menu), with the + // menus' entry points: Arrow Down on the closed button opens at the first + // option, Arrow Up at the last; Enter / Space open at the current language + function openMenu(at: 'current' | 'first' | 'last' = 'current') { setQuery('') - setFocusIdx(activeIdxIn(locales)) + setFocusIdx(at === 'first' ? 0 : at === 'last' ? Math.max(0, locales.length - 1) : activeIdxIn(locales)) setOpenState(true) } function close(returnFocus = true) { @@ -124,9 +127,9 @@ export function LanguageSwitch({ } const onTriggerKey = (e: KeyboardEvent) => { - if (e.key === 'ArrowDown' || e.key === 'Enter' || e.key === ' ') { + if (e.key === 'ArrowDown' || e.key === 'ArrowUp' || e.key === 'Enter' || e.key === ' ') { e.preventDefault() - openMenu() + openMenu(e.key === 'ArrowDown' ? 'first' : e.key === 'ArrowUp' ? 'last' : 'current') } } diff --git a/src/components/ModuleMenu.tsx b/src/components/ModuleMenu.tsx index bcf1a77c..a6a7b4d6 100644 --- a/src/components/ModuleMenu.tsx +++ b/src/components/ModuleMenu.tsx @@ -10,7 +10,7 @@ import { MODULE_KEY } from './moduleKeys' import type { ToolbarDialog } from './toolbar/dialogTypes' import { useMenuOpenStore } from './toolbar/menuOpenStore' import { useOutsideDismiss } from './toolbar/useOutsideDismiss' -import { useMenuKeyboard } from '../ui/useMenuKeyboard' +import { useMenuKeyboard, useMenuTrigger } from '../ui/useMenuKeyboard' import { Icon } from '../ui/icons' // docs/module-system.md §MS6 — the v1 assembly surface: an "Insert module ▾" @@ -59,10 +59,10 @@ export function ModuleMenu({ useOutsideDismiss(open, wrapRef, () => setOpen(false)) - // arrow / Home / End / Escape, including the focus return Escape used to - // skip (it left focus on `body`). One owner for the key -- see the hook. + // the menu keyboard contract (issue #307) -- see the hook const close = useCallback(() => setOpen(false), []) - useMenuKeyboard(open, popRef, btnRef, close) + const { entry, triggerProps } = useMenuTrigger(open, setOpen) + useMenuKeyboard(open, popRef, btnRef, close, entry) // review, Hanrim 2026-09-15 — announce open/closed so the palette can // suppress its own hover tooltip while this menu is up @@ -185,7 +185,7 @@ export function ModuleMenu({ className="btn" aria-haspopup="true" aria-expanded={open} - onClick={() => setOpen((v) => !v)} + {...triggerProps} > {t('modules.button')} diff --git a/src/components/SessionChip.tsx b/src/components/SessionChip.tsx index ad60501d..cde10f35 100644 --- a/src/components/SessionChip.tsx +++ b/src/components/SessionChip.tsx @@ -3,7 +3,7 @@ import { useT } from '../i18n' import { exportableDocument } from '../store/sessionActions' import { selectTemporary, useSessionStore } from '../store/sessionStore' import { downloadText } from '../ui/download' -import { useMenuKeyboard } from '../ui/useMenuKeyboard' +import { useMenuKeyboard, useMenuTrigger } from '../ui/useMenuKeyboard' import type { ToolbarDialog } from './toolbar/dialogTypes' import { useOutsideDismiss } from './toolbar/useOutsideDismiss' @@ -25,7 +25,8 @@ export function SessionChip({ onOpenDialog }: { onOpenDialog: (desc: ToolbarDial const menuId = useId() const close = useCallback(() => setOpen(false), []) useOutsideDismiss(open, wrapRef, close) - useMenuKeyboard(open, popRef, btnRef, close) + const { entry, triggerProps } = useMenuTrigger(open, setOpen) + useMenuKeyboard(open, popRef, btnRef, close, entry) if (!temporary) return null return ( @@ -36,10 +37,12 @@ export function SessionChip({ onOpenDialog }: { onOpenDialog: (desc: ToolbarDial className="rev-chip session-chip__btn" aria-haspopup="true" aria-expanded={open} - aria-controls={menuId} + // only while the menu exists (issue #307): an id that names nothing + // while closed stopped Narrator reading "expanded" once it opened + aria-controls={open ? menuId : undefined} title={t('session.temporary.chipTitle')} data-session-chip="temporary" - onClick={() => setOpen((v) => !v)} + {...triggerProps} > {t('session.temporary.chip')} diff --git a/src/components/Templates.tsx b/src/components/Templates.tsx index 499f6c98..4c9f000e 100644 --- a/src/components/Templates.tsx +++ b/src/components/Templates.tsx @@ -9,7 +9,7 @@ import { ConfirmDialog } from './ConfirmDialog' import { TEMPLATE_KEY } from './templateKeys' import { useMenuOpenStore } from './toolbar/menuOpenStore' import { useOutsideDismiss } from './toolbar/useOutsideDismiss' -import { useMenuKeyboard } from '../ui/useMenuKeyboard' +import { useMenuKeyboard, useMenuTrigger } from '../ui/useMenuKeyboard' import { Icon } from '../ui/icons' // Replacing the current diagram is confirmed through the shared in-app dialog — @@ -27,10 +27,10 @@ export function Templates() { useOutsideDismiss(open, wrapRef, () => setOpen(false)) - // arrow / Home / End / Escape, including the focus return Escape used to - // skip (it left focus on `body`). One owner for the key -- see the hook. + // the menu keyboard contract (issue #307) -- see the hook const close = useCallback(() => setOpen(false), []) - useMenuKeyboard(open, popRef, btnRef, close) + const { entry, triggerProps } = useMenuTrigger(open, setOpen) + useMenuKeyboard(open, popRef, btnRef, close, entry) // review, Hanrim 2026-09-15 — announce open/closed so the palette can // suppress its own hover tooltip while this menu is up @@ -70,7 +70,7 @@ export function Templates() { className="btn" aria-haspopup="true" aria-expanded={open} - onClick={() => setOpen((v) => !v)} + {...triggerProps} > {t('templates.button')} diff --git a/src/components/ThemeToggle.tsx b/src/components/ThemeToggle.tsx index d14385f0..c71b0991 100644 --- a/src/components/ThemeToggle.tsx +++ b/src/components/ThemeToggle.tsx @@ -1,5 +1,6 @@ -import { useEffect, useId, useRef, useState, type KeyboardEvent } from 'react' +import { useCallback, useEffect, useId, useRef, useState } from 'react' import { useT, type MessageKey } from '../i18n' +import { useMenuKeyboard, useMenuTrigger } from '../ui/useMenuKeyboard' import { useSideFlyoutPosition } from './toolbar/useAnchoredPosition' import { useOutsideDismiss } from './toolbar/useOutsideDismiss' import { storagePort } from '../storage/storagePort' @@ -52,7 +53,6 @@ export function ThemeToggle({ const wrapRef = useRef(null) const btnRef = useRef(null) const panelRef = useRef(null) - const itemRefs = useRef<(HTMLButtonElement | null)[]>([]) const menuId = useId() const flyoutPos = useSideFlyoutPosition(btnRef, panelRef, variant === 'row' && open) @@ -70,28 +70,14 @@ export function ThemeToggle({ useOutsideDismiss(variant === 'row' && open, wrapRef, () => setOpen(false)) - useEffect(() => { - if (variant !== 'row' || !open) return - const onKey = (e: KeyboardEvent | globalThis.KeyboardEvent) => { - if (e.key !== 'Escape') return - setOpen(false) - btnRef.current?.focus() - } - window.addEventListener('keydown', onKey as (e: globalThis.KeyboardEvent) => void) - return () => { - window.removeEventListener('keydown', onKey as (e: globalThis.KeyboardEvent) => void) - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [variant, open]) - - useEffect(() => { - if (variant !== 'row' || !open || !flyoutPos) return - itemRefs.current[MODES.indexOf(mode)]?.focus() - // depends on whether a position has landed, not the position object - // itself (a new `{top,left}` on every resize-triggered recompute would - // otherwise steal focus back to the current-mode item on every resize) - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [variant, open, Boolean(flyoutPos)]) + // the menu keyboard contract (issue #307), from the shared hook: opened from + // the keyboard, focus goes to the first option (the hook waits for the + // flyout to be positioned and visible); opened with the pointer it stays on + // the row. Escape closes this submenu alone and returns to the row, before + // Settings sees the key. The options are radio items: one is checked. + const close = useCallback(() => setOpen(false), [setOpen]) + const { entry, triggerProps } = useMenuTrigger(open, setOpen) + useMenuKeyboard(variant === 'row' && open, panelRef, btnRef, close, entry) const choose = (m: Mode) => { setMode(m) @@ -99,17 +85,6 @@ export function ThemeToggle({ btnRef.current?.focus() } - const onItemKeyDown = (e: KeyboardEvent) => { - const idx = itemRefs.current.findIndex((el) => el === document.activeElement) - if (e.key === 'ArrowDown') { - e.preventDefault() - itemRefs.current[(idx + 1 + MODES.length) % MODES.length]?.focus() - } else if (e.key === 'ArrowUp') { - e.preventDefault() - itemRefs.current[(idx - 1 + MODES.length) % MODES.length]?.focus() - } - } - if (variant === 'row') { return (
@@ -119,8 +94,10 @@ export function ThemeToggle({ className="settings-row" aria-haspopup="menu" aria-expanded={open} - aria-controls={menuId} - onClick={() => setOpen(!open)} + // only while the menu exists (issue #307): an id that names nothing + // while closed stopped Narrator reading "expanded" once it opened + aria-controls={open ? menuId : undefined} + {...triggerProps} > {t('theme.rowLabel')} @@ -141,18 +118,14 @@ export function ThemeToggle({ : { position: 'fixed', top: 0, left: 0, visibility: 'hidden' } } > - {MODES.map((m, i) => ( + {MODES.map((m) => ( {menuOpen && ( -
+
- - @@ -365,7 +384,7 @@ export function MobileMoreMenu({
{/* docs/large-graph-readability.md §LGR3.2 / §LGR9 — Filters + Reset view on mobile. Filters opens a sub-sheet; Reset view is a one-shot. */} - {/* docs/large-graph-readability.md §LGR6 / §LGR9 — on mobile the @@ -431,7 +450,7 @@ export function MobileMoreMenu({ {t('storage.menuLabel')} {temporary ? {t('session.temporary.chip')} : null} -
@@ -462,8 +481,10 @@ export function MobileMoreMenu({ return ( <> closeOverlay('templates')} + onEscape={backToMoreFrom('templates')} returnFocusTo={backToMore} > {TEMPLATES.map((tpl) => ( @@ -483,7 +504,7 @@ export function MobileMoreMenu({ if (overlay === 'export') { return ( <> - closeOverlay('export')} returnFocusTo={backToMore}> + closeOverlay('export')} onEscape={backToMoreFrom('export')} returnFocusTo={backToMore}> @@ -519,7 +540,7 @@ export function MobileMoreMenu({ if (overlay === 'help') { return ( <> - closeOverlay('help')} returnFocusTo={backToMore}> + closeOverlay('help')} onEscape={backToMoreFrom('help')} returnFocusTo={backToMore}> {open ? ( -