Skip to content

Fix controlled/uncontrolled validation for radio+checkbox inputs #7003

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jun 9, 2016

Conversation

jimfb
Copy link
Contributor

@jimfb jimfb commented Jun 9, 2016

Fix controlled/uncontrolled validation for radio+checkbox inputs

Fixes #6779

@jimfb
Copy link
Contributor Author

jimfb commented Jun 9, 2016

cc @spicyj

@jimfb jimfb force-pushed the uncontrolled-checkbox branch from 31c0b79 to 38f06f2 Compare June 9, 2016 01:54
function isControlled(props) {
return ((props.type === 'checkbox' || props.type === 'radio') && props.checked !== undefined) ||
(props.type !== 'checkbox' && props.type !== 'radio' && props.value !== undefined);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This spacing isn't FB style and this would be clearer anyway; can you change it?

function isControlled(props) {
  var usesChecked = props.type === 'checkbox' || props.type === 'radio';
  return usesChecked ? props.checked !== undefined : props.value !== undefined;
}

@jimfb jimfb force-pushed the uncontrolled-checkbox branch from 38f06f2 to 2152f5b Compare June 9, 2016 02:45
@ghost
Copy link

ghost commented Jun 9, 2016

@jimfb updated the pull request.

@jimfb jimfb merged commit 0bb0fe8 into facebook:master Jun 9, 2016
checked={false}
onChange={() => null}
/>, container);
console.log(console.error.calls.argsFor(0)[0]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean to leave this console.log call here? 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pssh, thanks. #7006

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a lint rule for that.

@sophiebits
Copy link
Collaborator

semver labels on this and #7006 please?

@zpao zpao added this to the 15-next milestone Jun 14, 2016
zpao pushed a commit that referenced this pull request Jun 14, 2016
@zpao zpao modified the milestones: 15-next, 15.2.0 Jun 14, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants