feat: add recommendPage to onChange when pageSize changes#708
Conversation
- Add third parameter recommendPage to onChange callback - calculatePage logic to preserve user's data position when changing pageSize - Related issue: ant-design/ant-design#30610 Co-Authored-By: Claude <noreply@anthropic.com>
|
Someone is attempting to deploy a commit to the afc163's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough分页尺寸变更时新增了推荐页码信息，并通过 ChangesonChange 新增 recommendPage 参数
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Pagination
participant InternalState
participant onChange回调
Pagination->>Pagination: 计算 preservedPage / newCurrent / recommendPage
Pagination->>InternalState: 更新 pageSize 与 current
Pagination->>onChange回调: onChange(nextCurrent, size, { recommendPage })
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 introduces a new recommendPage parameter to the onChange callback in the Pagination component, designed to help preserve the user's data position when the page size changes. The changes include updates to the component implementation, TypeScript interfaces, documentation, and test cases. The reviewer identified an issue with the recommended page calculation, noting that using Math.ceil can cause users to skip data when reducing the page size. They suggested a more precise formula using Math.floor to target the start of the previous page range, along with corresponding updates to the test assertions.
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)
tests/sizer.test.tsx (1)
53-70: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win补一个
recommendPage !== current的回归用例。当前断言只覆盖第一页，无法防止保留位置公式回退。建议增加一个后续页场景，例如
defaultCurrent={6}、20 -> 50时应回调(6, 50, 3)。建议补充测试
it('should onChange called when pageSize change', () => { const onChange = jest.fn(); const { container, getByRole } = render( <Pagination sizeChangerRender={sizeChangerRender} onChange={onChange} total={500} defaultPageSize={20} />, ); fireEvent.mouseDown(getByRole('combobox')); expect(container.querySelectorAll('.rc-select-item')[2]).toHaveTextContent( '50 条/页', ); const pageSize1 = container.querySelectorAll('.rc-select-item')[0]; fireEvent.click(pageSize1); expect(onChange).toHaveBeenCalled(); expect(onChange).toHaveBeenLastCalledWith(1, 10, 1); }); + + it('should recommend preserved page when pageSize changes from later page', () => { + const onChange = jest.fn(); + const { container, getByRole } = render( + <Pagination + sizeChangerRender={sizeChangerRender} + onChange={onChange} + total={500} + defaultCurrent={6} + defaultPageSize={20} + />, + ); + + fireEvent.mouseDown(getByRole('combobox')); + const pageSize50 = container.querySelectorAll('.rc-select-item')[2]; + fireEvent.click(pageSize50); + + expect(onChange).toHaveBeenLastCalledWith(6, 50, 3); + });🤖 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 `@tests/sizer.test.tsx` around lines 53 - 70, Add a regression test in the Pagination sizing spec to cover the recommendPage !== current case, since the current assertion only checks page 1 and won’t catch a preserved-position regression. Update the test around the existing Pagination/onChange flow in sizer.test.tsx to use a later page scenario such as defaultCurrent={6} with a page-size change from 20 to 50, and assert that the Pagination callback receives the recomputed page value (for example, (6, 50, 3)) instead of always snapping to 1.
🤖 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 `@tests/sizer.test.tsx`:
- Around line 53-70: Add a regression test in the Pagination sizing spec to
cover the recommendPage !== current case, since the current assertion only
checks page 1 and won’t catch a preserved-position regression. Update the test
around the existing Pagination/onChange flow in sizer.test.tsx to use a later
page scenario such as defaultCurrent={6} with a page-size change from 20 to 50,
and assert that the Pagination callback receives the recomputed page value (for
example, (6, 50, 3)) instead of always snapping to 1.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9bd13a2c-d84d-4cbc-b3cd-7650d3fd5bce
📒 Files selected for processing (3)
src/Pagination.tsxtests/simple.test.tsxtests/sizer.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/simple.test.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #708 +/- ##
=======================================
Coverage 99.69% 99.69%
=======================================
Files 4 4
Lines 329 331 +2
Branches 151 152 +1
=======================================
+ Hits 328 330 +2
Misses 1 1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…ution Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/interface.ts (1)
61-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value可选：将 info 对象类型提取为具名类型。
内联的
{ recommendPage?: number }类型目前仅在此处出现一次，若后续需要在外部（如消费者代码、文档）引用该类型，建议提取为具名接口并导出，便于复用和后续扩展字段。♻️ 提取具名类型示例
+export interface PaginationOnChangeInfo { + recommendPage?: number; +} + export interface PaginationProps extends Partial<PaginationData>, React.AriaAttributes { onChange?: ( page: number, pageSize: number, - info?: { recommendPage?: number }, + info?: PaginationOnChangeInfo, ) => void;🤖 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/interface.ts` around lines 61 - 65, The inline info parameter type in onChange is only used once and should be extracted into a named, exported type for reuse and future extension. Update the interface in interface.ts by introducing a dedicated type for the info object, give it a descriptive name that reflects recommendPage, and reference that type from onChange so consumers and docs can import it directly.
🤖 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/interface.ts`:
- Around line 61-65: The inline info parameter type in onChange is only used
once and should be extracted into a named, exported type for reuse and future
extension. Update the interface in interface.ts by introducing a dedicated type
for the info object, give it a descriptive name that reflects recommendPage, and
reference that type from onChange so consumers and docs can import it directly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 208f37a4-5af3-4aab-af18-d5035385054f
📒 Files selected for processing (6)
docs/examples/jumper.tsxsrc/Pagination.tsxsrc/interface.tstests/index.test.tsxtests/simple.test.tsxtests/sizer.test.tsx
✅ Files skipped from review due to trivial changes (3)
- tests/sizer.test.tsx
- tests/simple.test.tsx
- docs/examples/jumper.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/index.test.tsx
- src/Pagination.tsx
这个 PR 在做什么这个 PR 给 结论有条件合并 核心实现方向没看到明显逻辑问题； 问题清单🔴 高优先级（阻塞合并）无。 🟡 中优先级（建议修复）
🟢 低优先级（可选改进）
|
EmilyyyLiu
force-pushed
the
feat/recommend-page-on-size-change
branch
from
7632736 to
b8cc552
Compare
2 participants
需求背景
当用户切换 pageSize 时，当前页码有时会被重置为 1，用户体验不佳。特别是当用户浏览到很后面的页码，想增加每页显示数量时，需要重新回到前面查找之前看的数据。
相关 issue: ant-design/ant-design#30610
解决方案
给
onChange增加第三个参数recommendPage，只在切换 pageSize 时传递，用于计算保持用户当前数据位置的页码。onChange(page, pageSize)onChange(current, newSize, recommendPage)使用方可以选择是否采纳推荐页码，完全向后兼容。
更新日志
recommendPageparameter toonChangecallback when pageSize changesonChange增加recommendPage参数，切换 pageSize 时提供推荐页码Summary by CodeRabbit
onChange回调新增第三个参数info，可选包含recommendPage推荐页码，便于外部同步更合适的位置。onChange(page, pageSize, info?)及info.recommendPage的提供时机。