-
-
Notifications
You must be signed in to change notification settings - Fork 457
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
fix: https://github.com/ant-design/ant-design/issues/50642 #1084
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough此次更改主要集中在 Changes
Possibly related PRs
Poem
Warning There were issues while running the following tools: 🔧 eslintsrc/BaseSelect/index.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Cannot read config file: /.eslintrc.js
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: 1
🧹 Outside diff range and nitpick comments (2)
tests/Select.test.tsx (2)
619-633
: 多选模式下的搜索和失焦测试用例实现完整测试用例验证了在多选模式下:
- 搜索时会触发一次 handleSearch
- 失焦时会再次触发 handleSearch
- 使用 jest.runAllTimers() 确保异步行为被正确处理
建议添加对 handleSearch 调用参数的验证。
expect(handleSearch).toHaveBeenCalledTimes(2); +expect(handleSearch).toHaveBeenNthCalledWith(1, '1'); +expect(handleSearch).toHaveBeenNthCalledWith(2, '');
635-648
: 单选模式下的搜索和失焦测试用例实现完整测试用例验证了在单选模式下的搜索和失焦行为,与多选模式保持一致。
同样建议添加对 handleSearch 调用参数的验证。
expect(handleSearch).toHaveBeenCalledTimes(2); +expect(handleSearch).toHaveBeenNthCalledWith(1, '1'); +expect(handleSearch).toHaveBeenNthCalledWith(2, '');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/BaseSelect/index.tsx
(1 hunks)tests/Select.test.tsx
(1 hunks)
🔇 Additional comments (1)
tests/Select.test.tsx (1)
608-618
: 测试套件设置合理
测试套件正确使用了 jest 的定时器模拟,并在每个测试前后进行了适当的设置和清理。
src/BaseSelect/index.tsx
Outdated
if (mergedShowSearch) { | ||
onSearch('', { | ||
source: 'effect', | ||
}); | ||
} else { | ||
onSearch('', { | ||
source: 'blur', | ||
}); | ||
} |
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
需要补充失焦时搜索值处理的测试用例
BaseSelect 组件当前缺少针对 mergedShowSearch
条件下失焦时搜索值处理的测试用例。建议添加以下测试场景:
- 当
mergedShowSearch
为 true 时,失焦应触发onSearch
并传入source: 'effect'
- 当
mergedShowSearch
为 false 时,失焦应触发onSearch
并传入source: 'blur'
现有的测试文件(BaseSelect.test.tsx)中没有相关测试覆盖,而 Select.test.tsx 中的测试用例也未验证 source 参数的正确性。
🔗 Analysis chain
搜索值处理逻辑优化
此更改合理地区分了显示搜索和非显示搜索状态下的行为,使组件在失焦时的搜索值处理更加准确。
建议执行以下脚本验证相关测试用例是否完整:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证失焦时搜索值处理的测试覆盖情况
# 测试:搜索相关的测试用例
rg -l "describe.*blur.*search" tests/
Length of output: 39
Script:
#!/bin/bash
# 搜索所有与 blur 和 search 相关的测试文件
rg -l "blur.*search|search.*blur" tests/
# 查找与 BaseSelect 组件相关的测试文件
fd "BaseSelect.*test" tests/
# 在测试文件中搜索与 onSearch 和 blur 相关的测试用例
rg "test.*onSearch.*blur|describe.*blur.*onSearch" tests/ -A 5
Length of output: 186
Script:
#!/bin/bash
# 检查 BaseSelect.test.tsx 中的测试用例内容
cat tests/BaseSelect.test.tsx
# 检查 Select.test.tsx 中与 blur 和 search 相关的测试用例
rg "it\(.*blur.*search|it\(.*search.*blur" tests/Select.test.tsx -B 2 -A 10
Length of output: 4948
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.
合并成一个source: ? a: b如何
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.
@yoyo837,好建议!可以将代码合并成一个三元表达式,如下:
- if (mergedShowSearch) {
- onSearch('', {
- source: 'effect',
- });
- } else {
- onSearch('', {
- source: 'blur',
- });
- }
+ onSearch('', {
+ source: mergedShowSearch ? 'effect' : 'blur',
+ });
这样可以使代码更加简洁。
✏️ Learnings added
Learnt from: yoyo837
PR: react-component/select#1084
File: src/BaseSelect/index.tsx:608-616
Timestamp: 2024-11-11T06:10:20.695Z
Learning: 在适当的情况下,可以将简单的 if-else 语句合并为三元表达式,以使代码更简洁。
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1084 +/- ##
=======================================
Coverage 98.24% 98.24%
=======================================
Files 39 39
Lines 1482 1484 +2
Branches 418 448 +30
=======================================
+ Hits 1456 1458 +2
Misses 26 26 ☔ View full report in Codecov by Sentry. |
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)
tests/BaseSelect.test.tsx (2)
3-3
: 移除未使用的 useState 导入
useState
钩子已被导入但在测试代码中未被使用,建议将其移除以保持代码整洁。-import { forwardRef, act, useState } from 'react'; +import { forwardRef, act } from 'react';
127-175
: 测试用例设计合理,建议补充边缘场景测试测试用例很好地覆盖了
showSearch
启用和禁用时的核心场景,验证了onContainerBlur
在不同条件下的行为表现。建议考虑添加以下测试场景以提高覆盖率:
- 在失焦时搜索值为空的情况
- 在组件被禁用(disabled)时的行为
- 快速切换 showSearch 属性时的行为
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 (1)
tests/BaseSelect.test.tsx (1)
128-150
: showSearch 启用场景的测试用例实现完善测试用例验证了以下关键点:
- 输入值改变时触发 onSearch,source 为 'typing'
- 失焦时触发 onSearch,source 为 'effect'
建议补充一个边界场景测试。
it('BaseSelect with showSearch and empty input', () => { const onSearch = jest.fn(); const { container } = render( <BaseSelect // ... 其他属性保持不变 searchValue="" showSearch onSearch={onSearch} /> ); fireEvent.blur(container.querySelector('div.rc-select')); expect(onSearch).toHaveBeenCalledWith('', { source: 'effect' }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/BaseSelect/index.tsx
(1 hunks)tests/BaseSelect.test.tsx
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/BaseSelect/index.tsx
🔇 Additional comments (2)
tests/BaseSelect.test.tsx (2)
127-175
: 测试用例结构清晰,覆盖场景完整!
测试套件结构合理,分别验证了 showSearch 开启和关闭时的 onContainerBlur 行为,符合组件的预期功能。
152-174
: showSearch 禁用场景的测试用例实现准确
测试用例完整验证了在 showSearch 为 false 时的行为:
- 输入值改变时触发 onSearch,source 为 'typing'
- 失焦时触发 onSearch,source 为 'blur'
测试实现符合预期,无需修改。
@@ -606,7 +606,7 @@ const BaseSelect = React.forwardRef<BaseSelectRef, BaseSelectProps>((props, ref) | |||
} else if (mode === 'multiple') { | |||
// `multiple` mode only clean the search value but not trigger event | |||
onSearch('', { | |||
source: 'blur', | |||
source: mergedShowSearch ? 'effect' : 'blur', |
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.
感觉反了,应该是单选的时候 blur 触发的 onSearch 的 source 不对。那个应该是 blur
,这个 multiple 的是对的。
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.
感觉反了,应该是单选的时候 blur 触发的 onSearch 的 source 不对。那个应该是
blur
,这个 multiple 的是对的。
“多选下也需要重新触发搜索”,大佬,这是你给出的思路
Move to #1085 |
Summary by CodeRabbit
Summary by CodeRabbit
BaseSelect
组件的搜索状态管理逻辑,改进了组件失去焦点时的搜索值处理。BaseSelect
组件的测试覆盖率,新增测试用例以验证onContainerBlur
参数在不同条件下的行为。Select.test.tsx
中新增了测试用例,确保搜索功能在模糊和非模糊模式下的行为符合预期。fix ant-design/ant-design#50642