chore: sync antd 5.x branch to master #1349
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Walkthrough此变更引入可自定义的 measure 行渲染（measureRowRender）并通过 TableContext 贯通到 Body/MeasureRow；调整 FixedHolder 的水平滚动接口为 scrollX；VirtualTable 列宽计算支持 minWidth；docs 新增/扩展示例；样式移除 th/td 的 word-break；版本号升至 7.53.0；测试覆盖新增能力。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor App
participant Table as Table
participant Ctx as TableContext
participant Body as Body
participant MR as MeasureRow
App->>Table: render({ measureRowRender?, scroll, ... })
Table->>Ctx: provide { measureRowRender, ... }
Table->>Body: render body
Body->>MR: render base measure row
MR->>Ctx: read measureRowRender
alt measureRowRender provided
MR-->>App: return measureRowRender(baseMeasureRow)
else
MR-->>App: return baseMeasureRow
end
sequenceDiagram
autonumber
actor App
participant Table as Table
participant FH as FixedHolder
App->>Table: render({ scroll: { x: mergedScrollX }, ... })
Table->>FH: props { scrollX: mergedScrollX }
FH->>FH: apply styles { minWidth: '100%', width: scrollX }
FH-->>App: rendered fixed header/body sized by scrollX
sequenceDiagram
autonumber
participant Grid as VirtualTable.BodyGrid
participant Cols as Columns[]
Cols-->>Grid: { width?, minWidth?, key }...
Grid->>Grid: finalWidth = max(width||0, minWidth||0)
Grid-->>Cols: onColumnResize(key, finalWidth)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Flag potential breaking changes that are not documented:
1. Identify changes to public APIs/exports, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints (including removed/renamed items and changes to types, required params, return values, defaults, or behavior).
2. Ignore purely internal/private changes (e.g., code not exported from package entry points or marked internal).
3. Verify documentation exists: a "Breaking Change" section in the PR description and updates to CHANGELOG.md.Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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 @afc163, 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 integrates several enhancements and fixes into the rc-table component. The primary goal is to provide greater control over table rendering, particularly for complex layouts involving sticky headers and virtualized scrolling, while also improving the accuracy of column width calculations.
Highlights
- New measureRowRender Prop: Introduced a new measureRowRender prop for rc-table that allows developers to customize the rendering of the internal measurement row. This is particularly useful for scenarios where elements within table headers (like filter dropdowns) might interfere with layout calculations, enabling them to be hidden during measurement.
- Improved Column Width Calculation: Enhanced the logic for calculating column widths in virtualized tables to correctly account for minWidth properties, leading to more accurate and stable table layouts.
- Optimized colgroup Rendering: Modified the ColGroup component to conditionally render the HTML element only when there are actual columns, preventing the rendering of empty colgroup tags and potentially improving DOM structure.
- Refined Sticky Header Scrolling: Updated the handling of scroll.x in sticky headers to address column alignment issues, ensuring consistent layout when horizontal scrolling is enabled.
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 synchronizes the antd-5.x branch to master, introducing several features and fixes. Key changes include adding a measureRowRender prop for custom rendering of the measure row, updating dependencies, and fixing various layout and scrolling issues. My review focuses on potential bugs and improvements in type safety and styling. I've identified a couple of high-severity bugs related to style property assignment and width calculation that could break the UI under certain conditions. I've also pointed out some medium-severity issues regarding type-casting in examples and a CSS change that might have unintended side effects. Overall, the changes are beneficial, but these points should be addressed to ensure stability and code quality.
| ...scrollTableStyle, | ||
| minWidth: '100%', | ||
| // https://github.com/ant-design/ant-design/issues/54894 | ||
| width: scrollX, |
There was a problem hiding this comment.
The width style property is being set directly to scrollX. The scrollX prop can be true, which is not a valid CSS value for width and will cause rendering issues. The logic should handle the boolean case, for example by setting width to 'auto'.
| width: scrollX, | |
| width: scrollX === true ? 'auto' : scrollX, |
| const finalWidth = Math.max((width as number) || 0, (minWidth as number) || 0); | ||
| total += finalWidth; | ||
| return [key, finalWidth, total]; |
There was a problem hiding this comment.
The width property on a column can be a string (e.g., '100px'), but the current logic (width as number) only casts it in TypeScript and doesn't parse it at runtime. This can lead to NaN from Math.max if width is a string with units, causing incorrect layout calculations for the virtualized table. The width should be parsed to a number before being used in calculations.
const finalWidth = Math.max(parseInt(String(width), 10) || 0, minWidth || 0);
total += finalWidth;
return [key, finalWidth, total];
|
|
||
| padding: @cell-padding; | ||
| white-space: normal; | ||
| word-break: break-word; |
There was a problem hiding this comment.
The removal of word-break: break-word; could lead to UI issues where long, unbreakable words in table cells overflow their container. This might disrupt the table layout. It's recommended to retain this property or provide an alternative solution for handling long text, such as using overflow-wrap: break-word; which is the modern equivalent.
| [ | ||
| { key: '1', name: 'Test', age: 1, address: '2' }, | ||
| { key: '2', name: '0', age: 1, address: '2' }, | ||
| ] as any |
There was a problem hiding this comment.
Using as any bypasses TypeScript's type safety and should be avoided, especially in examples that serve as a reference for users. It would be better to define a proper type for the data and use it consistently for both the columns and the Table component to ensure type safety.
For example:
interface MyStickyRecord {
key: string;
name: string;
age: number;
address: string;
}
const columns3: ColumnType<MyStickyRecord>[] = [/* ... */];
// ... in component
<Table<MyStickyRecord>
columns={columns3}
data={[
{ key: '1', name: 'Test', age: 1, address: '2' },
{ key: '2', name: '0', age: 1, address: '2' },
]}
// ...
/>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/FixedHolder/index.tsx (1)
126-137: 在 useMemo 依赖中添加 direction缺失 direction 会导致 RTL/LTR 切换时 headerStickyOffsets 不重新计算：
- }, [combinationScrollBarSize, stickyOffsets, isSticky]); + }, [combinationScrollBarSize, stickyOffsets, isSticky, direction]);
🧹 Nitpick comments (11)
src/context/TableContext.tsx (1)
76-78: 新增 measureRowRender：请确保 TableContext 的 useMemo 依赖包含它请在 Table.tsx 里构造 TableContext 值时，将 measureRowRender 纳入 useMemo 依赖，避免外部更换渲染器但上下文未刷新导致测量行未更新。
docs/examples/scrollY.tsx (1)
79-81: 将纯文本 issue 地址改为链接，便于点击- <p>https://github.com/ant-design/ant-design/issues/54889</p> + <p><a href="https://github.com/ant-design/ant-design/issues/54889">Issue 54889</a></p>docs/examples/stickyHeader.tsx (1)
140-166: 示例可加入 minWidth 以覆盖本次引入的列最小宽能力为 columns3 的一两列添加
minWidth（如 160）更能直观展示minWidth对齐/滚动效果。src/Body/MeasureRow.tsx (1)
51-52: measureRowRender 的使用约束需明确，避免影响测量请在文档/注释中提示：自定义渲染器必须返回包含原始
<tr.rc-table-measure-row>的节点，且不要对该<tr>应用display: none（可用visibility: hidden/包裹层隐藏溢出），否则isVisible判断为不可见将导致列宽不更新。docs/examples/measureRowRender.tsx (4)
5-5: 为示例组件声明显式返回类型，避免隐式 any为示例组件加上 React.FC，类型更清晰。
-const MeasureRowRenderExample = () => { +const MeasureRowRenderExample: React.FC = () => {
34-34: 为 measureRowRender 显式标注类型并增强可访问性补齐参数类型可避免隐式 any；同时加上 aria-hidden，防止辅助技术读取被隐藏的测量节点。
- const measureRowRender = measureRow => <div style={{ display: 'none' }}>{measureRow}</div>; + const measureRowRender: (measureRow: React.ReactNode) => React.ReactNode = measureRow => ( + <div style={{ display: 'none' }} aria-hidden> + {measureRow} + </div> + );
11-14: 示例里双重隐藏可能引导误解，建议移除 header 内额外隐藏块既然 measureRowRender 已负责隐藏测量行，header 里这段演示性隐藏内容会造成“需要两处隐藏”的错觉，建议删掉以简化示例。
- <div className="filter-dropdown" style={{ display: 'none' }}> - Filter Content - </div>
41-41: 关于水平滚动的示例说明（可选）如果目标是让列宽按内容扩展，可在注释里提示也可使用 scroll={{ x: 'max-content' }} 或具体数值，避免读者误以为只有 true 可用。
tests/FixedHeader.spec.jsx (1)
284-294: 测试结束时恢复真实定时器以保持一致性本文件 beforeEach 会启用假定时器。虽然这是最后一个用例，但为避免后续维护时插入用例带来的隐患，建议在断言后恢复真实定时器。
expect(measureRowInWrapper).toHaveLength(1); + vi.useRealTimers(); });src/Table.tsx (2)
171-177: API 可见性与文档一致性需明确measureRowRender 被标注为 @private/“不要用于生产”，但同时在 docs/examples 中对外演示。建议二选一：
- 将注释改为“实验性（可能变更）”，或
- 将示例标注为内部/仅供验证，不对外推荐。
否则会引发使用预期混淆。
是否计划对外公开该能力？若是，建议在变更日志与类型注释中标注 experimental。
810-919: （可选）稳定回调引用，减少不必要的 Provider 更新如果调用方频繁以新引用传入 measureRowRender，可考虑用 useEvent 包裹后再放入 Context，降低无关重渲染风险（功能不变）。
- const TableContextValue = React.useMemo( + const stableMeasureRowRender = useEvent(measureRowRender); + const TableContextValue = React.useMemo( () => ({ ... - measureRowRender, + measureRowRender: stableMeasureRowRender, }), [ ... - measureRowRender, + stableMeasureRowRender, ], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
tests/__snapshots__/ExpandRow.spec.jsx.snapis excluded by!**/*.snaptests/__snapshots__/FixedColumn.spec.tsx.snapis excluded by!**/*.snaptests/__snapshots__/Table.spec.jsx.snapis excluded by!**/*.snap
📒 Files selected for processing (12)
assets/index.less(0 hunks)docs/examples/measureRowRender.tsx(1 hunks)docs/examples/scrollY.tsx(1 hunks)docs/examples/stickyHeader.tsx(5 hunks)package.json(1 hunks)src/Body/MeasureRow.tsx(3 hunks)src/ColGroup.tsx(1 hunks)src/FixedHolder/index.tsx(3 hunks)src/Table.tsx(5 hunks)src/VirtualTable/BodyGrid.tsx(1 hunks)src/context/TableContext.tsx(1 hunks)tests/FixedHeader.spec.jsx(1 hunks)
💤 Files with no reviewable changes (1)
- assets/index.less
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-08T12:53:09.293Z
Learnt from: bbb169
PR: react-component/table#1202
File: src/Table.tsx:903-904
Timestamp: 2024-11-08T12:53:09.293Z
Learning: 在 `src/Table.tsx` 文件的 React 组件 `Table` 中，即使 `bodyScrollLeft` 频繁更新，也需要在 `TableContextValue` 的 `useMemo` 依赖数组中包含 `bodyScrollLeft` 和 `headerCellRefs`，因为每次滚动时重新计算 `TableContextValue` 是解决该问题所必须的。
Applied to files:
src/context/TableContext.tsxdocs/examples/measureRowRender.tsxsrc/FixedHolder/index.tsxsrc/Body/MeasureRow.tsxdocs/examples/scrollY.tsxdocs/examples/stickyHeader.tsxsrc/Table.tsx
🧬 Code graph analysis (3)
docs/examples/scrollY.tsx (2)
tests/FixedColumn-IE.spec.jsx (1)
columns(28-41)tests/Scroll.spec.jsx (1)
data(8-11)
docs/examples/stickyHeader.tsx (1)
src/interface.ts (1)
ColumnType(105-120)
tests/FixedHeader.spec.jsx (1)
tests/utils.js (1)
safeAct(3-10)
🔇 Additional comments (8)
package.json (1)
3-3: 版本号升级到 7.53.0，请同步发布流程与变更说明建议在发布前确认：已打对应 git tag、更新 CHANGELOG，并在 README/docs 中体现新增 API（如 measureRowRender、scrollX）的最低兼容版本声明。
src/ColGroup.tsx (1)
43-44: 空 colgroup 将不再渲染：请确认样式与选择器无回归有些样式可能依赖
colgroup是否存在（如table > colgroup + thead）。建议回归检查 sticky/横向滚动场景是否受影响。docs/examples/stickyHeader.tsx (1)
350-360:scroll.x: true的语义需确认与实现一致若内部未将
true归一为具体宽度（如'max-content'或总列宽），建议示例改为显式字符串，避免在某些环境下无效：- scroll={{ x: true }} + scroll={{ x: 'max-content' }}tests/FixedHeader.spec.jsx (1)
238-294: 新增用例覆盖点合理，验证点到位验证了自定义 wrapper 存在、样式为 display: none 且包含 .rc-table-measure-row，能有效防回归。
src/Table.tsx (4)
219-221: 正确解构 measureRowRender 以透传到上下文解构位置合理，后续有加入依赖数组，避免闭包过期。
861-865: 将 measureRowRender 注入 TableContext 的位置与命名清晰上下文字段命名直观，Body/MeasureRow 可直接消费，符合预期。
915-918: 依赖数组包含 measureRowRender，避免闭包陈旧已纳入 useMemo 依赖，和我们过往对依赖完整性的复盘一致。
696-697: 确认 FixedHolder 接口与调用一致
FixedHolderProps 中已包含scrollX?: number | string | true，旧的scrollTableStyle仅在Table.tsx作本地样式计算，不再传入FixedHolder，且FixedHolder内部已根据scrollX正确设置width/minWidth。无需进一步修改。
| return flattenColumns.map(({ width, minWidth, key }) => { | ||
| const finalWidth = Math.max((width as number) || 0, (minWidth as number) || 0); | ||
| total += finalWidth; | ||
| return [key, finalWidth, total]; | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
列宽计算对 string 宽度不安全，且 scrollWidth 与 scroll.x: true 可能不兼容
- 这里对
width直接参与Math.max，当width为字符串（如'100%'/'max-content'/'100px'）将导致NaN，进而污染total与 offset。 TableContext.scrollX的类型包含true，但 VirtualList 的scrollWidth期望数值；当前传递true会在运行时成为布尔值，存在不兼容风险。
建议修复（仅变更本片段内逻辑）：
- return flattenColumns.map(({ width, minWidth, key }) => {
- const finalWidth = Math.max((width as number) || 0, (minWidth as number) || 0);
+ return flattenColumns.map(({ width, minWidth, key }) => {
+ const w = typeof width === 'number' ? width : 0;
+ const mw = typeof minWidth === 'number' ? minWidth : 0;
+ const finalWidth = Math.max(w, mw);
total += finalWidth;
return [key, finalWidth, total];
});并在 VirtualList 处（Line 245 附近）用总列宽兜底，避免把 true 透传为 scrollWidth：
// 建议将 scrollWidth 改成：
scrollWidth={
typeof scrollX === 'number'
? scrollX
: (columnsWidth[columnsWidth.length - 1]?.[2] ?? 0)
}🤖 Prompt for AI Agents
In src/VirtualTable/BodyGrid.tsx around lines 60-64 and the VirtualList usage
near line 245, the current logic uses width/minWidth directly in Math.max and
may pass TableContext.scrollX === true into VirtualList.scrollWidth; parse and
coerce widths to safe numbers and provide a numeric fallback for scrollWidth.
Specifically, in the map at lines ~60-64, convert width and minWidth into
numeric values (e.g., if typeof value === 'number' && isFinite(value) use it; if
typeof value === 'string' attempt parseFloat(value) and treat NaN as 0) then
compute finalWidth = Math.max(parsedWidth, parsedMinWidth, 0) and accumulate
total from that numeric finalWidth; and at the VirtualList call near line 245,
pass scrollWidth as typeof scrollX === 'number' ? scrollX :
(columnsWidth[columnsWidth.length - 1]?.[2] ?? 0) to avoid forwarding true.
Summary by CodeRabbit