Skip to content
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: autoClearSearchValue invalid #949

Closed
wants to merge 3 commits into from

Conversation

vaynevayne
Copy link

resolve #947

@vercel
Copy link

vercel bot commented Jun 10, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
select ❌ Failed (Inspect) Jun 10, 2023 11:15am

src/Select.tsx Outdated
@@ -544,7 +544,7 @@ const Select = React.forwardRef(
const newRawValues = Array.from(new Set<RawValueType>([...rawValues, formatted]));
triggerChange(newRawValues);
triggerSelect(formatted, true);
setSearchValue('');
if(autoClearSearchValue) setSearchValue('');
Copy link
Contributor

Choose a reason for hiding this comment

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

建议改为

if(autoClearSearchValue){
  setSearchValue('');
}

现在这样写不利于维护

@BoyYangzai
Copy link
Contributor

@vaynevayne 辛苦添加一下test case

@codecov
Copy link

codecov bot commented Jun 10, 2023

Codecov Report

Merging #949 (bbbaf98) into master (7940e17) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head bbbaf98 differs from pull request most recent head a82fc05. Consider uploading reports for the commit a82fc05 to get more accurate results

@@           Coverage Diff           @@
##           master     #949   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files          37       37           
  Lines        1356     1357    +1     
  Branches      393      394    +1     
=======================================
+ Hits         1351     1352    +1     
  Misses          4        4           
  Partials        1        1           
Impacted Files Coverage Δ
src/Select.tsx 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@vaynevayne
Copy link
Author

@BoyYangzai 你好 请问这个 pr 进度如何了, 业务那边比较着急

@BoyYangzai
Copy link
Contributor

@BoyYangzai 你好 请问这个 pr 进度如何了, 业务那边比较着急

召唤豆酱 @zombieJ

@@ -544,7 +544,9 @@ const Select = React.forwardRef(
const newRawValues = Array.from(new Set<RawValueType>([...rawValues, formatted]));
triggerChange(newRawValues);
triggerSelect(formatted, true);
setSearchValue('');
if(autoClearSearchValue) {
Copy link
Member

Choose a reason for hiding this comment

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

我给这两个 case 打了断点,不会跑到这里。确认一下是否是搞错地方了?

Copy link
Member

Choose a reason for hiding this comment

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

断点显示是来自上方的重置,再看看?

截屏2023-06-12 14 01 19

@zombieJ
Copy link
Member

zombieJ commented Jun 12, 2023

在这里回复了:ant-design/ant-design#42933 (comment)

autoClearSearchValue 是失去焦点触发,这个属性是没有错的。你需要的功能 HOC 受控一下 searchTextonSearch 即可。

@zombieJ zombieJ closed this Jun 12, 2023
@vaynevayne
Copy link
Author

看这个 pr, 失去焦点也有问题
#951

@vaynevayne
Copy link
Author

vaynevayne commented Jun 12, 2023

searchText 和 onSearch 的方式, 会引起新的问题
image

  1. 键入筛选值
  2. 设置 select 为 tag 类型
  3. 设置 select 为 multiple 类型

期望的行为: 选中项是空的, 因为这个过程我并没有选择任何东西
实际的行为: 选中项是我的searchValue

image

@vaynevayne
Copy link
Author

vaynevayne commented Jun 12, 2023

@zombieJ 按照我那个debug 步骤走一下就能复现,

讲道理你两处以 type submit 来调用的判断方式不一样

 if (which === KeyCode.ENTER /**here */ && mode === 'tags' && !compositionStatusRef.current && !open) {
      // When menu isn't open, OptionList won't trigger a value change
      // So when enter is pressed, the tag's input value should be emitted here to let selector know
      onSearchSubmit?.((event.target as HTMLInputElement).value);
    }
 const onContainerBlur: React.FocusEventHandler<HTMLElement> = (...args) => {
    ...

    if (mergedSearchValue) {
      // `tags` mode should move `searchValue` into values
      if (mode === 'tags') {
        onSearch(mergedSearchValue, { source: 'submit' }); // here 
      } 
    }

我有点不太理解, 为什么两段 type=submit 的判断逻辑不一致, 一个需要key===enter, 一个只需要 blur 即可(这也是导致我使用 searchValue 和 onSearch 会引出性 bug 的逻辑)

@zombieJ
Copy link
Member

zombieJ commented Jun 12, 2023

tag 模式下,输入的内容无论在不在 options 中都是 option。如果不需要为 option 应该是 multiple

@vaynevayne
Copy link
Author

tag 模式下,输入的内容无论在不在 options 中都是 option。如果不需要为 option 应该是 multiple

我按 enter 键,你把搜索值放到 options 里还正常, 问题是 blur 时为什么也要放入 options, 我觉得我觉得我描述的场景够清楚了

我也知道你的意思了,永远不会处理这个问题罢了

@zombieJ
Copy link
Member

zombieJ commented Jun 13, 2023

tag 模式下,输入的内容无论在不在 options 中都是 option。如果不需要为 option 应该是 multiple

我按 enter 键,你把搜索值放到 options 里还正常, 问题是 blur 时为什么也要放入 options, 我觉得我觉得我描述的场景够清楚了

我也知道你的意思了,永远不会处理这个问题罢了

这是两个问题:

  • blur 输入是交互设计问题
  • 这个 PR ref 的 issue 是 blur 后输入框值消失的问题

我理解这是个 XY 问题,真实要解的应该是交互设计,而不是给 autoClearSearchValue 打个洞去解交互问题。请再看一下我前面提到的关于 mode 的区别。对于不同的交互,是可以自行实现的:

https://codesandbox.io/s/duo-xuan-antd-5-6-1-forked-64q66p?file=/demo.tsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Select 自定义下拉面板, 当点击面板中的按钮+号时, 会丢失 搜索值
3 participants