-
Notifications
You must be signed in to change notification settings - Fork 534
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(user-interaction): event listeners have wrong this when listening for bubbled events #643
Conversation
… for bubbled events
Codecov Report
@@ Coverage Diff @@
## main #643 +/- ##
=======================================
Coverage 96.68% 96.68%
=======================================
Files 13 13
Lines 634 634
Branches 124 124
=======================================
Hits 613 613
Misses 21 21 |
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.
🤭 ... I should have made the tests better when I reworked the event listener instrumentation.
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.
lgtm
I was going to merge this but then saw you're still pushing changes. Is this good to merge? |
Yep |
Which problem is this PR solving?
this
inside event listeners gets changed toevent.target
, which is incorrect when listening to bubbled events (click on a child element)see: https://codesandbox.io/s/inspiring-bartik-6bc3y?file=/src/index.js
Short description of the changes
Use
this
the event listener is actually called with