[MUI] [Button,List,ToggleButton] Integrate useRovingTabIndex for Button, ButtonGroup, List, ListItemButton, ToggleButton, and ToggleButtonGroup #48321
Conversation
…oved keyboard navigation in Button, ButtonGroup, List, ListItemButton, ToggleButton, and ToggleButtonGroup components
Bundle size
Deploy previewhttps://deploy-preview-48321--material-ui.netlify.app/ Check out the code infra dashboard for more information about this PR. |
mj12albert
left a comment
mj12albert
left a comment
There was a problem hiding this comment.
- It would better if this PR was scoped to one component first
- The new behavior must be opt-in somehow, it would be better to discuss how to do this in the issue first
…ngTabIndex integration for simplified component structure
I have updated the pr to include only List component first |
| @@ -5,6 +5,7 @@ import clsx from 'clsx'; | |||
| import composeClasses from '@mui/utils/composeClasses'; | |||
There was a problem hiding this comment.
It's a huge breaking change for this to unconditionally become a roving focus parent
There was a problem hiding this comment.
You're absolutely right — this is a breaking change that shouldn't be unconditional.
how about this approach:
Add an optional prop: disableRovingTabIndex (or enableRovingTabIndex={false} by default)
Default to disabled (backward compatible)
Users explicitly opt-in when they want roving behavior
| {...rovingContainerProps} | ||
| {...other} |
There was a problem hiding this comment.
roving focus event handlers can be easily overwritten by accident silently
There was a problem hiding this comment.
should i Extracting event handlers separately and create wrapper functions that call both roving handlers and user callbacks?
2 participants
Uh oh!
There was an error while loading. Please reload this page.