Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
2 changes: 1 addition & 1 deletion src/hooks/useSearchConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export default function useSearchConfig(showSearch?: CascaderProps['showSearch']
};
}

if ((searchConfig.limit as number) <= 0) {
if ((searchConfig.limit as number) <= 0 && searchConfig.limit !== false) {
delete searchConfig.limit;

if (process.env.NODE_ENV !== 'production') {
Expand Down
76 changes: 76 additions & 0 deletions tests/search.limit.spec.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import React from 'react';
import Cascader from '../src';
import type { ReactWrapper } from './enzyme';
import { mount } from './enzyme';

describe('Cascader.Search', () => {
function doSearch(wrapper: ReactWrapper, search: string) {
wrapper.find('input').simulate('change', {
target: {
value: search,
},
});
}
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,
});
}

it('limit', () => {
const wrapper = mount(
<Cascader
options={options}
open
showSearch={{
limit: false,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
limit: false,
limit: 0,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

是不是用 0 比较好, pr修复的是<= 0的情况下,limit变成undefined然后又重新默认值50了,false本身是好的。

@dongbanban dongbanban Oct 16, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

image image

定位的问题出在limit = false时会走进 if的判断,导致 limit被 delete,所以测试用例里是用的limit:false, 或者直接再加一层判断,规避上述场景,这样原本的逻辑没变,false也可以当 no limit处理

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

如果误导你了不好意思,还是等大佬们建议。我评论的是测试用例用个 limit: 0,,因为之前limit: false的情况本来就是好的,是limit:0才出了问题, 感觉这里面的业务逻辑是 searchConfig.limit = 0; or searchConfig.limit = false; 可能都行。你改回了这个delete不是又导致了被默认赋值50的问题。

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

测试用例我加上 limit:0/false/20 了,limit:false 是有问题的,实际使用依然会走进 if,大佬你可以试一下,官网 demo 里改成limit:false 依然是50 条

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

delete之后会变成undefined所以重新赋值了,所以是不是给个默认复制 0 可以呢
image

image

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

image

所以这里的逻辑 limit = false或者<=0都是展示全量数据,我最开始的修改进入判断后都是直接limit设为 false应该就可以的

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

image image image image image

这样就可以了吧

}}
/>,
);

doSearch(wrapper, 'as');
const itemList = wrapper.find('div.rc-cascader-menu-item-content');
expect(itemList).toHaveLength(itemList.length);
});
Comment on lines +31 to +75

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue

测试用例可以更具体和描述性。

当前的测试用例结构正确,但可以进行以下改进:

  1. 测试描述应更具体,例如:"当showSearch.limit设置为false时,应显示所有匹配项"。

  2. 断言可以更明确。目前的断言只是检查项目列表的长度与自身相等,这并不能真正验证功能。建议改为:

    expect(itemList).toHaveLength(101); // 100个子项 + 1个父项
  3. 可以添加一个反例,测试当limit设置为一个数值时的行为。

  4. 考虑测试搜索结果的内容,而不仅仅是数量。

这些改进将使测试更加健壮和有意义。

如果您需要,我可以帮助您重写这个测试用例。


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(50);
});

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);
});
});