Map lowercase mouse events in #simulate #77
Conversation
| @@ -294,6 +295,7 @@ export default class ReactWrapper { | |||
| * @returns {ReactWrapper} | |||
| */ | |||
| simulate(event, ...args) { | |||
| event = mapNativeMouseEvent(event); | |||
There was a problem hiding this comment.
this approach seems both brittle and only will work on mouse events. There's lots of other kinds of events, like "dragstart", "touchstart", "keypress", etc that this could impact.
What about an explicit whitelist - just an object mapping lowercase names to react-cased ones?
There was a problem hiding this comment.
That thought had crossed my mind - happy to do that instead 👍
|
+1 I've been bit by this. Generates this not-so-helpful error: |
|
@ljharb just updated it - let me know what you think! |
|
This looks great as far as covering lowercase names - it would be nice to also have a nicer, more explicit error message when the event name wasn't valid (#79) - but that doesn't have to happen as part of this PR. The tests appear to be failing in most (but not all) of the React 0.13 builds, though. |
|
@ljharb happy to tackle #79 in a follow up PR 👍 I found this from the React 0.14 release notes:
Also see: facebook/react#1297 So it seemed that I could also in this or a follow up PR add an error message that's shown in the user is React 0.13 and they try to use Let me know what you think is best! |
|
@jackfranklin Could you conditionally have |
|
@ljharb 👍 will make that change now |
| const wrapper = mount(<Foo />); | ||
|
|
||
| wrapper.simulate('mouseenter'); | ||
| expect(spy.calledOnce).to.be.true; |
There was a problem hiding this comment.
You'll need to pull from master and changes these assertions to be to.equal(true). A change was merged that causes a linting error with these types of assertions!
|
@ljharb I've just tidied this up with a rebase and everything looks fine locally - I'm not entirely sure why the last Travis rebuild said test coverage had dropped so significantly mind. I think if I can fix that ( might need a little pointer in the right direction ) I hope this is now finally in a state where it could be merged. |
|
Also I'll squish the commits down when you're happy 👍 |
|
Awesome - I wouldn't worry too much about the coverage, it's still a bit flaky for some reason. This LGTM (pending a rebase) but I'll let @lelandrichardson look it over first. |
|
👍 this looks great. if you rebase, i'll merge this in and push out a release this afternoon. |
|
@jackfranklin can you rebase + squash so I can get this in? thanks! |
React camelcases a bunch of events that JS has in all lower case. This PR adds a map of them so people can call `simulate` with either the React version or the native event version. Fixes #29.
|
@lelandrichardson done! |
[new] Map lowercase mouse events in #simulate
React camelcases a bunch of events that JS has in all lower case. This
PR adds a map of them so people can call
simulatewith either theReact version or the native event version.
Fixes #29.
Thought I'd take a stab at the fix - let me know what you think!