Fix uncontrolled radios #10186
Fix uncontrolled radios #10186
Conversation
|
Just waiting on CI. |
|
I think we're going to have to update the |
|
If |
|
Let's wait with merging until we figure out why #10156 broke for us. |
|
👍
…On Sun, Jul 16, 2017, 1:30 PM Dan Abramov ***@***.***> wrote:
Let's wait with merging until we figure out why #10156
<#10156> broke for us.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#10186 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAkEOED-GIWEWB8v0IAUnkQP1xMTtYB0ks5sOki_gaJpZM4OYTa0>
.
|
|
fyi this should be fine as is. I don't think we want to backport #10208 and the Stack only v15 won't have the same bug, since it doesn't contain the offending logic branch |
|
Can we fix CI before merging? I would also prefer that we cherry-pick that test I added. |
|
I can cherry-pick that test. For CI I think we need to fixup the branch does someone want to do that? I'd prefer one of my first pushes to not be a force push :P lest the git gods betray me. |
|
I force pushed to 15.6-dev so it should be good now. |
|
Can you please rebase? |
|
|
||
| // We also check that we haven't missed a value update, such as a | ||
| // Radio group shifting the checked value to another named radio input. | ||
| inputValueTracking.updateValueIfChanged(this); |
There was a problem hiding this comment.
So in 15.6.2 this fix is no longer working...broadly when an radio changes it triggers an update in related (same name) radio inputs manually. Originally (unless i'm going crazy) those other updates hit this branch, .e.g radio1 triggered updateComponent and the other radios reset their inputTracking. Now it seems like this branch is missed entirely and ReactDOMInput.updateWrapper is called directly...I could totally be going crazy and this never worked originally but i'm certain we all tested it...
There was a problem hiding this comment.
I think we all tested this. The fixture in it's current state on 15.6.1 is completely broken on older builds of the fixtures (http://react-dom-test-fixtures.surge.sh/). Can we attribute this to a mixup pulling this code on to 15.x?
backport of #10156, not sure what the deal is with prettier but when I ran it, it gave me a ton of edits