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(select): 修复Select 通过 showClear 清空后,用户配置的 blur 失焦触发校验的Props 不生效的bug #1865

Closed
wants to merge 1 commit into from

Conversation

Jon-Millent
Copy link
Contributor

@Jon-Millent Jon-Millent commented Oct 19, 2023

中文模板 / Chinese Template

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Test Case
  • TypeScript definition update
  • Document improve
  • CI/CD improve
  • Branch sync
  • Other, please describe:

PR description

Fixes #1453

Changelog

🇨🇳 Chinese

  • Fix: 修复Select 通过 showClear 清空后,用户配置的 blur 失焦触发校验的Props 不生效的bug
    修复原因:
    因为Select在Open的时候,会注册registerClickOutsideHandler事件,来触发blur的规则校验,但是配置showClear之后,点击clear按钮,并没有open select,所以没有注册registerClickOutsideHandler,所以在clearSelected的时候注册registerClickOutsideHandler事件,这里不怕多次点击clear按钮会重复注册,因为注册事件的时候里面判断会解绑多余的注册事件,如下源码:

tooltip/index.ts

...
registerClickOutsideHandler: (cb: () => void) => {
   if (this.clickOutsideHandler) {
      this.adapter.unregisterClickOutsideHandler();
  }
...

🇺🇸 English

  • Fix: Fix the bug that the Props configured by the user to trigger the verification of blur out of focus do not take effect after the Select is cleared through showClear.
    Reason for repair:
    Because when Select is Open, it will register the registerClickOutsideHandler event to trigger blur rule verification. However, after configuring showClear, clicking the clear button does not open select, so the registerClickOutsideHandler is not registered, so the registerClickOutsideHandler event is registered when clearSelected. Don't be afraid here. Clicking the clear button multiple times will result in repeated registration, because when registering events, it will be judged to unbind redundant registration events, as shown in the following source code:

tooltip/index.ts

...
registerClickOutsideHandler: (cb: () => void) => {
   if (this.clickOutsideHandler) {
      this.adapter.unregisterClickOutsideHandler();
  }
...

Checklist

  • Test
  • Document no need
  • Changelog need

Other

  • Skip Changelog

Additional information

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7c913ca:

Sandbox Source
pr-story Configuration
Semi Design: Simple Story Configuration

@codecov-commenter
Copy link

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (205b13c) 88.02% compared to head (ca00d89) 88.00%.
Report is 1 commits behind head on release.

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

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff             @@
##           release    #1865      +/-   ##
===========================================
- Coverage    88.02%   88.00%   -0.03%     
===========================================
  Files          436      436              
  Lines        25503    25507       +4     
  Branches      6438     6438              
===========================================
- Hits         22449    22447       -2     
- Misses        3054     3060       +6     
Files Coverage Δ
packages/semi-ui/select/index.tsx 84.15% <0.00%> (ø)
packages/semi-foundation/select/foundation.ts 81.35% <25.00%> (-0.36%) ⬇️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@YannLynn
Copy link
Collaborator

这种修改存在 bad case,比如用户重复选择,并重复将选择值通过 clear icon 请空,这样就会导致重复注册 clickoutside 的问题。

@Jon-Millent
Copy link
Contributor Author

这种修改存在不好的情况,比如用户重复选择,并重复将选择值通过清除图标请空,这样可能会导致重复注册 clickoutside 的问题。

注册事件的时候里面判断会解绑多余的注册事件,如下源码:
tooltip/index.ts
...
registerClickOutsideHandler: (cb: () => void) => {
if (this.clickOutsideHandler) {
this.adapter.unregisterClickOutsideHandler();
}
...

@YannLynn
Copy link
Collaborator

YannLynn commented Jan 9, 2024

这种修改存在不好的情况,比如用户重复选择,并重复将选择值通过清除图标请空,这样可能会导致重复注册 clickoutside 的问题。

注册事件的时候里面判断会解绑多余的注册事件,如下源码: tooltip/index.ts ... registerClickOutsideHandler: (cb: () => void) => { if (this.clickOutsideHandler) { this.adapter.unregisterClickOutsideHandler(); } ...

showClear 请空后,焦点仍然在 Select 内,只有点击外部才会失去焦点,所以这个不是 bug

@YannLynn YannLynn closed this May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants