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

Radio buttons are not correctly checked when using multiple lists of radio buttons #7630

Closed
atimmer opened this issue Sep 1, 2016 · 8 comments · Fixed by #11227
Closed

Comments

@atimmer
Copy link

atimmer commented Sep 1, 2016

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

When there are two lists of radio buttons and they are conditionally shown the correct radio button isn't always checked. In the provided JSFiddle I have created two lists and put a button before them to toggle between the two lists. You can see when switching to the second list of radio buttons none of the radio buttons is checked even though the second one should be checked based on the state.

What is the expected behavior?

The correct radio button should be checked. The following JSFiddle shows the behaviour with React 15.3.0 where it still worked: https://jsfiddle.net/atimmer/Lzjs56sn/1/.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

✅ React 15.3.0: It works.
❌ React 15.3.1: It is broken.

@aweary
Copy link
Contributor

aweary commented Sep 1, 2016

Thanks for the report @atimmer, that does seem to be a regression somewhere. I'll take a look at it.

@atimmer
Copy link
Author

atimmer commented Sep 1, 2016

I had done some debugging prior to reporting this and it seems to be caused by the way the radios are updated. The radios in React 15.3.1 are updated in the DOM by first updating the attributes (name, id, etc) and then updating whether they are checked or not. So when going from a list where the third radio is checked to a list where the second one is checked the following happens:

  1. The first radio is updated, nothing of note happens.
  2. The second radio is updated, now the second and the third radio are both checked, but they have different names.
  3. The third radio is updated, in updating the attributes the browser detects that there is already a radio with the same name and unchecks that one, the second radio button. Then the third radio is unchecked because the React state has the radio as unchecked. Now no radios are checked.
  4. The fourth radio is updated, nothing of note happens.

@syranide
Copy link
Contributor

syranide commented Sep 1, 2016

Keying solves the problem, this is probably a problem with the input not realizing its name has changed/handling it correctly?

@aweary
Copy link
Contributor

aweary commented Sep 1, 2016

@atimmer yupp, that's about what I'm seeing as well. I verified this occurs on master too. Keying is probably your best bet as a workaround for now. I'm going to see what exactly caused this regression from 15.3.0 to 15.3.1.

@shuai-z
Copy link

shuai-z commented Oct 10, 2016

#7333 I guess? by moving ReactDOMInput.updateWrapper(this); (only set checked here) after this._updateDOMProperties(lastProps, nextProps, transaction); (update id, name, checked in order).

so another workaround is to put checked before name property, don't trust me 😄

maartenth added a commit to TND/react-jsonschema-form that referenced this issue Nov 25, 2016
n1k0 pushed a commit to rjsf-team/react-jsonschema-form that referenced this issue Nov 25, 2016
n1k0 added a commit to rjsf-team/react-jsonschema-form that referenced this issue Nov 25, 2016
* Update dependencies to their latest versions. (#386)
* Fix #385: Avoid dynamic requires. (#387)
* Fix #365: Tab stop for Add button + (#392)
* Add a single field form example in the playground (#390)
* Allow using field names containing a dot character (#397)
* Updated eslint config.
* Improve checkbox and radio button styles (#403)
* Add missing proptype for disabled (#416)
* Temporary fix for #349 and facebook/react#7630 radio widget bug (#423)
@landvibe
Copy link
Contributor

landvibe commented Sep 8, 2017

@aweary
I'm using shuai-z`s solution. It works well.
I wonder if React v16 has the same issue or not.

@gaearon
Copy link
Collaborator

gaearon commented Oct 4, 2017

This is still an issue in React 16.

landvibe pushed a commit to landvibe/react that referenced this issue Oct 14, 2017
nhunzaker pushed a commit that referenced this issue Nov 19, 2017
Fixes a case where changing the name and checked value of a radio button in the same update would lead to checking the wrong radio input. Also adds a DOM test fixture for related issue.

Related issues:
#7630
@nhunzaker
Copy link
Contributor

Fixed by #11227

Ethan-Arrowood pushed a commit to Ethan-Arrowood/react that referenced this issue Dec 8, 2017
Fixes a case where changing the name and checked value of a radio button in the same update would lead to checking the wrong radio input. Also adds a DOM test fixture for related issue.

Related issues:
facebook#7630
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants
@atimmer @nhunzaker @gaearon @syranide @landvibe @shuai-z @aweary and others