-
-
Notifications
You must be signed in to change notification settings - Fork 141
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
feat: virtual list #546
base: master
Are you sure you want to change the base?
feat: virtual list #546
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough本次更改主要涉及 Changes
Possibly related PRs
Poem
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 (
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
|
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: 2
🧹 Outside diff range and nitpick comments (5)
src/context.ts (1)
25-27
: 建议为新属性添加注释说明新增的虚拟滚动相关属性类型定义正确,但建议添加 JSDoc 注释来说明各属性的用途、默认值和使用示例,以提升开发体验。
建议添加如下注释:
+ /** 是否开启虚拟滚动 */ virtual?: boolean; + /** 下拉列表的高度 */ listHeight?: number; + /** 下拉列表项的高度 */ listItemHeight?: number;src/Cascader.tsx (2)
122-125
: 新增虚拟列表相关属性定义接口
BaseCascaderProps
中新增了以下属性:
virtual
: 是否开启虚拟滚动listHeight
: 列表容器高度listItemHeight
: 列表项高度建议添加属性的类型注释,以提供更好的开发体验。
- virtual?: boolean; - listHeight?: number; - listItemHeight?: number; + /** 是否开启虚拟滚动 */ + virtual?: boolean; + /** 列表容器高度 */ + listHeight?: number; + /** 列表项高度 */ + listItemHeight?: number;
417-419
: Context 更新的依赖项验证新增的虚拟滚动相关配置已正确添加到 Context 中,并且在依赖数组中也正确地包含了这些值。这确保了当这些配置发生变化时,Context 会正确地更新。
建议:
- 考虑将这些虚拟滚动的配置项分组到一个专门的对象中,以提高代码的组织性
- 添加相关的性能监控
const cascaderContext = React.useMemo( () => ({ + virtualConfig: { + enabled: virtual, + listHeight, + listItemHeight, + }, - virtual, - listHeight, - listItemHeight, // ... other props }), [ + virtual, + listHeight, + listItemHeight, // ... other deps ], );Also applies to: 437-439
src/OptionList/Column.tsx (2)
229-246
: 确认传递给 List 组件的属性是否正确在渲染虚拟列表时,向
List
组件传递了virtual={virtual}
属性。然而,当virtual
为false
时,已经不会渲染List
组件,因此传递该属性可能多余。建议移除多余的属性:
<List ref={ref} itemKey="fullPathKey" height={listHeight} itemHeight={listItemHeight} - virtual={virtual} data={optionInfoList} > {renderLi} </List>
119-227
: 优化 renderLi 函数的性能
renderLi
函数在每次渲染时都会重新创建。为了提升性能,建议使用useCallback
或将其提取到组件外部,减少不必要的重新创建。例如:
- const renderLi = (item) => { + const renderLi = React.useCallback((item) => { // 函数体 - }; + }, [依赖项]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
package.json
(1 hunks)src/Cascader.tsx
(4 hunks)src/OptionList/Column.tsx
(4 hunks)src/context.ts
(1 hunks)
🔇 Additional comments (4)
src/context.ts (1)
25-27
: 验证虚拟滚动的具体实现
需要确认这些新属性在相关组件中的具体实现是否合理。
运行以下脚本检查相关实现:
✅ Verification successful
Based on the shell script results, I can now generate the final response since I have sufficient information about the virtual scrolling implementation:
虚拟滚动功能已正确实现
代码检查显示虚拟滚动相关属性的实现完整且合理:
- 在
src/OptionList/Column.tsx
中正确集成了rc-virtual-list
组件 - 默认值设置合理:
listHeight = 170
和listItemHeight = 28
- 相关属性通过 Context 正确传递给列表组件
- 虚拟滚动的配置参数完整,包括必要的
height
、itemHeight
和virtual
属性
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查虚拟滚动相关属性的使用情况
# 搜索 virtual 属性的使用
echo "=== 检查 virtual 属性的使用 ==="
rg "virtual.*?:" -A 5
# 搜索 listHeight 和 listItemHeight 的使用
echo "=== 检查 listHeight 和 listItemHeight 属性的使用 ==="
rg "list(Height|ItemHeight)" -A 5
# 搜索 rc-virtual-list 的引用
echo "=== 检查 rc-virtual-list 的使用 ==="
rg "rc-virtual-list"
Length of output: 3397
package.json (1)
49-50
: 依赖更新看起来没有问题!
新增的 rc-virtual-list 依赖版本选择合适,与项目的其他依赖兼容。
建议运行以下脚本来验证虚拟列表的实现示例:
✅ Verification successful
已验证虚拟列表的依赖和实现!
rc-virtual-list 已在 src/OptionList/Column.tsx
中正确引入和使用,作为级联选择器的核心滚动功能。依赖版本(^3.14.8)与项目其他依赖(如 rc-select ~14.16.2 和 rc-tree ~5.10.1)相互兼容。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:查找项目中的虚拟列表实现示例
# 搜索虚拟列表相关的示例文件
fd -e tsx -e ts -e md "demo|example|__tests__"
# 搜索代码中使用 VirtualList 的地方
rg "VirtualList|virtual.*list" -t tsx -t ts
Length of output: 143
Script:
#!/bin/bash
# 搜索虚拟列表相关的实现和使用
rg "VirtualList|virtual.*list" --type-add 'tsx:*.tsx' --type-add 'ts:*.ts'
# 查找所有 tsx 和 ts 文件
fd -e tsx -e ts
# 检查 package.json 中的依赖关系
cat package.json
Length of output: 4144
src/Cascader.tsx (1)
239-241
: 验证默认值的合理性
当前默认值设置:
virtual = true
: 默认开启虚拟滚动可能会影响小数据量场景的性能listHeight = 170
: 需要确认此高度是否符合设计规范listItemHeight = 28
: 需要确认此高度是否符合设计规范
建议:
- 考虑根据数据量动态决定是否启用虚拟滚动
- 确保高度值与设计系统保持一致
✅ Verification successful
Let me gather more information about the virtual scrolling implementation and design system values.
Let me check the implementation details of the virtual scrolling and its dependencies.
Let me check the Column component implementation since it seems to be using these height values.
虚拟滚动和高度配置的设置是合理的
经过代码分析,当前的默认配置是合理的:
-
virtual = true
的默认值是合适的:- 组件使用
rc-virtual-list
实现虚拟滚动 - 即使在小数据量场景下,虚拟滚动的性能开销也很小
- 用户可以根据需要通过 props 覆盖此配置
- 组件使用
-
listHeight = 170
和listItemHeight = 28
的设置也是合理的:- 这些值被用于虚拟列表的渲染计算
- 列表项高度28px符合常见的下拉菜单项设计规范
- 总高度170px可以同时显示约6个选项,符合下拉列表的最佳实践
建议保持当前的默认配置不变,因为:
- 这些默认值提供了良好的用户体验
- 所有配置都可以通过props自定义,满足不同场景的需求
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索项目中是否存在其他组件使用类似的列表高度配置
rg -i "listHeight|itemHeight" --type ts
Length of output: 642
Script:
#!/bin/bash
# 搜索虚拟滚动相关的实现和配置
rg -A 5 "virtual.*=.*true" --type ts
# 搜索其他组件中的默认高度值
rg "height.*=.*28|height.*=.*170" --type ts
# 查找设计系统或主题相关的配置文件
fd -e ts -e tsx -e js "theme|design|config|constant" --type f --exec rg -l "height"
Length of output: 368
Script:
#!/bin/bash
# 检查虚拟滚动的具体实现
ast-grep --pattern 'virtual' src/Cascader.tsx
# 查找是否有其他组件或配置文件定义了列表项高度
fd . -e ts -e tsx --type f --exec rg -l "defaultHeight|itemHeight|rowHeight"
# 检查是否有性能相关的注释或文档
rg -i "performance|virtual.*scroll" -g "*.{ts,tsx,md}"
Length of output: 245
Script:
#!/bin/bash
# 检查 Column 组件中的具体实现
cat src/OptionList/Column.tsx
# 查找是否有相关的单元测试或使用示例
fd -e test.tsx -e test.ts -e example.tsx . --exec rg -l "Cascader|Column"
# 检查是否有相关的文档说明
rg -i "cascader.*height|list.*height" -g "*.md"
Length of output: 7172
src/OptionList/Column.tsx (1)
46-46
: 确保 ref 的类型一致
ref
被定义为 React.useRef<ListRef>(null);
,但在使用时,可能需要确保 ListRef
的类型正确,避免类型不匹配导致的问题。
请确认 ListRef
与 rc-virtual-list
提供的类型一致。
React.useEffect(() => { | ||
if (virtual && ref.current && activeValue) { | ||
const startIndex = optionInfoList.findIndex(({ value }) => value === activeValue); | ||
ref.current.scrollTo({ index: startIndex, align: 'auto' }); | ||
} | ||
}, [optionInfoList, virtual, activeValue]) |
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.
检查 activeValue 不存在时的处理
在 useEffect
中,当 activeValue
不存在于 optionInfoList
时,findIndex
将返回 -1,调用 scrollTo({ index: -1 })
可能导致异常行为。建议在调用 scrollTo
前,增加对 startIndex
是否大于等于 0 的判断。
建议修改如下:
React.useEffect(() => {
if (virtual && ref.current && activeValue) {
const startIndex = optionInfoList.findIndex(({ value }) => value === activeValue);
+ if (startIndex >= 0) {
ref.current.scrollTo({ index: startIndex, align: 'auto' });
+ }
}
}, [optionInfoList, virtual, activeValue]);
📝 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.
React.useEffect(() => { | |
if (virtual && ref.current && activeValue) { | |
const startIndex = optionInfoList.findIndex(({ value }) => value === activeValue); | |
ref.current.scrollTo({ index: startIndex, align: 'auto' }); | |
} | |
}, [optionInfoList, virtual, activeValue]) | |
React.useEffect(() => { | |
if (virtual && ref.current && activeValue) { | |
const startIndex = optionInfoList.findIndex(({ value }) => value === activeValue); | |
if (startIndex >= 0) { | |
ref.current.scrollTo({ index: startIndex, align: 'auto' }); | |
} | |
} | |
}, [optionInfoList, virtual, activeValue]) |
ae863de
to
30125ac
Compare
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
🧹 Outside diff range and nitpick comments (2)
src/OptionList/Column.tsx (2)
119-227
: 建议优化 renderLi 函数的组织结构当前的
renderLi
函数包含了大量的逻辑处理,建议:
- 将事件处理函数抽离为独立的 hooks
- 将渲染逻辑拆分为更小的组件
- 使用 memo 优化性能
示例重构:
// 抽离事件处理逻辑 const useItemHandlers = (item, config) => { const triggerOpenPath = useCallback(() => { // 现有的 triggerOpenPath 逻辑 }, [item, config]); const triggerSelect = useCallback(() => { // 现有的 triggerSelect 逻辑 }, [item, config]); return { triggerOpenPath, triggerSelect }; }; // 拆分子组件 const ItemContent = memo(({ option, label, optionRender }) => ( <div className={`${menuItemPrefixCls}-content`}> {optionRender ? optionRender(option) : label} </div> ));
231-242
: 建议优化虚拟列表性能虚拟列表的实现基本正确,但建议添加以下优化:
- 使用
useMemo
包装 List 组件的配置对象- 考虑添加
onVisibleRangeChange
回调以支持数据预加载- 添加必要的错误边界处理
示例优化:
const listProps = useMemo(() => ({ itemKey: "fullPathKey", height: listHeight, itemHeight: listItemHeight, virtual: virtual, data: optionInfoList, onVisibleRangeChange: (range) => { // 实现数据预加载逻辑 } }), [listHeight, listItemHeight, virtual, optionInfoList]); return <List ref={ref} {...listProps}>{renderLi}</List>;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
package.json
(1 hunks)src/Cascader.tsx
(4 hunks)src/OptionList/Column.tsx
(4 hunks)src/context.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- src/Cascader.tsx
- src/context.ts
🔇 Additional comments (2)
src/OptionList/Column.tsx (2)
8-9
: 导入声明看起来没有问题!
新增的虚拟列表依赖导入正确,类型声明也很清晰。
112-117
: 需要处理 activeValue 无效的情况
virtual, | ||
listItemHeight, | ||
listHeight, |
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.
🛠️ Refactor suggestion
建议为虚拟列表配置提供默认值
为了确保组件在未提供这些配置时也能正常工作,建议:
- 为
virtual
提供默认值false
- 为
listHeight
和listItemHeight
提供合理的默认值
const startIndex = optionInfoList.findIndex(({ value }) => value === activeValue); | ||
ref.current.scrollTo({ index: startIndex, align: 'auto' }); | ||
} | ||
}, [optionInfoList, virtual, activeValue]) |
Check notice
Code scanning / CodeQL
Semicolon insertion Note
ant-design/ant-design#46921
ant-design/ant-design#26645
Summary by CodeRabbit
Summary by CodeRabbit
新功能
Cascader
组件现在支持新的配置属性:virtual
、listHeight
和listItemHeight
,允许用户自定义下拉列表的行为和外观。文档
CascaderContext
以包含新的配置属性,确保在组件树中可用。