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
bug fix on input radio #11227
bug fix on input radio #11227
Changes from 8 commits
4ce1fb0
0e15659
6f3a000
aad2f64
03b1b30
69cbb05
25c0fc4
e03b117
9ee15eb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
const React = window.React; | ||
const noop = n => n; | ||
|
||
class RadioNameChangeFixture extends React.Component { | ||
state = { | ||
updated: false, | ||
}; | ||
onClick = () => { | ||
this.setState(state => { | ||
return {updated: !state.updated}; | ||
}); | ||
}; | ||
render() { | ||
const {updated} = this.state; | ||
const radioName = updated ? 'firstName' : 'secondName'; | ||
return ( | ||
<div> | ||
<label> | ||
<input | ||
type="radio" | ||
name={radioName} | ||
onChange={noop} | ||
checked={updated === true} | ||
/> | ||
First Radio | ||
</label> | ||
|
||
<label> | ||
<input | ||
type="radio" | ||
name={radioName} | ||
onChange={noop} | ||
checked={updated === false} | ||
/> | ||
Second Radio | ||
</label> | ||
|
||
<div> | ||
<button type="button" onClick={this.onClick}>Toggle</button> | ||
</div> | ||
</div> | ||
); | ||
} | ||
} | ||
|
||
export default RadioNameChangeFixture; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -144,6 +144,20 @@ export function initWrapperState(element: Element, props: Object) { | |
}; | ||
} | ||
|
||
export function updateChecked( | ||
element: Element, | ||
props: Object, | ||
targetType: ?string, | ||
) { | ||
var node = ((element: any): InputWithWrapperState); | ||
if (!targetType || targetType === node.type) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In function updateNamedCousins(rootNode, props) {
var name = props.name;
if (props.type === 'radio' && name != null) {
// ...
}
// ...
} Can you use that check here? That would allow us to remove the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added the code in updateProperties because updateWrapper also calls updateChecked |
||
var checked = props.checked; | ||
if (checked != null) { | ||
DOMPropertyOperations.setValueForProperty(node, 'checked', checked); | ||
} | ||
} | ||
} | ||
|
||
export function updateWrapper(element: Element, props: Object) { | ||
var node = ((element: any): InputWithWrapperState); | ||
if (__DEV__) { | ||
|
@@ -183,14 +197,7 @@ export function updateWrapper(element: Element, props: Object) { | |
} | ||
} | ||
|
||
var checked = props.checked; | ||
if (checked != null) { | ||
DOMPropertyOperations.setValueForProperty( | ||
node, | ||
'checked', | ||
checked || false, | ||
); | ||
} | ||
updateChecked(element, props); | ||
|
||
var value = props.value; | ||
if (value != null) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to figure out why this is necessary (but it is definitely effective).
updateNamedCousins
inReactDOMInput
should take care of this, but it looks like it doesn't run in the test case you've provided.@jquense
updateNamedCousins
gets called inrestoreControlledState
, but for what ever reason it only fires on thebutton
, which I guess is clicked and responsible for the last trigger. The radio input never gets targeted byrestoreControlledState
.It feels like there's something deeper here that we need to fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think updateNamedCousins is called only on the button because of this code
ReactTestUtils.Simulate.click(buttonNode);
I checked that updateNamedCousins is not called when I run your fixture.