-
-
Notifications
You must be signed in to change notification settings - Fork 259
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
Ensure blur
event fires on prior focused element even if click target is not focusable
#1033
Conversation
Fixes #1032 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is generally the correct behavior (hopefully the plain HTML demo will prove that to us), we should consider doing this inside focus itself. I suspect other of our interaction helpers would need this same thing (e.g. doubleClick
, tap
, etc).
1bac7d4
to
5766d0e
Compare
Moved 'isFocusable' check and blur into focus. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall seems great, thanks for working on this!
5766d0e
to
10e4091
Compare
The root cause of the issue is that the blur event is not triggered on the current active element in the __focus__ helper in case if the clicked (double-clicked, tapped) element is not focusable. If the clicked (double-clicked, tapped) element is focusable the previous active element gets blur event trough the __focus__ helper. A call to __blur__ was added to the __focus__ helper for the case when the clicked (double-clicked, tapped) element element is not focusable.
10e4091
to
1e0c6a4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job, thank you!!
blur
event fires on prior focused element even if click target is not focusable
Hey @rwjblue and @helgablazhkun, this is a great improvement, and definitely moves things in the right direction! However, it has turned out to be a pretty significantly breaking change for my test suite -- see #1126 and #1127. I guess it's already released, but maybe we can fast-track those PRs to minimize the time that the latest version of |
The root cause of the issue is that the blur event is not triggered on
the current active element in the click helper in case if the clicked
element is not focusable.
If the clicked element is focusable the previous active element gets
blur event trough the focus helper.
A call to blur was added for the case when the clicked element
is not focusable.