fix: do not render scrollbar column when table is empty#1477
fix: do not render scrollbar column when table is empty#1477hblab-tiennm wants to merge 2 commits intoreact-component:masterfrom
Conversation
|
@hblab-tiennm is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
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 ignored due to path filters (1)
📒 Files selected for processing (1)
Walkthrough在 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 分钟 Possibly related PRs
Suggested reviewers
兔子的庆祝诗
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 modifies the table header logic to prevent the scrollbar column from being appended when the table is empty. A new test case and updated snapshots were added to verify this behavior. However, feedback indicates that the implementation is incomplete because the sticky offset calculations for fixed columns still account for the scrollbar width even when it is not rendered, resulting in layout gaps in the empty state.
| className: `${prefixCls}-cell-scrollbar`, | ||
| }), | ||
| }; | ||
| const shouldAppendScrollBarColumn = combinationScrollBarSize && !noData; |
There was a problem hiding this comment.
While this change correctly prevents the scrollbar column from being appended to the header when the table is empty, it is incomplete. The headerStickyOffsets calculation (around line 156) still uses combinationScrollBarSize directly, which means fixed columns will still have sticky offsets that account for a non-existent scrollbar. This leads to an incorrect layout gap in the empty state, as seen in the updated snapshots (e.g., inset-inline-end: 15px remaining despite the column removal). You should also update the headerStickyOffsets logic to use shouldAppendScrollBarColumn instead of combinationScrollBarSize.
There was a problem hiding this comment.
Good catch. This finding was valid.
I updated headerStickyOffsets to use the same shouldAppendScrollBarColumn condition, so the scrollbar width is only included when the scrollbar placeholder column is actually appended.
This was pushed in ca3af2e, and I re-ran pnpm test tests/FixedColumn.spec.tsx -u to verify the empty-state fixed-column layout stays in sync.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/FixedHolder/index.tsx`:
- Around line 134-143: The header sticky offset calculation is unconditionally
adding the scrollbar width even when noData prevents appending ScrollBarColumn,
causing misaligned fixed columns; update the headerStickyOffsets computation to
use the same shouldAppendScrollBarColumn condition (the same boolean used for
columnsWithScrollbar and flattenColumnsWithScrollbar) so the scrollbar width is
only included in headerStickyOffsets when shouldAppendScrollBarColumn is true
(reference shouldAppendScrollBarColumn, columnsWithScrollbar,
flattenColumnsWithScrollbar, headerStickyOffsets).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7cddf4c1-8ffc-44ca-9478-1d28cfff3dfa
⛔ Files ignored due to path filters (1)
tests/__snapshots__/FixedColumn.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/FixedHolder/index.tsxtests/FixedColumn.spec.tsx
1 participant
Chinese template / 中文版模板
This is a ...
Related Issues
antd@5.29.3withscroll.yenabled and empty data.Background and Solution
When
scroll.yis enabled,FixedHolderalways appendsScrollBarColumnto header columns whencombinationScrollBarSizeis truthy.In empty state, the table body renders the placeholder row instead of a scrollable body, so the extra scrollbar placeholder column is not needed. Keeping it in the header causes an extra
rc-table-cell-scrollbarheader cell to be rendered.This change skips appending
ScrollBarColumnwhennoDatais true, and adds a regression test to cover the empty-state case.Changelog
scroll.yis enabled and data is empty.scroll.y且数据为空时，表头额外渲染 scrollbar 占位列的问题。Summary by CodeRabbit
Bug Fixes
Tests