Fix/gap fixed columns shadow #1202
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough本次更改主要涉及表格组件的样式和功能增强。具体来说，移除了 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Warning Tool Failures:Tool Failure Count:Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (6)
src/context/TableContext.tsx (1)
69-69: 新增的 headerCellRefs 属性设计合理headerCellRefs 使用 MutableRefObject 存储表头单元格引用是合适的选择，这对于实现固定列的阴影效果是必要的。建议在使用时注意以下几点：
- 确保在组件卸载时正确清理引用
- 考虑在列重新排序时更新引用
- 建议添加相关文档说明用途
src/Body/BodyRow.tsx (1)
Line range hint
1-240: 建议：考虑添加单元测试建议为固定列的阴影效果添加相应的单元测试，确保在不同的列位置和滚动状态下，阴影效果都能正常显示。
需要我帮您生成相关的测试用例吗？可以包括：
- 测试固定列的阴影是否正确显示
- 测试多个固定列的堆叠效果
- 测试滚动时阴影的动态变化
src/Cell/index.tsx (2)
185-194: 优化了固定列的阴影逻辑这段代码通过计算单元格的实际位置来确定是否应该显示阴影。建议添加以下注释来提高可维护性：
let mergedLastFixLeft = lastFixLeft; const { current } = headerCellRefs; const dom = current[colIndex]; if (lastFixLeft && dom) { + // 计算单元格相对于父元素的偏移量 const offsetLeft = dom.getBoundingClientRect().x - dom.parentElement.getBoundingClientRect().x || 0; + // 只有当单元格确实处于粘性定位状态时才显示阴影 mergedLastFixLeft = typeof fixLeft === 'number' && offsetLeft === fixLeft + bodyScrollLeft; }
185-194: 建议添加单元测试这个新的阴影计算逻辑是关键功能，建议添加相应的单元测试来确保其可靠性。
需要我帮助编写测试用例吗？可以包括以下场景：
- 正常滚动时的阴影显示
- 边界条件下的阴影行为
- 不同列宽度下的阴影效果
tests/FixedColumn.spec.tsx (2)
28-52: getBoundingClientRect 模拟实现看起来不错，建议小幅改进模拟实现合理地提供了表格行和单元格的尺寸数据，这对于测试固定列阴影效果很有帮助。不过建议考虑以下几点改进：
- 为不同类型的元素（如 TH、TD）添加更多特定的尺寸处理
- 添加注释说明每个尺寸值的用途
277-290: 批量调整大小的测试实现有效，建议扩展测试场景当前测试很好地验证了固定列的样式类应用，但建议添加以下测试场景：
- 测试不同滚动位置时阴影的显示状态
- 验证多个固定列堆叠时的阴影效果
- 添加边界情况的测试，如列宽很窄或很宽的情况
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
assets/index.less(0 hunks)src/Body/BodyRow.tsx(1 hunks)src/Cell/index.tsx(8 hunks)src/Header/HeaderRow.tsx(2 hunks)src/Table.tsx(4 hunks)src/context/TableContext.tsx(1 hunks)tests/FixedColumn.spec.tsx(2 hunks)
💤 Files with no reviewable changes (1)
- assets/index.less
🧰 Additional context used
🪛 Biome
src/Header/HeaderRow.tsx
[error] 70-70: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (11)
src/context/TableContext.tsx (1)
71-71: bodyScrollLeft 属性的添加有助于解决阴影问题
这个可选属性用于跟踪表格体的水平滚动位置，对于控制固定列的阴影显示至关重要。
运行以下脚本验证实现：
✅ Verification successful
Let me gather more information about the usage of bodyScrollLeft in TypeScript files.
Based on the search results, I can now provide a final response:
bodyScrollLeft 属性的添加确实对固定列阴影效果至关重要
代码分析显示该属性在以下关键场景中发挥作用：
- 在
src/Table.tsx中用于跟踪表格体的水平滚动位置 - 在
src/Cell/index.tsx中用于计算固定列的阴影效果 - 与
assets/index.less中定义的阴影样式配合使用，实现了固定列的视觉分隔效果
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 bodyScrollLeft 的使用情况
# 预期：在滚动处理和阴影效果相关的代码中会使用到这个属性
# 搜索与滚动相关的实现
rg -A 5 "bodyScrollLeft|onScroll|scrollLeft" --type typescript
# 搜索阴影相关的样式定义
rg -A 5 "box-shadow|shadow" --type css --type less
Length of output: 2104
Script:
#!/bin/bash
# Search for bodyScrollLeft in all files (since typescript extension wasn't recognized)
rg -A 5 "bodyScrollLeft"
# Search for scroll related implementations
rg -A 5 "onScroll|scrollLeft"
Length of output: 33839
src/Header/HeaderRow.tsx (1)
35-39: 验证 headerCellRefs 的使用情况
新增的 headerCellRefs 上下文值看起来实现正确。建议验证其在其他组件中的使用情况。
✅ Verification successful
Let me gather more information about how headerCellRefs is used in these files.
headerCellRefs 的实现和使用验证完成
经过验证，headerCellRefs 的实现和使用是合理的：
- 在 Table 组件中正确初始化为 useRef 数组
- 在 TableContext 中定义了正确的类型
- 在 HeaderRow 中用于存储单元格引用
- 在 Cell 组件中用于计算固定列的位置
实现遵循了 React 的最佳实践，没有发现任何问题。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：验证 headerCellRefs 的使用情况
# 预期：找到所有使用 headerCellRefs 的地方
# 搜索所有使用 headerCellRefs 的文件
rg "headerCellRefs" -l
# 检查 TableContext 的定义
ast-grep --pattern 'interface TableContextProps {
$$$
headerCellRefs: $_
$$$
}'
Length of output: 197
Script:
#!/bin/bash
# 检查 headerCellRefs 在各个文件中的具体使用情况
rg "headerCellRefs" -A 3 -B 3
# 检查 TableContext 中 headerCellRefs 的定义
rg "headerCellRefs.*:" src/context/TableContext.tsx
# 检查 Table 组件中 headerCellRefs 的初始化
ast-grep --pattern 'const headerCellRefs = $_'
Length of output: 3310
src/Body/BodyRow.tsx (1)
170-170: 新增 colIndex 属性传递到 Cell 组件
这个改动为 Cell 组件添加了列索引信息，这对于实现固定列的阴影效果是必要的。通过 colIndex，Cell 组件可以确定自己在表格中的位置，从而正确应用阴影样式。
让我们验证一下这个改动是否与其他文件的变更相配合：
✅ Verification successful
colIndex 属性的添加是合理且必要的
通过代码分析，我们可以确认：
- Cell 组件的接口定义中已包含
colIndex属性（src/Cell/index.tsx） colIndex在多个组件中被正确使用，包括：- 用于固定列信息的计算（src/hooks/useFixedInfo.ts）
- 用于单元格属性的确定（src/Body/BodyRow.tsx）
- 用于列宽度的计算（src/VirtualTable/VirtualCell.tsx）
- 阴影样式定义存在于 assets/index.less 中
这个改动是整个固定列阴影功能实现的重要组成部分，与其他组件的实现保持一致。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查 Cell 组件是否正确处理了 colIndex 属性，以及是否有相关的样式定义
# 测试：查找 Cell 组件中 colIndex 的使用
ast-grep --pattern 'interface CellProps {
$$$
colIndex?: number;
$$$
}'
# 测试：查找与固定列阴影相关的样式定义
rg -l "box-shadow|shadow" --type css
Length of output: 267
Script:
#!/bin/bash
# 查找 Cell 组件的定义和实现
ast-grep --pattern 'interface CellProps {
$$$
}'
# 查找 Cell 组件中使用 colIndex 的地方
ast-grep --pattern 'function Cell($$$) {
$$$
}'
# 查找样式文件中的阴影定义
fd -e css -e less -e scss -x rg -l "box-shadow|shadow" {}
# 查找其他使用 colIndex 的组件
rg "colIndex" -A 3 -B 3
Length of output: 7820
src/Cell/index.tsx (5)
25-26: 新增 colIndex 属性以支持列索引
属性定义清晰，注释说明充分。这个改动有助于跟踪单元格在表格中的水平位置。
77-80: 组件现在支持 ref 转发
通过 React.ForwardedRef 实现了对底层 DOM 元素的引用访问，这是一个很好的改进。
129-136: 扩展了上下文使用
新增了 bodyScrollLeft 和 headerCellRefs 的上下文消费，这些对于处理固定列的阴影效果是必要的。
216-217: 更新了固定列的类名逻辑
使用 mergedLastFixLeft 替代 lastFixLeft 使得阴影效果的应用更加准确。
282-284: 完善了组件导出方式
使用 React.memo 和 React.forwardRef 优化了组件的性能和可用性。TypeScript 类型定义也很完善。
src/Table.tsx (3)
438-438: 初始化 bodyScrollLeft 状态
正确地初始化了 bodyScrollLeft 状态，用于跟踪表格主体的横向滚动位置。
445-445: 在滚动事件中更新 bodyScrollLeft
在 onInternalScroll 函数中正确地更新了 bodyScrollLeft，确保滚动位置能够被实时跟踪。
801-801: 初始化 headerCellRefs 引用
通过 React.useRef() 初始化了 headerCellRefs，用于存储表头单元格的引用，便于后续操作。
|
看下上面 CR Bot 的评论 |
已resolve 解决此问题必须要每次滚动时重新计算，无解，目前看没有性能问题 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1202 +/- ##
==========================================
+ Coverage 97.75% 97.77% +0.01%
==========================================
Files 75 75
Lines 7484 7540 +56
Branches 1121 1144 +23
==========================================
+ Hits 7316 7372 +56
Misses 162 162
Partials 6 6 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/Cell/index.tsx (3)
Line range hint
77-136: 建议补充新增上下文属性的用途说明新增的
bodyScrollLeft和headerCellRefs对实现固定列阴影效果很重要，建议添加注释说明它们的具体用途，以便其他开发者理解。const { + // 用于计算固定列阴影效果 supportSticky, allColumnsFixedLeft, rowHoverable, + // 表格水平滚动位置，用于计算固定列状态 bodyScrollLeft, + // 列头单元格的引用，用于获取固定列位置信息 headerCellRefs, } = useContext(TableContext, [/*...*/]);
185-197: 建议优化 mergedLastFixLeft 的性能当前实现在每次渲染时都会进行 DOM 测量，这可能会影响性能。建议考虑以下优化方案：
- 使用 ResizeObserver 监听位置变化
- 在必要时才触发重新计算
- 缓存计算结果
+ // 使用 ResizeObserver 监听位置变化 + const [offsetLeft, setOffsetLeft] = React.useState(0); + + React.useEffect(() => { + const { current } = headerCellRefs; + const dom = current[colIndex]; + + if (dom) { + const observer = new ResizeObserver(() => { + const newOffset = dom.getBoundingClientRect().x - + dom.parentElement.getBoundingClientRect().x || 0; + setOffsetLeft(newOffset); + }); + + observer.observe(dom); + return () => observer.disconnect(); + } + }, [colIndex, headerCellRefs]); + const mergedLastFixLeft = React.useMemo(() => { - const { current } = headerCellRefs; - const dom = current[colIndex]; - if (lastFixLeft && dom && typeof fixLeft === 'number') { - const offsetLeft = - dom.getBoundingClientRect().x - dom.parentElement.getBoundingClientRect().x || 0; + if (lastFixLeft && typeof fixLeft === 'number') { return offsetLeft === fixLeft + bodyScrollLeft; } return lastFixLeft; - }, [bodyScrollLeft, colIndex, fixLeft, headerCellRefs, lastFixLeft]); + }, [bodyScrollLeft, offsetLeft, fixLeft, lastFixLeft]);
285-287: 建议为组件导出添加 JSDoc 注释为了提高代码可读性，建议添加完整的 JSDoc 注释说明组件的用途、参数和使用方式。
+/** + * 表格单元格组件 + * @template RecordType 数据记录类型 + * @param {CellProps<RecordType>} props 组件属性 + * @param {React.ForwardedRef<HTMLTableCellElement>} ref 转发的 ref + * @returns {React.JSX.Element} 渲染的单元格组件 + */ export default React.memo(React.forwardRef(Cell)) as <RecordType>( props: CellProps<RecordType> & { ref?: React.ForwardedRef<HTMLTableCellElement> }, ) => React.JSX.Element;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/Cell/index.tsx(8 hunks)src/Header/HeaderRow.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Header/HeaderRow.tsx
🔇 Additional comments (2)
src/Cell/index.tsx (2)
25-26: 新增的 colIndex 属性定义清晰明确！
类型定义和注释说明都很到位，有助于代码的可维护性。
219-220: 类名逻辑更新准确到位！
使用 mergedLastFixLeft 来控制固定列阴影的类名添加逻辑非常合理，完美解决了堆叠固定列的阴影问题。
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
tests/Scroll.spec.jsx (2)
53-116: 测试设置的改进建议测试设置中的 mock 实现非常全面，但建议进行以下优化：
- 建议将 mock 值提取为常量，便于维护和重用
- 建议添加更多边界情况的测试用例
建议这样重构：
+ const MOCK_DIMENSIONS = { + scrollWidth: 200, + clientWidth: 100, + offsetWidth: 200 + }; beforeEach(() => { vi.useFakeTimers(); domSpy = spyElementPrototypes(HTMLElement, { // ... 使用 MOCK_DIMENSIONS 中的值 }); });
174-243: 固定列阴影类名测试的改进建议这个测试用例验证了固定列阴影的行为，但有以下几点建议：
- 注释掉的代码应该被清理（175-180行）
console.log调试代码应该被移除（219行）- 建议添加更多滚动位置的测试点
建议移除以下代码：
- // const cellSpy = spyElementPrototypes(HTMLElement, { - // offsetParent: { - // get: () => ({}), - // }, - // }); - console.log(wrapper.find('th').first(), wrapper.find('th').at(2));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
tests/Scroll.spec.jsx(2 hunks)
🔇 Additional comments (1)
tests/Scroll.spec.jsx (1)
6-6: 从 utils 导入 safeAct 函数
引入 safeAct 工具函数有助于处理异步更新，这是一个很好的改进。
|
|
||
| vi.runAllTimers(); | ||
| await safeAct(wrapper); | ||
| console.log(wrapper.find('th').first(), wrapper.find('th').at(2)); |
There was a problem hiding this comment.
移除调试代码
请移除 console.log 语句，这些调试代码不应该出现在生产代码中。
| it('scroll event', () => { | ||
| const newColumns = [ | ||
| { title: 'title1', dataIndex: 'a', key: 'a', width: 100, fixed: 'left' }, | ||
| { title: 'title2', dataIndex: 'b', key: 'b' }, | ||
| { title: 'title3', dataIndex: 'c', key: 'c' }, | ||
| { title: 'title4', dataIndex: 'd', key: 'd', width: 100, fixed: 'right' }, | ||
| ]; | ||
| const newData = [ | ||
| { a: '123', b: 'xxxxxxxx', c: 3, d: 'hehe', key: '1' }, | ||
| { a: 'cdd', b: 'edd12221', c: 3, d: 'haha', key: '2' }, | ||
| ]; | ||
| const wrapper = mount( | ||
| <Table | ||
| columns={newColumns} | ||
| data={newData} | ||
| scroll={{ | ||
| x: 200, | ||
| y: 200, | ||
| }} | ||
| />, | ||
| ); | ||
|
|
||
| vi.runAllTimers(); | ||
| // Use `onScroll` directly since simulate not support `currentTarget` | ||
| act(() => { | ||
| const headerDiv = wrapper.find('div.rc-table-header').instance(); | ||
|
|
||
| const wheelEvent = new WheelEvent('wheel'); | ||
| Object.defineProperty(wheelEvent, 'deltaX', { | ||
| get: () => 10, | ||
| }); | ||
|
|
||
| headerDiv.dispatchEvent(wheelEvent); | ||
| vi.runAllTimers(); | ||
| }); | ||
|
|
||
| expect(setScrollLeft).toHaveBeenCalledWith(undefined, 10); | ||
| setScrollLeft.mockReset(); | ||
|
|
||
| act(() => { | ||
| wrapper | ||
| .find('.rc-table-body') | ||
| .props() | ||
| .onScroll({ | ||
| currentTarget: { | ||
| scrollLeft: 33, | ||
| scrollWidth: 200, | ||
| clientWidth: 100, | ||
| }, | ||
| }); | ||
| }); | ||
| vi.runAllTimers(); | ||
| expect(setScrollLeft).toHaveBeenCalledWith(undefined, 33); | ||
| }); | ||
| vi.runAllTimers(); | ||
| expect(setScrollLeft).toHaveBeenCalledWith(undefined, 33); | ||
| setScrollLeft.mockReset(); | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
需要补充滚动事件边界测试用例
经过代码分析，当前的滚动事件测试确实缺少关键的边界场景测试。建议在 tests/Scroll.spec.jsx 中补充以下测试用例：
- 负值滚动：测试
deltaX和scrollLeft为负数时的处理 - 最大值边界：测试
scrollLeft等于或超过scrollWidth - clientWidth的情况 - 快速连续滚动：测试短时间内多次触发滚动事件的行为
当前测试只验证了基本的滚动值（10和33），未能完整覆盖边界情况，可能导致边界条件下的滚动行为异常。
🔗 Analysis chain
滚动事件测试需要补充边界情况
当前的滚动事件测试覆盖了基本场景，但缺少以下边界情况的测试：
- 负数滚动值
- 超出最大滚动范围的值
- 快速连续滚动的情况
Let me check the content of these scroll-related test files to see if they cover the boundary cases mentioned in the review comment.
Let me check the specific scroll event handling implementation to better understand the current test coverage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有其他滚动相关的测试用例
rg -l "scroll" tests/
Length of output: 468
Script:
#!/bin/bash
# Check scroll event test cases in all test files
rg "scroll.*test|test.*scroll" tests/ -B 2 -A 2
# Specifically look for boundary test cases
rg "scroll.*(negative|max|boundary|exceed|consecutive|continuous)" tests/ -B 2 -A 2
Length of output: 3258
Script:
#!/bin/bash
# Check scroll event handling implementation
ast-grep --pattern 'onScroll($$$) {
$$$
}'
# Check the content of Scroll.spec.jsx for all scroll-related tests
cat tests/Scroll.spec.jsx
Length of output: 7976
fix: ant-design/ant-design#47834
Summary by CodeRabbit
新功能
summary属性，允许用户提供自定义摘要。Cell和HeaderRow组件，增强了对列索引的支持。修复
文档