fix: Render ColGroup even if table has no data to maintain layout consistency#1490
fix: Render ColGroup even if table has no data to maintain layout consistency#1490wanpan11 wants to merge 2 commits into
Conversation
…sistency Ensures FixedHolder renders the colgroup as long as column widths are available, fixing layout issues when the table is empty but has fixed columns. Fixes #57916.
|
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 selected for processing (1)
💤 Files with no reviewable changes (1)
Walkthrough将 FixedHolder 中用于决定是否渲染 ColGroup 的 isColGroupEmpty 条件从同时检查 noData 与列宽,改为仅基于列宽(mergedColumnWidth),并在 props 解构中移除 noData。 变更内容列组渲染逻辑优化
🎯 3 (Moderate) | ⏱️ ~20 minutes 相关 PR
建议审阅者
总体说明(更新)本 PR 对 🐰 诗歌
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Removes the noData condition from the isColGroupEmpty check in FixedHolder so that the calculated ColGroup widths are used even when the table has no data, keeping Header/Body/Summary layouts aligned.
Changes:
- Drop
noDatafrom theisColGroupEmptymemo and its dependency array. - Update the inline comment to describe the new behavior and reference ant-design issue #57916.
- Update the
FixedColumnsnapshot to reflect the new colgroup widths emitted for the scrollXY-without-data case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/FixedHolder/index.tsx | Removes noData from the ColGroup-empty check so calculated widths apply even with no data. |
| tests/snapshots/FixedColumn.spec.tsx.snap | Updates the no-data snapshot to match the new col widths produced by the change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@wanpan11 is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Code Review
This pull request updates FixedHolder to render the ColGroup as long as column widths are available, even when there is no data, ensuring layout consistency. A review comment correctly points out that noData is now an unused variable in FixedHolder and suggests resolving this to prevent potential ESLint warnings.
|
Actionable comments posted: 0 |
|
@gemini-code-assist review |
There was a problem hiding this comment.
Code Review
This pull request updates the FixedHolder component to render the ColGroup as long as column widths are available, even when there is no data, which maintains layout consistency. A review comment notes that removing noData from the destructuring assignment could cause it to leak into restProps and be forwarded to DOM elements, potentially triggering React console warnings. It is recommended to keep noData in the destructuring to prevent this issue.
| const { | ||
| className, | ||
| style, | ||
| noData, | ||
| columns, |
There was a problem hiding this comment.
By removing noData from the destructuring assignment, it now implicitly falls into restProps. Since restProps is passed down to the children render prop (which eventually renders the header elements), noData will be forwarded down and may end up being spread onto DOM elements, triggering React console warnings (e.g., React does not recognize the noData prop on a DOM element).
To prevent this prop leakage, keep noData in the destructuring assignment so it is excluded from restProps, even if it is not directly used within FixedHolder.
| const { | |
| className, | |
| style, | |
| noData, | |
| columns, | |
| const { | |
| className, | |
| style, | |
| noData, | |
| columns, |
|
Actionable comments posted: 0 |
Ensures FixedHolder renders the colgroup as long as column widths are available, fixing layout issues when the table is empty but has fixed columns. Fixes ant-design/ant-design/issues/57916
Summary by CodeRabbit
改进