-
Notifications
You must be signed in to change notification settings - Fork 48.5k
Make Simulate.mouseEnter/Leave use direct dispatch #1366
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
Conversation
Did you see how enter/leave actually has its own dispatch path? A leave/enter from |
Yeah, I saw. Enter/leave are now in the spec and I think they work the same way you did? I didn't have great ideas for a simulation API for that though -- see #1297. I figured direct dispatch is simple and is 99% of the time what you need in a test. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Yay or nay? |
@spicyj Do you recall our conversation about this and what my thoughts were? I seem to recall saying that the test cases should operate exactly like the actual code that runs in applications. |
Any update on this? This is a pretty big issue for me since I do commonly use mouseenter/mouseleave and right now I can write any unit tests for those pieces of code. |
@@ -314,17 +314,24 @@ function makeSimulator(eventType) { | |||
node = domComponentOrNode; | |||
} | |||
|
|||
var dispatchConfig = ReactEventEmitter.eventNameDispatchConfigs[eventType]; |
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.
It looks like the latest code (0.12.1) uses ReactBrowserEventEmitter
instead of ReactEventEmitter
Any reason why this is not being merged? |
Another month has gone by, any update on this (this would help greatly with my unit tests)? |
Fixes facebook#1297. onMouseEnter and onMouseLeave shouldn't *actually* use direct dispatch, but doing so is more useful than doing nothing (and I don't think it precludes adding proper enter/leave dispatching later, either). Test Plan: grunt test
2c0dcfc
to
6b03975
Compare
@spicyj Ok, its been about 2 more months so I figure I would give another nudge. Personally I think it would be better to have something that works than something that does not work. I would rather be able to test now and then have to update my tests if the way to test this event changes then not being able to test this event at all. |
Make Simulate.mouseEnter/Leave use direct dispatch
@ryanzec Thanks for the nudge. :) |
Will this be a 0.14 fix? |
Yes. |
Thanks! :] |
Fixes #1297.
onMouseEnter and onMouseLeave shouldn't actually use direct dispatch, but doing so is more useful than doing nothing (and I don't think it precludes adding proper enter/leave dispatching later, either).
Test Plan: grunt test