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

Controlled Radio Button Component Does not Update #4930

Closed
benglass opened this Issue Sep 22, 2015 · 8 comments

Comments

Projects
None yet
4 participants
@benglass

We are running across an issue in one of our components used for showing a group of radio buttons. The radio button elements are controlled (using checked and not defaultChecked) and they use a callback prop to change the state in a parent component which determines which one is selected.

When you first click a radio button, it works.

The second and any subsequent clicks do not update the radio button to show as checked.

Here is our example code that exhibits the issue.

http://jsbin.com/royolo/edit?js,console,output

I thought it might be related to the key of each element but we are using the radio button value as key on the element which is unique.

Any help is appreciated.

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Sep 22, 2015

Member

I dug into this for like half an hour and couldn't figure it out. Very weird.

cc @zpao @syranide

Member

sophiebits commented Sep 22, 2015

I dug into this for like half an hour and couldn't figure it out. Very weird.

cc @zpao @syranide

@syranide

This comment has been minimized.

Show comment
Hide comment
@syranide

syranide Sep 22, 2015

Contributor

Uhm... I'm getting errors in the console when clicking which could explain a lot of it... are you not seeing those? Since it happens during render React then starts throwing internal after that point which should explain the rest.

royolo.js:23 Uncaught TypeError: Cannot read property 'apply' of undefined
Contributor

syranide commented Sep 22, 2015

Uhm... I'm getting errors in the console when clicking which could explain a lot of it... are you not seeing those? Since it happens during render React then starts throwing internal after that point which should explain the rest.

royolo.js:23 Uncaught TypeError: Cannot read property 'apply' of undefined
@benglass

This comment has been minimized.

Show comment
Hide comment
@benglass

benglass Sep 22, 2015

@syranide sorry I was trying to get the jsbin working and apparently it saved an incomplete change I had made.

It is reverted to the original case where it doesnt work and there are no js errors.

@syranide sorry I was trying to get the jsbin working and apparently it saved an incomplete change I had made.

It is reverted to the original case where it doesnt work and there are no js errors.

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Sep 22, 2015

Member

e.preventDefault() in your change handler is doing this. We have to actually hijack the click event to make onChange work for radios and checkboxes. The downside of this has come up a few times - that preventDefault in there actually prevents the browser from updating its state. See some prior discussions in #3005.

You don't actually need to call preventDefault here, and we've actually seen a lot of people blindly calling it without actually knowing why. I would recommend doing a spot check of your own handlers and seeing if they're actually needed throughout.

Member

zpao commented Sep 22, 2015

e.preventDefault() in your change handler is doing this. We have to actually hijack the click event to make onChange work for radios and checkboxes. The downside of this has come up a few times - that preventDefault in there actually prevents the browser from updating its state. See some prior discussions in #3005.

You don't actually need to call preventDefault here, and we've actually seen a lot of people blindly calling it without actually knowing why. I would recommend doing a spot check of your own handlers and seeing if they're actually needed throughout.

@zpao zpao closed this Sep 22, 2015

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Sep 22, 2015

Member

@zpao Thanks.

Member

sophiebits commented Sep 22, 2015

@zpao Thanks.

@benglass

This comment has been minimized.

Show comment
Hide comment
@benglass

benglass Sep 22, 2015

@zpao Thanks so much for the explanation.

We are definitely calling this blindly in this case and its certainly not necessary but is there somewhere in the react docs where this could perhaps be added as a caveat? Perhaps in the TIPs section as "event.preventDefault breaks checkboxes/radio buttons" or in the controlled input section here https://facebook.github.io/react/docs/forms.html#uncontrolled-components

I'll submit a PR if you think its worth adding but my opinion would be that its so hard to debug if you run into this issue that it would be worth pointing out in the official docs.

@zpao Thanks so much for the explanation.

We are definitely calling this blindly in this case and its certainly not necessary but is there somewhere in the react docs where this could perhaps be added as a caveat? Perhaps in the TIPs section as "event.preventDefault breaks checkboxes/radio buttons" or in the controlled input section here https://facebook.github.io/react/docs/forms.html#uncontrolled-components

I'll submit a PR if you think its worth adding but my opinion would be that its so hard to debug if you run into this issue that it would be worth pointing out in the official docs.

@benglass

This comment has been minimized.

Show comment
Hide comment
@benglass

benglass Sep 22, 2015

I can see someone did add some docs here but I'm not sure if a PR was ever created

WickyNilliams@5bf8cda

I can see someone did add some docs here but I'm not sure if a PR was ever created

WickyNilliams@5bf8cda

@benglass

This comment has been minimized.

Show comment
Hide comment
@benglass

benglass Sep 22, 2015

Here is the PR #3069

Here is the PR #3069

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