Skip to content

feat: implement keyboard accessibility for Menu (P0 #1) - #10

Open
krantighatpande wants to merge 7 commits into
mainfrom
feat/phase3-p0-keyboard-accessibility
Open

krantighatpande wants to merge 7 commits into
mainfrom
feat/phase3-p0-keyboard-accessibility

Conversation

@krantighatpande

Copy link
Copy Markdown
Collaborator
  • Convert from ARIA Menu pattern to W3C Disclosure Navigation pattern
  • Replace role="menu" with semantic element
  • Add native Tab/Shift+Tab keyboard navigation support
  • Implement disclosure buttons for expandable items without links
  • Add separately focusable chevron buttons for items with links and children
  • Add Space key support for disclosure toggle
  • Remove tabIndex manipulation that blocked native keyboard behavior
  • Add 5 comprehensive keyboard accessibility tests
  • Update Navbar to use navigation role consistently

Implements Phase 3 P0 #1 - Keyboard Accessibility
Follows W3C Disclosure Navigation Pattern
All 86 tests passing

Summary

Type

  • Bug fix
  • Accessibility
  • Security
  • Documentation
  • Performance
  • API change
  • Feature

Validation

  • Unit tests
  • Storybook
  • Playwright
  • Accessibility
  • Package/build

Documentation

  • README updated if needed
  • WDG metadata regenerated if needed

- Convert from ARIA Menu pattern to W3C Disclosure Navigation pattern
- Replace role="menu" with semantic <nav> element
- Add native Tab/Shift+Tab keyboard navigation support
- Implement disclosure buttons for expandable items without links
- Add separately focusable chevron buttons for items with links and children
- Add Space key support for disclosure toggle
- Remove tabIndex manipulation that blocked native keyboard behavior
- Add 5 comprehensive keyboard accessibility tests
- Update Navbar to use navigation role consistently

Implements Phase 3 P0 #1 - Keyboard Accessibility
Follows W3C Disclosure Navigation Pattern
All 86 tests passing
@krantighatpande
krantighatpande requested a review from a team as a code owner October 29, 2026 07:10
- Remove role prop from MenuProps (component always uses <nav>)
- Fix nested interactive elements: chevron button now sibling of link
- Add controls wrapper for link + chevron button layout
- Ensure disabled state respected on all buttons
- Add regression tests for disabled state handling
- Update Navbar and Sidebar to not pass role prop

All 86 tests passing
Fixes nested button-in-link accessibility violation
- Remove Space and Enter handlers from Menu handleKeyDown
- Let native button and link elements handle activation
- Keep optional arrow key navigation for Phase 3
- Clarify that arrow keys are optional enhancements
- Tab/Shift+Tab, Space, and Enter now fully native

All 86 tests passing
Ensures no interference with native keyboard behavior
@krantighatpande
krantighatpande force-pushed the feat/phase3-p0-keyboard-accessibility branch from 442e353 to 0450ce6 Compare October 30, 2026 05:17
…losure button only

Final corrections for Phase 3 P0 #1 review feedback:
- Remove redundant nav landmarks in Sidebar and Navbar
- Keep aria-expanded only on disclosure button, not anchor
- Add regression tests for both corrections
- All 89 tests passing
Final review corrections for Phase 3 P0 #1 Keyboard Accessibility:

- Remove incomplete arrow key handlers (Arrow keys, Home, End) from Menu component
- MenuItem no longer uses focused state for tabIndex, so handlers prevented default without moving focus
- Native Tab/Shift+Tab navigation is sufficient per W3C Disclosure Navigation Pattern
- Add deprecated 'role' prop for backwards API compatibility (Menu always renders as <nav>)
- Add :focus-visible styles for disclosure buttons to ensure visible focus indicators
- Add regression tests for focus indicators on all interactive elements
- Update arrow key tests to reflect handler removal
- All 89 tests passing, TypeScript clean, build successful

Addresses final review feedback items 1, 2, and 3 for PR #10
Final corrections for Phase 3 P0 #1 Keyboard Accessibility:

- Align Menu forwardRef type with rendered <nav> element (HTMLElement not HTMLDivElement)
- Update menuRef type from HTMLDivElement to HTMLElement
- Remove @ts-expect-error from deprecated role prop test (TypeScript correctly accepts the prop)
- All 89 tests passing, TypeScript clean, build successful

Addresses final review feedback for PR #10
Consistency fix for Phase 3 P0 #1 Keyboard Accessibility:

- Navbar: Change forwardRef from HTMLDivElement to HTMLElement (renders <nav>)
- Sidebar: Change forwardRef from HTMLDivElement to HTMLElement (renders <aside>)
- Update useRef types to match forwardRef types
- Ensures all components in this PR have correctly aligned ref types
- All 89 tests passing, TypeScript clean, build successful

Maintains consistency with Menu component forwardRef type alignment

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant