fix: correct pagination semantics on default render path#713
fix: correct pagination semantics on default render path#713ZQDesigned wants to merge 14 commits into
Conversation
# Conflicts: # tests/__snapshots__/demo.test.tsx.snap
|
@ZQDesigned is attempting to deploy a commit to the afc163's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
Walkthrough本次改动为分页组件引入基于原生 button 的默认渲染路径，并同步调整 Pager/Pagination 的交互语义、样式选择器和测试。 Changes默认按钮渲染重构
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the accessibility and semantic structure of the Pagination component by introducing semantic elements for navigation items (prev, next, and jump controls) and supporting both Enter and Space keys for keyboard interaction. The feedback focuses on several key areas of improvement: preventing potential string coercion of undefined in pageLabel which would degrade screen reader experiences, cleaning up unused code (such as the type parameter in renderDefaultControlButton and the PaginationItemType definition), and consistently using undefined instead of null for React props to correctly omit attributes from the DOM.
[!IMPORTANT]
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Pager.tsx (1)
60-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议以显式布尔值判断 disabled，避免依赖类名字符串匹配。
disabled={cls.includes(${prefixCls}-disabled)}依赖对完整类名字符串做子串匹配来推断禁用态。这一方式之所以“能工作”，是因为noPager场景通过className传入的rc-pagination-item-disabled恰好等于${prefixCls}-disabled；但仅用!page又会漏掉该场景。这种与类名字符串的隐式耦合较脆弱，后续若调整类名或新增包含该子串的类名可能引入难以察觉的回归。建议由父组件显式传入禁用标记并直接使用。♻️ 可选实现思路（引入显式 disabled 语义）
export interface PagerProps extends Pick<PaginationProps, 'itemRender'> { rootPrefixCls: string; page: number; pageLabel?: string; defaultItemRender?: boolean; + disabled?: boolean; active?: boolean;- disabled={cls.includes(`${prefixCls}-disabled`)} + disabled={!page || disabled}调用处（
Pagination.tsx的noPager分支）以disabledprop 取代className={${prefixCls}-item-disabled}传值。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Pager.tsx` around lines 60 - 75, `Pager` 里按钮的 `disabled` 现在通过 `cls.includes(...)` 依赖类名字符串推断，和样式命名耦合过紧。请在 `Pager.tsx` 的 `defaultItemRender`/按钮渲染逻辑中改为直接使用显式的禁用标记，并让 `Pagination.tsx` 的 `noPager` 分支把这个状态作为 prop 传给 `Pager`，不要再用 `className` 间接表达。这样 `disabled` 的判断只依赖业务状态而不是 `prefixCls` 生成的类名。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Pager.tsx`:
- Around line 60-75: `Pager` 里按钮的 `disabled` 现在通过 `cls.includes(...)`
依赖类名字符串推断，和样式命名耦合过紧。请在 `Pager.tsx` 的 `defaultItemRender`/按钮渲染逻辑中改为直接使用显式的禁用标记，并让
`Pagination.tsx` 的 `noPager` 分支把这个状态作为 prop 传给 `Pager`，不要再用 `className` 间接表达。这样
`disabled` 的判断只依赖业务状态而不是 `prefixCls` 生成的类名。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6b08f5be-46ed-4571-b853-5ad498f1c856
⛔ Files ignored due to path filters (2)
tests/__snapshots__/demo.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/simple.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
assets/index.lesssrc/Pager.tsxsrc/Pagination.tsxtests/index.test.tsxtests/itemRender.test.tsxtests/jumper.test.tsx
There was a problem hiding this comment.
Pull request overview
This PR refines rc-pagination’s default DOM structure to use native <button> controls for pagination interactions (prev/next/jump/page), keeping <li> elements as list semantics only, and updates styling/tests/snapshots accordingly to address remaining accessibility issues after #690.
Changes:
- Move default pagination interaction/ARIA attributes from wrapper
<li>nodes to inner native<button>elements. - Flatten interactive icon wrappers on the default path to avoid nested interactive controls when consumers pass
<button>/<a>icons. - Update Less styles plus unit tests and snapshots to match the new semantic DOM.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/jumper.test.tsx | Adjust DOM queries to target the new inner <button> for next navigation. |
| tests/itemRender.test.tsx | Add coverage for custom itemRender fallback wrapper interaction; update imports for new events. |
| tests/index.test.tsx | Update assertions and keyboard interaction tests to use the new button-based DOM (including userEvent). |
| tests/snapshots/simple.test.tsx.snap | Snapshot updates reflecting button-based semantics and moved ARIA attributes. |
| tests/snapshots/demo.test.tsx.snap | Snapshot updates reflecting button-based semantics and moved ARIA attributes across demos. |
| src/Pagination.tsx | Core logic to render native buttons on the default path; flatten icon wrappers; move ARIA/title to actual controls. |
| src/Pager.tsx | Page items render as native buttons on the default path; non-default path keeps wrapper-driven interaction. |
| assets/index.less | Style updates so <button>-based items match existing visuals, including focus/hover/disabled states. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
This is a follow-up fix for the accessibility changes in #690.
The previous update improved labels, but
antdintegration still exposed a few a11y issues on the default render path:ul > liprevIcon/nextIconThis change keeps the useful a11y improvements from #690, while fixing the remaining semantic issues.
Changes
buttonsprev/next/jump-prev/jump-nextrender inner nativebuttonslikeeps list semantics onlyaria-label,aria-current, andtitleto the real interactive elementitemRendercompatibility fallback for custom renderersrole="button"aria-hiddenbuttonoraasprevIcon/nextIcon/ jump icons, only their content is reusedTests
itemRenderfallback behaviorVerification
Verified in
rc-pagination:npx rc-test --no-watchmanVerified against
antdintegration with the packed local build:components/pagination/__tests__/a11y.test.tscomponents/list/__tests__/a11y.test.tscomponents/table/__tests__/a11y.test.tsRelated
Summary by CodeRabbit
defaultItemRender控制是否走默认渲染分支，并调整页码标签回退逻辑。