Skip to content

Conversation

niyati34
Copy link

…eyboard navigation (Arrow keys, Home/End); add tests and types

Problem: Toolbar-like groups aren’t fully accessible out of the box; the docs DemoToolbar uses useToolbar to enable Arrow/Home/End navigation but this isn’t available in shipped components.

Approach: Add an opt-in rovingFocus prop to ButtonGroup and ToggleButtonGroup implementing roving focus keyboard navigation:

  • ArrowLeft/Right (or Up/Down for vertical), with RTL-aware swapping

  • Home moves focus to first; End to last

  • No visual/UI regressions; pure behavior addition
    Why this instead of moving docs useToolbar:

  • Maintainer guidance suggested improving ButtonGroup/ToggleButtonGroup

  • Keeps the solution general, composable, and avoids coupling demo-only code into core

  • Backwards compatibility: Default behavior unchanged. rovingFocus is opt-in.

  • Types: Added rovingFocus?: boolean to ButtonGroupProps and ToggleButtonGroupProps.

  • Tests: Added focused tests to verify Arrow/Home/End behavior.

  • Docs: Prop will surface in API docs via existing generation pipeline.

Files changed (key)

  • packages/mui-material/src/ButtonGroup/ButtonGroup.js: add rovingFocus handling; RTL/orientation-aware keyboard navigation.
  • packages/mui-material/src/ButtonGroup/ButtonGroup.d.ts: add rovingFocus?: boolean.
  • packages/mui-material/src/ButtonGroup/ButtonGroup.test.js: add roving focus test.
  • packages/mui-material/src/ToggleButtonGroup/ToggleButtonGroup.js: mirror rovingFocus behavior.
  • packages/mui-material/src/ToggleButtonGroup/ToggleButtonGroup.d.ts: add rovingFocus?: boolean.
  • packages/mui-material/src/ToggleButtonGroup/ToggleButtonGroup.test.js: add roving focus test.

How reviewers can test

  • Typecheck:
npx --yes [email protected] -F @mui/material typescript
  • Docs playground (local dev): already validated; this PR doesn’t include playground edits.

If you want, I can push this branch to your fork and open the PR. Run:

  • Set your fork as origin (if not already), then:
git push -u origin feat/button-groups-roving-focus
  • Open a PR against mui/material-ui master, paste the title and description above.

…eyboard navigation (Arrow keys, Home/End); add tests and types
@mui-bot
Copy link

mui-bot commented Aug 19, 2025

Netlify deploy preview

https://deploy-preview-46789--material-ui.netlify.app/

Bundle size report

Bundle Parsed size Gzip size
@mui/material 🔺+2.02KB(+0.39%) 🔺+819B(+0.54%)
@mui/lab 0B(0.00%) 0B(0.00%)
@mui/system 0B(0.00%) 0B(0.00%)
@mui/utils 0B(0.00%) 0B(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against 9b86c3d

@zannager zannager added scope: toggle button Changes related to the toggle button. component: ButtonGroup The React component. labels Aug 20, 2025
@ZeeshanTamboli ZeeshanTamboli changed the title material[ButtonGroup|ToggleButtonGroup] Add optional roving focus keyboard navigation [ButtonGroup|ToggleButtonGroup] Add optional roving focus keyboard navigation Aug 23, 2025
@ZeeshanTamboli ZeeshanTamboli requested review from siriwatknp and removed request for ZeeshanTamboli August 23, 2025 06:42
@ZeeshanTamboli ZeeshanTamboli changed the title [ButtonGroup|ToggleButtonGroup] Add optional roving focus keyboard navigation [ButtonGroup][ToggleButtonGroup] Add optional roving focus keyboard navigation Aug 23, 2025
@siriwatknp
Copy link
Member

Thanks for submitting the PR but I think the roving logic should live in developer land, not the Material UI.

The reference I'm using is from WAI ARIA - Toolbar where multiple controls are used within the same container.

In your case, I think it makes more sense to export a hook (instead of putting the logic within the component) to let user compose it with components sound like a better approach to me.

I think for next step, it's better to open an issue to discuss what's the proper solution we want to have before creating a PR.
I am closing this PR.

Anyway, thank you for your effort. Your work has a lot of value, so don't me wrong that this PR is closed. Let's proceed by creating an issue first.

@siriwatknp siriwatknp closed this Aug 25, 2025
@siriwatknp siriwatknp reopened this Aug 25, 2025
@siriwatknp siriwatknp closed this Aug 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ButtonGroup The React component. scope: toggle button Changes related to the toggle button.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants