Skip to content

fix: https://github.com/ant-design/ant-design/issues/50642 #1084

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

fix: https://github.com/ant-design/ant-design/issues/50642 #1084

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3f8c624
fix: multiple 下清空时少触发一次
su-muzhi Nov 6, 2024
9a44b6e
test: add unit tests for BaseSelct
su-muzhi Nov 11, 2024
20e01d6
refactor: use ternary operator instead of if-else
su-muzhi Nov 11, 2024
f02c24d
chore: resolve ESLint errors
su-muzhi Nov 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/BaseSelect/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,9 +605,15 @@
onSearch(mergedSearchValue, { source: 'submit' });
} else if (mode === 'multiple') {
// `multiple` mode only clean the search value but not trigger event
onSearch('', {
source: 'blur',
});
if (mergedShowSearch) {
onSearch('', {
source: 'effect',
});
} else {
onSearch('', {

Check warning on line 613 in src/BaseSelect/index.tsx

View check run for this annotation

Codecov / codecov/patch

src/BaseSelect/index.tsx#L612-L613 

Added lines #L612 - L613 were not covered by tests
source: 'blur',
});
}
Copy link
Contributor

@coderabbitai coderabbitai bot Nov 6, 2024 •
edited
Loading

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

合并成一个source: ? a: b如何

Copy link
Contributor

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.

}
}

Expand Down
43 changes: 43 additions & 0 deletions tests/Select.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,49 @@ describe('Select.Basic', () => {
jest.useRealTimers();
});

describe('should call handleSearch twice on search and blur', () => {
injectRunAllTimers(jest);

beforeEach(() => {
jest.useFakeTimers();
});

afterEach(() => {
jest.useRealTimers();
});

it('multiple mode should call handleSearch twice on search and blur', async () => {
const handleSearch = jest.fn();
const { container } = render(
<Select showSearch onSearch={handleSearch} mode="multiple">
<Option value="1">1</Option>
<Option value="2">2</Option>
</Select>,
);
fireEvent.change(container.querySelector('input')!, { target: { value: '1' } });
// 模拟失去焦点（blur）事件
fireEvent.blur(container.querySelector('input'));
jest.runAllTimers();
expect(handleSearch).toHaveBeenCalledTimes(2);
jest.useRealTimers();
});

it('not multiple mode should call handleSearch twice on search and blur', async () => {
const handleSearch = jest.fn();
const { container } = render(
<Select showSearch onSearch={handleSearch}>
<Option value="1">1</Option>
<Option value="2">2</Option>
</Select>,
);
fireEvent.change(container.querySelector('input')!, { target: { value: '1' } });
fireEvent.blur(container.querySelector('input'));
jest.runAllTimers();
expect(handleSearch).toHaveBeenCalledTimes(2);
jest.useRealTimers();
});
});

it('should render 0 as text properly', () => {
const data = [
{ text: 0, value: '=0' },
Expand Down
Loading