[table] Improve a11y for the pagination buttons - #49112
Conversation
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
There was a problem hiding this comment.
🔵 Needs a closer look
It changes focus-management behavior in a core component and should get final human validation across real-world browser/AT scenarios despite the added tests.
Pull request overview
This PR updates TablePaginationActions to replace native title attributes with MUI Tooltip for action labels and adds focus management so keyboard focus moves to the nearest enabled pagination action when the currently focused action becomes disabled.
Changes:
- Wrap pagination action buttons with
Tooltipwhile preservingaria-labelbehavior, RTL slot mapping, and slot customization. - Add layout-effect-based focus recovery when a focused action becomes disabled (including via
slotProps). - Expand unit/browser-only coverage for tooltip visibility, slot/title preservation, and focus management scenarios.
File summaries
| File | Description |
|---|---|
| packages/mui-material/src/TablePaginationActions/TablePaginationActions.js | Replace native titles with Tooltip, add focus tracking + focus relocation when a focused action becomes disabled, and preserve slot/ref behavior. |
| packages/mui-material/src/TablePaginationActions/TablePaginationActions.test.js | Add tests for tooltip behavior (hover/focus), custom slot/title preservation (LTR/RTL), and non-jsdom focus management behaviors. |
Review details
Suppressed comments (1)
packages/mui-material/src/TablePaginationActions/TablePaginationActions.js:162
getItemAriaLabel('last', page)is currently evaluated in the slotProps destructuring even whenshowLastButtonis false, which does extra work for a button that won’t render. Guarding the default initializer avoids callinggetItemAriaLabelwhen the last button is hidden.
const { title: lastButtonTitle = getItemAriaLabel('last', page), ...lastButtonSlotProps } =
(isRtl ? slotProps.firstButton : slotProps.lastButton) ?? {};
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
59d49d8 to
fba526f
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Custom function slots without forwarded refs break tooltip display and focus restoration on supported React versions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
mnajdova
left a comment
There was a problem hiding this comment.
Can we just show one Tooltip at a time? If one button is focused and the other hovered they overlap. I know in Base UI we have this feature about tooltip provider, I don't remember if we have something similar in Material UI, or if it is easy to achieve. If not, let's add comment to fix this in the next major once we migrate the Tooltip to Base UI.
| import useForkRef from '../utils/useForkRef'; | ||
| import { getTablePaginationActionsUtilityClass } from './tablePaginationActionsClasses'; | ||
|
|
||
| function isDisabled(button) { |
There was a problem hiding this comment.
I can see this logic in other places too, could be useful to have it extracted in @mui/material/utils so potentially it can be reused.
There was a problem hiding this comment.
I couldn't find other places, found a couple which partially use some of it, but that's it.
| other.onBlur?.(event); | ||
| }; | ||
|
|
||
| useEnhancedEffect(() => { |
There was a problem hiding this comment.
Let's add a comment what the effect is about.
There was a problem hiding this comment.
🟡 Changes recommended
The generated PropTypes additions need corresponding TypeScript declarations and regeneration.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
410002f to
7ef866a
Compare
There was a problem hiding this comment.
🟡 Changes recommended
A nullish custom aria-label can now leave an action without an accessible name.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
| } | ||
|
|
||
| // Prefer following actions, then fall back to preceding actions in reverse order. | ||
| const nextButton = [...buttons.slice(index + 1), ...buttons.slice(0, index).reverse()].find( |
mnajdova
left a comment
There was a problem hiding this comment.
Logic looks good, I left few comments on improving the docs. Please resolve them before merging.
| ### Custom pagination actions | ||
|
|
||
| The `ActionsComponent` prop of the `TablePagination` component allows the implementation of custom actions. | ||
| A replacement action component is responsible for its own tooltips and focus management, unless it composes the built-in `TablePaginationActions` component. |
There was a problem hiding this comment.
| A replacement action component is responsible for its own tooltips and focus management, unless it composes the built-in `TablePaginationActions` component. | |
| The replacement action component is responsible for its own tooltips and focus management, unless it composes the built-in `TablePaginationActions` component. |
| The default buttons include tooltips. | ||
| When replacing `firstButton`, `previousButton`, `nextButton`, or `lastButton` through `slots.actions`, the custom component receives a `title` prop and is responsible for rendering its own tooltip. | ||
|
|
||
| When an update to pagination props or action slot props disables the focused action, `TablePaginationActions` moves focus to the next enabled action in DOM order, or searches backward if none follows. |
There was a problem hiding this comment.
| When an update to pagination props or action slot props disables the focused action, `TablePaginationActions` moves focus to the next enabled action in DOM order, or searches backward if none follows. | |
| When an update to pagination props or action slot props disables the focused action, `TablePaginationActions` moves focus to the nearest enabled action in the DOM order. |
| Actions with `tabIndex={-1}` are skipped. | ||
| Focus stays where it is if no action is available or your application has already moved focus elsewhere. | ||
|
|
||
| Custom button slots participate in focus restoration by forwarding the supplied `data-mui-pagination-action` prop to their focusable element. |
There was a problem hiding this comment.
| Custom button slots participate in focus restoration by forwarding the supplied `data-mui-pagination-action` prop to their focusable element. | |
| In order for custom button slots to participate in focus restoration you need to pass the `data-mui-pagination-action` prop to their focusable element. |
Maybe also list the available values for it, as they are not documented anywhere at this moment.
ecff5e7 to
d15266c
Compare
d15266c to
f089ea2
Compare
Wrap buttons in Tooltip
Replace native
titlehints on TablePaginationActions with Tooltip so action labels appear on hover and keyboard focus.Restore focus when buttons become disabled
When a focused action becomes disabled, move focus to the nearest enabled action instead of losing it to the document body. For example, navigating to the last page moves focus from Next to Previous. The search also handles actions disabled independently through
slotPropsand respects focus moved elsewhere by the application.Preserves accessible labels, custom titles, button slots, refs, and RTL behavior.