Skip to content

Fix uncontrolled radios #10156

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.

Sign up for GitHub

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

Merged
nhunzaker merged 6 commits into from
Jul 13, 2017
Merged

Fix uncontrolled radios #10156

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ce70f13
Add fixture
jquense Jul 12, 2017
25cff31
Fix Uncontrolled radio groups
jquense Jul 12, 2017
84bda97
address feedback
jquense Jul 12, 2017
48fc77f
Merge branch 'master' into fix-uncontrolled-radios
jquense Jul 12, 2017
15228e8
fix tests; prettier
jquense Jul 12, 2017
dede4b6
Update TestCase.js
jquense Jul 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Merge branch 'master' into fix-uncontrolled-radios
  • Loading branch information
@jquense
jquense authored Jul 12, 2017
commit 48fc77f6ee960c59d5c60ff5b4be838a235ffe74
57 changes: 21 additions & 36 deletions fixtures/dom/src/components/TestCase.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const propTypes = {
title: PropTypes.node.isRequired,
resolvedIn: semverString,
introducedIn: semverString,
resolvedBy: PropTypes.string
resolvedBy: PropTypes.string,
};

class TestCase extends React.Component {
Expand Down Expand Up @@ -41,12 +41,9 @@ class TestCase extends React.Component {

let {complete} = this.state;

const { version } = parse(window.location.search);
const isTestFixed = (
!version ||
!resolvedIn ||
semver.gte(version, resolvedIn)
);
const {version} = parse(window.location.search);
const isTestFixed =
!version || !resolvedIn || semver.gte(version, resolvedIn);

complete = !isTestFixed || complete;
Copy link
Contributor

@nhunzaker nhunzaker Jul 13, 2017

Choose a reason for hiding this comment

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

Doing some local testing, looks like this causes a failure on line 103:
https://github.com/jquense/react/blob/15228e8b5f9d056d475955a54723cd906ffa5619/fixtures/dom/src/components/TestCase.js#L103

Copy link
Contributor Author

@jquense jquense Jul 13, 2017

Choose a reason for hiding this comment

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

Last time I do merge conflict resolve on github :P man.


Expand All @@ -65,29 +62,16 @@ class TestCase extends React.Component {
</h2>

<dl className="test-case__details">
{introducedIn && (
<dt>First broken in: </dt>)}
{introducedIn && (
<dd>
<a href={'https://github.com/facebook/react/tag/v' + introducedIn}>
<code>{introducedIn}</code>
</a>
</dd>
)}

{resolvedIn && (
<dt>First supported in: </dt>)}
{resolvedIn && (
<dd>
<a href={'https://github.com/facebook/react/tag/v' + resolvedIn}>
<code>{resolvedIn}</code>
</a>
</dd>
)}

{resolvedBy && (
<dt>Fixed by: </dt>)}
{resolvedBy && (
{introducedIn && <dt>First broken in: </dt>}
{introducedIn &&
<dd>
<a href={'https://github.com/facebook/react/tag/v' + introducedIn}>
<code>{introducedIn}</code>
</a>
</dd>}

{resolvedIn && <dt>First supported in: </dt>}
{resolvedIn &&
<dd>
<a href={'https://github.com/facebook/react/tag/v' + resolvedIn}>
<code>{resolvedIn}</code>
Expand Down Expand Up @@ -115,12 +99,13 @@ class TestCase extends React.Component {
</p>

<div className="test-case__body">
{!isTestFixed && (
<p className="test-case__invalid-version">
<strong>Note:</strong> This test case was fixed in a later version of React.
This test is not expected to pass for the selected version, and that's ok!
</p>
)}
{!isTestRelevant &&
<p className="test-case__invalid-version">
<strong>Note:</strong>
{' '}
This test case was fixed in a later version of React.
This test is not expected to pass for the selected version, and that's ok!
</p>}

{children}
</div>
Expand Down
You are viewing a condensed version of this merge commit. You can view the full changes here.