chore: sync antd 5.x branch to master #1350
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该变更新增 measureRowRender 钩子以允许自定义测量行渲染；调整 FixedHolder 水平滚动 API 为 scrollX；ColGroup 在无列时不再渲染空 colgroup；更新样式移除表格单元的 word-break；补充/新增示例与用例，覆盖新能力与粘性表头场景。 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Caller
participant Table as Table
participant Ctx as TableContext
participant Body as Body.MeasureRow
Dev->>Table: render(<Table measureRowRender?>)
Table->>Ctx: 提供 { measureRowRender }
Body->>Ctx: useContext(measureRowRender)
alt 提供了 measureRowRender
Body->>Body: 生成默认 measureRow
Body->>Dev: 调用 measureRowRender(measureRow)
Dev-->>Body: 返回包装后的节点
Body->>Body: 渲染包装后的测量行
else 未提供
Body->>Body: 渲染默认测量行
end
sequenceDiagram
autonumber
participant Table as Table
participant FH as FixedHolder
participant TC as TableComponent (inner)
Table->>FH: props { scrollX: mergedScrollX }
FH->>TC: style { minWidth:'100%', width: scrollX }
TC->>TC: 应用宽度以支持横向滚动/粘性表头
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 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 enhances the rc-table component by providing more control over its internal rendering mechanisms and improving its layout and scrolling capabilities. It introduces a new prop for custom measure row rendering, refines how horizontal scrolling is handled, and optimizes the rendering of table column groups. These changes aim to increase the component's flexibility and resolve specific display inconsistencies.
Highlights
- New
measureRowRenderProp: Introduced a newmeasureRowRenderprop to theTablecomponent, allowing developers to customize the rendering of the internalMeasureRowcomponent. This enables advanced use cases, such as hiding the measure row for specific scenarios, as demonstrated in a new example. - Improved Table Scrolling and Layout: Refactored the handling of table
scroll.xinFixedHolderandTablecomponents. ThescrollTableStyleprop has been replaced withscrollX, which is now directly applied to the table'swidthstyle, alongsidemin-width: '100%', to address column alignment and scrolling issues. - Conditional
colgroupRendering: TheColGroupcomponent has been updated to only render the<colgroup>HTML element if there are actual columns present. This prevents the rendering of emptycolgroupelements, leading to cleaner HTML output. - Styling and Test Updates: Removed the
word-break: break-wordCSS property fromassets/index.lessfor table cells. Additionally, new examples and test cases have been added to demonstrate and verify themeasureRowRenderfunctionality, column alignment fixes, andscroll.xbehavior. Existing tests and snapshots were also updated to reflect these changes. - Build Configuration Update: The Vitest configuration file has been renamed from
vitest.config.tstovitest.config.mts.
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 syncs changes from the antd-5.x branch, introducing a measureRowRender prop to customize column width measurement and fixing some scrolling issues. The changes are mostly good, but I've found a critical rendering bug related to the new scroll logic, a potential layout issue from a CSS change, and a type-safety problem in one of the examples. Please review the comments for details.
| padding: 0; | ||
|
|
||
| padding: @cell-padding; | ||
| white-space: normal; |
There was a problem hiding this comment.
The removal of word-break: break-word; could lead to layout issues where long words in table cells overflow their containers. It's advisable to re-introduce word wrapping. Consider using overflow-wrap: break-word;, which is the current standard for this purpose.
white-space: normal;
overflow-wrap: break-word;
| data={ | ||
| [ | ||
| { 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 checking and should be avoided. The issue stems from columns3 being typed with RecordType, which doesn't include properties like name, age, or address used in the data. To fix this properly, you should:
- Define a new interface for this specific data structure (e.g.,
StickyData). - Update the type of
columns3to use this new interface (e.g.,ColumnType<StickyData>[]). - Remove the
as anycast here.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1350 +/- ##
=======================================
Coverage 96.10% 96.11%
=======================================
Files 57 57
Lines 3440 3447 +7
Branches 628 630 +2
=======================================
+ Hits 3306 3313 +7
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/FixedHolder/index.tsx (1)
159-176: 严重：当未计算列宽时返回组件类型导致渲染错误
return ColGroup;返回的是组件函数本身而非 React 元素，作为子节点会导致 “Objects are not valid as a React child”/空渲染。应始终返回元素。- if ( - noData || - !mergedColumnWidth || - mergedColumnWidth.length === 0 || - mergedColumnWidth.every(width => !width) - ) { - return ColGroup; - } - return ( - <ColGroup - colWidths={[...mergedColumnWidth, combinationScrollBarSize]} - columCount={columCount + 1} - columns={flattenColumnsWithScrollbar} - /> - ); + const useMerged = !!( + mergedColumnWidth && + mergedColumnWidth.length > 0 && + !mergedColumnWidth.every(width => !width) + ); + + const widths = useMerged ? mergedColumnWidth : colWidths; + const withScrollbar = combinationScrollBarSize ? [...widths, combinationScrollBarSize] : widths; + const count = combinationScrollBarSize ? columCount + 1 : columCount; + const cols = combinationScrollBarSize ? flattenColumnsWithScrollbar : flattenColumns; + + return <ColGroup colWidths={withScrollbar} columCount={count} columns={cols} />;src/Table.tsx (1)
869-926: useMemo 依赖缺失：classNames 与 styles 未纳入，可能导致主题/样式变更不生效两者参与了
TableContextValue，但未出现在依赖数组，应补充。const TableContextValue = React.useMemo( () => ({ // Scroll scrollX: mergedScrollX, scrollInfo, - classNames, - styles, + classNames, + styles, … }), [ // Scroll mergedScrollX, scrollInfo, + // Style/Class + classNames, + styles, … ], );Also applies to: 927-979
🧹 Nitpick comments (13)
src/context/TableContext.tsx (1)
81-82: 新增 measureRowRender API 合理，但建议抽出类型别名以便复用为避免在多文件重复声明、提升可读性，可在该文件顶部定义并导出类型别名后复用。
+export type MeasureRowRender = (measureRow: React.ReactNode) => React.ReactNode; export interface TableContextProps<RecordType = any> { … - measureRowRender?: (measureRow: React.ReactNode) => React.ReactNode; + measureRowRender?: MeasureRowRender; }docs/examples/scrollY.tsx (1)
99-101: 文档示例可读性：将裸 URL 改为可点击链接便于读者直接跳转到 issue。
- <p>https://github.com/ant-design/ant-design/issues/54889</p> + <p> + <a href="https://github.com/ant-design/ant-design/issues/54889" target="_blank" rel="noreferrer"> + https://github.com/ant-design/ant-design/issues/54889 + </a> + </p>src/FixedHolder/index.tsx (1)
39-39: Header/Holder API 从 scrollTableStyle 切到 scrollX — 请同步文档与类型注释对外（或内部）使用方需要知道从样式对象改为数值/字符串/true 的新语义。
src/Body/MeasureRow.tsx (1)
51-52: 渲染钩子需保障可测量性提醒：外层包装不要使用
display: none直接隐藏整行，否则ResizeObserver获取的offsetWidth会为 0，列宽测量失真。建议在文档中强调使用visibility: hidden/opacity: 0之类方案。docs/examples/stickyHeader.tsx (1)
347-360: 新增 “scroll.x is true” 用例合理，但可加断言说明预期行为建议补一句注释或简单断言（在测试中）阐明该场景预期：宽度应撑满且不出现错位。
src/Table.tsx (1)
415-438: body 区域scrollTableStyle对scroll.x === true使用'auto'— 与预期一致与 Holder 中的修复（建议改为
'auto'）保持一致可避免行为分歧。tests/FixedHeader.spec.jsx (7)
31-35: 在 afterEach 恢复真实计时器，避免跨用例计时器状态泄漏。
beforeEach 已 useFakeTimers；建议在每个用例结束后恢复，提升隔离性。beforeEach(() => { measureWidth = 100; vi.useFakeTimers(); visible = true; }); + +afterEach(() => { + vi.useRealTimers(); +});
53-61: 用 safeAct 统一时序刷新；避免对测量行数量的强假设。
- 以 for-of 遍历测量行触发 resize，减少对固定下标的依赖。
- 统一用 safeAct(container) 代替手写 act+runAllTimers。
- await triggerResize(measureCells[0]); - await triggerResize(measureCells[1]); - await triggerResize(measureCells[2]); - act(() => { - vi.runAllTimers(); - }); + for (const row of measureCells) { + await triggerResize(row); + } + await safeAct(container);
64-66: parseInt 请显式指定基数。
微调以避免潜在歧义。-expect(parseInt(container.querySelectorAll('colgroup col')[0].style.width)).toEqual(100); -expect(parseInt(container.querySelectorAll('colgroup col')[1].style.width)).toEqual(100); -expect(parseInt(container.querySelectorAll('colgroup col')[2].style.width)).toEqual(100); +expect(parseInt(container.querySelectorAll('colgroup col')[0].style.width, 10)).toEqual(100); +expect(parseInt(container.querySelectorAll('colgroup col')[1].style.width, 10)).toEqual(100); +expect(parseInt(container.querySelectorAll('colgroup col')[2].style.width, 10)).toEqual(100);
106-109: 用 safeAct 简化时序处理。
当前 act+runAllTimers 可工作；用 safeAct 更一致。- await act(async () => { - vi.runAllTimers(); - await Promise.resolve(); - }); + await safeAct(container);
128-139: 统一 radix 且用 safeAct。
两处小改更一致也更健壮。-expect(parseInt(container.querySelector('col').style.width)).toEqual(100); +expect(parseInt(container.querySelector('col').style.width, 10)).toEqual(100); @@ - await act(async () => { - vi.runAllTimers(); - await Promise.resolve(); - }); + await safeAct(container); @@ -expect(parseInt(container.querySelector('col').style.width)).toEqual(100); +expect(parseInt(container.querySelector('col').style.width, 10)).toEqual(100);
199-255: measureRowRender 用例方向正确；补充一条“无重复 MeasureRow”的断言并提升可读性。
- 额外断言容器内仅有 1 个 .rc-table-measure-row，避免重复渲染回归。
- 使用 getByTestId/within 可读性更好。
- const { container } = render( + const { container, getByTestId } = render( <Table columns={columns} data={data} sticky scroll={{ x: true }} measureRowRender={measureRowRender} />, ); @@ - const measureRowWrapper = container.querySelectorAll('[data-testid="measure-row-wrapper"]'); - expect(measureRowWrapper).toHaveLength(1); - expect(measureRowWrapper[0].style.display).toBe('none'); + const wrapper = getByTestId('measure-row-wrapper'); + expect(wrapper).toBeTruthy(); + expect(wrapper.style.display).toBe('none'); @@ - const measureRowInWrapper = measureRowWrapper[0].querySelectorAll('.rc-table-measure-row'); + const measureRowInWrapper = wrapper.querySelectorAll('.rc-table-measure-row'); expect(measureRowInWrapper).toHaveLength(1); + + // 不应在 wrapper 外重复渲染 MeasureRow + expect(container.querySelectorAll('.rc-table-measure-row')).toHaveLength(1);
200-206: 避免与外层变量同名（可读性）。
局部组件参数 visible 与全局测试控制变量 visible 同名，虽作用域隔离但易混淆。-const FilterDropdown = ({ visible, onVisibleChange }) => ( - <div className="test-filter-dropdown" style={{ display: visible ? 'block' : 'none' }}> +const FilterDropdown = ({ open, onOpenChange }) => ( + <div className="test-filter-dropdown" style={{ display: open ? 'block' : 'none' }}> Filter Content - <button onClick={() => onVisibleChange && onVisibleChange(!visible)}>Toggle</button> + <button onClick={() => onOpenChange && onOpenChange(!open)}>Toggle</button> </div> ); @@ - <FilterDropdown visible={true} onVisibleChange={() => {}} /> + <FilterDropdown open={true} onOpenChange={() => {}} />Also applies to: 213-213
📜 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 (10)
assets/index.less(0 hunks)docs/examples/measureRowRender.tsx(1 hunks)docs/examples/scrollY.tsx(1 hunks)docs/examples/stickyHeader.tsx(5 hunks)src/Body/MeasureRow.tsx(3 hunks)src/ColGroup.tsx(1 hunks)src/FixedHolder/index.tsx(3 hunks)src/Table.tsx(5 hunks)src/context/TableContext.tsx(1 hunks)tests/FixedHeader.spec.jsx(4 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:
docs/examples/measureRowRender.tsxsrc/context/TableContext.tsxsrc/Body/MeasureRow.tsxdocs/examples/scrollY.tsxsrc/FixedHolder/index.tsxdocs/examples/stickyHeader.tsxsrc/Table.tsx
🧬 Code graph analysis (3)
docs/examples/scrollY.tsx (2)
tests/FixedColumn-IE.spec.jsx (1)
columns(27-40)tests/Scroll.spec.jsx (1)
data(8-11)
docs/examples/stickyHeader.tsx (1)
src/interface.ts (1)
ColumnType(118-133)
tests/FixedHeader.spec.jsx (1)
tests/utils.js (1)
safeAct(3-8)
⏰ 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 (13)
src/ColGroup.tsx (1)
43-43: 避免渲染空 colgroup 的处理正确空列时直接返回 null，避免无意义 DOM。该变更与浏览器行为一致，不会影响布局计算。
src/Body/MeasureRow.tsx (1)
23-23: 从上下文读取 measureRowRender 的方式正确按需订阅键减少无关重渲染，符合 @rc-component/context 的最佳实践。
docs/examples/stickyHeader.tsx (3)
140-166: 新增大宽度列组用于复现横向滚动问题 — LGTM用于覆盖
scroll.x为true的场景，列定义合理。
303-315: 示例数据从空对象改为带 key 的记录 — LGTM更贴近真实使用，避免
rowKey警告。
332-342: 示例：宽度未定义列 + 非空数据 — LGTM有助于观察 sticky header 与列宽推断的配合。
src/Table.tsx (4)
181-187: 新增内部属性 measureRowRender — 与上下文保持一致与
TableContextProps对齐，便于下游（MeasureRow）自定义包装。
231-233: props 解构增加 measureRowRender — LGTM
731-731: 向 FixedHolder 传递 scrollX — LGTM匹配新 API 的期望入参。
924-926: 上下文透传 measureRowRender 与依赖项关联 — LGTMAlso applies to: 978-979
tests/FixedHeader.spec.jsx (4)
1-3: 导入组织合理，依赖精简，LGTM。
覆盖了 RTL、ResizeObserver stub 与 domHook 的必要依赖。
20-29: 原型 stub 方案稳健。
对 offsetParent/offsetWidth 的 getter mock 足以模拟可见性与宽度量测。
71-92: INTERNAL_COL_DEFINE 透传校验到位，LGTM。
同时覆盖 header 与 body，能有效防回归。
144-197: 嵌套表头固定列掩蔽用例清晰，LGTM。
断言 fix-start/fix-end 数量到位。
| // 自定义 MeasureRow 渲染，隐藏弹层内容 | ||
| const measureRowRender = measureRow => <div style={{ display: 'none' }}>{measureRow}</div>; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
不要用 display: none 包裹测量行，会导致列宽测量为 0
测量行需要参与布局才能被 ResizeObserver 正确观测。建议使用 visibility: hidden 或 “视觉隐藏” 技术。
- // 自定义 MeasureRow 渲染，隐藏弹层内容
- const measureRowRender = measureRow => <div style={{ display: 'none' }}>{measureRow}</div>;
+ // 自定义 MeasureRow 渲染，隐藏但仍参与布局，保证可测量
+ const measureRowRender = measureRow => (
+ <div style={{ visibility: 'hidden', pointerEvents: 'none' }}>{measureRow}</div>
+ );📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 自定义 MeasureRow 渲染，隐藏弹层内容 | |
| const measureRowRender = measureRow => <div style={{ display: 'none' }}>{measureRow}</div>; | |
| // 自定义 MeasureRow 渲染，隐藏但仍参与布局，保证可测量 | |
| const measureRowRender = measureRow => ( | |
| <div style={{ visibility: 'hidden', pointerEvents: 'none' }}>{measureRow}</div> | |
| ); |
🤖 Prompt for AI Agents
In docs/examples/measureRowRender.tsx around lines 33 to 35, the measurement row
is wrapped with style display: 'none' which removes it from layout and causes
column width measurements to be 0; replace display: 'none' with style that
preserves layout (e.g., visibility: 'hidden') or apply a "visually hidden" CSS
utility so the element remains in the flow for ResizeObserver while remaining
invisible to users.
| minWidth: '100%', | ||
| // https://github.com/ant-design/ant-design/issues/54894 | ||
| width: scrollX, | ||
| }} |
There was a problem hiding this comment.
严重：width: scrollX 可能为 boolean，类型不兼容且会生成无效 CSS
当 scrollX === true 时传入 boolean，将导致 TS 类型错误并在浏览器生成 width: true 的无效样式。与 Table.tsx 的逻辑保持一致应使用 'auto'。
<TableComponent
style={{
tableLayout,
minWidth: '100%',
// https://github.com/ant-design/ant-design/issues/54894
- width: scrollX,
+ width: scrollX === true ? 'auto' : scrollX,
}}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| minWidth: '100%', | |
| // https://github.com/ant-design/ant-design/issues/54894 | |
| width: scrollX, | |
| }} | |
| <TableComponent | |
| style={{ | |
| tableLayout, | |
| minWidth: '100%', | |
| // https://github.com/ant-design/ant-design/issues/54894 | |
| width: scrollX === true ? 'auto' : scrollX, | |
| }} | |
| > |
🤖 Prompt for AI Agents
In src/FixedHolder/index.tsx around lines 200 to 203, the code sets width:
scrollX which can be a boolean and will produce invalid CSS (e.g. width: true)
and TypeScript errors; change the width expression to mirror Table.tsx by using
'auto' when scrollX is true (e.g. width: typeof scrollX === 'boolean' ? 'auto' :
scrollX) so width is always a valid string/number and update typing if needed to
satisfy CSSProperties.
2 participants
Summary by CodeRabbit