Skip to content
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

Add range input to ChangeEventPlugin - Fixes #554 #1594

Closed
wants to merge 1 commit into from
Closed

Add range input to ChangeEventPlugin - Fixes #554 #1594

wants to merge 1 commit into from

Conversation

eddhannay
Copy link

  • Added type check for range inputs in to ChangeEventPlugin
  • Added tests to check that native change events trigger synthetic events on range inputs but
    not text inputs

This replaces #1561, which enabled ChangeEventPlugin for all inputs.

 - Added type check for range inputs in to ChangeEventPlugin
 - Added tests to check that native change events trigger synthetic events on range inputs but
   not text inputs

This replaces #1561, which enabled ChangeEventPlugin for all inputs.
@sophiebits
Copy link
Collaborator

For range inputs, we need to listen to both the input event and change event because as your research in #554 showed, not all browsers fire change. (We should not fire two events for a single change though even if we get both native events.)

@eddhannay
Copy link
Author

Input events will still be tracked correctly, the difference is that React will no longer fire a synthetic input and synthetic change whenever a native input is triggered. All browsers will eventually trigger change, the difference is that IE10 will trigger it whenever the value changes, where the others will trigger it when the range loses focus.

I have made a JSFiddle here: http://jsfiddle.net/vqnMU/, and uploaded a copy of React with the patch here: https://edd.fm/test/react/react.min-fix.js, which you can switch to in the external resources panel. The result across browsers will be:

Browser event React 0.10.0 This patch
Input Input, Change Input
Change - Change

There isn't a time at which we'll receive both native events simultaneously - at least I haven't seen a browser that fires both change and input for the exact same interaction, although they do both occur close together on most browsers when a value is select with the mouse. Is this somewhere that React's events should differ from the native events?

@sophiebits
Copy link
Collaborator

This patch isn't quite what we want – we'd instead like onChange to fire whenever the value changes (which probably means listening to both native events). If you want to take this on feel free to open a new PR – it'll probably take a bit of refactoring to ChangeEventPlugin.

@sophiebits sophiebits closed this Jun 13, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants