Skip to content

Feature/pagination button hover - #374

Merged
ako3131 merged 7 commits into
mainfrom
feature/pagination-button-hover
Oct 29, 2026
Merged

ako3131 merged 7 commits into
mainfrom
feature/pagination-button-hover

Conversation

@ako3131

@ako3131 ako3131 commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

ref: https://app.clickup.com/t/9014802374/86bcfkrr0

What

Makes the table pagination controls behave the same on hover and keyboard focus, and fixes a hidden Tab stop.

  • Hover: the page button gets a grey hover so first-time users see it's clickable. The arrows and the Per Page dropdown use the same hover.
  • Keyboard focus: all three show the same darker grey on Tab.
  • One shared style: PAGINATION_BUTTON_SX in table-styles.js is used by all three.
  • Per Page dropdown: its border stays grey on hover and focus. The base theme turned it black.
  • Collapsed slider: it's removed from the Tab order and hidden from screen readers. Before, Tab landed on the invisible slider, and the arrow keys could change the page.

@ako3131
ako3131 requested review from santipalenque and a balanced review from Copilot October 27, 2026 21:26
@ako3131 ako3131 self-assigned this Oct 27, 2026
@coderabbitai

coderabbitai Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: ba58f1ee-4df4-4201-8875-7fb407b5b990
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The slider collapses while retaining keyboard focus, allowing arrow keys to change pages invisibly; focus must return to the page button before collapse, with a keyboard regression test covering this path.

1 open issue
What changed in this PR

This PR aligns hover and keyboard-focus styling across the table pagination controls and removes the collapsed slider from the Tab order and accessibility tree.

Changes:

  • Shares hover and focus styles across arrows, the page button, and the Per Page dropdown.
  • Keeps the dropdown border grey on hover and focus.
  • Adds collapsed-state accessibility attributes to the slider.
File Description
src/​components/​mui/​tables/​components/​table-styles.js Defines shared pagination hover and focus styles.
src/​components/​mui/​tables/​components/​SliderPagination.js Applies shared styles and controls collapsed-slider accessibility.
src/​components/​mui/​tables/​components/​CustomTablePagination.js Styles the Per Page dropdown and preserves its grey border.

Review effort: Balanced

Comment thread src/components/mui/tables/components/SliderPagination.js
@ako3131
ako3131 requested a review from santipalenque October 27, 2026 21:49
};

// grey on hover, darker grey on keyboard focus; shared by the pagination arrows, page button and per-page select
export const PAGINATION_BUTTON_SX = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ako3131 can we set these styles in customTheme instead targeting all pagination buttons ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify, you mean move the styling into summit admin?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no @ako3131 there is a custom theme base in uicore that you can use to set these buttons styles

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no @ako3131 there is a custom theme base in uicore that you can use to set these buttons styles

@santipalenque

Copy link
Copy Markdown
Contributor

@ako3131 please put the ticket link in PR description

@ako3131
ako3131 requested a review from smarcet October 28, 2026 20:39
@ako3131
ako3131 force-pushed the feature/pagination-button-hover branch from ae2c7d8 to 4ddaf27 Compare October 29, 2026 00:30
@ako3131
ako3131 requested review from santipalenque and a balanced review from Copilot October 29, 2026 17:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Disabling focus ripples leaves pagination arrows without a keyboard-focus indicator under MUI’s default theme because replacement styles depend on a separately installed custom theme.

2 open issues
1 resolved since last review

Review effort: Balanced

onClick={prev}
disabled={currentPage <= 1}
aria-label={T.translate("mui_table.previous_page")}
disableFocusRipple
},
// per-page select: keep the border grey on rest, hover and focus
"&.pagination-button": {
...paginationButtonStyles(theme),

@santipalenque santipalenque left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ako3131
ako3131 force-pushed the feature/pagination-button-hover branch from 4ddaf27 to 9e6479d Compare October 29, 2026 20:01
@ako3131
ako3131 merged commit 196f1f5 into main Oct 29, 2026
5 checks passed
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.

3 participants