-
-
Notifications
You must be signed in to change notification settings - Fork 595
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
feat: expandedRowClassName support receive a string #1195
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Warning Rate limit exceeded@li-jia-nan has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Walkthrough此次更改涉及对 Changes
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 (
|
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
🧹 Outside diff range and nitpick comments (1)
src/Body/BodyRow.tsx (1)
149-151
: 条件应用computedExpandedRowClassName
的改进这个改动很好地优化了
computedExpandedRowClassName
的应用逻辑。通过只在indent >= 1
时应用这个类名,确保了扩展行样式只在实际扩展的行上生效。这比之前的实现更加精确,避免了不必要的类名应用。为了进一步提高代码的可读性,您可以考虑将条件提取为一个命名常量:
const isExpandedRow = indent >= 1;然后在类名对象中使用这个常量:
{ [computedExpandedRowClassName]: isExpandedRow, }这样可以使代码的意图更加明确。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- src/Body/BodyRow.tsx (2 hunks)
- src/context/TableContext.tsx (1 hunks)
- tests/ExpandRow.spec.jsx (1 hunks)
🔇 Additional comments (4)
src/context/TableContext.tsx (1)
40-40
: 批准更改,但建议验证使用情况这个更改增加了
expandedRowClassName
属性的灵活性,允许它接受字符串或RowClassName<RecordType>
函数。这与上面的rowClassName
属性保持一致,是一个很好的改进。为了确保这个更改不会在其他地方引起问题,请运行以下脚本来验证
expandedRowClassName
的使用情况:这将帮助我们确认是否需要在其他文件中进行相应的更新。
✅ Verification successful
修正搜索
expandedRowClassName
的脚本之前的搜索命令因未识别类型
typescript
而失败。请使用以下修正后的命令重新搜索:
修正搜索
expandedRowClassName
的脚本之前的搜索命令因未识别类型
tsx
而失败。请使用以下修正后的命令重新搜索:
验证通过,无问题发现
expandedRowClassName
属性的类型已成功修改为string | RowClassName<RecordType>
,并且在代码库中的所有使用情况均已正确处理,没有引入任何问题。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:验证 expandedRowClassName 的所有使用情况 # 测试:搜索 expandedRowClassName 的使用。预期:找到所有使用该属性的地方。 rg --type typescript -A 5 $'expandedRowClassName'Length of output: 83
Script:
#!/bin/bash # 描述:验证 expandedRowClassName 的所有使用情况 # 修正:使用正确的文件类型 rg -A 5 'expandedRowClassName' -t ts -t tsxLength of output: 71
Script:
#!/bin/bash # 描述:验证 expandedRowClassName 的所有使用情况 # 修正:使用通配符匹配 .ts 和 .tsx 文件 rg -A 5 'expandedRowClassName' -g '*.ts' -g '*.tsx'Length of output: 6038
src/Body/BodyRow.tsx (2)
129-137
: 使用 React.useMemo 优化性能,很好!这个改动通过使用 React.useMemo 来优化
computedExpandedRowClassName
的计算,是一个很好的性能改进。它可以避免在每次渲染时都重新计算这个值,只有当依赖项(expandedRowClassName
,record
,index
, 和indent
)发生变化时才会重新计算。此外,这个实现正确地处理了
expandedRowClassName
可能是字符串或函数的情况,增加了代码的灵活性。
Line range hint
129-151
: 总结:性能和正确性的显著改进这些更改通过使用 React.useMemo 和更精确的类名应用逻辑,显著提高了组件的性能和正确性。主要改进包括:
- 使用 React.useMemo 优化
computedExpandedRowClassName
的计算。- 更精确地应用扩展行类名,只在实际扩展的行(indent >= 1)上应用。
这些改动不仅提高了性能,还增强了代码的可维护性和灵活性。建议的小改进(使用命名常量来增加可读性)可以在未来的重构中考虑。
总的来说,这是一个高质量的改进,符合 React 的最佳实践。
tests/ExpandRow.spec.jsx (1)
418-431
: 新测试用例看起来不错,但可以进一步改进这个新的测试用例很好地验证了当
expandedRowClassName
是字符串时的行为。这与PR的目标一致,即支持expandedRowClassName
接收字符串类型。建议:
- 考虑添加一个反面测试用例,验证当提供无效的
expandedRowClassName
时(如数字或对象)的行为。- 可以测试多个展开的行,以确保类名正确应用于所有展开的行。
- 为了提高可读性,可以将
expandedRowClassName
声明为常量,放在测试用例外部。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1195 +/- ##
=======================================
Coverage 97.70% 97.71%
=======================================
Files 75 75
Lines 7420 7451 +31
Branches 1112 1116 +4
=======================================
+ Hits 7250 7281 +31
Misses 164 164
Partials 6 6 ☔ 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/VirtualTable/BodyLine.tsx (1 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
src/VirtualTable/BodyLine.tsx
[warning] 48-49: src/VirtualTable/BodyLine.tsx#L48-L49
Added lines #L48 - L49 were not covered by tests
[warning] 51-52: src/VirtualTable/BodyLine.tsx#L51-L52
Added lines #L51 - L52 were not covered by tests
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
🧹 Outside diff range and nitpick comments (2)
docs/examples/virtual.tsx (2)
198-199
: 同步滚动功能已实现,但可以进一步优化。这个更改实现了两个表格同时滚动到底部的功能,这很好。但是,使用一个非常大的数值(9999999999999)作为滚动位置可能不是最佳做法。
建议:
- 考虑使用
scrollHeight
属性来获取实际的滚动高度。- 或者,如果 API 支持,使用类似
scrollToBottom()
的方法可能更加清晰和可维护。
214-215
: 滚动到特定键的功能已更新,但可以考虑进一步增强。这个更改实现了两个表格同时滚动到数据的最后一个索引,这很好。然而,当前的实现限制了只能滚动到最后一个索引。
建议:
- 考虑允许用户指定要滚动到的索引,而不是固定到最后一个。
- 可以添加一个输入框,让用户输入想要滚动到的特定索引或键。
这样可以提高功能的灵活性和实用性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- docs/examples/virtual.tsx (2 hunks)
- src/interface.ts (1 hunks)
- tests/Virtual.spec.tsx (1 hunks)
🔇 Additional comments (8)
docs/examples/virtual.tsx (5)
192-193
: 代码更改看起来不错!这个更改引入了两个独立的表格引用(
table1Ref
和table2Ref
)。这种方法允许对两个虚拟表格进行独立控制,为后续的功能增强奠定了基础。
206-211
: 新增的滚动到顶部功能很好!这个新增的功能很好地补充了之前的滚动到底部的功能。实现简洁明了,同时保持了两个表格的同步。这种对称的设计提高了用户体验。
221-222
: VirtualTable 组件的更改看起来不错。
- 将 ref 更改为
table1Ref
与之前的修改保持一致,很好。- 添加的
style
属性通过设置上边距,改善了表格之间的视觉分隔,这有助于提高用户体验。这些更改都是合理的,有助于改善组件的结构和外观。
238-241
: getContainerWidth 函数的重构很棒!这次重构提高了代码的可读性,同时保持了原有的功能:
- 使用解构赋值来获取
borderInlineStartWidth
,使代码更简洁。- 变量名
mergedWidth
更具描述性,清楚地表明了它的用途。- 代码结构更清晰,易于理解。
这些小改动体现了良好的编码实践,值得赞赏。
243-264
: 新增的第二个 VirtualTable 组件很好地展示了新功能!这个新增的 VirtualTable 组件有以下几个亮点:
- 使用
table2Ref
作为引用,与之前的修改保持一致。- 大部分属性与第一个表格相同,确保了两个表格的行为一致性。
expandedRowClassName
属性使用了字符串值 'good-one-string',这很好地展示了 PR 中提到的新功能。这个例子不仅演示了如何同步两个虚拟表格,还直观地展示了
expandedRowClassName
支持字符串的新特性。这对于文档和测试都很有价值。src/interface.ts (2)
250-250
: 扩展了expandedRowClassName
的类型定义,提高了灵活性。这个改动允许
expandedRowClassName
接受字符串或函数类型,增加了组件的灵活性。优点:
- 用户可以直接传入一个字符串作为类名,简化了简单场景的使用。
- 保留了原有的函数类型,仍然支持复杂的动态类名逻辑。
建议:
- 更新相关文档,说明这个新的用法。
- 考虑在其他相似的属性上也采用这种模式,以保持一致性。
Line range hint
1-250
: 总体评估:积极的改进,提高了 API 的灵活性。这个改动虽小,但影响深远:
- 增强了
expandedRowClassName
的使用灵活性,同时保持了向后兼容性。- 符合组件库设计的最佳实践,允许简单和复杂的使用场景。
- 可能需要更新文档和测试用例以反映这一变化。
建议:
- 确保此更改在 CHANGELOG 中得到适当记录。
- 考虑是否有其他类似的属性可以采用这种模式,以提高整体 API 的一致性。
tests/Virtual.spec.tsx (1)
166-166
: 代码更改符合预期,建议小幅改进这个更改很好地实现了允许
expandedRowClassName
接收字符串类型的功能。测试用例正确地设置了expandedRowClassName
并验证了其应用。建议:
- 考虑添加一个额外的测试用例,以验证当
expandedRowClassName
为函数时的行为,确保两种用法都能正常工作。- 可以将断言更改为使用
toHaveClass()
方法,这样可以更明确地表达意图:expect(container.querySelector<HTMLElement>('.rc-table-expanded-row')).toHaveClass('bamboo');Also applies to: 173-173
为啥需要支持 string? |
|
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- src/Body/BodyRow.tsx (4 hunks)
- src/VirtualTable/BodyLine.tsx (3 hunks)
- src/utils/expandUtil.tsx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Body/BodyRow.tsx
- src/VirtualTable/BodyLine.tsx
🔇 Additional comments (2)
src/utils/expandUtil.tsx (2)
3-3
: 导入语句更新正确导入
ExpandableConfig
类型是合理的,因为它在新添加的computedExpandedClassName
函数中被使用。这个更改与现有的代码风格保持一致。
Line range hint
1-68
: 总体评价:变更有效实现了新功能此次更改成功地为
expandedRowClassName
添加了字符串类型支持,与 PR 的目标和讨论保持一致。新增的computedExpandedClassName
函数设计合理,能够处理字符串和函数类型的输入。代码结构良好,遵循了现有的编码风格,新函数在文件中的位置也很合适。这个更改提高了组件的灵活性和易用性,使得开发者可以更方便地为展开行设置类名。
为了确保这个更改不会影响其他部分的代码,建议运行以下脚本来检查
expandedRowClassName
的使用情况:这将帮助我们确认其他地方的代码是否需要相应的更新。
expandedRowClassName 支持 string 类型
Summary by CodeRabbit
expandable.expandedRowClassName
属性的类型声明,支持字符串和函数两种形式。expandedRowClassName
为字符串时,扩展行类名的正确渲染。expandedRowClassName
属性并进行断言。