-
Notifications
You must be signed in to change notification settings - Fork 45.7k
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
Modern Event System: fix EnterLeave plugin logic #18830
Conversation
// We use ensureListeningTo because the container might | ||
// be a COMMENT_NODE and this function handles events | ||
// on comments for us. | ||
ensureListeningTo(container, 'onMouseEnter'); |
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.
This is similar to what we did with the fix in #18720.
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 2be41ca:
|
Details of bundled changes.Comparing: 7992ca1...2be41ca react-dom
ReactDOM: size: 0.0%, gzip: -0.0% Size changes (experimental) |
Details of bundled changes.Comparing: 7992ca1...2be41ca react-dom
Size changes (stable) |
584652f
to
751f4f5
Compare
Add regression tests Fix root code Fix logic in DOMComponent Fix unused var
// If we are not processing the first ancestor, then we | ||
// should not process the same nativeEvent again, as we | ||
// will have already processed it in the first ancestor. | ||
const nativeTargetInst = getClosestInstanceFromNode(nativeEventTarget); |
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.
This is basically the same as what we do on:
Addressed feedback
Today we found an internal issue with the modern event system and the EnterLeave plugin. Specifically, we didn't track
mouseout
on the container, only in the portal. That's because the portal had a child containedonMouseEnter
. This bug is actually very similar to that of the recent fix to #18720.Making this change, so that we now register this plugin on each root container fixed that issue, but also brought about another issue. This was great though, as we had already solved this in the legacy EnterLeave plugin (I commented on it below). Taking that logic and bringing it to the modern event system fixed the failing issues and aligned the modern event system with the legacy one.