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(module:nz-select): fix nz-select blur event will not be emitted #4458

Closed

Conversation

msyesyan
Copy link
Contributor

@msyesyan msyesyan commented Nov 21, 2019

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #4452

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@msyesyan
Copy link
Contributor Author

msyesyan commented Nov 21, 2019

请看附件,focus 和 blur 事件本身是不能冒泡的,所以靠 nzSelectTopControlDOM.blur()来触发 nzBlur应该是不可行的。既然这样,也没必要在html模板上去handle这两个事件。这个问题在 8.2时还不是这么写的代码,现在我们升级之后,影响了项目的部分功能

Screen Shot 2019-11-21 at 6 39 51 PM
Screen Shot 2019-11-21 at 6 40 13 PM

@msyesyan msyesyan force-pushed the chenqian-fix-nz-select-blur-event branch from 1fc6f38 to 69b29d3 Compare November 21, 2019 10:45
@netlify
Copy link

netlify bot commented Nov 21, 2019

Deploy preview for ng-zorro-master ready!

Built with commit 1fc6f38

https://deploy-preview-4458--ng-zorro-master.netlify.com

@codecov
Copy link

codecov bot commented Nov 21, 2019

Codecov Report

Merging #4458 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4458      +/-   ##
==========================================
+ Coverage   92.23%   92.23%   +<.01%     
==========================================
  Files         522      522              
  Lines       11107    11109       +2     
  Branches     2018     2018              
==========================================
+ Hits        10244    10246       +2     
  Misses        436      436              
  Partials      427      427
Impacted Files Coverage Δ
components/select/nz-select.component.ts 93.18% <100%> (+0.1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3aa68d3...69b29d3. Read the comment docs.

@vthinkxie
Copy link
Member

这个 pr 与 #3708 需求冲突,估计要重新讨论这部分的实现,建议先不要依赖 nzBlur,改用 fromEvent 操作

@msyesyan
Copy link
Contributor Author

msyesyan commented Nov 21, 2019

@vthinkxie 其实不看需求,但看我添加的测试的话,以前的代码是不能过的,也就是说原来只是靠让一些代码失效来解决一些问题,感觉也不太合理呀。^_^

@vthinkxie
Copy link
Member

核心问题在于对nzBlur的理解,之前很多issue的提出者都觉得nzBlur应该针对的是当前组件的外壳,而不是里面的input,你认为nzBlur应该针对的是里面的input,而不是外壳,这就是冲突的地方,上一次改动是按照了之前issue提出者的希望进行了修改。

@msyesyan
Copy link
Contributor Author

@vthinkxie 谢谢,已经用 fromEvent解决了。其实你可以看我 issue的链接,https://ng-zorro-antd-start-jkfcwi.stackblitz.io 这个例子很明显能看出现在的行为不合理。当我点击这个选择框,它明明是打开了,而且焦点也一直focus,却触发了 blur函数,这应该不是壳子还是input的理解的问题了

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

Successfully merging this pull request may close these issues.

None yet

4 participants