Skip to content

Conversation

@quantizor
Copy link
Contributor

Although it is unreasonable to set every possible property for simulated events, type is useful for shared event handlers that have different behaviors.

Closes #6100

Although it is unreasonable to set every possible property for
simulated events, `type` is useful for event handlers that are shared
between types and potentially have different behaviors.
@gaearon
Copy link
Collaborator

gaearon commented Mar 28, 2016

Thanks for contributing! I’m closing because I think this should already be possible for any property, so there is no need to add a special support for type: #6100 (comment). Please let me know if I’m wrong!

Cheers.

@gaearon gaearon closed this Mar 28, 2016
@quantizor
Copy link
Contributor Author

quantizor commented Mar 29, 2016 via email

@gaearon
Copy link
Collaborator

gaearon commented Mar 29, 2016

Yeah perhaps you’re right. Since we’re setting target anyway, might as well set the type.
I don’t feel strongly either way. I wonder why this hasn’t come up before though.

Should this line have the same addition too for consistency? I don’t know this code well so can’t say.

I’ll defer to @zpao on whether we want this.

@gaearon gaearon reopened this Mar 29, 2016
@quantizor
Copy link
Contributor Author

@gaearon The tests don't break without that other line, so ¯\_(ツ)_/¯

@gaearon
Copy link
Collaborator

gaearon commented Mar 30, 2016

Yeah but you’re not using SimulateNative, are you? I’m not sure who’s using it because it’s undocumented but quite extensive. Maybe Facebook uses it internally; I haven’t checked.

@quantizor
Copy link
Contributor Author

@gaearon Yes that's true, I've only ever used Simulate in my testing.

@quantizor
Copy link
Contributor Author

@gaearon anything else you want me to do here?

@gaearon gaearon merged commit 5a20d44 into facebook:master Jun 26, 2016
@gaearon
Copy link
Collaborator

gaearon commented Jun 26, 2016

Let’s get it in, don’t see any harm in this. 👍

@zpao zpao modified the milestones: 15-next, 15.3.0 Jul 7, 2016
zpao pushed a commit that referenced this pull request Jul 13, 2016
Although it is unreasonable to set every possible property for
simulated events, `type` is useful for event handlers that are shared
between types and potentially have different behaviors.
(cherry picked from commit 5a20d44)
@uxtx
Copy link

uxtx commented Aug 3, 2016

Hi all, I've encountered a few errors in a few external apps (react-select is a public-facing one) due to mutating the event type with the toLowerCase method. Replacing this with toString() appears to resolve the issue - I've created a pr for this. #7419

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants