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

input type=range onChange should fire when changing the value using the keyboard arrow keys #554

Closed
subtleGradient opened this Issue Nov 17, 2013 · 33 comments

Comments

Projects
None yet
@subtleGradient
Contributor

subtleGradient commented Nov 17, 2013

No description provided.

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Nov 17, 2013

Member

Chrome doesn't fire the input event when a range is modified using arrow keys, but Firefox does.

Member

sophiebits commented Nov 17, 2013

Chrome doesn't fire the input event when a range is modified using arrow keys, but Firefox does.

@jessep

This comment has been minimized.

Show comment
Hide comment
@jessep

jessep Jan 15, 2014

Chrome fires a 'change' event when you change it with keyboard. Possible to use 'change' instead of input?

jessep commented Jan 15, 2014

Chrome fires a 'change' event when you change it with keyboard. Possible to use 'change' instead of input?

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Mar 11, 2014

Member

In Chrome beta and Firefox, 'change' doesn't fire when dragging until mouseup so we should listen to input here too.

Member

sophiebits commented Mar 11, 2014

In Chrome beta and Firefox, 'change' doesn't fire when dragging until mouseup so we should listen to input here too.

@eddhannay

This comment has been minimized.

Show comment
Hide comment
@eddhannay

eddhannay May 3, 2014

I made a JSFiddle to test this: http://jsfiddle.net/vqnMU/embedded/result/

React seems to be listening for input, and triggering input & change whenever it occurs.

Focusing the slider and pressing right:

Browser Event React Event
Chrome 34.0.1847.131** Change -
Safari 7.0.3 Change -
Chrome Canary Input, Change Input, Change
Firefox 29* Input Input, Change
IE 10 Change -

* Firefox triggers change on blur

Additionally for dragging:

Browser Event React Event
Chrome 34.0.1847.131 Input* Input, Change
Safari 7.0.3 Input, Change Input, Change
Chrome Canary Input* Input, Change
Firefox 29 Input* Input, Change
IE 10 Change -

* These browsers all trigger change on drag end

Editing ChangeEventPlugin to listen for change events on all input types (35e7aa4) appears to fix React's change & input event handling for all of the above browsers. Should I make a pull request, or is there a reason that the plugin was only listening for change events on file inputs that I'm missing?

eddhannay commented May 3, 2014

I made a JSFiddle to test this: http://jsfiddle.net/vqnMU/embedded/result/

React seems to be listening for input, and triggering input & change whenever it occurs.

Focusing the slider and pressing right:

Browser Event React Event
Chrome 34.0.1847.131** Change -
Safari 7.0.3 Change -
Chrome Canary Input, Change Input, Change
Firefox 29* Input Input, Change
IE 10 Change -

* Firefox triggers change on blur

Additionally for dragging:

Browser Event React Event
Chrome 34.0.1847.131 Input* Input, Change
Safari 7.0.3 Input, Change Input, Change
Chrome Canary Input* Input, Change
Firefox 29 Input* Input, Change
IE 10 Change -

* These browsers all trigger change on drag end

Editing ChangeEventPlugin to listen for change events on all input types (35e7aa4) appears to fix React's change & input event handling for all of the above browsers. Should I make a pull request, or is there a reason that the plugin was only listening for change events on file inputs that I'm missing?

@locks

This comment has been minimized.

Show comment
Hide comment
@locks

locks May 18, 2014

I just ran into this problem when trying to port a widget I have to React.
Can someone in the core give some feedback on whether @eddhannay's suggestion would/wouldn't work? Thanks :)

locks commented May 18, 2014

I just ran into this problem when trying to port a widget I have to React.
Can someone in the core give some feedback on whether @eddhannay's suggestion would/wouldn't work? Thanks :)

@syranide

This comment has been minimized.

Show comment
Hide comment
@syranide

syranide May 18, 2014

Contributor

@locks I would recommend implementing your own, it seems like all the HTML input elements come with a bunch of inconsistencies that can be hard to normalize and are basically unstyle:able.

Contributor

