fix: should pass colGroup #1358
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Walkthrough为 FixedHolder 新增可选的 colGroup 属性，并在 Table 中将 bodyColGroup 传入头部与汇总的 FixedHolder。渲染逻辑调整：当内部列组为空时优先渲染传入的 colGroup，否则沿用原有 ColGroup（含宽度与滚动条处理）。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Table
participant FH as FixedHolder
participant CG as 内部ColGroup
Note over T: 渲染表格
T->>FH: props(..., colGroup=bodyColGroup)
alt isColGroupEmpty == true
FH-->>T: 使用传入的 colGroup 渲染
else
FH->>CG: 使用 mergedColumnWidth/滚动条 渲染内部 ColGroup
CG-->>FH: 返回内部列组
FH-->>T: 渲染内部 ColGroup
end
Note over T,FH: 头部与汇总的 FixedHolder 同步使用 bodyColGroup
Estimated code review effort🎯 3 (中等) | ⏱️ ~25 分钟 Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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.
Summary of Changes
Hello @li-jia-nan, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a fix to ensure that column grouping (colGroup) information is correctly propagated and rendered within the FixedHolder component, which is utilized for fixed table headers and summaries. By enabling FixedHolder to accept and render a custom colGroup prop, the changes resolve potential layout inconsistencies and improve the structural integrity of fixed table elements, leading to more accurate column alignment and styling.
Highlights
- FixedHolder Component Enhancement: The
FixedHoldercomponent now accepts an optionalcolGroupprop, allowing custom column group definitions to be passed for rendering. - ColGroup Rendering Logic Update: The internal rendering logic within
FixedHolderhas been updated to conditionally render the providedcolGroupprop when the default column group is determined to be empty, ensuring proper column structure. - Table Component Integration: The
Tablecomponent now consistently passes itsbodyColGroupto theFixedHoldercomponents responsible for both the fixed header and fixed summary sections, ensuring uniform column sizing and alignment across all table parts.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request correctly addresses an issue where the colGroup was not being passed to the FixedHolder component, which could cause layout problems in tables with fixed headers or footers. The changes involve adding a colGroup prop and using it as a fallback until column widths are measured. The implementation is sound. I have one minor suggestion to improve code clarity around a variable name, which would enhance future maintainability.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1358 +/- ##
=======================================
Coverage 96.10% 96.10%
=======================================
Files 57 57
Lines 3437 3441 +4
Branches 629 630 +1
=======================================
+ Hits 3303 3307 +4
Misses 129 129
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/FixedHolder/index.tsx (2)
162-169: 命名易混淆：建议更贴近语义
isColGroupEmpty实际含义是“在无数据或未计算出有效列宽时，应使用外部传入的 colGroup”，建议重命名为shouldUseExternalColGroup（或类似）以减少认知负担。
190-198: 当未传 colGroup 时将渲染空节点，建议添加兜底以维持既有行为当前分支在
isColGroupEmpty为true且未传入colGroup时，结果是“无任何 colgroup”，可能回退到浏览器默认宽度推断，带来不可预期的对齐差异。建议增加兜底，至少回退到内部ColGroup（不依赖已计算列宽）。可在本分支内做空值回退：
- {isColGroupEmpty ? ( - colGroup - ) : ( + {isColGroupEmpty ? ( + colGroup ?? <ColGroup columns={flattenColumnsWithScrollbar} /> + ) : ( <ColGroup colWidths={[...mergedColumnWidth, combinationScrollBarSize]} columCount={columCount + 1} columns={flattenColumnsWithScrollbar} /> )}此外，fixHeader 且存在垂直滚动条时，表头会追加一个“滚动条占位列”（
ScrollBarColumn）。当采用外部colGroup时，该占位列对应的<col>没有被追加，极端情况下可能出现 1 个th无对应<col>的轻微对齐抖动。建议验证这一场景；若出现偏差，可考虑在使用外部colGroup且combinationScrollBarSize > 0时附加一个仅含占位宽度的额外<colgroup>，或改为使用内部ColGroup进行兜底拼接。请在本地用以下场景快速核验是否存在表头/主体列对齐抖动：
- 有固定表头（
scroll={{ y: 300 }}）且数据足够产生垂直滚动条；- 列未显式指定
width，首屏渲染时依赖本 PR 的colGroup传递路径；- 观察表头最后一列与主体右侧是否存在 1 个滚动条宽度的对齐差。
src/Table.tsx (1)
745-745: 稳定 bodyColGroup 引用，减少无谓重渲染（可选）
bodyColGroup每次 render 都会新建一个 ReactElement。虽然成本不高，但可用useMemo稳定引用，避免子树不必要的重渲染。可在定义处做轻量包裹（在现有 653-655 行附近）：
- const bodyColGroup = ( - <ColGroup colWidths={flattenColumns.map(({ width }) => width)} columns={flattenColumns} /> - ); + const bodyColGroup = React.useMemo( + () => ( + <ColGroup + colWidths={flattenColumns.map(({ width }) => width)} + columns={flattenColumns} + /> + ), + [flattenColumns.map(({ width }) => width).join('_'), flattenColumns] + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/FixedHolder/index.tsx(3 hunks)src/Table.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test / react component workflow
- GitHub Check: test / react component workflow
🔇 Additional comments (5)
src/FixedHolder/index.tsx (2)
43-44: 新增可选 colGroup 属性：合理扩展为 FixedHolder 暴露
colGroup?: React.ReactNode很合理，可解耦表头/汇总与主体的列组对齐问题。
58-59: 正确解构 colGroup，不会透传到 children在
...restProps之外单独解构了colGroup，避免误传入Header等子组件，保持类型与行为安全。src/Table.tsx (3)
745-745: 将 bodyColGroup 传入 Header 的 FixedHolder：对齐问题可解向表头 FixedHolder 传
colGroup={bodyColGroup}能在未计算列宽时复用主体的列组，提升首屏对齐稳定性。
761-761: 将 bodyColGroup 传入 Summary 的 FixedHolder：一致性良好汇总区与表头一致复用
bodyColGroup，可避免三段（表头/主体/汇总）列宽策略不一致。
745-745: 已确认：所有 FixedHolder 调用均已传入 colGroup仓库搜索仅发现 src/Table.tsx 的两处 FixedHolder 调用（header 行 739–747；summary 行 755–763），均包含 colGroup={bodyColGroup}；src/FixedHolder/index.tsx 的类型定义也声明了 colGroup?: React.ReactNode。无需变更或补充 colGroup。
2 participants
Summary by CodeRabbit