-
-
Notifications
You must be signed in to change notification settings - Fork 618
[WIP]get columnHeader's real width #1368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…o the real columnHeader.
Walkthrough在 MeasureCell 增加可选的 columnIndex 属性并移除 title。MeasureCell 在布局时通过 columnIndex 从表头 DOM 读取对应列宽并设置自身宽度,同时保留挂载时触发 onColumnResize。MeasureRow 改为计算 columnIndex 并传给 MeasureCell,停止传递 title。 Changes
Sequence Diagram(s)sequenceDiagram
participant Row as MeasureRow
participant Cell as MeasureCell
participant DOM as Header DOM
participant CB as onColumnResize
Row->>Cell: render({ columnIndex })
Note over Cell: 初始渲染
alt columnIndex 已定义
Cell->>DOM: 查询对应表头单元格宽度
DOM-->>Cell: width(px)
Cell->>Cell: 设置内部容器样式 width
else 未定义
Cell->>Cell: 使用自身测量宽度
end
Cell->>CB: onMount: onColumnResize(offsetWidth)
Note over Cell: 维持原有挂载回调
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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 |
Summary of ChangesHello @Linkodt, 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! 此拉取请求旨在解决表格列头宽度测量不准确的问题,特别是在存在合并表头的情况下。通过引入一种新的宽度测量机制,即利用 Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
本次 PR 旨在通过获取表头单元格的实际宽度来解决复杂表头(如合并表头)下的列宽测量问题。这是一个很好的思路,但目前的实现存在一些问题,可能导致在某些情况下无法正常工作或代码脆弱。
我的主要反馈集中在 MeasureCell.tsx
中:
- 硬编码的 CSS 类名: 直接使用了
.rc-table-thead
和.rc-table-cell
,这会使得在自定义prefixCls
时组件失效。 - 不够健壮的 DOM 查询:
querySelectorAll
的选择器对于分组/合并表头不够精确,可能导致获取到错误的单元格。 useLayoutEffect
依赖项缺失:useLayoutEffect
的依赖数组为空,这会导致 effect 只在组件挂载时运行一次,无法响应 props 的变化。
我提供了一些具体的代码建议来修复这些问题,主要是通过传递 prefixCls
并使用更精确的 DOM 选择器来增强代码的健壮性和正确性。
<MeasureCell | ||
key={columnKey} | ||
columnIndex={columnIndex} | ||
columnKey={columnKey} | ||
onColumnResize={onColumnResize} | ||
title={column?.title} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Body/MeasureCell.tsx (1)
16-27
: 修正测量与上报逻辑:限定作用域到当前表、避免首次 0 宽度上报,并处理 columnIndex = -1当前用 document.querySelectorAll 跨文档查询且硬编码类名,可能跨多表实例误读;同时在 setWidth 之前调用 onColumnResize,首次很容易上报 0 宽度;还未过滤 findIndex 返回 -1 的情况。建议:
- 以当前单元格为起点,限定到最近的 table 再查找最后一行表头(叶子列)。
- 检查 columnIndex >= 0。
- 先测量再上报,或依赖 ResizeObserver 的变更回调;下方示例选择“测量后立即上报”,并添加合理依赖。
- useLayoutEffect(() => { - if (columnIndex !== undefined) { - setWidth( - document - .querySelectorAll('.rc-table-thead >tr > .rc-table-cell') - [columnIndex]?.getClientRects()[0].width || 0, - ); - } - if (cellRef.current) { - onColumnResize(columnKey, cellRef.current.offsetWidth); - } - }, []); + useLayoutEffect(() => { + if (cellRef.current && columnIndex != null && columnIndex >= 0) { + const table = cellRef.current.closest('table'); + const headerCells = table?.querySelectorAll( + `.${prefixCls}-thead > tr:last-child > .${prefixCls}-cell`, + ); + const nextWidth = + headerCells?.[columnIndex]?.getBoundingClientRect().width || 0; + setWidth(nextWidth); + // 直接使用表头测得宽度上报,避免首次 0 宽度 + onColumnResize(columnKey, nextWidth); + } + }, [columnIndex, prefixCls]);
🧹 Nitpick comments (2)
src/Body/MeasureCell.tsx (2)
5-9
: 不要硬编码 rc-table 类名;通过 props 传入 prefixCls 以定位当前表头为支持自定义前缀及多表场景,需要将 prefixCls 传入 MeasureCell 并用于选择器拼装。
export interface MeasureCellProps { columnKey: React.Key; onColumnResize: (key: React.Key, width: number) => void; - columnIndex?: number; + columnIndex?: number; + prefixCls: string; } const MeasureCell: React.FC<MeasureCellProps> = props => { - const { columnKey, onColumnResize, columnIndex } = props; + const { columnKey, onColumnResize, columnIndex, prefixCls } = props; const [width, setWidth] = React.useState(0);Also applies to: 12-13
35-37
: 去掉 fontWeight,减少不必要的样式干扰测量已直接使用表头宽度,内部占位无需加粗,避免潜在的布局差异。
- <div style={{ height: 0, overflow: 'hidden', fontWeight: 'bold', width: `${width}px` }}> + <div style={{ height: 0, overflow: 'hidden', width: `${width}px` }}>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/Body/MeasureCell.tsx
(2 hunks)src/Body/MeasureRow.tsx
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Body/MeasureRow.tsx (1)
tests/FixedColumn-IE.spec.jsx (1)
columns
(27-40)
{columnsKey.map(columnKey => { | ||
const column = columns.find(col => col.key === columnKey); | ||
const columnIndex = columns.findIndex(col => col.key === columnKey); | ||
return ( | ||
<MeasureCell | ||
key={columnKey} | ||
columnIndex={columnIndex} | ||
columnKey={columnKey} | ||
onColumnResize={onColumnResize} | ||
title={column?.title} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
findIndex 容易失配(列 key 可能缺失或规则不同),直接使用 map 的 index 作为列序更稳,并下传 prefixCls
当列未显式设置 key 或内部生成规则不同,findIndex 可能返回 -1,导致测量失败。使用遍历索引与表头叶子列顺序一致,更可靠。
- {columnsKey.map(columnKey => {
- const columnIndex = columns.findIndex(col => col.key === columnKey);
- return (
- <MeasureCell
- key={columnKey}
- columnIndex={columnIndex}
- columnKey={columnKey}
- onColumnResize={onColumnResize}
- />
- );
- })}
+ {columnsKey.map((columnKey, columnIndex) => (
+ <MeasureCell
+ key={columnKey}
+ columnIndex={columnIndex}
+ prefixCls={prefixCls}
+ columnKey={columnKey}
+ onColumnResize={onColumnResize}
+ />
+ ))}
📝 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.
{columnsKey.map(columnKey => { | |
const column = columns.find(col => col.key === columnKey); | |
const columnIndex = columns.findIndex(col => col.key === columnKey); | |
return ( | |
<MeasureCell | |
key={columnKey} | |
columnIndex={columnIndex} | |
columnKey={columnKey} | |
onColumnResize={onColumnResize} | |
title={column?.title} | |
/> | |
{columnsKey.map((columnKey, columnIndex) => ( | |
<MeasureCell | |
key={columnKey} | |
columnIndex={columnIndex} | |
prefixCls={prefixCls} | |
columnKey={columnKey} | |
onColumnResize={onColumnResize} | |
/> | |
))} |
🤖 Prompt for AI Agents
In src/Body/MeasureRow.tsx around lines 37 to 45, using columns.findIndex(col =>
col.key === columnKey) can return -1 when keys are missing or differ; change to
use the map iteration index (the second argument of map) as the stable
columnIndex to match header leaf order, and pass the component's prefixCls down
to MeasureCell (add prefixCls={prefixCls}) so styling/namespace is preserved.
Ensure you replace columnIndex assignment with the map index, remove reliance on
findIndex, and include prefixCls in the MeasureCell props.
The latest updates on your projects. Learn more about Vercel for GitHub.
|
@afc163 @crazyair 大佬们看看呢?
可以在 useLayoutEffect 内通过 getClientRects 获取header 宽度。
当前碰到的问题:
Summary by CodeRabbit
新增功能
Bug 修复
样式