Repository navigation
spec(a11y): DropdownMenu and MoreMenu triggers adopt APG Menu Button, arrow-key opening as a named exception - #7459
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
the
spec-auto-merge
|
/approve-spec 4190302 |
…s required parts, with arrow-key opening as a named exception DropdownMenu AR2 and DEC-9: a button with a name, aria-haspopup=menu, aria-expanded and aria-controls; Enter or Space opens with focus on the first enabled row; ArrowDown (first) and ArrowUp (last) opening stay as the named exception the component already ships. MoreMenu AR1 adopts the same through DropdownMenu. ContextMenu and NavMenu are out of scope.
4190302 to
6f70d26
Compare
|
/approve-spec ef8a43b |
PR Analysis ReportView Storybook for this PR · View Sandbox for this PR
Generated by PR Enrichment workflow | View current CI run |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested: the Menu Button adoption matches what DropdownMenu and MoreMenu ship today, but four statements in the new requirements need correcting before this can land.
- AR2 says a link row is exposed as a link, but in the pointer menu the anchor carries
role="menuitem"(FR10), so only the touch sheet's row is a link. - AR4 says a
renderTriggercontrol inherits thebuttonrole and accessible name through the spread props, but those props carry neither; the caller's control supplies them. - The new AR4 and MoreMenu AR1 verification rows cite a Menu Button accessibility contract and bindings that do not exist yet, so cite the existing keyboard-open tests and mark the contract as pending.
- AR4 cites
spec:AST-020/FR15, which is not onmainyet, so land that rule first or drop the citation.
Consider also scoping MoreMenu AR1 to the anchored presentation, since its bottom-sheet trigger says aria-haspopup="dialog".
[Automated review]
| - **AR2 — A row that navigates is a link.** A row with an `href` is the anchor | ||
| itself, so assistive technology exposes it as a link with the row's name, a | ||
| modified click or Enter keeps the browser's meaning, and a disabled link row | ||
| is not activatable. |
There was a problem hiding this comment.
In the pointer menu this anchor carries role="menuitem" (FR10 above; DropdownMenuItem.tsx renders it that way), so it is exposed as a menu item, not as a link. Only the touch sheet's row is a link. Suggested wording: "…is the anchor itself, exposed with the row's name as a menu item (role="menuitem") in the pointer menu and as a link in the touch sheet; …"
| Back, after a pointer open on the drilled list (the `menu`); Back, Escape, or | ||
| ArrowLeft return to the parent view with focus restored. | ||
| - **AR4 — The trigger adopts the APG Menu Button pattern's required parts** | ||
| (`spec:AST-020/FR15`): the control that opens a pointer menu is a `button` |
There was a problem hiding this comment.
spec:AST-020/FR15 is not on main yet. AST-020 currently ends at FR14 and DEC-3, and FR15/DEC-4 live only in the open record PR. If this merges first, the citation dangles and "a named exception under that rule" has no rule. Either land that record first or drop the citation until it is current.
| focus on the first enabled row. As a named exception under that rule, the | ||
| trigger also opens on ArrowDown (focus on the first enabled row) and ArrowUp | ||
| (focus on the last enabled row), the keyboard this component already ships. | ||
| A `renderTrigger` control inherits all of it through the spread props (FR9). |
There was a problem hiding this comment.
DropdownMenuTriggerProps carries ref, id, the handlers, aria-haspopup, aria-expanded and aria-controls, but no role and no name. The custom-trigger test's span has to add role="button" and tabIndex={0} itself. A chip, avatar or list-row control therefore does not inherit the button role or its name. Suggested wording: "A renderTrigger control receives the disclosure attributes and keyboard opens through the spread props (FR9); its button role and accessible name are the caller's control's own."
| | Theming anatomy map | `scripts/check-knowledge.mjs` | Canonical anatomy and current target inventory | Missing, extra, prefixed, stale, or unclassified mappings fail repository validation. | `audit:DropdownMenu/theming` | | ||
| | FR8 | `DropdownMenu.test.tsx` press model suite (trigger cases) | Press-open + drag-release, unsettled opening release, trigger toggle, held touch, tap | A menu that does not open on a mouse press, an unsettled release that acts, or a trigger that reopens in the same gesture fails. | `audit:DropdownMenu/behavior` | | ||
| | FR9 | `DropdownMenu.test.tsx` custom trigger suite; `DropdownMenuSubMenu.test.tsx` custom-trigger Back row cases | Icon button, chip, list row; open, toggle, keyboard open, name; drill-in Back row | A rendered control missing `aria-haspopup`, `aria-expanded` or `aria-controls`, an unnamed menu, a lost keyboard open, or a "Back to Menu" row, fails. | `audit:DropdownMenu/anatomy` | | ||
| | AR4 | Menu Button accessibility contract (`internal/a11y-spec`) bound to the pointer trigger, in jsdom and Chromium; `DropdownMenu.test.tsx` keyboard-open suites | Button and `renderTrigger` controls; closed/open; Enter, Space, ArrowDown, ArrowUp open | The trigger loses its name, `aria-haspopup="menu"`, `aria-expanded` or `aria-controls`; a keyboard open lands focus somewhere other than the first (or, for ArrowUp, last) enabled row | `audit:DropdownMenu/accessibility` | |
There was a problem hiding this comment.
No Menu Button contract exists in internal/a11y-spec/src/patterns, and nothing binds DropdownMenu to one, so this row describes a gate that does not exist yet. The existing jsdom tests cover Enter, ArrowDown and ArrowUp opening; Space is untested. Please cite the existing tests and mark the contract binding as pending.
| | FR1, FR4 | `MoreMenu.test.tsx` render, pointer, bottom-sheet, placement, and controlled suites | Default, custom icon, pointer, touch | Replacing the composition or changing forwarded behavior fails existing render/interaction tests. | `audit:MoreMenu/anatomy` | | ||
| | FR2 | MoreMenu target test, source inspection, and theming target inventories | Closed trigger and composed menu panel | Moving `more-menu` onto the trigger or removing it from the panel fails target evidence. | `audit:MoreMenu/theming` | | ||
| | FR3 | MoreMenu, DropdownMenu, Button, Icon, and List source/owner tests | Trigger, icon branches, pointer rows, touch rows | A composed part loses its owner target or is documented as a new MoreMenu-owned descendant. | `audit:MoreMenu/theming` | | ||
| | AR1 | Menu Button accessibility contract (`internal/a11y-spec`) bound to the MoreMenu trigger, in jsdom and Chromium | Default and custom-icon triggers; closed/open; Enter, Space, ArrowDown, ArrowUp | The trigger loses its label, `aria-haspopup="menu"`, `aria-expanded` or `aria-controls`, or a keyboard open does not land on the first (ArrowUp: last) enabled row | `audit:MoreMenu/accessibility` | |
There was a problem hiding this comment.
Same as DropdownMenu's AR4 row: there is no Menu Button contract or MoreMenu binding yet, and MoreMenu.test.tsx has no keyboard-open or aria-controls case. Please mark this row as pending, or cite the evidence that exists.
| - **AR1 — The trigger is a Menu Button through DropdownMenu.** MoreMenu's | ||
| icon-only trigger adopts the APG Menu Button pattern's required parts as | ||
| `component:DropdownMenu/AR4` records them — `button` role, its accessible | ||
| label as the name, `aria-haspopup="menu"`, `aria-expanded`, `aria-controls`, |
There was a problem hiding this comment.
Non-blocking: MoreMenu forwards presentation. In the bottom-sheet presentation its trigger carries aria-haspopup="dialog" and no aria-controls, and ArrowDown and ArrowUp do not open the sheet. DropdownMenu AR4 is scoped to the pointer menu, so consider "In the anchored presentation, MoreMenu's icon-only trigger…".
… rows keep menuitem in the pointer menu; renderTrigger receives the popup props; verification cites the shipped keyboard-open tests
|
/approve-spec 23875f2 |
There was a problem hiding this comment.
Changes requested: the Menu Button adoption and its arrow-key exception now match what DropdownMenu and MoreMenu ship, but two things need fixing before this lands.
- AR4 (and DEC-9's "adoption rule") cites
spec:AST-020/FR15, which is not onmainyet, so merge #7458 first or drop the citation. - MoreMenu's AR1 verification row cites a pointer-open test as keyboard-open evidence, and DropdownMenu's AR4 row leaves out the existing Enter-open and trigger-attribute tests; cite the tests that cover each part and keep the rest marked pending.
[Automated review]
| Back, after a pointer open on the drilled list (the `menu`); Back, Escape, or | ||
| ArrowLeft return to the parent view with focus restored. | ||
| - **AR4 — The anchored trigger adopts the APG Menu Button pattern's required | ||
| parts** (`spec:AST-020/FR15`): the control that opens the anchored pointer |
There was a problem hiding this comment.
spec:AST-020/FR15 is not on main: AST-020 currently ends at FR14 and DEC-3, and FR15/DEC-4 live only in the open #7458. If this merges first, AR4's basis and the "named exception under that rule" (and DEC-9's "adoption rule") point at a clause that does not exist. Merge #7458 first and rebase, or drop the citation until FR15 is current.
| | Theming anatomy map | `scripts/check-knowledge.mjs` | Canonical anatomy and current target inventory | Missing, extra, prefixed, stale, or unclassified mappings fail repository validation. | `audit:DropdownMenu/theming` | | ||
| | FR8 | `DropdownMenu.test.tsx` press model suite (trigger cases) | Press-open + drag-release, unsettled opening release, trigger toggle, held touch, tap | A menu that does not open on a mouse press, an unsettled release that acts, or a trigger that reopens in the same gesture fails. | `audit:DropdownMenu/behavior` | | ||
| | FR9 | `DropdownMenu.test.tsx` custom trigger suite; `DropdownMenuSubMenu.test.tsx` custom-trigger Back row cases | Icon button, chip, list row; open, toggle, keyboard open, name; drill-in Back row | A rendered control missing `aria-haspopup`, `aria-expanded` or `aria-controls`, an unnamed menu, a lost keyboard open, or a "Back to Menu" row, fails. | `audit:DropdownMenu/anatomy` | | ||
| | AR4 | `DropdownMenu.test.tsx` keyboard-open cases ("walks into a mounted-open menu with ArrowDown from the focused trigger", "keyboard open via ArrowDown skips a disabled leading item", "ArrowUp on the trigger opens with the last row highlighted"); Menu Button accessibility contract (`internal/a11y-spec`, pending) bound to the anchored trigger | Button and `renderTrigger` controls; closed/open; Enter, Space, ArrowDown, ArrowUp open | The trigger loses its name, `aria-haspopup="menu"`, `aria-expanded` or `aria-controls`; a keyboard open lands focus somewhere other than the first (or, for ArrowUp, last) enabled row | `audit:DropdownMenu/accessibility` | |
There was a problem hiding this comment.
This row lists Enter and the trigger's name and ARIA among its states but cites only ArrowDown/ArrowUp cases. Existing cases cover more: "keyboard open via Enter focuses the first enabled item", "has aria-haspopup and aria-expanded attributes", and the custom-trigger suite's aria-controls assertion. Nothing covers Space yet; leave that to the pending contract.
| | FR1, FR4 | `MoreMenu.test.tsx` render, pointer, bottom-sheet, placement, and controlled suites | Default, custom icon, pointer, touch | Replacing the composition or changing forwarded behavior fails existing render/interaction tests. | `audit:MoreMenu/anatomy` | | ||
| | FR2 | MoreMenu target test, source inspection, and theming target inventories | Closed trigger and composed menu panel | Moving `more-menu` onto the trigger or removing it from the panel fails target evidence. | `audit:MoreMenu/theming` | | ||
| | FR3 | MoreMenu, DropdownMenu, Button, Icon, and List source/owner tests | Trigger, icon branches, pointer rows, touch rows | A composed part loses its owner target or is documented as a new MoreMenu-owned descendant. | `audit:MoreMenu/theming` | | ||
| | AR1 | `MoreMenu.test.tsx` "has aria-haspopup and aria-expanded attributes" and "first ArrowDown after a pointer open moves focus to the first item"; Menu Button accessibility contract (`internal/a11y-spec`, pending) bound to the anchored MoreMenu trigger | Default and custom-icon triggers; closed/open; Enter, Space, ArrowDown, ArrowUp | The trigger loses its label, `aria-haspopup="menu"`, `aria-expanded` or `aria-controls`, or a keyboard open does not land on the first (ArrowUp: last) enabled row | `audit:MoreMenu/accessibility` | |
There was a problem hiding this comment.
"first ArrowDown after a pointer open…" opens the menu by click and then checks ArrowDown inside the menu, so it is not evidence for a keyboard open from the trigger; no MoreMenu case opens by keyboard or asserts aria-controls. Cite the DropdownMenu keyboard-open cases MoreMenu inherits (or add a MoreMenu case), and keep the contract marked pending for the rest.
…nd attribute tests exactly
|
/approve-spec 9bceebb |
There was a problem hiding this comment.
Changes requested: the Menu Button adoption and its arrow-key exception match what DropdownMenu and MoreMenu ship, and the new verification rows cite passing tests, but AR4 and DEC-9 rest on spec:AST-020/FR15, which is not on main yet. Please land #7458 first (no change is needed here if FR15 lands as proposed), or drop the citation and the "under that rule" wording until FR15 is current.
[Automated review]
There was a problem hiding this comment.
This review found no blocking issues: with the adoption rule (AST-020 FR15) now on main, the DropdownMenu and MoreMenu Menu Button adoption, its ArrowDown/ArrowUp exception and the AR2/AR3 definitions match that rule and the shipped behavior, and every test the new verification rows cite exists and passes. Two optional follow-ups: AR3 (like FR11) names ArrowLeft as the return key although right-to-left layouts use ArrowRight, and #7352 and #7319 also add a DEC-9 to the DropdownMenu record, so whichever lands later needs to renumber.
[Automated review]
| Back, after a pointer open on the drilled list (the `menu`); Back, Escape, or | ||
| ArrowLeft return to the parent view with focus restored. |
There was a problem hiding this comment.
Optional: in right-to-left layouts the drilled-in list returns on ArrowRight, not ArrowLeft (DropdownMenuSubMenu.tsx picks the close key by direction). FR11 has the same wording, so the two could change together, for example "Back, Escape, or ArrowLeft (ArrowRight in right-to-left)".
This branch was successfully deployed
1 participant
Exact decision needed
Whether DropdownMenu's pointer trigger and MoreMenu's trigger adopt the APG Menu Button pattern's required parts —
buttonrole, accessible name,aria-haspopup="menu",aria-expanded,aria-controls, Enter or Space opening with focus on the first enabled row — plus, as a named exception under the adoption rule, ArrowDown (first row) and ArrowUp (last row) opening, which DropdownMenu already ships. ContextMenu and NavMenu are out of scope.User impact
A keyboard or screen-reader user on a DropdownMenu or MoreMenu trigger gets what the Menu Button pattern promises — a named button that says it opens a menu and whether it is open, and Enter/Space/ArrowDown/ArrowUp landing them on a row — and these records now gate it, so a regression in any of those parts fails the accessibility contract. Nothing changes on screen or in behaviour: every part is shipped today.
Existing authority searched
component:DropdownMenu(draft; FR9 custom-trigger wiring, FR12 arrow wrap inside the open menu, AR1 custom-trigger naming),component:MoreMenu(draft; its Accessibility section delegates to DropdownMenu),spec:AST-020FR15/DEC-4 (the adoption rule: required parts where the role is already played; extras a component already ships are named exceptions) in the open record pull request #7458, APG Menu Button pattern as published (button witharia-haspopup,aria-expanded; Enter/Space/Down Arrow open with focus on the first item, Up Arrow optionally opens with focus on the last). No record decided the adoption before.Decision
Owner decision, 2026-10-13: adopt, with the arrow-key opening exception.
component:DropdownMenuAR4 names the required parts (AR2 and AR3, which the verification map already cited, are now defined: link rows are anchors; a drilled-in sub-menu stays one menu) and the named exception; arenderTriggercontrol inherits them through FR9's spread props. DEC-9 records why the exception stands (shipped behaviour; removing a working keyboard path would cost its users) and the rejected shapes. Verification map gains an AR4 row bound to the Menu Button accessibility contract.component:MoreMenuAR1 adopts the same through DropdownMenu and adds nothing of its own; verification row added.Both records stay
draft; this adds one accessibility requirement to each. The owner's approval is explicit for the exception.Delta
packages/core/src/DropdownMenu/DropdownMenu.spec.md: AR2–AR4, verification row, DEC-9.packages/core/src/MoreMenu/MoreMenu.spec.md: AR1, verification row.Writing rubric results
#digits,/pull/,/issues/as standing content): none.pnpm check:knowledge: clean.