syranide commented May 18, 2014

@locks I would recommend implementing your own, it seems like all the HTML input elements come with a bunch of inconsistencies that can be hard to normalize and are basically unstyle:able.

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits May 19, 2014

Member

@syranide I think we should attempt to support all the built-in inputs at least mostly-reasonably.

Member

sophiebits commented May 19, 2014

@syranide I think we should attempt to support all the built-in inputs at least mostly-reasonably.

@syranide

This comment has been minimized.

Show comment
Hide comment
@syranide

syranide May 19, 2014

Contributor

@spicyj Certainly! But there are lots of weird things that we can't fix... like not being able to read the value of type="number" if it isn't strictly numeric, etc.

Contributor

syranide commented May 19, 2014

@spicyj Certainly! But there are lots of weird things that we can't fix... like not being able to read the value of type="number" if it isn't strictly numeric, etc.

@dmmalam

This comment has been minimized.

Show comment
Hide comment
@dmmalam

dmmalam Jan 29, 2015

+1

Any workaround for IE10+ to get react to fire a synth onChange, on a browser onchange for range inputs

dmmalam commented Jan 29, 2015

+1

Any workaround for IE10+ to get react to fire a synth onChange, on a browser onchange for range inputs

@chooven

This comment has been minimized.

Show comment
Hide comment
@chooven

chooven Feb 11, 2015

+1 for input on IE10+ firing onChange

chooven commented Feb 11, 2015

+1 for input on IE10+ firing onChange

@kwhitaker

This comment has been minimized.

Show comment
Hide comment
@kwhitaker

kwhitaker Feb 16, 2015

+1 to all the +1

kwhitaker commented Feb 16, 2015

+1 to all the +1

@luisrudge

This comment has been minimized.

Show comment
Hide comment
@luisrudge

luisrudge Jun 17, 2015

any news? range input is still broken in IE 11.
Works great on Edge, though.

luisrudge commented Jun 17, 2015

any news? range input is still broken in IE 11.
Works great on Edge, though.

@rippo

This comment has been minimized.

Show comment
Hide comment
@rippo

rippo Jun 25, 2015

+1 for this to be applied. Doesn't work for me even on Edge

rippo commented Jun 25, 2015

+1 for this to be applied. Doesn't work for me even on Edge

@Genert

This comment has been minimized.

Show comment
Hide comment
@Genert

Genert Aug 27, 2015

Does not work in IE 11.

Genert commented Aug 27, 2015

Does not work in IE 11.

@JeanCarriere

This comment has been minimized.

Show comment
Hide comment
@JeanCarriere

JeanCarriere commented Sep 4, 2015

+1

@bsudekum

This comment has been minimized.

Show comment
Hide comment
@bsudekum

bsudekum Sep 15, 2015

For what it's worth, I created a work around component while this is worked out in core: https://github.com/mapbox/react-range

bsudekum commented Sep 15, 2015

For what it's worth, I created a work around component while this is worked out in core: https://github.com/mapbox/react-range

@dancoates

This comment has been minimized.

Show comment
Hide comment
@dancoates

dancoates Oct 8, 2015

+1, Love the work that everyone is doing on react, but I do feel like this bug should be a higher priority to fix seeing as we are approaching the 2 year mark from its discovery.

dancoates commented Oct 8, 2015

+1, Love the work that everyone is doing on react, but I do feel like this bug should be a higher priority to fix seeing as we are approaching the 2 year mark from its discovery.

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Oct 8, 2015

Member

@dancoates I'm sorry that this is a lower priority for us than for you. Feel free to send a pull request though! ChangeEventPlugin is a little hairy but my blog post at http://benalpert.com/2013/06/18/a-near-perfect-oninput-shim-for-ie-8-and-9.html should give you an idea of the general strategy. For range elements, we should listen to both input and change events and fire events when either happens (but only once even if both happen).

Member

sophiebits commented Oct 8, 2015

