-
Notifications
You must be signed in to change notification settings - Fork 271
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(pagination): data abstraction via hooks #2988
base: feat_v3.x
Are you sure you want to change the base?
Conversation
Walkthrough该提交引入了一个新的分页钩子(usePagination),用于集中管理分页相关的逻辑。新的钩子在 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户
participant PC as 分页组件
participant UP as usePagination 钩子
User->>PC: 点击分页按钮或前/后翻页按钮
PC->>UP: 调用 usePagination(total, current, ...options)
UP-->>PC: 返回 pages 与 pageCount
PC->>PC: 更新状态(current)并重新渲染分页组件
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #2988 +/- ##
=============================================
+ Coverage 85.79% 85.84% +0.04%
=============================================
Files 281 282 +1
Lines 18527 18589 +62
Branches 2810 2818 +8
=============================================
+ Hits 15896 15957 +61
- Misses 2626 2627 +1
Partials 5 5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/packages/pagination/pagination.tsx (1)
142-142
: 静态分析提示测试覆盖率不足。
第 142 行涉及渲染处理,可能在测试中未覆盖mode="lite"
或者特殊场景。可添加对这部分渲染和点击交互的测试用例,以提高整体测试覆盖率。需要帮助编写测试用例吗?我可以提供一个基础测试方案来确保此代码路径得到充分验证。
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 142-142: src/packages/pagination/pagination.tsx#L142
Added line #L142 was not covered by testssrc/packages/pagination/pagination.taro.tsx (1)
95-99
: 前一页按钮的可用状态样式逻辑清晰,但可考虑辅助文本提示。
建议在禁用时加上相应的aria-disabled
等属性以增强可访问性。<View className={classNames({ ... })} + aria-disabled={current === 1} onClick={() => prevPage()} >src/hooks/use-pagination.ts (2)
30-32
:human2Machine
函数命名直观,但使用自减运算符可能引发误解。
减少一行后再返回,容易让读者对返回值及副作用判断不清,可将number - 1
直接写在 return 中以提升可读性。-function human2Machine(number: number) { - return --number +function human2Machine(number: number) { + return number - 1 }
52-53
: 静态分析提示分支场景的测试覆盖率不足。
在calculateButtons
内部,当尾部触达极限或首部边界时,会触发不同的start/end
调整逻辑;静态分析的警告显示这部分可能缺乏测试用例。可通过添加极端场景(大分页数、小分页数等)进行验证,确保所有分支能正确执行。是否需要我为这些分支逻辑提供示例测试代码,以便及时补充测试覆盖率?
Also applies to: 55-56
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 52-53: src/hooks/use-pagination.ts#L52-L53
Added lines #L52 - L53 were not covered by tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/nutui-taro-demo/config/index.js
(1 hunks)packages/nutui-taro-demo/tsconfig.json
(1 hunks)src/hooks/use-pagination.ts
(1 hunks)src/packages/pagination/pagination.taro.tsx
(4 hunks)src/packages/pagination/pagination.tsx
(4 hunks)tsconfig.h5.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tsconfig.h5.json
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/pagination/pagination.tsx
[warning] 142-142: src/packages/pagination/pagination.tsx#L142
Added line #L142 was not covered by tests
src/hooks/use-pagination.ts
[warning] 52-53: src/hooks/use-pagination.ts#L52-L53
Added lines #L52 - L53 were not covered by tests
[warning] 55-56: src/hooks/use-pagination.ts#L55-L56
Added lines #L55 - L56 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (9)
src/packages/pagination/pagination.tsx (3)
59-64
: 在usePropsValue
中使用current
状态没有问题,命名清晰且与组件语义相符。
此处逻辑简单明了,能够有效同步外部value
与内部状态,不存在明显问题。
74-78
: 点击事件的边界检查非常必要,避免了无效操作。
对于超出范围或已经选中的页码进行了短路处理,可读性和安全性都较好。
125-125
: 在简单模式下显示当前页码与总页数,能保持简洁,但仍需注意用例覆盖。
建议在测试中包含mode="simple"
的场景,确保current
与pageCount
的计算与渲染在极端值时也能正确工作。src/packages/pagination/pagination.taro.tsx (2)
76-80
: 点击事件边界校验合理,可避免无效或重复操作。
与普通版组件的要点保持一致,防止用户在边界外翻页导致状态异常。
142-143
: 与行 142-143 相关的颜色设置需完善测试覆盖率。
同样地,如果在某些模式下next
按钮在边界不可用,需测试并验证addColorForHarmony
对配色的处理是否符合预期。src/hooks/use-pagination.ts (1)
76-97
: “省略符号”逻辑设计良好,可读性高。
在项目需求中常见此模式,可灵活隐藏中间页码。整体实现足以应对大部分分页场景,无需额外改动。packages/nutui-taro-demo/config/index.js (1)
123-123
: 新增 hooks 别名配置正确!新增的
@/hooks
别名配置遵循了项目现有的别名命名规范,路径解析也正确指向了 hooks 目录。这个改动很好地支持了通过 hooks 实现数据抽象的目标。packages/nutui-taro-demo/tsconfig.json (2)
34-36
: TypeScript 路径映射配置完善!新增的
@/hooks
路径映射与现有配置保持一致,正确使用了通配符模式。这个配置确保了 TypeScript 能够正确解析 hooks 模块的导入。
123-123
:✅ Verification successful
验证 hooks 配置的一致性
建议验证 hooks 目录结构是否已经建立,确保配置能够正常工作。
Also applies to: 34-36
🏁 Script executed:
#!/bin/bash # 验证 hooks 目录是否存在且包含文件 test -d src/hooks && find src/hooks -type f -name "*.ts" -o -name "*.tsx"Length of output: 102
验证 hooks 配置一致性
经过验证,项目中
src/hooks/
目录已存在,并且包含诸如src/hooks/use-pagination.ts
文件,证明 hooks 配置在各文件中是一致且正确的。同时,其他提到的配置(如 34-36 行)也保持了相应的一致性。请确保所有 hooks 文件维持该配置要求。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/packages/pagination/pagination.tsx (2)
74-86
: 建议优化事件处理函数的结构!通过使用提前返回(early return)模式可以使代码更清晰:
const prevPage = () => { - const prev = current - 1 - prev >= 1 && setCurrent(prev) + if (current <= 1) return + setCurrent(current - 1) } const nextPage = () => { - const next = current + 1 - next <= pageCount && setCurrent(next) + if (current >= pageCount) return + setCurrent(current + 1) }
142-142
: 需要补充测试用例!
lite
模式的渲染逻辑缺少测试覆盖。建议添加以下场景的测试:
- 验证当前页码显示
- 验证分隔符显示
- 验证总页数显示
需要我帮您生成测试用例吗?
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 142-142: src/packages/pagination/pagination.tsx#L142
Added line #L142 was not covered by testssrc/packages/pagination/pagination.taro.tsx (2)
130-130
: 可以简化字符串模板的使用!-{`${current}/${pageCount}`} +{current}/{pageCount}当前的模板字符串用法过于复杂,可以简化为直接的 JSX 表达式。
1-160
: 建议提取共享逻辑!Web 版本和 Taro 版本的分页组件存在大量重复代码。建议:
- 创建一个共享的基础组件类,包含通用的分页逻辑
- Web 和 Taro 版本继承这个基础类,只处理平台特定的渲染逻辑
- 使用组合模式将通用的计算逻辑与平台特定的 UI 渲染分离
这样可以:
- 减少代码重复
- 简化维护工作
- 确保跨平台行为一致性
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/packages/pagination/pagination.taro.tsx
(4 hunks)src/packages/pagination/pagination.tsx
(4 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/packages/pagination/pagination.tsx
[warning] 142-142: src/packages/pagination/pagination.tsx#L142
Added line #L142 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (1)
src/packages/pagination/pagination.tsx (1)
1-74
: 代码重构改进了关注点分离!通过引入
usePagination
钩子将分页逻辑抽象到独立的模块中,提高了代码的可维护性和可重用性。同时使用ButtonItem
类型增强了类型安全性。
{addColorForHarmony( | ||
prev || locale.pagination.prev, | ||
currentPage === 1 ? '#c2c4cc' : '#ff0f23' | ||
current === 1 ? '#c2c4cc' : '#ff0f23' | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
建议将颜色值移至主题配置!
当前硬编码的颜色值 #c2c4cc
和 #ff0f23
违反了 DRY 原则。建议:
- 将颜色值移至主题配置中
- 使用语义化的变量名(如
disabled-text-color
,active-text-color
) - 通过
useConfig
钩子访问这些主题变量
-current === 1 ? '#c2c4cc' : '#ff0f23'
+current === 1 ? theme.disabledTextColor : theme.activeTextColor
Also applies to: 141-144
Summary by CodeRabbit