-
-
Notifications
You must be signed in to change notification settings - Fork 616
feat: Table scrollTo support offset #1301
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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough本次变更为表格组件的滚动功能引入了 offset(偏移量)参数,支持在按 index 或 key 滚动时,额外指定垂直偏移像素。相关类型定义、核心实现、示例和测试均已同步更新,确保 offset 能正确影响滚动行为,并在不同场景下被验证。 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户
participant Button as 按钮
participant TableRef as 表格引用
participant Table as Table组件
participant DOM as DOM容器
User->>Button: 点击“滚动到指定行(带offset)”
Button->>TableRef: 调用 scrollTo({ index/key, offset })
TableRef->>Table: 触发 scrollTo 方法
Table->>DOM: 计算目标元素 offsetTop
alt 提供 offset
Table->>DOM: 滚动到 offsetTop + offset
else 未提供 offset
Table->>DOM: scrollIntoView()
end
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/VirtualTable/BodyGrid.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. tests/Virtual.spec.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "prettier" to extend from. Please check that the name of the config is correct. The config "prettier" was referenced from the config file in "/.eslintrc.js". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1301 +/- ##
==========================================
+ Coverage 95.69% 95.71% +0.02%
==========================================
Files 57 57
Lines 3391 3410 +19
Branches 655 660 +5
==========================================
+ Hits 3245 3264 +19
Misses 121 121
Partials 25 25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
🧹 Nitpick comments (1)
tests/refs.spec.tsx (1)
66-111
: 测试用例设计良好,建议增加边缘情况覆盖。新增的 offset 功能测试用例结构清晰,正确验证了以下核心场景:
top
参数忽略offset
(绝对定位逻辑正确)index/key
参数正确应用offset
(相对偏移逻辑正确)- 无
offset
时回退到scrollIntoView
(向后兼容性正确)测试断言准确,假设
offsetTop
为 0 在测试环境中是合理的。建议考虑增加以下边缘情况的测试覆盖:
+ // Test negative offset + ref.current.scrollTo({ + index: 0, + offset: -10, + }); + expect(scrollParam.top).toEqual(-10); + + // Test with larger offset + ref.current.scrollTo({ + key: 'bamboo', + offset: 100, + }); + expect(scrollParam.top).toEqual(100);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/examples/scrollY.tsx
(2 hunks)docs/examples/virtual.tsx
(1 hunks)src/Table.tsx
(1 hunks)src/interface.ts
(1 hunks)tests/Virtual.spec.tsx
(1 hunks)tests/refs.spec.tsx
(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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` 是解决该问题所必须的。
tests/Virtual.spec.tsx (1)
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` 是解决该问题所必须的。
src/interface.ts (1)
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` 是解决该问题所必须的。
docs/examples/virtual.tsx (1)
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` 是解决该问题所必须的。
docs/examples/scrollY.tsx (1)
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` 是解决该问题所必须的。
tests/refs.spec.tsx (1)
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` 是解决该问题所必须的。
src/Table.tsx (1)
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` 是解决该问题所必须的。
🧬 Code Graph Analysis (3)
tests/Virtual.spec.tsx (1)
src/interface.ts (1)
Reference
(48-51)
tests/refs.spec.tsx (1)
src/interface.ts (1)
Reference
(48-51)
src/Table.tsx (1)
src/utils/valueUtil.tsx (1)
validNumberValue
(40-42)
⏰ 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 (7)
src/interface.ts (1)
32-46
: 接口设计良好,文档完整
ScrollConfig
类型的扩展设计合理:
offset
属性是可选的,保证了向后兼容性- JSDoc 文档清晰地说明了
offset
的行为和限制- 类型定义正确,符合 TypeScript 最佳实践
docs/examples/scrollY.tsx (2)
6-6
: 数据增加合理将数据数量从 10 增加到 20 能更好地演示滚动功能。
71-90
: 示例实现正确,演示效果好新增的两个按钮正确演示了 offset 功能:
- 分别测试了 index 和 key 模式
- 使用 -10 偏移量能提供良好的视觉效果
- 与接口设计保持一致
docs/examples/virtual.tsx (1)
202-207
: 虚拟表格示例实现正确新增按钮正确演示了虚拟表格的 offset 功能:
- 与 scrollY 示例保持一致的实现方式
- 使用不同的测试值(index 10, key '50')增加了示例的多样性
- 与 VirtualTable 组件集成良好
src/Table.tsx (3)
345-345
: 正确解构 offset 参数从 config 中正确解构了
offset
参数,与接口定义保持一致。
347-349
: 正确处理 top 模式在 top 模式下正确忽略了 offset 参数,符合接口文档的规定。注释清晰地说明了这一行为。
352-364
: offset 逻辑实现正确实现逻辑完善且符合预期:
- 当没有 offset 时,保持原有的
scrollIntoView()
行为,确保向后兼容- 当有 offset 时,使用
elementTop + offset
计算滚动位置,实现精确定位- 正确处理了目标元素不存在的情况
- 类型安全,与接口定义保持一致
tests/Virtual.spec.tsx
Outdated
it('scrollTo with offset should pass', async () => { | ||
const tblRef = React.createRef<Reference>(); | ||
getTable({ ref: tblRef }); | ||
|
||
// Test index with offset | ||
tblRef.current.scrollTo({ | ||
index: 50, | ||
offset: 20, | ||
}); | ||
|
||
await waitFakeTimer(); | ||
|
||
expect(global.scrollToConfig).toEqual({ | ||
index: 50, | ||
offset: 20, | ||
}); | ||
|
||
// Test key with offset | ||
tblRef.current.scrollTo({ | ||
key: '25', | ||
offset: -10, | ||
}); | ||
|
||
await waitFakeTimer(); | ||
|
||
expect(global.scrollToConfig).toEqual({ | ||
key: '25', | ||
offset: -10, | ||
}); | ||
|
||
// Test top with offset (offset should be passed to virtual list) | ||
tblRef.current.scrollTo({ | ||
top: 100, | ||
offset: 30, | ||
}); | ||
|
||
await waitFakeTimer(); | ||
|
||
expect(global.scrollToConfig).toEqual({ | ||
top: 100, | ||
offset: 30, | ||
}); | ||
}); |
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.
💡 Verification agent
🧩 Analysis chain
测试覆盖全面,但需要确认 top 模式的预期行为
测试用例很好地覆盖了三种滚动模式,但对于 top 模式的测试需要确认:
根据接口文档,当使用 top
模式时,offset
应该被忽略。但当前测试验证 scrollToConfig
包含 offset: 30
,这可能不符合预期行为。
请运行以下脚本验证 Table 组件中 top 模式下 offset 的实际处理逻辑:
🏁 Script executed:
#!/bin/bash
# 查找 Table.tsx 中处理 top 模式的逻辑
ast-grep --pattern $'if (validNumberValue(top)) {
$$$
} else {
$$$
}'
Length of output: 1239
请修正 “top” 模式下的 offset 预期
根据 src/Table.tsx
(第 347 行)中的实现,top
模式下会忽略 offset
,仅调用:
scrollBodyRef.current?.scrollTo({ top });
但当前测试却期望 global.scrollToConfig
包含 offset: 30
,与组件逻辑不符。
需要更新的地方:
- 文件:
tests/Virtual.spec.tsx
- 行号:371-413
- 将最后一个断言由:
修改为:
expect(global.scrollToConfig).toEqual({ top: 100, offset: 30, });
expect(global.scrollToConfig).toEqual({ top: 100, });
🤖 Prompt for AI Agents
In tests/Virtual.spec.tsx lines 371 to 413, the test for scrollTo with top and
offset incorrectly expects global.scrollToConfig to include offset: 30, but
according to src/Table.tsx line 347, offset is ignored in top mode. Fix this by
changing the last expect assertion to only check for { top: 100 } without the
offset property.
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: 0
🧹 Nitpick comments (1)
src/VirtualTable/BodyGrid.tsx (1)
81-92
: 注意用户 align 参数被静默覆盖的问题当前实现在存在
offset
时会强制将align
设为'top'
,这可能会静默覆盖用户显式提供的align
参数,导致意外行为。建议考虑以下改进方案:
- 在文档中明确说明当提供
offset
时align
会被强制设为'top'
- 或者在用户同时提供两个参数时给出警告
- 或者允许用户的
align
设置优先于默认的'top'
const { offset, ...restConfig } = config; -// If offset is provided, force align to 'top' for consistent behavior if (offset) { + // If offset is provided, default align to 'top' for consistent behavior unless user specifies otherwise listRef.current?.scrollTo({ ...restConfig, offset, - align: 'top', + align: restConfig.align || 'top', }); } else { listRef.current?.scrollTo(config); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/VirtualTable/BodyGrid.tsx
(1 hunks)tests/Virtual.spec.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Virtual.spec.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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` 是解决该问题所必须的。
src/VirtualTable/BodyGrid.tsx (1)
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` 是解决该问题所必须的。
⏰ 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 (1)
src/VirtualTable/BodyGrid.tsx (1)
83-83
: 请验证 rc-virtual-list 的 scrollTo 方法是否支持 offset 参数在 VirtualTable/BodyGrid.tsx 第 83 行使用了
offset
,需要确认底层依赖对该参数的支持情况,以避免运行时错误。请:
- 查阅当前安装的
rc-virtual-list
版本文档或源码中的ListRef
接口定义,确认scrollTo
方法签名是否包含offset
参数。- 如发现不支持
offset
,需调整实现逻辑或升级到支持该参数的版本。
ref: ant-design/ant-design#54322
Summary by CodeRabbit
新功能
文档
测试