@dancoates I'm sorry that this is a lower priority for us than for you. Feel free to send a pull request though! ChangeEventPlugin is a little hairy but my blog post at http://benalpert.com/2013/06/18/a-near-perfect-oninput-shim-for-ie-8-and-9.html should give you an idea of the general strategy. For range elements, we should listen to both input and change events and fire events when either happens (but only once even if both happen).

@dancoates

This comment has been minimized.

Show comment
Hide comment
@dancoates

dancoates Oct 8, 2015

No need to be sorry, I realise it is a tricky one. I'll take a look at ChangeEventPlugin and see if I can help.
Perhaps a caveats section in the docs would help folks wondering why changes aren't firing? Though I guess this thread is easy enough to find.

dancoates commented Oct 8, 2015

No need to be sorry, I realise it is a tricky one. I'll take a look at ChangeEventPlugin and see if I can help.
Perhaps a caveats section in the docs would help folks wondering why changes aren't firing? Though I guess this thread is easy enough to find.

@raphaelparent

This comment has been minimized.

Show comment
Hide comment
@raphaelparent

raphaelparent Feb 24, 2016

👍 would love to see this fixed!

How I dealt with the problem for now
For my use, I simply added the same event on a onMouseUp event and it works for all IE 10 and above and Edge.

It doesn't give the exact same result because the value is only changed on the mouseup event but I think (for my case anyway) that it degrades beautifully.

raphaelparent commented Feb 24, 2016

👍 would love to see this fixed!

How I dealt with the problem for now
For my use, I simply added the same event on a onMouseUp event and it works for all IE 10 and above and Edge.

It doesn't give the exact same result because the value is only changed on the mouseup event but I think (for my case anyway) that it degrades beautifully.

@zanarmstrong

This comment has been minimized.

Show comment
Hide comment
@zanarmstrong

zanarmstrong Mar 3, 2016

I also ran into this & would love to see it fixed.

Thanks for the suggestions for workarounds. Adding an "onClick" or "onMouseup" event was very helpful, but it's still suboptimal that nothing happens until the user let's go of the slider.

zanarmstrong commented Mar 3, 2016

I also ran into this & would love to see it fixed.

Thanks for the suggestions for workarounds. Adding an "onClick" or "onMouseup" event was very helpful, but it's still suboptimal that nothing happens until the user let's go of the slider.

@kevinfrei

This comment has been minimized.

Show comment
Hide comment
@kevinfrei

kevinfrei Apr 11, 2016

Check out this on StackOverflow which explains a way to address getting updates while the user is dragging...

kevinfrei commented Apr 11, 2016

Check out this on StackOverflow which explains a way to address getting updates while the user is dragging...

@jimfb jimfb closed this in #5746 Apr 15, 2016

jimfb added a commit that referenced this issue Apr 15, 2016

