-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
React events: fix press end event dispatching #15500
React events: fix press end event dispatching #15500
Conversation
Regression tests? |
Details of bundled changes.Comparing: d1f667a...f3aa134 react-events
Generated by 🚫 dangerJS |
packages/react-events/src/Press.js
Outdated
if (pointerType === 'keyboard') { | ||
if (!isValidKeyPress(nativeEvent.key)) { | ||
return; | ||
} | ||
// If the event isn't within the event target, check if we're still within the responder region. |
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.
Shouldn't this be:
If the event target isn't within the press target
?
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.
The implementation makes sense. It would be good to have another test or two before, as I feel like this is likely to break when we refactor Press in the future.
37e9737
to
de53925
Compare
What additional test coverage do you want? |
de53925
to
1f3a8bd
Compare
@necolas I see you added tests. I was probably looking at an older commit. |
This patch fixes an issue related to determining whether the end event occurs within the responder region. Previously we only checked if the event target was within the responder region for moves, otherwise we checked if the target was within the event component. Since the dimensions of the child element can change after activation, we need to recalculate the responder region before deactivation as well if the target is not within the event component.
1f3a8bd
to
f3aa134
Compare
This patch fixes an issue related to determining whether the end event occurs
within the responder region. Previously we only checked if the event target was
within the responder region for moves, otherwise we checked if the target was
within the event component. Since the dimensions of the child element can
change after activation, we need to recalculate the responder region before
deactivation as well if the target is not within the event component.
Ref #15257