-
-
Notifications
You must be signed in to change notification settings - Fork 142
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: Cascader.Panel support default expand cells #515
feat: Cascader.Panel support default expand cells #515
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
这个改动是 实现 Panel 支持 通过传参 的方式 控制 其展开 对应的导航树 |
来个大佬review下,有点久了 |
src/Cascader.tsx
Outdated
@@ -159,6 +159,7 @@ export interface CascaderProps< | |||
value: GetValueType<OptionType, ValueField, Multiple>, | |||
selectOptions: OptionType[], | |||
) => void; | |||
defaultActiveValueCells?: React.Key[]; |
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.
这个名字改一下?active value cells 是内部命名。如果是要默认展开的话最好是 defaultActiveKey: Key[]
src/OptionList/List.tsx
Outdated
if (defaultActiveValueCells && defaultActiveValueCells?.length > 0) { | ||
setActiveValueCells(defaultActiveValueCells) | ||
} | ||
}, [defaultActiveValueCells]); |
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.
不能用 effect,default
就是默认的意思。你如果是 effect 的话,我动态改 defaultActiveKey 这里就会动态的去展开了。
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.
不能用 effect,
default
就是默认的意思。你如果是 effect 的话,我动态改 defaultActiveKey 这里就会动态的去展开了。
已改 0b2c795
* feat: optimize search * feat: test * feat: test * feat: test * feat: test * feat: add test
* feat: test * feat: test
Co-authored-by: afc163 <afc163@gmail.com>
* fix: antd-issue-51248 * feat: add test for 51248 * feat: update test for limit * feat: update test for limit * feat: update test for limit * feat: update test for limit
Walkthrough本次更改引入了多个文件的更新,包括创建新的资金支持配置文件、重构CI工作流、更新依赖项、添加新的文档和示例组件,以及修改现有组件的属性和测试用例。这些更改旨在增强项目的可配置性、测试覆盖率和用户体验,同时确保项目的依赖项保持最新。 Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
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: 7
🧹 Outside diff range and nitpick comments (12)
.github/workflows/main.yml (1)
6-6
: 需要在文件末尾添加换行符根据 YAML 规范,文件末尾应该有一个换行符。
name: ✅ test on: [push, pull_request] jobs: test: uses: react-component/rc-test/.github/workflows/test.yml@main - secrets: inherit + secrets: inherit +🧰 Tools
🪛 yamllint
[error] 6-6: no new line character at the end of file
(new-line-at-end-of-file)
src/OptionList/useActive.ts (1)
18-18
: 建议添加状态初始化的相关文档说明状态初始化的实现逻辑正确,使用空值合并运算符(??)来处理默认值是个很好的选择。建议在函数注释中添加
defaultActiveKey
参数的使用说明。建议添加如下文档:
/** * Control the active open options path. + * @param multiple - Enable multiple selection mode + * @param open - Control dropdown open state + * @param defaultActiveKey - Set initial active options path */examples/multiple-search.tsx (1)
44-48
: 组件实现可以进一步完善
- 添加类型定义:
-const Demo = () => { +const Demo: React.FC = () => {
建议添加组件文档注释,说明用途和属性用法。
考虑展示更多 Cascader 的特性,例如:
- return <Cascader checkable showSearch options={options} />; + return ( + <Cascader + checkable + showSearch + options={options} + defaultValue={['zhejiang', 'hangzhou']} + onChange={(value, selectedOptions) => { + console.log(value, selectedOptions); + }} + /> + );tests/search.limit.spec.tsx (1)
7-13
: 建议增强 doSearch 函数的类型安全性建议对
doSearch
函数进行以下改进:
- 为
search
参数添加类型注解- 为模拟的事件对象添加适当的类型
建议修改如下:
- function doSearch(wrapper: ReactWrapper, search: string) { + function doSearch(wrapper: ReactWrapper, search: string): void { wrapper.find('input').simulate('change', { - target: { + target: { value: search, - }, + } as HTMLInputElement, }); }examples/panel.tsx (1)
92-92
: 新增 defaultActiveKey 属性展示这个示例很好地展示了新增的 defaultActiveKey 功能。建议考虑以下几点完善:
- 添加注释说明该属性的作用
- 展示不同的默认展开场景
建议添加如下注释:
value={value2} options={addressOptions} onChange={nextValue => { console.log('Change:', nextValue); setValue2(nextValue); }} + // 设置默认展开的节点,数组中的值对应options中的value defaultActiveKey={['bj', 'haidian']}
tests/Panel.spec.tsx (1)
82-93
: 测试用例需要更全面的断言测试用例验证了基本功能,但建议增加以下测试点以提高覆盖率:
- 验证展开项的正确性
- 验证交互后的状态变化
- 测试边界情况
建议按如下方式扩展测试用例:
it('multiple with defaultActiveKey', () => { const onChange = jest.fn(); const { container } = render( <Cascader.Panel checkable options={options} onChange={onChange} defaultActiveKey={['bamboo', 'little']} />, ); expect(container.querySelectorAll('.rc-cascader-menu')).toHaveLength(2); + + // 验证默认展开的选项 + const menus = container.querySelectorAll('.rc-cascader-menu'); + expect(menus[0].querySelector('.rc-cascader-menu-item-active')?.textContent).toBe('Bamboo'); + expect(menus[1].querySelector('.rc-cascader-menu-item-active')?.textContent).toBe('Little'); + + // 验证交互 + fireEvent.click(menus[0].querySelectorAll('.rc-cascader-menu-item')[0]); + expect(container.querySelectorAll('.rc-cascader-menu')).toHaveLength(1); + + // 测试无效的defaultActiveKey + const { container: invalidContainer } = render( + <Cascader.Panel + checkable + options={options} + onChange={onChange} + defaultActiveKey={['invalid']} + />, + ); + expect(invalidContainer.querySelectorAll('.rc-cascader-menu')).toHaveLength(1); });tests/selector.spec.tsx (1)
Line range hint
19-142
: 建议添加 defaultActiveKey 相关的测试用例现有的测试用例主要集中在清除、选择和选项修改等功能上,建议添加针对 defaultActiveKey 参数的测试场景,以确保新功能的正确性。
建议添加以下测试场景:
- 验证传入 defaultActiveKey 时的初始展开状态
- 验证 defaultActiveKey 与用户交互的结合
- 验证无效的 defaultActiveKey 值的处理
it('should expand cells with defaultActiveKey', () => { const wrapper = mount( <Cascader options={addressOptions} defaultActiveKey={['fj']} /> ); expect((global as any).activeValueCells).toEqual(['fj']); });src/Panel.tsx (1)
72-72
: 建议添加默认值建议为
defaultActiveKey
添加默认值,以确保在未传入该属性时组件行为可预期。- defaultActiveKey + defaultActiveKey = [],src/OptionList/List.tsx (1)
Line range hint
169-182
: 建议优化滚动性能当前实现在每次
activeValueCells
或searchValue
变化时都会触发滚动计算。建议考虑以下优化:
- 使用防抖处理滚动操作
- 只在必要时(如展开/收起时)触发滚动
建议添加防抖处理:
+ import { debounce } from 'lodash'; // >>>>> Active Scroll - React.useEffect(() => { + const handleScroll = React.useCallback( + debounce(() => { if (searchValue) { return; } for (let i = 0; i < activeValueCells.length; i += 1) { const cellPath = activeValueCells.slice(0, i + 1); const cellKeyPath = toPathKey(cellPath); const ele = containerRef.current?.querySelector<HTMLElement>( `li[data-path-key="${cellKeyPath.replace(/\\{0,2}"/g, '\\"')}"]`, ); if (ele) { scrollIntoParentView(ele); } } + }, 100), + [activeValueCells, searchValue] + ); + React.useEffect(() => { + handleScroll(); return () => { + handleScroll.cancel(); }; }, [activeValueCells, searchValue]);tests/search.spec.tsx (1)
Line range hint
196-213
: 测试用例结构清晰,但建议增强测试覆盖率测试用例验证了
changeOnSelect
和multiple
组合使用时的功能正确性,逻辑完整。建议增加以下场景的测试:
- 取消选中选项的情况
- 选中父节点时的行为验证
建议添加如下测试场景:
// 测试取消选中 wrapper.find('.rc-cascader-menu-item').first().simulate('click'); expect(onChange).toHaveBeenCalledWith([], expect.anything()); // 测试选中父节点 doSearch(wrapper, 'bamboo'); wrapper.find('.rc-cascader-menu-item').first().simulate('click'); expect(onChange).toHaveBeenCalledWith([['bamboo']], expect.anything());src/Cascader.tsx (1)
285-285
: 建议添加注释说明逻辑变更的原因
changeOnSelect || multiple
的修改增加了对多选模式的支持,但建议添加注释说明这个改动的具体原因和影响。建议添加如下注释:
searchOptions( mergedSearchValue, mergedOptions, mergedFieldNames, dropdownPrefixCls || prefixCls, searchConfig, - changeOnSelect || multiple, + // 支持多选模式下的即时选择 + changeOnSelect || multiple, );tests/index.spec.tsx (1)
1025-1103
: 测试用例实现正确,但建议改进测试用例正确验证了悬停和搜索功能的交互,但可以通过以下方式增强测试覆盖率:
- 添加更多边界条件测试,如空搜索结果的悬停行为
- 验证搜索结果的正确性
- 测试键盘导航交互
建议重构测试用例以提高覆盖率:
it('hover + search', () => { let getOffesetTopTimes = 0; const spyElement = spyElementPrototypes(HTMLElement, { offsetTop: { get: () => (getOffesetTopTimes++ % 2 === 0 ? 100 : 0), }, scrollTop: { get: () => 0, }, offsetHeight: { get: () => 10, }, }); const wrapper = render( <Cascader expandTrigger="hover" options={[/* ... */]} showSearch checkable open />, ); // 测试空搜索结果 + fireEvent.change(wrapper.container.querySelector('input') as HTMLElement, { + target: { value: 'nonexistent' }, + }); + expect(wrapper.container.querySelectorAll('.rc-cascader-menu-item')).toHaveLength(1); + expect(wrapper.container.querySelector('.rc-cascader-menu-item')?.textContent).toBe('Not Found'); + + // 测试有效搜索 fireEvent.change(wrapper.container.querySelector('input') as HTMLElement, { target: { value: 'w' }, }); const items = wrapper.container.querySelectorAll('.rc-cascader-menu-item'); + expect(items.length).toBeGreaterThan(0); + + // 测试悬停行为 fireEvent.mouseEnter(items[9]); expect(mockScrollTo).toHaveBeenCalledTimes(0); + + // 测试键盘导航 + fireEvent.keyDown(wrapper.container.querySelector('input') as HTMLElement, { key: 'ArrowDown' }); + expect(wrapper.container.querySelector('.rc-cascader-menu-item-active')).toBeTruthy(); spyElement.mockRestore(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
tests/__snapshots__/search.spec.tsx.snap
is excluded by!**/*.snap
📒 Files selected for processing (21)
- .github/FUNDING.yml (1 hunks)
- .github/workflows/main.yml (1 hunks)
- .gitignore (1 hunks)
- README.md (1 hunks)
- bunfig.toml (1 hunks)
- docs/demo/multiple-search.md (1 hunks)
- examples/multiple-search.tsx (1 hunks)
- examples/panel.tsx (1 hunks)
- package.json (3 hunks)
- src/Cascader.tsx (3 hunks)
- src/OptionList/Column.tsx (1 hunks)
- src/OptionList/List.tsx (4 hunks)
- src/OptionList/useActive.ts (1 hunks)
- src/Panel.tsx (3 hunks)
- src/hooks/useSearchConfig.ts (1 hunks)
- src/hooks/useSearchOptions.ts (3 hunks)
- tests/Panel.spec.tsx (1 hunks)
- tests/index.spec.tsx (2 hunks)
- tests/search.limit.spec.tsx (1 hunks)
- tests/search.spec.tsx (2 hunks)
- tests/selector.spec.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (5)
- .github/FUNDING.yml
- .gitignore
- README.md
- bunfig.toml
- docs/demo/multiple-search.md
🧰 Additional context used
🪛 yamllint
.github/workflows/main.yml
[error] 6-6: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (22)
.github/workflows/main.yml (2)
1-2
: 工作流配置简化得当!工作流程名称更新为更直观的表情符号,触发器配置涵盖所有分支的推送和拉取请求,这样的改动使配置更加清晰和全面。
4-6
: 验证共享工作流引用使用共享工作流是个好主意,但需要确保
react-component/rc-test
仓库的工作流稳定可靠。✅ Verification successful
共享工作流引用已验证安全可靠
该共享工作流来自 react-component 组织的官方测试仓库,具有以下特点:
- 工作流配置完善,包含了完整的测试、构建和覆盖率检查流程
- 仓库持续维护中,最近更新时间为 2024-10-21
- 使用标准的 Node.js 工具链,运行环境稳定
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 验证共享工作流的可用性 gh api \ -H "Accept: application/vnd.github+json" \ repos/react-component/rc-test/contents/.github/workflows/test.yml \ --jq '.content' | base64 -dLength of output: 215
Script:
#!/bin/bash # 验证共享工作流的可用性和稳定性 gh api \ -H "Accept: application/vnd.github+json" \ repos/react-component/rc-test/contents/.github/workflows/test.yml # 检查仓库的活跃度 gh api \ -H "Accept: application/vnd.github+json" \ repos/react-component/rc-test \ --jq '[.updated_at, .pushed_at, .stargazers_count, .open_issues_count]'Length of output: 1878
🧰 Tools
🪛 yamllint
[error] 6-6: no new line character at the end of file
(new-line-at-end-of-file)
src/hooks/useSearchConfig.ts (1)
Line range hint
25-29
: 建议优化警告信息并验证行为变更代码改动将
limit
从删除属性改为设置为false
,并添加了开发环境下的警告提示,这是个很好的改进。不过有以下几点建议:
- 警告信息
'limit' of showSearch should be positive number or false
可能会造成混淆,因为它说明false
是有效值,但同时又在limit <= 0
时触发警告。建议将消息改为更明确的表述。- 从删除属性改为设置
false
的行为变更可能会影响到下游组件的处理逻辑。运行以下脚本来验证相关组件对
limit: false
的处理:✅ Verification successful
Let me analyze the search results. I can see that
limit
beingfalse
is actually a valid value inuseSearchOptions.ts
and is handled correctly. Let me verify the warning message implementation to ensure it's triggered appropriately.
Let me try a different approach to check the warning implementation and its usage:
Based on the search results, I can now generate the final response as we have sufficient information. The code shows that:
- The warning is properly imported from 'rc-util/lib/warning'
limit: false
is correctly handled inuseSearchOptions.ts
- There are tests specifically for warning cases
代码变更验证完毕,警告信息实现合理
经过验证,这个改动是合理的:
useSearchOptions.ts
中已经正确处理了limit: false
的情况,不会影响组件功能。- 警告信息的实现符合组件库的开发模式警告规范。
- 警告信息 "'limit' of showSearch should be positive number or false" 的表述准确,因为它只在
limit <= 0
且不是false
时触发。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 搜索相关组件中对 limit 属性的处理逻辑 rg -A 5 "limit.*=.*false"Length of output: 1198
Script:
#!/bin/bash # Search for the warning message implementation and its context ast-grep --pattern 'warning($_, $msg)'Length of output: 40
Script:
#!/bin/bash # Search for warning imports and usage rg "warning.*" -A 3Length of output: 4774
src/OptionList/useActive.ts (1)
10-10
: 参数签名更改正确!新增的
defaultActiveKey
参数类型定义恰当,可选参数的处理方式符合 TypeScript 最佳实践。examples/panel.tsx (3)
83-83
: 添加了说明性文本,有助于理解示例这个说明文本清晰地展示了示例中使用的 defaultActiveKey 的值,对用户理解该功能很有帮助。
89-90
: 代码格式调整更加规范onChange 处理函数的格式调整使代码更加清晰易读,同时保持了原有功能。
83-92
: 验证新功能的实现效果需要验证 defaultActiveKey 功能在各种场景下是否正常工作。
✅ Verification successful
defaultActiveKey 功能已经得到充分验证
根据代码库分析,defaultActiveKey 功能已经有完整的实现和测试:
- 在
tests/Panel.spec.tsx
中有专门的测试用例 "multiple with defaultActiveKey"- 该功能在以下关键文件中都有完整实现:
src/Panel.tsx
src/OptionList/List.tsx
src/OptionList/useActive.ts
- 测试用例验证了多选模式下的默认展开功能
- 示例代码中的使用方式正确
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查是否有相关的测试用例 # 查找与 defaultActiveKey 相关的测试 rg -l "defaultActiveKey.*Panel" "tests/" # 检查是否有其他使用示例 rg -l "defaultActiveKey.*Panel" "examples/"Length of output: 84
Script:
#!/bin/bash # 扩大搜索范围,查找所有与 defaultActiveKey 相关的测试和使用示例 rg -i "defaultActiveKey" --type ts --type tsx -A 5 # 查找 Cascader.Panel 的所有使用示例 rg "Cascader\.Panel" --type ts --type tsx -A 5 # 查找测试文件中的相关内容 fd "cascader.*test.*" --type fLength of output: 188
Script:
#!/bin/bash # 不指定文件类型,直接搜索所有文件 rg -i "defaultActiveKey" -A 5 # 搜索 Cascader.Panel 的使用 rg "Cascader\.Panel" -A 5 # 使用 fd 查找所有可能的测试文件 fd -e test.js -e test.ts -e test.tsx -e spec.js -e spec.ts -e spec.tsxLength of output: 8824
package.json (3)
45-49
: 核心依赖包版本更新幅度较大以下依赖包的版本更新幅度较大,需要特别关注:
- @babel/runtime: ^7.12.5 → ^7.25.7
- rc-select: ~14.15.0 → ~14.16.2
- rc-tree: ~5.8.1 → ~5.10.1
- rc-util: ^5.37.0 → ^5.43.0
建议:
- 确认这些依赖的更新是否与新功能相关
- 验证是否有破坏性更新
- 进行充分的回归测试
#!/bin/bash # 检查依赖包的使用情况 echo "检查 @babel/runtime 的使用..." rg "require\(['\"]@babel/runtime" echo "检查 rc-select 的使用..." rg "from ['\"](rc-select|@rc-component/select)" echo "检查 rc-tree 的使用..." rg "from ['\"](rc-tree|@rc-component/tree)" echo "检查 rc-util 的使用..." rg "from ['\"](rc-util|@rc-component/util)"
62-63
: 新增开发依赖包的用途需要说明新增了两个开发依赖:
- array-tree-filter@^3.0.2
- cheerio@1.0.0-rc.12
建议:
- 在提交信息或文档中说明这些依赖的用途
- 确认这些依赖仅用于开发/测试环境
#!/bin/bash # 检查新增依赖的使用情况 echo "检查 array-tree-filter 的使用..." rg "require\(['\"]array-tree-filter['\"]" echo "检查 cheerio 的使用..." rg "require\(['\"]cheerio['\"]"
3-3
: 版本号更新需要符合语义化版本规范版本从 3.27.0 升级到 3.29.1,这是一个次要版本和补丁版本的组合更新。根据语义化版本规范,这表明添加了向后兼容的新功能并修复了一些问题。
✅ Verification successful
版本更新符合规范且有完整的版本历史记录
从版本历史记录可以看出,版本更新遵循了渐进式的升级路径:
3.27.0 -> 3.27.1 -> 3.28.0 -> 3.28.1 -> 3.28.2 -> 3.29.0 -> 3.29.1版本号的递增符合语义化版本规范,通过小版本号(3.28.x、3.29.x)和补丁版本号的有序递增,表明这是一个稳健的版本演进过程。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查 CHANGELOG.md 中是否记录了相应的版本更新信息 rg -A 10 "3\.29\.1|3\.28\.0" CHANGELOG.mdLength of output: 336
Script:
#!/bin/bash # 检查 CHANGELOG 相关文件 fd -g "*CHANGELOG*" -g "!node_modules" # 检查 git tag 历史中的版本信息 git tag | grep -E "v?3\.(27|28|29)" # 检查 package.json 的历史版本变更 git log -p package.json | grep -E "\"version\".*3\.(27|28|29)"Length of output: 554
src/hooks/useSearchOptions.ts (2)
90-90
: 依赖数组更新正确
useMemo
的依赖数组已正确更新,包含了新的enableHalfPath
参数。
92-93
: 导出语句改进符合最佳实践将默认导出改为具名函数导出是一个很好的改进,这样可以在调试和开发工具中看到更有意义的函数名。
tests/selector.spec.tsx (1)
8-11
: 更新了 useActive mock 实现以支持 defaultActiveKeymock 函数已正确更新以支持新的 defaultActiveKey 参数,并且正确地将其传递给了原始的 hook。这与组件的新功能保持一致。
不过建议验证一下新参数的测试覆盖率。
✅ Verification successful
已验证 defaultActiveKey 参数的测试覆盖情况
在
tests/Panel.spec.tsx
中已经包含了针对defaultActiveKey
参数的专门测试用例,测试了多选模式下的默认展开功能。mock 实现和测试用例都已经正确覆盖了这个新参数。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查是否有针对 defaultActiveKey 参数的测试用例 rg -l "defaultActiveKey.*=.*\[.*\].*Cascader" tests/Length of output: 54
Script:
#!/bin/bash # 搜索测试文件中包含 defaultActiveKey 的相关测试用例 rg "defaultActiveKey" tests/ -C 3 # 查找所有与 Cascader 相关的测试文件 fd ".*\.spec\." tests/Length of output: 1807
src/Panel.tsx (2)
38-39
: 类型定义正确添加
defaultActiveKey
的类型定义位置合适,与其他UI控制相关的属性放在一起,保持了良好的代码组织。
205-205
: 验证组件兼容性需要确认
RawOptionList
组件是否正确处理defaultActiveKey
属性。✅ Verification successful
Based on the shell script results, I can now generate the final response since I have found clear evidence that the
RawOptionList
component does support thedefaultActiveKey
prop. The interface definition and usage patterns are clearly visible in the codebase.
defaultActiveKey
属性验证完成,使用安全
RawOptionList
组件完全支持defaultActiveKey
属性:
- 在
src/OptionList/List.tsx
中已定义该属性类型为React.Key[]
- 在组件内部通过
useActive
hook 正确处理了该属性- 属性值会被用于初始化活动项状态
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:验证 RawOptionList 组件是否支持 defaultActiveKey 属性 # 测试:搜索 RawOptionList 的属性定义 ast-grep --pattern 'interface $PROPS { $$$ defaultActiveKey$_ $$$ }' # 测试:搜索 defaultActiveKey 的使用 rg -A 5 'defaultActiveKey' src/OptionList/Length of output: 1849
src/OptionList/Column.tsx (2)
Line range hint
118-124
: 代码简化提高了可读性!通过移除冗余的条件检查,使代码更加清晰和直观。这个改动与支持默认展开单元格的目标相符合,同时保持了核心功能的完整性。
Line range hint
1-205
: 建议验证搜索功能的完整性由于移除了
searchValue
属性,建议:
- 确认搜索相关功能是否仍然正常工作
- 验证这个改动是否会影响现有的实现
- 考虑在文档中说明这个变更
src/OptionList/List.tsx (3)
25-25
: 类型定义正确且完整!
defaultActiveKey
的类型定义为可选的React.Key[]
符合 TypeScript 最佳实践。
28-28
: 属性解构合理!
defaultActiveKey
的解构位置正确,与其他相关属性放在一起,保持了代码的一致性。
92-92
: 请确认 useActive 钩子的行为符合预期根据之前的评审意见,
defaultActiveKey
应该只在初始化时生效,而不是在值变化时触发展开。请确保useActive
钩子的实现符合这个要求。✅ Verification successful
useActive 钩子的实现符合预期行为
根据源码分析,
useActive
钩子的实现是正确的:
defaultActiveKey
仅在初始化时通过useState
设置默认值:const [activeValueCells, setActiveValueCells] = React.useState<React.Key[]>(defaultActiveKey ?? []);
- 后续的值更新是由
setActiveValueCells
触发,与defaultActiveKey
无关- 在非多选模式下,活动项会跟随选中值自动更新,这是预期的行为
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查 useActive 钩子的实现 ast-grep --pattern 'export default function useActive($_) { $$$ }'Length of output: 70
Script:
#!/bin/bash # 搜索 useActive 的定义和实现 rg -g '*.{ts,tsx}' "useActive" -A 10 # 搜索包含 defaultActiveKey 的相关代码 rg -g '*.{ts,tsx}' "defaultActiveKey" -A 5Length of output: 7982
src/Cascader.tsx (2)
162-162
:defaultActiveKey
属性的类型定义正确且命名恰当属性类型定义为
React.Key[]
符合 React 的类型系统规范,且命名遵循了之前代码评审的建议。Also applies to: 179-179
162-162
: 验证新增属性的使用场景新增的
defaultActiveKey
属性和搜索逻辑的变更可能会影响现有功能,建议验证以下场景:
- 默认展开项的正确性
- 多选模式下搜索功能的表现
Also applies to: 179-179, 285-285
✅ Verification successful
defaultActiveKey
属性已得到充分验证和测试根据代码分析结果,
defaultActiveKey
属性已经:
- 在
useActive
hook 中得到正确实现和处理- 在多选模式下有专门的测试用例覆盖
- 在示例代码中有实际使用场景展示
搜索功能与
defaultActiveKey
属性相互独立,不会产生功能冲突。🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查 defaultActiveKey 的使用方式 echo "检查 defaultActiveKey 的使用场景..." rg "defaultActiveKey.*=.*\[.*\]" -g "!*.{test,spec}.{ts,tsx,js,jsx}" # 检查多选模式下的搜索相关测试用例 echo "检查多选模式的测试覆盖..." rg "describe.*multiple.*search" -g "*.{test,spec}.{ts,tsx,js,jsx}" -A 5Length of output: 503
Script:
#!/bin/bash # 检查 defaultActiveKey 的使用和相关测试 echo "检查 defaultActiveKey 的实现和测试..." rg "defaultActiveKey" -g "*.{ts,tsx}" -A 5 # 检查多选模式相关的测试用例 echo "检查多选模式的测试用例..." rg "multiple.*mode" -g "*.{test,spec}.{ts,tsx}" -A 5 # 检查搜索功能相关的测试 echo "检查搜索功能的测试..." rg "search.*test" -g "*.{test,spec}.{ts,tsx}" -A 5Length of output: 5397
const options = [ | ||
{ | ||
value: 'zhejiang', | ||
label: 'Zhejiang', | ||
children: [ | ||
{ | ||
value: 'hangzhou', | ||
label: 'Hangzhou', | ||
children: [ | ||
{ | ||
value: 'xihu', | ||
label: 'West Lake', | ||
}, | ||
{ | ||
value: 'xiasha', | ||
label: 'Xia Sha', | ||
}, | ||
], | ||
}, | ||
], | ||
}, | ||
{ | ||
value: 'jiangsu', | ||
label: 'Jiangsu', | ||
children: [ | ||
{ | ||
value: 'nanjing', | ||
label: 'Nanjing', | ||
children: [ | ||
{ | ||
value: 'zhonghuamen', | ||
label: 'Zhong Hua men', | ||
}, | ||
], | ||
}, | ||
], | ||
}, | ||
]; |
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
建议添加类型定义并分离数据
- 建议为选项数据添加 TypeScript 接口定义,提高代码的类型安全性:
interface CascaderOption {
value: string;
label: string;
children?: CascaderOption[];
}
- 建议将模拟数据移至单独的文件(如
mock/cascaderData.ts
),以提高代码的可维护性和复用性。
import React from 'react'; | ||
import '../assets/index.less'; | ||
import Cascader from '../src'; |
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
建议使用路径别名优化导入语句
当前的相对路径导入方式可能会在项目结构变化时造成维护困难。建议配置 TypeScript 的 path aliases,使用 @
或类似的别名来简化导入路径。
例如:
-import '../assets/index.less';
-import Cascader from '../src';
+import '@/assets/index.less';
+import Cascader from '@/src';
Committable suggestion was skipped due to low confidence.
const options = [ | ||
{ | ||
children: [] as any[], | ||
isParent: true, | ||
label: 'Asia', | ||
value: 'Asia', | ||
}, | ||
]; | ||
for (let i = 0; i < 100; i++) { | ||
options[0].children.push({ | ||
label: 'label' + i, | ||
value: 'value' + i, | ||
}); | ||
} |
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
建议优化测试数据的设置方式
当前的测试数据设置存在以下可改进之处:
- 魔法数字 100 应该提取为常量
- 可以使用更简洁的数组生成方法
- 类型
any[]
应该更具体
建议修改如下:
+ const TOTAL_OPTIONS = 100;
const options = [
{
- children: [] as any[],
+ children: [] as Array<{ label: string; value: string }>,
isParent: true,
label: 'Asia',
value: 'Asia',
},
];
- for (let i = 0; i < 100; i++) {
- options[0].children.push({
+ options[0].children = Array.from({ length: TOTAL_OPTIONS }, (_, i) => ({
label: 'label' + i,
value: 'value' + i,
- });
- }
+ }));
📝 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.
const options = [ | |
{ | |
children: [] as any[], | |
isParent: true, | |
label: 'Asia', | |
value: 'Asia', | |
}, | |
]; | |
for (let i = 0; i < 100; i++) { | |
options[0].children.push({ | |
label: 'label' + i, | |
value: 'value' + i, | |
}); | |
} | |
const TOTAL_OPTIONS = 100; | |
const options = [ | |
{ | |
children: [] as Array<{ label: string; value: string }>, | |
isParent: true, | |
label: 'Asia', | |
value: 'Asia', | |
}, | |
]; | |
options[0].children = Array.from({ length: TOTAL_OPTIONS }, (_, i) => ({ | |
label: 'label' + i, | |
value: 'value' + i, | |
})); |
it('limit', () => { | ||
const wrapper = mount( | ||
<Cascader | ||
options={options} | ||
open | ||
showSearch={{ | ||
limit: false, | ||
}} | ||
/>, | ||
); | ||
|
||
doSearch(wrapper, 'as'); | ||
const itemList = wrapper.find('div.rc-cascader-menu-item-content'); | ||
expect(itemList).toHaveLength(100); | ||
}); | ||
|
||
it('limit', () => { | ||
const wrapper = mount( | ||
<Cascader | ||
options={options} | ||
open | ||
showSearch={{ | ||
limit: 0, | ||
}} | ||
/>, | ||
); | ||
|
||
doSearch(wrapper, 'as'); | ||
const itemList = wrapper.find('div.rc-cascader-menu-item-content'); | ||
expect(itemList).toHaveLength(100); | ||
}); | ||
|
||
it('limit', () => { | ||
const wrapper = mount( | ||
<Cascader | ||
options={options} | ||
open | ||
showSearch={{ | ||
limit: 20, | ||
}} | ||
/>, | ||
); | ||
|
||
doSearch(wrapper, 'as'); | ||
const itemList = wrapper.find('div.rc-cascader-menu-item-content'); | ||
expect(itemList).toHaveLength(20); | ||
}); |
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
建议重构测试用例结构
当前测试用例存在以下问题:
- 测试名称不够具体,三个测试都叫 "limit"
- 测试设置代码重复
- 可以使用
test.each
来简化测试结构
建议重构如下:
- it('limit', () => {
- const wrapper = mount(
- <Cascader
- options={options}
- open
- showSearch={{
- limit: false,
- }}
- />,
- );
-
- doSearch(wrapper, 'as');
- const itemList = wrapper.find('div.rc-cascader-menu-item-content');
- expect(itemList).toHaveLength(100);
- });
-
- it('limit', () => {
- // ... 重复的测试
- });
-
- it('limit', () => {
- // ... 重复的测试
- });
+ const createWrapper = (limit: number | boolean) =>
+ mount(
+ <Cascader
+ options={options}
+ open
+ showSearch={{ limit }}
+ />
+ );
+
+ test.each([
+ ['显示所有选项当 limit 为 false', false, 100],
+ ['显示所有选项当 limit 为 0', 0, 100],
+ ['限制显示 20 个选项', 20, 20],
+ ])('%s', (_, limit, expectedLength) => {
+ const wrapper = createWrapper(limit);
+ doSearch(wrapper, 'as');
+ const itemList = wrapper.find('div.rc-cascader-menu-item-content');
+ expect(itemList).toHaveLength(expectedLength);
+ });
📝 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.
it('limit', () => { | |
const wrapper = mount( | |
<Cascader | |
options={options} | |
open | |
showSearch={{ | |
limit: false, | |
}} | |
/>, | |
); | |
doSearch(wrapper, 'as'); | |
const itemList = wrapper.find('div.rc-cascader-menu-item-content'); | |
expect(itemList).toHaveLength(100); | |
}); | |
it('limit', () => { | |
const wrapper = mount( | |
<Cascader | |
options={options} | |
open | |
showSearch={{ | |
limit: 0, | |
}} | |
/>, | |
); | |
doSearch(wrapper, 'as'); | |
const itemList = wrapper.find('div.rc-cascader-menu-item-content'); | |
expect(itemList).toHaveLength(100); | |
}); | |
it('limit', () => { | |
const wrapper = mount( | |
<Cascader | |
options={options} | |
open | |
showSearch={{ | |
limit: 20, | |
}} | |
/>, | |
); | |
doSearch(wrapper, 'as'); | |
const itemList = wrapper.find('div.rc-cascader-menu-item-content'); | |
expect(itemList).toHaveLength(20); | |
}); | |
const createWrapper = (limit: number | boolean) => | |
mount( | |
<Cascader | |
options={options} | |
open | |
showSearch={{ limit }} | |
/> | |
); | |
test.each([ | |
['显示所有选项当 limit 为 false', false, 100], | |
['显示所有选项当 limit 为 0', 0, 100], | |
['限制显示 20 个选项', 20, 20], | |
])('%s', (_, limit, expectedLength) => { | |
const wrapper = createWrapper(limit); | |
doSearch(wrapper, 'as'); | |
const itemList = wrapper.find('div.rc-cascader-menu-item-content'); | |
expect(itemList).toHaveLength(expectedLength); | |
}); |
// If is changeOnSelect or multiple | ||
enableHalfPath |
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
条件判断逻辑简化建议
当前条件判断 enableHalfPath
可以与上面的叶子节点判断合并,使代码更简洁。
建议重构为:
- if (
- // If is leaf option
- !children ||
- children.length === 0 ||
- // If is changeOnSelect or multiple
- enableHalfPath
- ) {
+ if (!children?.length || enableHalfPath) {
📝 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.
// If is changeOnSelect or multiple | |
enableHalfPath | |
// If is changeOnSelect or multiple | |
if (!children?.length || enableHalfPath) { |
const useSearchOptions = ( | ||
search: string, | ||
options: DefaultOptionType[], | ||
fieldNames: InternalFieldNames, | ||
prefixCls: string, | ||
config: ShowSearchType, | ||
changeOnSelect?: boolean, | ||
enableHalfPath?: boolean, |
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.
💡 Codebase verification
需要更新多个文件以反映参数变更
根据验证结果,changeOnSelect
参数在多个重要文件中被使用:
- README.md 中的 API 文档需要更新
- docs/demo/change-on-select.md 示例文档需要修改
- CHANGELOG.md 需要添加此破坏性变更说明
- 多个示例文件(如 examples/change-on-select.tsx)需要更新
- 相关测试文件需要同步修改
建议在合并此更改之前,确保完成以上文档和示例的更新,以保持文档与代码的一致性。
🔗 Analysis chain
参数命名更改需要更新相关文档
参数从 changeOnSelect
更改为 enableHalfPath
反映了功能语义的变化。建议:
- 确保更新相关的API文档
- 在 CHANGELOG 中记录这个破坏性变更
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有相关文档需要更新
rg -l "changeOnSelect" docs/
Length of output: 28
Script:
#!/bin/bash
# 检查整个代码库中是否有相关文档或测试文件包含 changeOnSelect
rg -l "changeOnSelect"
# 检查是否有 markdown 或其他文档文件
fd -e md -e mdx
Length of output: 1054
Script:
#!/bin/bash
# 检查文档中 changeOnSelect 的具体使用情况
rg "changeOnSelect" docs/demo/change-on-select.md -A 5
rg "changeOnSelect" README.md -A 5
rg "changeOnSelect" CHANGELOG.md
Length of output: 401
it('onChange should be triggered when click option with multiple', () => { | ||
const onChange = jest.fn(); | ||
const wrapper = mount(<Cascader checkable options={options} onChange={onChange} showSearch />); | ||
doSearch(wrapper, 'toy'); | ||
wrapper.find('.rc-cascader-menu-item').first().simulate('click'); | ||
wrapper.find('.rc-cascader-menu-item').first().simulate('mousedown'); | ||
expect(onChange).toHaveBeenCalledWith([['bamboo', 'little', 'fish']], expect.anything()); | ||
|
||
doSearch(wrapper, 'light'); | ||
wrapper.find('.rc-cascader-menu-item').first().simulate('click'); | ||
wrapper.find('.rc-cascader-menu-item').first().simulate('mousedown'); | ||
expect(onChange).toHaveBeenCalledWith( | ||
[['bamboo', 'little', 'fish'], ['light']], | ||
expect.anything(), | ||
); |
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
测试用例存在重复代码,建议重构
该测试用例与前一个测试用例有大量重复代码,仅配置项不同。建议提取共同的测试逻辑到辅助函数中。
建议重构为:
const testMultipleSelection = (props) => {
const onChange = jest.fn();
const wrapper = mount(<Cascader {...props} options={options} onChange={onChange} showSearch />);
// 测试选中第一个选项
doSearch(wrapper, 'toy');
wrapper.find('.rc-cascader-menu-item').first().simulate('click');
wrapper.find('.rc-cascader-menu-item').first().simulate('mousedown');
expect(onChange).toHaveBeenCalledWith([['bamboo', 'little', 'fish']], expect.anything());
// 测试选中第二个选项
doSearch(wrapper, 'light');
wrapper.find('.rc-cascader-menu-item').first().simulate('click');
wrapper.find('.rc-cascader-menu-item').first().simulate('mousedown');
expect(onChange).toHaveBeenCalledWith(
[['bamboo', 'little', 'fish'], ['light']],
expect.anything(),
);
};
it('onChange should be triggered when click option with changeOnSelect + multiple', () => {
testMultipleSelection({ checkable: true, changeOnSelect: true });
});
it('onChange should be triggered when click option with multiple', () => {
testMultipleSelection({ checkable: true });
});
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #515 +/- ##
==========================================
+ Coverage 99.67% 99.83% +0.16%
==========================================
Files 23 23
Lines 607 611 +4
Branches 183 186 +3
==========================================
+ Hits 605 610 +5
+ Misses 2 1 -1 ☔ View full report in Codecov by Sentry. |
PR 乱了,解决一下冲突? |
我看下 |
* feat: add disabled api for panel * feat: add test case * docs: add demo * fix: test case fixed * feat: adjust demo position * feat: improve demo * chore: reorganize code for better readability * test: update snapshot * fix: lint fix * chore: adjust code structure * test: update snapshot * chore: code optimization --------- Co-authored-by: 小豪 <aojunhao@cai-inc.com>
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: 3
🧹 Outside diff range and nitpick comments (4)
examples/panel.tsx (1)
99-100
: 代码格式需要调整
console.log
语句的缩进不一致,建议保持统一的缩进风格。建议修改为:
onChange={nextValue => { - console.log('Change:', nextValue); - setValue2(nextValue); + console.log('Change:', nextValue); + setValue2(nextValue); }}tests/Panel.spec.tsx (1)
82-93
: 建议增强测试用例的覆盖范围测试用例验证了
defaultActiveKey
属性的基本功能,但建议添加以下断言以提高测试覆盖率:
- 验证激活项的状态(例如检查
aria-checked
属性)- 验证菜单项的内容是否正确显示
- 验证用户交互后的行为
建议添加以下测试代码:
it('multiple with defaultActiveKey', () => { const onChange = jest.fn(); const { container } = render( <Cascader.Panel checkable options={options} onChange={onChange} defaultActiveKey={['bamboo', 'little']} />, ); expect(container.querySelectorAll('.rc-cascader-menu')).toHaveLength(2); + // 验证激活项 + const activeItems = container.querySelectorAll('.rc-cascader-menu-item-active'); + expect(activeItems).toHaveLength(2); + expect(activeItems[0].textContent).toBe('Bamboo'); + expect(activeItems[1].textContent).toBe('Little'); + + // 验证用户交互 + fireEvent.click(container.querySelectorAll('.rc-cascader-menu-item')[0]); + expect(onChange).toHaveBeenCalled(); });src/Panel.tsx (1)
Line range hint
1-213
: 建议添加属性文档和类型验证为了提高代码的可维护性和使用体验,建议:
- 为新增的
defaultActiveKey
和disabled
属性添加 JSDoc 文档说明- 考虑添加属性类型验证
需要我帮您生成相关的文档注释和类型验证代码吗?
src/OptionList/List.tsx (1)
120-127
: 建议优化禁用状态的判断逻辑当前实现是可行的,但可以通过合并条件判断来优化代码:
- if (disabled) { - return false; - } - - const { disabled: optionDisabled } = option; - const isMergedLeaf = isLeaf(option, fieldNames); - - return !optionDisabled && (isMergedLeaf || changeOnSelect || multiple); + const { disabled: optionDisabled } = option; + const isMergedLeaf = isLeaf(option, fieldNames); + return !disabled && !optionDisabled && (isMergedLeaf || changeOnSelect || multiple);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
examples/panel.tsx
(2 hunks)package.json
(3 hunks)src/OptionList/Column.tsx
(7 hunks)src/OptionList/List.tsx
(5 hunks)src/Panel.tsx
(3 hunks)tests/Panel.spec.tsx
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/OptionList/Column.tsx
🧰 Additional context used
🪛 Biome
examples/panel.tsx
[error] 103-104: This JSX property is assigned multiple times.
This attribute is assigned again here.
(lint/suspicious/noDuplicateJsxProps)
src/Panel.tsx
[error] 74-74: expected ,
but instead found disabled
Remove disabled
(parse)
[error] 75-75: Declarations inside of a const
declaration may not have duplicates
a second declaration of defaultActiveKey
is not allowed
defaultActiveKey
is first declared here
(parse)
[error] 208-208: This JSX property is assigned multiple times.
This attribute is assigned again here.
(lint/suspicious/noDuplicateJsxProps)
🪛 eslint
examples/panel.tsx
[error] 104-104: No duplicate props allowed
(react/jsx-no-duplicate-props)
🔇 Additional comments (9)
examples/panel.tsx (3)
64-65
: 状态管理的良好实践!
新增的 disabled
状态变量使用了 React 的 useState
Hook,实现了组件的可控性,这是一个很好的实践。
76-82
: 用户交互控制完善!
新增的按钮提供了良好的交互体验,允许用户动态切换面板的禁用状态。
107-107
: RTL 支持的良好实践!
添加 direction="rtl"
支持右到左布局是一个很好的国际化实践。
tests/Panel.spec.tsx (1)
114-125
: 测试用例设计合理
该测试用例完整地覆盖了禁用状态的核心功能:
- 验证了禁用样式的存在
- 验证了禁用状态下点击事件被阻止
- 验证了回调函数未被触发
src/Panel.tsx (1)
38-40
: 类型定义扩展正确
新增的 defaultActiveKey
和 disabled
属性扩展了 PickType
类型,这些改动符合组件功能的需求。
src/OptionList/List.tsx (4)
24-32
: 类型定义更新合理!
RawOptionListProps
类型的扩展符合组件需求,新增的 disabled
和 defaultActiveKey
属性定义清晰。
35-45
: 属性解构合理!
新增属性的解构符合代码风格,顺序合理。
109-109
: 很好地采纳了之前的反馈!
将 defaultActiveKey
直接传递给 useActive
hook 而不是使用 effect,这样确保了默认值不会随着属性变化而动态改变,符合 "default" 的语义。
Line range hint 190-203
: 滚动逻辑优化合理!
在搜索状态下跳过滚动操作的优化很好,可以避免不必要的 DOM 操作。代码实现清晰,维护了原有功能。
examples/panel.tsx
Outdated
defaultActiveKey={['bj', 'haidian']} | ||
disabled={disabled} | ||
defaultActiveKey={['bj', 'haidian']} |
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.
发现重复的属性设置
defaultActiveKey
属性在组件中被重复定义了两次,这可能会导致意外的行为。
建议移除重复的属性:
<Cascader.Panel
checkable
value={value2}
options={addressOptions}
onChange={nextValue => {
console.log('Change:', nextValue);
setValue2(nextValue);
}}
defaultActiveKey={['bj', 'haidian']}
disabled={disabled}
- defaultActiveKey={['bj', 'haidian']}
/>
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome
[error] 103-104: This JSX property is assigned multiple times.
This attribute is assigned again here.
(lint/suspicious/noDuplicateJsxProps)
🪛 eslint
[error] 104-104: No duplicate props allowed
(react/jsx-no-duplicate-props)
src/Panel.tsx
Outdated
defaultActiveKey={defaultActiveKey} | ||
disabled={disabled} | ||
defaultActiveKey={defaultActiveKey} |
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.
修复 RawOptionList 组件的重复属性
defaultActiveKey
属性在 RawOptionList 组件中被重复传递了。
建议按照以下方式修改:
<RawOptionList
prefixCls={prefixCls}
searchValue=""
multiple={multiple}
toggleOpen={noop}
open
direction={direction}
defaultActiveKey={defaultActiveKey}
disabled={disabled}
- defaultActiveKey={defaultActiveKey}
/>
📝 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.
defaultActiveKey={defaultActiveKey} | |
disabled={disabled} | |
defaultActiveKey={defaultActiveKey} | |
defaultActiveKey={defaultActiveKey} | |
disabled={disabled} |
🧰 Tools
🪛 Biome
[error] 208-208: This JSX property is assigned multiple times.
This attribute is assigned again here.
(lint/suspicious/noDuplicateJsxProps)
src/Panel.tsx
Outdated
defaultActiveKey | ||
disabled, | ||
defaultActiveKey |
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.
修复属性解构中的重复声明
代码中存在以下问题:
defaultActiveKey
被重复声明了两次disabled
的声明位置不正确
建议按照以下方式修改:
const {
prefixCls = 'rc-cascader',
style,
className,
options,
checkable,
defaultValue,
value,
fieldNames,
changeOnSelect,
onChange,
showCheckedStrategy,
loadData,
expandTrigger,
expandIcon = '>',
loadingIcon,
direction,
notFoundContent = 'Not Found',
- defaultActiveKey
- disabled,
- defaultActiveKey
+ defaultActiveKey,
+ disabled
} = props as Pick<InternalCascaderProps, PickType>;
📝 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.
defaultActiveKey | |
disabled, | |
defaultActiveKey | |
prefixCls = 'rc-cascader', | |
style, | |
className, | |
options, | |
checkable, | |
defaultValue, | |
value, | |
fieldNames, | |
changeOnSelect, | |
onChange, | |
showCheckedStrategy, | |
loadData, | |
expandTrigger, | |
expandIcon = '>', | |
loadingIcon, | |
direction, | |
notFoundContent = 'Not Found', | |
defaultActiveKey, | |
disabled |
🧰 Tools
🪛 Biome
[error] 74-74: expected ,
but instead found disabled
Remove disabled
(parse)
[error] 75-75: Declarations inside of a const
declaration may not have duplicates
a second declaration of defaultActiveKey
is not allowed
defaultActiveKey
is first declared here
(parse)
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
🧹 Outside diff range and nitpick comments (2)
examples/panel.tsx (2)
76-82
: 用户界面交互设计合理切换按钮的文本会根据当前状态动态改变,这提供了良好的用户体验。建议考虑添加一些视觉反馈,比如禁用状态下的样式变化。
<button onClick={() => { setDisabled(prev => !prev); }} + style={{ + opacity: disabled ? 0.5 : 1, + cursor: disabled ? 'not-allowed' : 'pointer' + }} > {disabled ? 'enable panel' : 'disable panel'} </button>
99-100
: 代码格式需要调整
console.log
和setValue2
的缩进不一致,影响代码的可读性。onChange={nextValue => { - console.log('Change:', nextValue); - setValue2(nextValue); + console.log('Change:', nextValue); + setValue2(nextValue); }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
examples/panel.tsx
(2 hunks)
🔇 Additional comments (3)
examples/panel.tsx (3)
64-65
: 状态管理的良好实践
使用 useState
来管理 disabled
状态是一个很好的做法,这样可以统一控制所有面板的禁用状态。
102-103
: 验证默认展开功能的实现
需要确保 defaultActiveKey
和 disabled
属性在组件内部正确处理,建议添加相应的单元测试。
✅ Verification successful
Let me gather more information about the test coverage for the disabled
prop.
defaultActiveKey
和 disabled
属性已有测试覆盖
根据测试文件的分析,这两个属性已经有相应的单元测试覆盖:
defaultActiveKey
在multiple with defaultActiveKey
测试用例中验证了多选模式下的默认展开功能disabled
属性在测试中验证了禁用状态下的渲染和点击行为
无需添加额外的测试用例。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:检查是否存在相关的测试用例
# 查找测试文件中的相关测试用例
rg -A 5 "defaultActiveKey|disabled.*Panel" tests/
Length of output: 1284
Script:
#!/bin/bash
# 查找与 Panel 组件的 disabled 属性相关的测试用例
rg -A 10 "disabled.*Panel|Panel.*disabled" tests/
# 查找 Panel 组件的实现,了解 disabled 属性的处理
ast-grep --pattern 'interface PanelProps {
$$$
disabled?: $_;
$$$
}'
Length of output: 679
106-106
: RTL支持的完整性检查
在添加 RTL 支持时,建议确保所有子组件和样式都正确支持从右到左的布局。
已改 |
新的 pull-request : https://github.com/react-component/cascader/pull/547/commits |
Summary by CodeRabbit
新功能
.github/FUNDING.yml
文件,支持多种资金筹集平台。defaultActiveKey
属性,增强Cascader
组件的配置选项。multiple-search.md
文档,展示多个搜索示例。multiple-search.tsx
示例组件,演示层级选项的使用。Cascader.Panel
组件的测试用例,验证defaultActiveKey
和disabled
属性的功能。Cascader.Search
组件的测试用例,验证搜索限制功能。Cascader
组件在使用悬停触发和搜索功能时的表现。修复
Cascader
组件在不同条件下的行为正确。文档
README.md
文件中的描述,增加句点。依赖更新
package.json
中的多个依赖版本,提升项目兼容性和功能性。