Only fire input value change events when the value changes (#5746)
* Only fire input value change events when the value changes

fixes #554, fixes #1471, fixes #2185 (still trying to figure out why)

* catch programmatic value changes

* move value tracking to seperate module

@zpao zpao added the Component: DOM label Aug 3, 2016

ajreed79 pushed a commit to ajreed79/react that referenced this issue Aug 29, 2016

Only fire input value change events when the value changes (#5746)
* Only fire input value change events when the value changes

fixes #554, fixes #1471, fixes #2185 (still trying to figure out why)

* catch programmatic value changes

* move value tracking to seperate module
@AlexReff

This comment has been minimized.

Show comment
Hide comment
@AlexReff

AlexReff Oct 21, 2016

Why is this closed? This is still a problem - onChange does not fire at all in IE11 in our testing. Forced to use https://github.com/mapbox/react-range

AlexReff commented Oct 21, 2016

Why is this closed? This is still a problem - onChange does not fire at all in IE11 in our testing. Forced to use https://github.com/mapbox/react-range

@jvanderberg

This comment has been minimized.

Show comment
Hide comment
@jvanderberg

jvanderberg Jan 10, 2017

We are using 15.3.2, and are experiencing this issue. Looking at the release notes for the three minor releases since then, I don't see that this has been fixed. Why is it closed? Issue #8168 seems to be the tracking case for it, but it seems like this issue has a much more thorough description of the problem.

jvanderberg commented Jan 10, 2017

We are using 15.3.2, and are experiencing this issue. Looking at the release notes for the three minor releases since then, I don't see that this has been fixed. Why is it closed? Issue #8168 seems to be the tracking case for it, but it seems like this issue has a much more thorough description of the problem.

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jan 10, 2017

Member

If you look at the history above, you will see that the issue was closed by #5746.
If you open this PR, you will see that its milestone is set to 16.

So the fix will be in 16, and we can’t put it in 15.x because it introduces a breaking change in behavior.

I don’t know if it’s possible to backport it to 15.x. @jquense and @nhunzaker would likely be able to answer this.

Member

gaearon commented Jan 10, 2017

If you look at the history above, you will see that the issue was closed by #5746.
If you open this PR, you will see that its milestone is set to 16.

So the fix will be in 16, and we can’t put it in 15.x because it introduces a breaking change in behavior.

I don’t know if it’s possible to backport it to 15.x. @jquense and @nhunzaker would likely be able to answer this.

@jquense

This comment has been minimized.

Show comment
Hide comment
@jquense

jquense Jan 10, 2017

Collaborator

It is possible! There is even a PR :) already #8575

Collaborator

jquense commented Jan 10, 2017

It is possible! There is even a PR :) already #8575

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Jan 10, 2017

Member

Oh nice. Can you make a list of DOM PRs that are hanging in there awaiting review?
Then add them to umbrella in #8583 so that the relevant cases have manual tests.

Member

gaearon commented Jan 10, 2017

Oh nice. Can you make a list of DOM PRs that are hanging in there awaiting review?
Then add them to umbrella in #8583 so that the relevant cases have manual tests.

@aweary

This comment has been minimized.

Show comment
Hide comment
@aweary

aweary Jan 10, 2017

Member

#8575 is on my todo list. Just need to test some stuff locally and then I think we'll be 👍 to merge. I'll make sure to add some relevant cases to #8583 when I get a chance to review it

Member

aweary commented Jan 10, 2017

#8575 is on my todo list. Just need to test some stuff locally and then I think we'll be 👍 to merge. I'll make sure to add some relevant cases to #8583 when I get a chance to review it

@nhunzaker

This comment has been minimized.

Show comment
Hide comment
@nhunzaker

nhunzaker Jan 21, 2017

Collaborator

Anyone working on a browser test fixture for this? If not, I've got some time to write one up.

Collaborator

nhunzaker commented Jan 21, 2017

Anyone working on a browser test fixture for this? If not, I've got some time to write one up.

@jquense

This comment has been minimized.

Show comment
Hide comment
@jquense

jquense Jan 21, 2017

Collaborator

I was planning on it but haven had a moment yet. feel free to jump in

Collaborator

jquense commented Jan 21, 2017

I was planning on it but haven had a moment yet. feel free to jump in

@nhunzaker

This comment has been minimized.

Show comment
Hide comment
@nhunzaker

nhunzaker Jan 23, 2017

Collaborator

No worries. There's plenty of other test cases to write up. I've got a PR out related to #8308. I'll shift gears to that.

Collaborator

nhunzaker commented Jan 23, 2017

No worries. There's plenty of other test cases to write up. I've got a PR out related to #8308. I'll shift gears to that.

@c0debreaker

This comment has been minimized.

Show comment
Hide comment
@c0debreaker

c0debreaker May 27, 2017

I encountered this issue today on my Windows phone. It does not call the method I specified in onChange. When is the fix going to be applied?

c0debreaker commented May 27, 2017

I encountered this issue today on my Windows phone. It does not call the method I specified in onChange. When is the fix going to be applied?

@gaearon

This comment has been minimized.

Show comment
Hide comment
@gaearon

gaearon Nov 24, 2017

Member

This has been released in 15.6.x.

Member

gaearon commented Nov 24, 2017

This has been released in 15.6.x.

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