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: trigger onBlur does not require reset focused #980

Merged
merged 5 commits into from
Sep 14, 2023

Conversation

Yuiai01
Copy link
Contributor

@Yuiai01 Yuiai01 commented Sep 13, 2023

close ant-design/ant-design#44619

问题出现原因:在 Select 触发 onBlur 后其 disabled 为 true 时触发了重置 focused 的操作,将 onBlur 要执行的回调给清除掉了。

@vercel
Copy link

vercel bot commented Sep 13, 2023

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

Name Status Preview Comments Updated (UTC)
select ❌ Failed (Inspect) Sep 14, 2023 5:39am

@MadCcc
Copy link
Member

MadCcc commented Sep 13, 2023

来个测试用例

@Yuiai01
Copy link
Contributor Author

Yuiai01 commented Sep 13, 2023

来个测试用例

好了~

@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #980 (8df4ae7) into master (28a3dab) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head 8df4ae7 differs from pull request most recent head 4e8848f. Consider uploading reports for the commit 4e8848f to get more accurate results

@@           Coverage Diff           @@
##           master     #980   +/-   ##
=======================================
  Coverage   99.78%   99.78%           
=======================================
  Files          38       38           
  Lines        1370     1373    +3     
  Branches      400      400           
=======================================
+ Hits         1367     1370    +3     
  Misses          3        3           
Files Changed Coverage Δ
src/BaseSelect.tsx 100.00% <100.00%> (ø)

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

fireEvent.focus(container.querySelector('input'));
jest.runAllTimers();

expect(container.querySelector('.rc-select-focused')).toBeTruthy();
Copy link
Member

Choose a reason for hiding this comment

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

这个用例感觉不对,上一步 focus input 之后这里必然是 truthy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

最开始的 focus 有触发 onBlur 取消掉,后续重新 focus input 到这里应当是 truthy。这个问题是由于 disabled 重置了 focused 导致无法再次触发 focus,所以这里逻辑应该没问题。

Copy link
Member

@MadCcc MadCcc Sep 14, 2023

Choose a reason for hiding this comment

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

感觉上应该是判断 onFocus 事件有没有触发?focus 状态是有的,但是原问题是内部的 focus 状态错误地保留了导致没有触发 foucs 事件。

tests/focus.test.tsx Fixed Show fixed Hide fixed
tests/focus.test.tsx Outdated Show resolved Hide resolved
tests/focus.test.tsx Outdated Show resolved Hide resolved
@MadCcc MadCcc merged commit b5fccbd into react-component:master Sep 14, 2023
8 of 9 checks passed
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.

onFocus not triggered when switching "disabled" property
2 participants