From e961efe8002ea6ec33ed75938fd87034ec1098da Mon Sep 17 00:00:00 2001 From: Hanrim <148833226+MerciHanrim@users.noreply.github.com> Date: Mon, 5 Oct 2026 17:58:31 +0900 Subject: [PATCH] fix(menus): one keyboard contract for every menu, and the phone's sheets step back one level (v0.18.1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of #307. Version 0.18.1, release date 2026-10-06. Every desktop menu button (Templates, Insert module, File, Data, Help, the temporary-session chip, Settings > Theme and the Distribution export) shares useMenuTrigger and useMenuKeyboard in src/ui/useMenuKeyboard.ts: Enter, Space or a screen reader's activation opens at the first item, Arrow Down or Arrow Up on the closed button at the first or last; inside, the arrows wrap, Home and End jump, and disabled, hidden and separator entries are skipped; Escape closes and returns to the button; Tab and Shift+Tab close the menu (flushSync, so the popup is gone before the default action) and move to the element after or before the button, never between items; typeahead is left out. After a keyboard choice that opens no dialog, focus returns to the button; a dialog takes focus and returns it there; a pointer open or choice never moves focus. Theme's three choices are menuitemradio with aria-checked. Settings, the overflow button, File, the temporary-session chip and Theme carry aria-controls only while their panel exists (it mounts on open): measured with Narrator and Edge on a comparison page, a button whose aria-controls named an absent panel while closed was never read as expanded after opening, and with the attribute only while open both expanded and collapsed were read at once. Settings and the overflow button are disclosures: no role="menu", aria-expanded and aria-controls on the button, a labelled group, Tab-only movement, focus left on the button when opened, Escape back to it; a keyboard choice in a group nested in the overflow returns to the overflow button. Language keeps its combobox with real focus in the search field and aria-activedescendant; Arrow Down or Arrow Up on its closed row opens it at the first or last language. Help keeps its issue #306 behaviour on the shared hook. On a phone a sheet is not modal (docs/mobile.md MV-D11, MV-D18 and MV-D19 are unchanged): no aria-modal and no Tab trap, so the run bar and the PWA update bar stay usable by pointer, keyboard and screen reader while it is open, Play runs without closing it, and Tab follows the page's order out of it; only what its scrim covers for the pointer (the canvas, the top bar's More button, the open-file card, marked data-covered-by-sheets) is inert while it is open. Focus enters a sheet when it opens; Escape in a sheet opened from More (Templates, Export, Filters, Help, Share) goes back to More with focus on the row that opened it, while Close and the scrim still close everything. Every dialog and sheet registers in src/ui/overlayStack.ts through useDialogFocus: the top one alone answers Escape, and a real dialog (aria-modal) is modal for real at any width: while it is the top one everything outside it is inert, the run bar and the PWA update bar included, except pure live regions (no control inside), and what mounts meanwhile is made inert as it arrives (MutationObserver); with two modals only the top one is reachable. "The dialog" is its whole modal layer, the nearest data-modal-layer ancestor, which the guided tour sets so its own scrim and spotlight are not made inert with the page. The stack releases only the inert it set itself, and only when the entry that set it closes. The update bar's z-index moves below the tour and the dialogs (sheet < run bar < update bar < tour < dialogs), so a real dialog covers it; it keeps its state and is back and usable when the dialog closes (docs/mobile.md MV8a rewritten to say so). A dialog opened from a sheet returns focus on close to the control inside the sheet that opened it, and the sheet is non-modal again. New e2e/modal-inert.spec.ts checks pointer, Tab and the accessibility tree with About, an update arriving during About, the guided tour, the phone's MC dialog and the Storage dialog over the More sheet, before and after closing, the tour's own scrim taking its click, what a sheet's scrim covers staying inert after a dialog over it closes, and, driven through the stack, two nested modals, an element inert before anything opened, a late element, and no observer left once the last modal closes. Three release-note lines in 18 languages; the 16 other than English and Korean have not been reviewed by a native speaker. The per-language copy tests move their pinned counts by the three keys and declare the key names (Home, End, Tab, Escape) where each guard asks; two of the three pt-BR and pt-PT lines are worded the same in both, so the pt-PT audit's differing keys (DELTA) move from 268 to 269 and the differing keys outside the 33 password keys (restDelta) from 240 to 241, still under the unchanged bound of a quarter of the 966 non-password keys (241.5). New e2e/menu-keyboard.spec.ts; existing specs follow the new roles, entry points and Escape steps. Docs: docs/toolbar-responsive.md (the menu keyboard contract), docs/mobile.md §MV5, CHANGELOG.md and README.md. --- .changes/menu-keyboard.json | 1 + CHANGELOG.md | 11 + README.md | 24 +- docs/guided-tour.md | 8 +- docs/mobile.md | 45 +- docs/toolbar-responsive.md | 54 ++ e2e/data-import-guide.spec.ts | 4 +- e2e/menu-keyboard.spec.ts | 569 +++++++++++++++++++ e2e/mobile.spec.ts | 18 +- e2e/modal-inert.spec.ts | 292 ++++++++++ e2e/storage-sessions.spec.ts | 3 +- e2e/theme-boot.spec.ts | 2 +- e2e/theme-submenu.spec.ts | 23 +- e2e/toolbar-responsive.spec.ts | 4 +- e2e/whats-new.spec.ts | 2 +- package-lock.json | 4 +- package.json | 2 +- src/components/Canvas.tsx | 1 + src/components/DistributionPanel.tsx | 22 +- src/components/GuidedTour.tsx | 8 +- src/components/HelpMenu.tsx | 16 +- src/components/LanguageSwitch.tsx | 11 +- src/components/ModuleMenu.tsx | 10 +- src/components/SessionChip.tsx | 11 +- src/components/Templates.tsx | 10 +- src/components/ThemeToggle.tsx | 61 +- src/components/dataImport/DataImportMenu.tsx | 32 +- src/components/mobile/MobileMoreMenu.tsx | 39 +- src/components/mobile/MobileOpenFileHint.tsx | 2 +- src/components/mobile/MobileSheet.tsx | 19 +- src/components/mobile/MobileTopBar.tsx | 1 + src/components/toolbar/FileMenu.tsx | 11 +- src/components/toolbar/OverflowMenu.tsx | 16 +- src/components/toolbar/SettingsMenu.tsx | 15 +- src/components/useDialogFocus.ts | 36 +- src/i18n/es419Copy.test.ts | 4 + src/i18n/itCopy.test.ts | 9 +- src/i18n/locales/ar/ui.ts | 3 + src/i18n/locales/de/ui.ts | 3 + src/i18n/locales/en/ui.ts | 3 + src/i18n/locales/es-419/ui.ts | 3 + src/i18n/locales/es-ES/ui.ts | 3 + src/i18n/locales/fr/ui.ts | 3 + src/i18n/locales/it/ui.ts | 3 + src/i18n/locales/ja/ui.ts | 3 + src/i18n/locales/ko/ui.ts | 3 + src/i18n/locales/nl/ui.ts | 3 + src/i18n/locales/pt-BR/ui.ts | 3 + src/i18n/locales/pt-PT/ui.ts | 3 + src/i18n/locales/ru/ui.ts | 3 + src/i18n/locales/th/ui.ts | 3 + src/i18n/locales/tr/ui.ts | 3 + src/i18n/locales/vi/ui.ts | 3 + src/i18n/locales/zh-Hans/ui.ts | 3 + src/i18n/locales/zh-Hant/ui.ts | 3 + src/i18n/nlCopy.test.ts | 8 +- src/i18n/ptBrCopy.test.ts | 2 + src/i18n/ptPtCopy.test.ts | 9 +- src/i18n/ruCopy.test.ts | 2 +- src/i18n/thCopy.test.ts | 15 +- src/i18n/trCopy.test.ts | 2 +- src/i18n/viCopy.test.ts | 16 +- src/index.css | 18 +- src/releaseNotes/releaseNotes.ts | 9 + src/ui/overlayStack.ts | 132 +++++ src/ui/useMenuKeyboard.ts | 209 +++++-- 66 files changed, 1636 insertions(+), 240 deletions(-) create mode 100644 .changes/menu-keyboard.json create mode 100644 e2e/menu-keyboard.spec.ts create mode 100644 e2e/modal-inert.spec.ts create mode 100644 src/ui/overlayStack.ts 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 ? ( -