feat: TreeSelect support maxCount #596
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此拉取请求主要对 Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #596 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 546 561 +15
Branches 162 169 +7
=========================================
+ Hits 546 561 +15 ☔ View full report in Codecov by Sentry. |
aojunhao123
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: 5
🧹 Outside diff range and nitpick comments (18)
tests/util.tsx (1)
13-16: 建议重构键盘事件处理函数当前的
keyUp函数实现与keyDown非常相似，建议提取共同逻辑以减少代码重复。建议重构为：
+type KeyEventType = 'keyDown' | 'keyUp'; + +function createKeyboardEvent( + element: HTMLElement, + key: string, + eventType: KeyEventType +) { + const event = createEvent[eventType](element, { key }); + fireEvent(element, event); +} + -export function keyDown(element: HTMLElement, keyCode: number) { - const event = createEvent.keyDown(element, { keyCode }); - fireEvent(element, event); +export function keyDown(element: HTMLElement, key: string) { + createKeyboardEvent(element, key, 'keyDown'); } -export function keyUp(element: HTMLElement, keyCode: number) { - const event = createEvent.keyUp(element, { keyCode }); - fireEvent(element, event); +export function keyUp(element: HTMLElement, key: string) { + createKeyboardEvent(element, key, 'keyUp'); }src/TreeSelectContext.ts (1)
17-17: 建议添加 JSDoc 注释说明属性用途为了提高代码可维护性和开发体验，建议为
maxCount属性添加 JSDoc 注释，说明其用途和使用场景。建议添加如下注释：
+ /** 多选模式下用户最多可以选择的节点数量 */ maxCount?: number;src/utils/valueUtil.ts (1)
39-40: 函数实现正确但缺少文档说明函数实现简洁且符合逻辑，但建议添加 JSDoc 文档说明函数的用途、参数和返回值。
建议添加如下文档：
+/** + * 检查给定的数值是否有效（已定义且不是 NaN） + * @param value - 要检查的数值 + * @returns 如果数值有效则返回 true，否则返回 false + */ export const isValidCount = (value?: number) => typeof value !== 'undefined' && !Number.isNaN(value);examples/mutiple-with-maxCount.tsx (5)
1-5: 文件名称中存在拼写错误文件名
mutiple-with-maxCount.tsx中的 "mutiple" 应该改为 "multiple"。这个拼写错误可能会影响示例文档的可发现性。
7-35: 建议优化示例数据的可读性当前的示例数据中，标题使用了简单的数字（如 "1"，"2"，"3"），建议使用更具描述性的标题来提高示例的可读性和教学价值。
建议修改如下：
const treeData = [ { key: '1', value: '1', - title: '1', + title: '父节点 1', children: [ { key: '1-1', value: '1-1', - title: '1-1', + title: '子节点 1-1', }, ], }, { key: '2', value: '2', - title: '2', + title: '节点 2', }, // ... 其他节点类似修改 ];
37-39: 建议添加值的验证逻辑当前的
onChange处理函数直接设置状态，建议添加值的验证逻辑以确保数据的正确性。建议修改如下：
const onChange = (val: string[]) => { + if (!Array.isArray(val)) { + console.warn('预期接收数组类型的值'); + return; + } setValue(val); };
44-51: 移除或说明注释掉的代码第一个 TreeSelect 示例中包含注释掉的
onChange和value属性。如果这是有意为之，建议添加注释说明原因；如果不是，建议删除这些注释掉的代码。
41-74: 优化示例代码的结构和可读性建议进行以下改进：
- 为每个 TreeSelect 示例添加注释，说明其配置的特点和用途
- 统一示例之间的间距
- 将重复的样式提取为常量
建议修改如下：
+const treeSelectStyle = { width: 300 }; + return ( <> <h2>multiple with maxCount</h2> + {/* 基础多选示例：最多选择3项 */} <TreeSelect - style={{ width: 300 }} + style={treeSelectStyle} multiple maxCount={3} treeData={treeData} /> <h2>checkable with maxCount</h2> + {/* 可勾选多选示例：最多选择3项，严格勾选模式 */} <TreeSelect - style={{ width: 300 }} + style={treeSelectStyle} multiple treeCheckable treeCheckStrictly maxCount={3} treeData={treeData} onChange={onChange} value={value} /> + {/* 简单数据示例：最多选择2项 */} <TreeSelect - style={{ width: 300 }} + style={treeSelectStyle} treeData={[ { key: '0', value: '0', title: '0 label' }, { key: '1', value: '1', title: '1 label' }, { key: '2', value: '2', title: '2 label' }, ]} multiple maxCount={2} /> </> );tests/Select.maxCount.spec.tsx (3)
15-17: 建议改进 renderTreeSelect 函数的类型定义为了提高代码的可维护性和类型安全性，建议添加适当的类型定义。
- const renderTreeSelect = (props?: any) => { + const renderTreeSelect = (props?: Partial<TreeSelectProps>) => {
26-106: 建议添加初始渲染时的预选值测试用例当前的测试用例已经很全面，但建议添加一个测试用例来验证组件初始化时带有预选值的情况。这对于确保组件在所有场景下都能正确处理 maxCount 限制很重要。
it('should handle pre-selected values correctly', () => { const { container } = renderTreeSelect({ value: ['0', '1'], maxCount: 2 }); const dropdownList = container.querySelector('.rc-tree-select-dropdown'); const option3 = within(dropdownList).getByText('2 label'); expect(option3.closest('div')).toHaveClass('rc-tree-select-tree-treenode-disabled'); });
107-157: 建议补充键盘取消选择的测试用例键盘操作的测试覆盖了选择操作，但缺少使用键盘取消选择的测试用例。建议添加相关测试以确保完整的键盘操作支持。
it('should allow keyboard deselection when maxCount is reached', () => { const { container } = render( <TreeSelect treeData={treeData} multiple open maxCount={2} value={['0', '1']} /> ); const input = container.querySelector('input'); // 模拟键盘取消选择第一个选项 keyDown(input, KeyCode.BACKSPACE); // 验证第三个选项是否变为可选状态 expect(container.querySelector('.rc-tree-select-tree-treenode-disabled')).toBeFalsy(); });tests/Select.multiple.spec.js (2)
2-2: 新增测试工具导入需要更新相关测试用例新增了
fireEvent和within测试工具的导入，但现有测试用例仍在使用simulate。建议统一使用@testing-library提供的事件触发方式。-wrapper.simulate('keyDown', { which: KeyCode.BACKSPACE }); +fireEvent.keyDown(wrapper.getByRole('textbox'), { key: 'Backspace', keyCode: KeyCode.BACKSPACE });
35-38: 代码格式调整建议链式调用的格式调整提高了可读性，但建议在整个测试文件中保持一致的格式。目前只有部分代码块进行了调整。
建议统一所有类似的链式调用格式：
-wrapper.find('input').first().simulate('keyDown', ...); +wrapper + .find('input') + .first() + .simulate('keyDown', ...);Also applies to: 65-73
src/TreeSelect.tsx (1)
75-75: 需要为新属性添加JSDoc文档注释请为
maxCount属性添加JSDoc文档注释，说明其用途、类型和示例用法。这将帮助其他开发者更好地理解和使用这个新功能。+ /** + * 限制多选模式下最多可选中的节点数量 + * @default undefined + */ maxCount?: number;README.md (3)
64-64: 文档描述中的语法问题需要修正以下几处描述需要改进：
dropdownMatchSelectWidth属性描述中的 "with" 应为 "width"treeData描述中的语法不通顺key属性描述中的 "it's" 应为 "its"建议修改如下：
- whether dropdown's with is same with select + whether dropdown's width is same as select - treeNodes data Array, if set it then you need not to construct children TreeNode + treeNodes data array. If set, you don't need to construct children TreeNode - it's value must be unique + its value must be uniqueAlso applies to: 97-97, 116-116
111-119: 建议优化 TreeNode props 部分的提示样式为了提高可读性，建议将使用
treeData的提示更加醒目。-> note: you'd better to use `treeData` instead of using TreeNode. +> **注意：** 推荐使用 `treeData` 属性而不是 TreeNode 方式。🧰 Tools
🪛 LanguageTool
[grammar] ~116-~116: Did you mean to use the possessive pronoun “its”?
Context: ...bool | false | | key | it's value must be unique across the tree's ...(IT_S_ITS)
122-126: 建议改进性能优化建议部分的格式性能优化建议部分的格式可以更加规范，同时建议补充与新增
maxCount功能相关的性能考虑。建议调整格式如下：
- 1. Optimization tips(when there are large amounts of data, like more than 5000 nodes) - - Do not Expand all nodes. - - Recommend not exist many `TreeSelect` components in a page at the same time. - - Recommend not use `treeCheckable` mode, or use `treeCheckStrictly`. + ## 性能优化建议 + + 当数据量较大时（如超过 5000 个节点），请注意以下几点： + + 1. 避免展开所有节点 + 2. 避免在同一页面中同时使用多个 TreeSelect 组件 + 3. 避免使用 `treeCheckable` 模式，或使用 `treeCheckStrictly` 模式 + 4. 使用 `maxCount` 时建议设置合理的限制值，过大的显示数量可能影响性能🧰 Tools
🪛 LanguageTool
[uncategorized] ~125-~125: The preposition “on” seems more likely in this position than the preposition “in”.
Context: ... not exist manyTreeSelectcomponents in a page at the same time. - Recommend...(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_IN_ON)
src/OptionList.tsx (1)
85-91: 考虑优化traverse函数的性能递归遍历整个树以设置节点的
disabled状态，可能在大型数据集上导致性能问题。建议对树数据进行缓存或优化，提升性能。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (10)
README.md(2 hunks)docs/demo/mutiple-with-maxCount.md(1 hunks)examples/mutiple-with-maxCount.tsx(1 hunks)src/OptionList.tsx(7 hunks)src/TreeSelect.tsx(5 hunks)src/TreeSelectContext.ts(1 hunks)src/utils/valueUtil.ts(1 hunks)tests/Select.maxCount.spec.tsx(1 hunks)tests/Select.multiple.spec.js(4 hunks)tests/util.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/demo/mutiple-with-maxCount.md
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~85-~85: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...ust show parent treeNode). Default just show child. | enum{TreeSelect.SHOW_ALL, Tree...
(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[typographical] ~97-~97: Consider adding a comma.
Context: ... | treeNodes data Array, if set it then you need not to construct children Tree...
(IF_THEN_COMMA)
[grammar] ~97-~97: Did you mean “construct”?
Context: ...data Array, if set it then you need not to construct children TreeNode. (value should be uni...
(NEEDNT_TO_DO_AND_DONT_NEED_DO)
[uncategorized] ~102-~102: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...t.body;} | | autoClearSearchValue | auto clear search input value when multiple select...
(AUTO_HYPHEN)
[grammar] ~116-~116: Did you mean to use the possessive pronoun “its”?
Context: ...bool | false | | key | it's value must be unique across the tree's ...
(IT_S_ITS)
[uncategorized] ~125-~125: The preposition “on” seems more likely in this position than the preposition “in”.
Context: ... not exist many TreeSelect components in a page at the same time. - Recommend...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_IN_ON)
🪛 Markdownlint
README.md
99-99: Expected: 4; Actual: 3; Too few cells, row will be missing data
Table column count
(MD056, table-column-count)
🔇 Additional comments (14)
tests/util.tsx (2)
1-1: 导入语句看起来不错！
导入了必要的 createEvent 函数用于创建键盘事件，同时保持了原有的 fireEvent 导入。
8-16: 请确保新增函数的测试覆盖率
建议验证这些新增的键盘事件模拟函数是否有充分的测试覆盖。
✅ Verification successful
键盘事件函数已得到充分测试覆盖
keyDown 和 keyUp 函数在多个测试文件中都有使用，包括：
Select.maxCount.spec.tsx中用于测试按键选择和最大选择数量限制Select.spec.tsx中用于测试键盘导航和选择功能Select.SearchInput.spec.js中用于测试搜索输入的键盘交互Select.multiple.spec.js中用于测试多选模式的键盘操作
这些测试涵盖了各种键盘事件场景，包括 Enter、ESC、上下键导航、退格键等，测试覆盖率充分。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查键盘事件相关的测试用例
# 搜索使用这些函数的测试文件
echo "检查 keyDown 和 keyUp 函数的使用情况："
rg -l "keyDown|keyUp" "tests/**/*.(test|spec).(ts|tsx|js|jsx)"
Length of output: 208
Script:
#!/bin/bash
# 检查测试文件的位置
echo "查找所有测试文件:"
fd -e test.tsx -e test.ts -e test.jsx -e test.js -e spec.tsx -e spec.ts -e spec.jsx -e spec.js
echo -e "\n检查 keyDown 和 keyUp 函数的使用情况："
rg "keyDown|keyUp" -l
Length of output: 746
Script:
#!/bin/bash
# 检查 keyDown 和 keyUp 函数的具体使用情况
echo "在测试文件中查找 keyDown 和 keyUp 函数的使用："
rg "keyDown|keyUp" tests/Select.maxCount.spec.tsx tests/Select.multiple.spec.js tests/Select.spec.tsx tests/Select.SearchInput.spec.js -B 2 -A 2
Length of output: 7868
src/TreeSelectContext.ts (1)
17-17: 代码变更看起来不错！
maxCount 属性的添加符合需求，类型定义正确，且保持了可选特性。这与 Select 组件中的实现保持一致。
src/utils/valueUtil.ts (1)
39-40: 建议验证函数的测试覆盖情况
作为 maxCount 功能的关键验证函数，建议确保测试用例完整覆盖各种边界情况。
运行以下脚本检查测试覆盖：
✅ Verification successful
Let me gather more information about the test coverage and usage patterns.
Let me check one more aspect to see if there are any integration tests or related test files in the test directory.
Let me check the test directory content for any relevant tests.
测试覆盖情况已得到验证，无需担心
isValidCount 函数虽然没有直接的单元测试，但在 tests/Select.maxCount.spec.tsx 中有完整的集成测试覆盖：
- 测试了当选择达到 maxCount 限制时的行为
- 验证了 maxCount 为 0 时的情况
- 验证了 maxCount 大于选项数量时的情况
- 包含了键盘操作相关的测试场景
这些测试用例充分验证了 isValidCount 函数在各种边界情况下的正确性。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查 isValidCount 函数的测试覆盖情况
# 搜索测试文件中对 isValidCount 的测试
rg -l "isValidCount" --glob "**/*.test.{ts,tsx}"
# 检查实际使用情况
rg "isValidCount" --type ts
Length of output: 493
Script:
#!/bin/bash
# 检查 OptionList.tsx 中的具体使用场景
rg -B 5 -A 5 "isValidCount" src/OptionList.tsx
# 检查是否存在其他相关的测试文件
fd "option.*test\.(ts|tsx)$"
# 检查 valueUtil.ts 的测试文件
fd "valueUtil.*test\.(ts|tsx)$"
# 查看 OptionList 组件的测试文件
fd "OptionList.*test\.(ts|tsx)$"
Length of output: 886
Script:
#!/bin/bash
# 搜索所有测试文件中可能包含 maxCount 相关测试的内容
rg -B 3 -A 3 "maxCount" "**/*.test.{ts,tsx}"
# 检查是否存在专门的测试目录
fd -t d "tests?"
# 检查 __tests__ 目录下的文件
fd . -t f "__tests__"
Length of output: 248
Script:
#!/bin/bash
# 检查 tests 目录下的所有测试文件
fd . tests/
# 在 tests 目录中搜索与 maxCount 或 isValidCount 相关的测试
rg -B 3 -A 3 "maxCount|isValidCount" tests/
# 检查 Select.test 相关文件
fd "Select.*test" tests/
Length of output: 6630
tests/Select.maxCount.spec.tsx (2)
1-6: 测试工具选择恰当！
使用 @testing-library/react 作为测试工具是个很好的选择，它提供了模拟用户行为的优秀 API。导入的依赖项都是必要的，没有多余导入。
1-157: 测试文件整体评估：代码质量优秀！
整体测试覆盖率很好，包含了：
- 基础功能测试
- 边界条件测试
- 键盘操作测试
- 事件处理测试
代码结构清晰，测试用例设计合理，符合测试最佳实践。
src/TreeSelect.tsx (1)
563-563:
验证maxCount在选择逻辑中的实现
maxCount已被添加到依赖数组中，但在onOptionSelect函数中似乎缺少相关的选择限制逻辑。需要确保在达到最大选择数量时，正确处理用户的选择操作。
Also applies to: 615-615, 629-629
src/OptionList.tsx (7)
12-12: 正确导入新工具函数 isValidCount
导入了 isValidCount 函数，确保了对 maxCount 值的有效性验证。
48-48: 将 maxCount 添加到上下文
从 TreeSelectContext 中解构出 maxCount，以便在组件中使用该属性。
80-83: 正确计算 isOverMaxCount 的值
使用 useMemo 计算 isOverMaxCount，逻辑清晰，依赖项合理。
93-97: 正确地生成 processedTreeData
使用 useMemo 对处理后的树数据进行缓存，确保数据在依赖项变化时更新。
207-213: 验证 onActiveChange 中的条件
当超过最大选择数量且当前节点未被选中时，onActiveChange 将不更新 activeKey。请确认此行为符合预期，并考虑在界面上给予用户反馈。
310-310: 更新 Tree 组件的 treeData
将 treeData 属性更新为 processedTreeData，以应用节点的禁用状态。
332-332: 添加自定义的 onActiveChange 处理函数
将 onActiveChange 函数传递给 Tree 组件，控制激活节点的更新逻辑。
| export function keyDown(element: HTMLElement, keyCode: number) { | ||
| const event = createEvent.keyDown(element, { keyCode }); | ||
| fireEvent(element, event); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议改进键盘事件的实现
当前实现使用了已废弃的 keyCode 属性，建议使用现代的 key 或 code 属性来提高代码的可维护性。
建议按照以下方式修改：
-export function keyDown(element: HTMLElement, keyCode: number) {
- const event = createEvent.keyDown(element, { keyCode });
+export function keyDown(element: HTMLElement, key: string) {
+ const event = createEvent.keyDown(element, { key });
fireEvent(element, event);
}📝 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.
| export function keyDown(element: HTMLElement, keyCode: number) { | |
| const event = createEvent.keyDown(element, { keyCode }); | |
| fireEvent(element, event); | |
| } | |
| export function keyDown(element: HTMLElement, key: string) { | |
| const event = createEvent.keyDown(element, { key }); | |
| fireEvent(element, event); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
tests/Select.SearchInput.spec.js (1)
224-224: 键盘事件处理需要同时测试 keyUp为了更完整地模拟键盘交互，在测试 Enter 键选择时添加了 keyUp 事件。这样的改进使测试更接近真实用户的交互场景。
建议在其他键盘事件测试用例中也添加相应的 keyUp 事件测试，以保持一致性。
Also applies to: 231-231
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
tests/Select.SearchInput.spec.js(1 hunks)tests/Select.maxCount.spec.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Select.maxCount.spec.tsx
🔇 Additional comments (1)
tests/Select.SearchInput.spec.js (1)
234-239: 完善了选择第三个选项的测试用例
通过添加对第三个选项的测试，提高了测试覆盖率。这个测试用例验证了在有多个选项时，Enter 键仍然能够正确选择匹配的非禁用节点。
运行以下脚本检查是否还有其他未覆盖的选择场景：
✅ Verification successful
Let me analyze the test coverage based on the results. I can see comprehensive test coverage for selection scenarios including:
- Search and select functionality
- Multiple selection scenarios
- Keyboard navigation (UP/DOWN/ENTER)
- Max count limitations
- Tree selection behaviors
- Selection with disabled items
The specific test case in question (selecting the third option) is part of a broader test suite that already covers various selection scenarios. Let me generate the final response:
测试覆盖已经很完善，无需额外补充
当前的测试套件已经全面覆盖了选择组件的各种场景，包括：
- 搜索选择功能
- 多选操作
- 键盘导航（上下键和回车键）
- 最大选择数限制
- 树形选择行为
- 禁用项的选择处理
新增的测试用例是对现有测试覆盖的有益补充，与其他测试用例共同构成了完整的测试体系。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：检查测试文件中的选择相关测试用例覆盖情况
# 搜索所有包含 select 相关的测试用例
rg "it\(['\"].*select.*['\"]" tests/
# 检查键盘事件相关的测试用例
rg "keyDown|keyUp" tests/
Length of output: 6061
|
ci 挂了，看看。 |
react-component/tree#890 前置pr在这里 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/OptionList.tsx (2)
274-284: 完善键盘导航的错误处理当前实现在以下场景可能存在问题：
- 没有处理
nextNode为空的情况- 缺少错误处理机制
建议添加错误处理：
if (isOverMaxCount) { event.preventDefault(); const direction = which === KeyCode.UP || which === KeyCode.LEFT ? 'prev' : 'next'; const nextNode = getNextMatchingNode(activeKey, direction); - if (nextNode) { + try { + if (!nextNode) { + // 当没有可用节点时，保持当前选中状态 + return; + } setActiveKey(nextNode[fieldNames.value]); treeRef.current?.scrollTo({ key: nextNode[fieldNames.value] }); + } catch (error) { + console.error('键盘导航失败:', error); } }
345-381: Tree 组件集成建议建议在 Tree 组件配置中添加以下改进：
- 添加适当的
aria-*属性以提升可访问性- 考虑添加错误边界处理
<Tree ref={treeRef} focusable={false} + aria-multiselectable={multiple} + aria-label="树形选择器" prefixCls={`${prefixCls}-tree`} // ... rest of props />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/OptionList.tsx(7 hunks)
🔇 Additional comments (2)
src/OptionList.tsx (2)
162-171: 🛠️ Refactor suggestion
简化节点禁用逻辑
当前实现存在以下问题：
- 重复创建
selectedValues数组 - 使用
includes进行查找效率较低
建议优化实现：
const nodeDisabled = (node: DataNode) => {
if (isOverMaxCount) {
- const selectedValues = memoDisplayValues;
- if (!selectedValues.includes(node[fieldNames.value])) {
+ if (!memoDisplayValues.includes(node[fieldNames.value])) {
return true;
}
}
return undefined;
};Likely invalid or redundant comment.
198-236: 🛠️ Refactor suggestion
优化节点收集和导航逻辑
当前实现存在以下问题：
- 使用
useRef存储节点可能导致状态不同步 - 每次渲染都会重新收集节点
- 遍历逻辑可以更高效
建议使用 useMemo 替代当前实现：
- const availableNodesRef = React.useRef<EventDataNode<any>[]>([]);
+ const availableNodes = React.useMemo(() => {
+ const nodes: EventDataNode<any>[] = [];
+ const selectedValueSet = new Set(memoDisplayValues);
+
+ const collectNodes = (nodeList: EventDataNode<any>[]) => {
+ nodeList.forEach(node => {
+ if (!node.disabled && node.selectable !== false) {
+ if (selectedValueSet.has(node[fieldNames.value])) {
+ nodes.push(node);
+ }
+ }
+ if (node[fieldNames.children]) {
+ collectNodes(node[fieldNames.children]);
+ }
+ });
+ };
+
+ collectNodes(memoTreeData);
+ return nodes;
+ }, [memoDisplayValues, memoTreeData, fieldNames]);
const getNextMatchingNode = (
currentKey: Key | null,
direction: 'next' | 'prev' = 'next',
): EventDataNode<any> | null => {
- const availableNodes = availableNodesRef.current;
const currentIndex = availableNodes.findIndex(
node => node[fieldNames.value] === currentKey
);
// ... rest of the function
};Likely invalid or redundant comment.
|
New and removed dependencies detected. Learn more about Socket for GitHub ↗︎
🚮 Removed packages: npm/[email protected] |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
src/OptionList.tsx (1)
274-284: 完善键盘导航的错误处理当前实现缺少边界情况处理，建议添加错误处理：
if (isOverMaxCount) { event.preventDefault(); const direction = which === KeyCode.UP || which === KeyCode.LEFT ? 'prev' : 'next'; const nextNode = getNextMatchingNode(activeKey, direction); - if (nextNode) { + try { + if (!nextNode) { + return; + } setActiveKey(nextNode[fieldNames.value]); treeRef.current?.scrollTo({ key: nextNode[fieldNames.value] }); + } catch (error) { + console.error('键盘导航失败:', error); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/OptionList.tsx(7 hunks)
🔇 Additional comments (3)
src/OptionList.tsx (3)
198-236: 🛠️ Refactor suggestion
建议重构节点导航实现
当前使用 useRef 和 useEffect 的实现可能导致状态不同步，建议使用 useMemo 优化：
- const availableNodesRef = React.useRef<EventDataNode<any>[]>([]);
+ const availableNodes = React.useMemo(() => {
+ const nodes: EventDataNode<any>[] = [];
+ const collectNodes = (nodeList: EventDataNode<any>[]) => {
+ nodeList.forEach(node => {
+ if (!node.disabled && node.selectable !== false) {
+ if (memoDisplayValues.has(node[fieldNames.value])) {
+ nodes.push(node);
+ }
+ }
+ if (node[fieldNames.children]) {
+ collectNodes(node[fieldNames.children]);
+ }
+ });
+ };
+ collectNodes(memoTreeData);
+ return nodes;
+ }, [memoDisplayValues, memoTreeData, fieldNames]);Likely invalid or redundant comment.
162-171: 🛠️ Refactor suggestion
优化节点禁用逻辑的实现
当前实现存在以下问题：
- 重复创建
selectedValues数组 - 使用
includes进行查找效率较低
建议使用 Set 结构优化性能：
const nodeDisabled = (node: DataNode) => {
if (isOverMaxCount) {
- const selectedValues = memoDisplayValues;
- if (!selectedValues.includes(node[fieldNames.value])) {
+ if (!memoDisplayValues.has(node[fieldNames.value])) {
return true;
}
}
return undefined;
};Likely invalid or redundant comment.
345-381: 验证 Tree 组件集成的正确性
需要验证 nodeDisabled 是否正确传递给所有子节点，以及是否正确处理了禁用状态。
✅ Verification successful
Let me gather more information about the UnstableContext and its usage.
Let me try a different approach to find the context usage and Tree component.
Let me try one last time without file type restrictions.
Based on the search results, I can now provide a final response:
nodeDisabled 功能已正确集成并验证
nodeDisabled函数通过UnstableContext.Provider正确传递给 Tree 组件- 禁用状态在整个组件树中得到正确处理，包括：
- 通过
treeData的disabled属性 - 通过
TreeNode组件的disabled属性 - 通过
maxCount限制时的动态禁用
- 通过
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 验证 nodeDisabled 的使用
ast-grep --pattern 'UnstableContext.Provider value={{ nodeDisabled }}'
# 检查是否有其他组件使用了 nodeDisabled
rg "nodeDisabled" -A 5
Length of output: 736
Script:
#!/bin/bash
# Search for UnstableContext definition and usage
ast-grep --pattern 'const UnstableContext = createContext'
# Search for nodeDisabled usage in Tree component
rg "useContext.*UnstableContext" -A 5
# Look for Tree component implementation
ast-grep --pattern 'function Tree($$$) {
$$$
}'
Length of output: 152
Script:
#!/bin/bash
# Search for UnstableContext in all files
rg "UnstableContext" --type ts --type tsx -B 2 -A 2
# Search for Tree component imports
rg "import.*Tree" --type ts --type tsx
# Look for disabled related logic in Tree
rg "disabled.*tree" -i --type ts --type tsx
Length of output: 222
Script:
#!/bin/bash
# Search for UnstableContext in all files
rg "UnstableContext" -B 2 -A 2
# Search for Tree component imports and implementation
rg "import.*Tree|export.*Tree" -A 5
# Look for disabled related logic
rg "disabled.*tree|tree.*disabled" -i
Length of output: 25179
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/OptionList.tsx (1)
296-332: 注意UnstableContext的使用在第 296 行引入并使用了
UnstableContext。由于UnstableContext属于不稳定的 API，可能在未来版本中更改或移除。建议确认其稳定性，或者考虑使用稳定的替代方案，以确保组件的长期可维护性。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/OptionList.tsx(6 hunks)tests/Select.maxCount.spec.tsx(1 hunks)
🧰 Additional context used
🪛 eslint
tests/Select.maxCount.spec.tsx
[error] 107-107: 'treeData' is already declared in the upper scope on line 8 column 9.
(@typescript-eslint/no-shadow)
🔇 Additional comments (3)
tests/Select.maxCount.spec.tsx (2)
90-96: 建议添加对 maxCount 边界情况的测试
目前缺少对 maxCount 的边界条件和错误处理的测试。建议添加以下测试场景：
maxCount为负数的情况maxCount为非整数的情况maxCount为undefined或null的情况
107-118: 避免变量命名冲突，防止变量遮蔽
在第 107 行重新声明了 treeData，但在第 8 行已经声明过该变量。这可能导致变量遮蔽，增加代码维护难度和潜在错误。建议将第 107 行的 treeData 重命名为 nestedTreeData，以避免与外部作用域的变量冲突。
🧰 Tools
🪛 eslint
[error] 107-107: 'treeData' is already declared in the upper scope on line 8 column 9.
(@typescript-eslint/no-shadow)
src/OptionList.tsx (1)
83-87: 良好的性能优化
在第 83-87 行，使用 React.useMemo 对 memoDisplayValues 进行缓存，防止每次渲染都重新计算，优化了性能表现。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/OptionList.tsx (2)
166-168: 建议优化节点禁用逻辑当前实现在每次检查时都需要遍历数组，建议使用 Set 结构优化性能：
const nodeDisabled = useEvent((node: DataNode) => { - return isOverMaxCount && !memoRawValues.includes(node[fieldNames.value]); + const valueSet = new Set(memoRawValues); + return isOverMaxCount && !valueSet.has(node[fieldNames.value]); });
237-239: 建议优化代码结构当前实现的条件判断可以更简洁：
- const isNodeDisabled = nodeDisabled(activeEntity.node); - const { selectable, value, disabled } = activeEntity?.node || {}; - if (selectable !== false && !disabled && !isNodeDisabled) { + const { node } = activeEntity; + if (node.selectable !== false && !node.disabled && !nodeDisabled(node)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/OptionList.tsx(6 hunks)
🔇 Additional comments (3)
src/OptionList.tsx (3)
50-51: 代码结构清晰，上下文值获取正确！
从 TreeSelectContext 中获取 displayValues 和 isOverMaxCount 的实现方式正确。
83-86: 🛠️ Refactor suggestion
建议优化性能和代码结构
- 建议将此逻辑移至 TreeSelectContext 中
- 使用
displayValues || []替代可选链操作，可以减少 babel 编译后的代码量
- const memoRawValues = React.useMemo(
- () => (displayValues || []).map(v => v.value),
- [displayValues],
- );
+ const { memoRawValues } = React.useContext(TreeSelectContext);Likely invalid or redundant comment.
293-329: 谨慎使用 UnstableContext
使用 UnstableContext 可能带来潜在风险，建议：
- 关注 rc-tree 的更新，确保 API 稳定性
- 添加错误处理机制
- 考虑使用更稳定的替代方案
✅ Verification successful
Let me gather more information about the UnstableContext usage and its implications.
Let me check one more thing about the stability of this API in rc-tree.
UnstableContext 的使用是安全的
基于以下发现，当前对 UnstableContext 的使用是可接受的：
- rc-tree 版本稳定在 5.11.0，这是一个成熟的主版本
- UnstableContext 仅用于传递 nodeDisabled 功能，使用范围受控
- nodeDisabled 的实现是本地的，且具有明确的类型定义
- 没有发现相关的重大变更警告
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查 rc-tree 中 UnstableContext 的使用情况
rg -A 5 "UnstableContext"
Length of output: 987
Script:
#!/bin/bash
# Check rc-tree dependency version and any related documentation
cat package.json | jq '.dependencies["rc-tree"]'
# Search for any comments or documentation about UnstableContext
rg -i "unstable.*context" -g "*.{md,mdx,ts,tsx,js,jsx}"
# Look for nodeDisabled usage patterns
ast-grep --pattern 'nodeDisabled'
Length of output: 512
Script:
#!/bin/bash
# Check the implementation and usage of nodeDisabled
ast-grep --pattern 'function $_(node: DataNode) {
$$$
nodeDisabled($$$)
$$$
}'
# Look for any migration guides or breaking changes
rg -i "breaking.*changes|migration|upgrade" -g "CHANGELOG.md"
Length of output: 254
Add maxCount support for TreeSelect
Related Issue
Screen Shot
Description
Added
maxCountproperty support to limit the maximum number of selectable items in TreeSelect component's multiple mode. When the selected count reaches maxCount, other unselected options will be disabled.Main updates include:
API
Usage Example
Features
Test Coverage
tests/Select.maxCount.spec.tsxtest fileBreaking Changes
None
Notes
Summary by CodeRabbit
更新后的版本
新功能
TreeSelect组件中添加了maxCount属性，以限制可选择的最大项数。mutiple-with-maxCount.md，提供关于maxCount功能的示例和说明。文档
README.md，增强了 API 文档的可读性，添加了maxCount属性的说明，并明确了其他属性的描述。测试
Select.maxCount.spec.tsx，覆盖TreeSelect组件的maxCount功能，确保选择限制的正确性。