fix: fix first rows scroll down when expand#1256
fix: fix first rows scroll down when expand#1256Vinh241 wants to merge 2 commits intoreact-component:masterfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改主要在 Changes
Sequence Diagram(s)sequenceDiagram
participant User as 用户操作
participant Grid as 网格组件
participant Effect as useEffect钩子
participant Render as extraRender函数
participant Scroll as handleScroll函数
User->>Grid: 触发行展开/收缩
Grid->>Effect: 监测expandedKeys和data变化
Effect->>Grid: 检查第一行展开状态变化
alt 第一行状态发生变化
Effect-->>Grid: 更新 firstRowExpandChangeRef 为 true, 设置 shouldPreserveScroll 为 false
else 非第一行或无变化
Effect-->>Grid: 保持shouldPreserveScroll为true
end
Grid->>Render: 执行extraRender逻辑
Render->>Grid: 根据状态重置或恢复滚动位置
Grid->>Scroll: 触发onScroll事件，更新scrollPositionRef
Assessment against linked issues
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. Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 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 (
|
Vinh241
changed the title
Vinh241
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/VirtualTable/BodyGrid.tsx (1)
69-94: 监控展开状态变化的副作用实现这个 useEffect 的实现很好地监控了展开状态的变化，特别是区分了第一行和其他行的展开情况。通过比较前后的展开状态，能够判断是否需要保持滚动位置。
建议：当数据量非常大时，每次都创建新的 Set 可能会有性能影响，可以考虑只在必要时更新 prevExpandedKeysRef。
- prevExpandedKeysRef.current = new Set(expandedKeys); + // 只在键集合有变化时才创建新的 Set + if (!setsAreEqual(prevExpandedKeysRef.current, expandedKeys)) { + prevExpandedKeysRef.current = new Set(expandedKeys); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/VirtualTable/BodyGrid.tsx(4 hunks)tests/Virtual.spec.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/VirtualTable/BodyGrid.tsx (3)
src/interface.ts (1)
Key(21-21)src/hooks/useFlattenRecords.ts (1)
useFlattenRecords(57-88)tests/Table.spec.jsx (1)
data(10-13)
🔇 Additional comments (5)
tests/Virtual.spec.tsx (1)
563-690: 测试用例全面覆盖了展开行时的滚动问题测试很好地验证了在展开非第一行时，滚动位置能够被正确保留，而不会意外滚动到表格顶部。测试方法通过监听 scrollTo 方法调用来确认没有不必要的滚动发生。
建议：
- 测试中仅检查了展开行的情况，建议也添加折叠行的测试用例，确保两种情况都能正常工作
- 可以考虑在测试中加入滚动到中间位置的步骤，以更好地模拟真实用户场景
src/VirtualTable/BodyGrid.tsx (4)
54-64: 新增的引用变量用于跟踪滚动位置和展开状态这些引用变量很好地解决了展开行时滚动位置的管理问题。每个变量都有明确的用途：
- scrollPositionRef：跟踪当前滚动位置
- prevExpandedKeysRef：存储之前的展开键以便比较
- firstRowExpandChangeRef：标记是否是第一行的展开状态变化
- shouldPreserveScroll：决定是否应保留滚动位置
代码结构清晰，变量命名准确反映了其用途。
151-178: 滚动位置恢复逻辑这段代码很好地解决了两种情况下的滚动位置处理：
- 第一行展开/折叠时，滚动到顶部
- 非第一行变化时，保持滚动位置
使用 requestAnimationFrame 确保滚动操作在渲染完成后执行，这是一个很好的实践。
建议：可以考虑添加一些调试日志（在开发环境），以便于排查潜在的滚动问题。
272-282: 滚动事件处理函数handleScroll 函数实现了两个功能：
- 记录当前滚动位置
- 调用原始的滚动回调
逻辑简洁明了，依赖项设置正确。这样的封装方式不会影响原有功能，同时增加了新的行为。
321-321: 更新 VirtualList 的 onScroll 属性使用新的 handleScroll 函数替换原有的 onTablePropScroll，确保滚动位置可以被正确记录和恢复。这种方式不会破坏原有功能，同时增加了新的滚动位置管理能力。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1256 +/- ##
========================================
Coverage 98.03% 98.04%
========================================
Files 76 76
Lines 7425 7558 +133
Branches 1146 1164 +18
========================================
+ Hits 7279 7410 +131
- Misses 140 142 +2
Partials 6 6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/VirtualTable/BodyGrid.tsx (2)
54-65: 在多处使用 Ref 时注意命名与职责划分。
这些 Ref 变量能够清晰追踪滚动位置和首行展开状态，整体实现思路合理。但建议确认命名是否足够直观，并评估后续是否会有更多行的展开逻辑需求，以避免后期改动较大。
259-270: 可考虑同时记录水平滚动位置。
目前只存储并更新scrollTop，若后续需要在首行展开时保持水平位置，可能需要scrollLeft同步。可根据需求决定是否要补充对横向滚动的跟踪。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/VirtualTable/BodyGrid.tsx(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/VirtualTable/BodyGrid.tsx (2)
src/interface.ts (1)
Key(21-21)src/hooks/useFlattenRecords.ts (1)
useFlattenRecords(57-88)
🔇 Additional comments (3)
src/VirtualTable/BodyGrid.tsx (3)
69-94: 确保首行逻辑在数据变化时不会产生意外行为。
当前仅通过比较首行记录的展开键来判断，若数据顺序或首行记录发生变动（例如数据被重新排序），可能会导致误判。请验证是否需在其他场景下也对首行展开进行特殊处理，或保证数据的首行恒定不变。
151-166: 检查注释内容与 scrollTo({ top: 0 }) 行为是否一致。
注释中写到“不要进行自动滚动”，但实际代码仍将滚动位置设置为顶部，这可能与预期不符。请确认是故意将视图重置到顶部，还是应保持原有滚动位置。
308-308: 改为使用自定义事件时，请注意回调执行顺序。
此处替换为onScroll={handleScroll}后，滚动回调会在handleScroll中先更新scrollPositionRef，再调用原先的onTablePropScroll。如需在外部组件监听更改后的滚动位置，应保证调用次序满足需求。
yoyo837
changed the title
| if (expanded) { | ||
| expandedRowKeys = [...expandedRowKeys, record.name]; | ||
| } else { | ||
| expandedRowKeys = expandedRowKeys.filter(key => key !== record.name); |
| requestAnimationFrame(() => { | ||
| // Keep the scroll position at its current position rather than jumping | ||
| if (listRef.current) { | ||
| listRef.current.scrollTo({ top: 0 }); |
There was a problem hiding this comment.
This seems be the rc-virtual-list length extends bug. Not a better place to fix in rc-table
3 participants
fix ant-design/ant-design#53334
Summary by CodeRabbit
Summary by CodeRabbit
新功能
Tests