-
Notifications
You must be signed in to change notification settings - Fork 47.1k
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
Remove loose check on non-number controlled inputs. Fix trailing dot issue. #9584
Changes from all commits
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,62 @@ | ||
const React = window.React; | ||
|
||
import Fixture from '../../Fixture'; | ||
|
||
class InputTestCase extends React.Component { | ||
static defaultProps = { | ||
type: 'text', | ||
defaultValue: '', | ||
parseAs: 'text' | ||
} | ||
|
||
constructor () { | ||
super(...arguments); | ||
|
||
this.state = { | ||
value: this.props.defaultValue | ||
}; | ||
} | ||
|
||
onChange = (event) => { | ||
const raw = event.target.value; | ||
|
||
switch (this.props.type) { | ||
case 'number': | ||
const parsed = parseFloat(event.target.value, 10); | ||
|
||
this.setState({ value: isNaN(parsed) ? '' : parsed }); | ||
|
||
break; | ||
default: | ||
this.setState({ value: raw }); | ||
} | ||
} | ||
|
||
render() { | ||
const { children, type, defaultValue } = this.props; | ||
const { value } = this.state; | ||
|
||
return ( | ||
<Fixture> | ||
<div>{children}</div> | ||
|
||
<div className="control-box"> | ||
<fieldset> | ||
<legend>Controlled {type}</legend> | ||
<input type={type} value={value} onChange={this.onChange} /> | ||
<p className="hint"> | ||
Value: {JSON.stringify(this.state.value)} | ||
</p> | ||
</fieldset> | ||
|
||
<fieldset> | ||
<legend>Uncontrolled {type}</legend> | ||
<input type={type} defaultValue={defaultValue} /> | ||
</fieldset> | ||
</div> | ||
</Fixture> | ||
); | ||
} | ||
} | ||
|
||
export default InputTestCase; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,62 +1,92 @@ | ||
const React = window.React; | ||
|
||
import Fixture from '../../Fixture'; | ||
import FixtureSet from '../../FixtureSet'; | ||
import TestCase from '../../TestCase'; | ||
import InputTestCase from './InputTestCase'; | ||
|
||
class TextInputFixtures extends React.Component { | ||
state = { | ||
color: '#ffaaee', | ||
}; | ||
render() { | ||
return ( | ||
<FixtureSet | ||
title="Inputs" | ||
description="Common behavior across controled and uncontrolled inputs" | ||
> | ||
<TestCase title="Numbers in a controlled text field with no handler"> | ||
<TestCase.Steps> | ||
<li>Move the cursor to after "2" in the text field</li> | ||
<li>Type ".2" into the text input</li> | ||
</TestCase.Steps> | ||
|
||
renderControlled = (type) => { | ||
let id = `controlled_${type}`; | ||
<TestCase.ExpectedResult> | ||
The text field's value should not update. | ||
</TestCase.ExpectedResult> | ||
|
||
let onChange = e => { | ||
let value = e.target.value; | ||
if (type === 'number') { | ||
value = value === '' ? '' : parseFloat(value, 10) || 0; | ||
} | ||
this.setState({ | ||
[type] : value, | ||
}); | ||
}; | ||
<Fixture> | ||
<div className="control-box"> | ||
<fieldset> | ||
<legend>Value as number</legend> | ||
<input value={2} onChange={() => {}} /> | ||
</fieldset> | ||
|
||
let state = this.state[type] || ''; | ||
<fieldset> | ||
<legend>Value as string</legend> | ||
<input value={"2"} onChange={() => {}} /> | ||
</fieldset> | ||
</div> | ||
</Fixture> | ||
|
||
return ( | ||
<div key={type} className="field"> | ||
<label className="control-label" htmlFor={id}>{type}</label> | ||
<input id={id} type={type} value={state} onChange={onChange} /> | ||
→ {JSON.stringify(state)} | ||
</div> | ||
); | ||
} | ||
<p className="footnote"> | ||
This issue was first introduced when we added extra logic | ||
to number inputs to prevent unexpected behavior in Chrome | ||
and Safari (see the number input test case). | ||
</p> | ||
</TestCase> | ||
|
||
renderUncontrolled = (type) => { | ||
let id = `uncontrolled_${type}`; | ||
return ( | ||
<div key={type} className="field"> | ||
<label className="control-label" htmlFor={id}>{type}</label> | ||
<input id={id} type={type} /> | ||
</div> | ||
); | ||
} | ||
<TestCase title="Cursor when editing email inputs"> | ||
<TestCase.Steps> | ||
<li>Type "[email protected]"</li> | ||
<li>Select "@"</li> | ||
<li>Type ".", to replace "@" with a period</li> | ||
</TestCase.Steps> | ||
|
||
render() { | ||
// https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input | ||
let types = [ | ||
'text', 'email', 'number', 'url', 'tel', | ||
'color', 'date', 'datetime-local', | ||
'time', 'month', 'week', 'range', 'password', | ||
]; | ||
return ( | ||
<form onSubmit={event => event.preventDefault()}> | ||
<fieldset> | ||
<legend>Controlled</legend> | ||
{types.map(this.renderControlled)} | ||
</fieldset> | ||
<fieldset> | ||
<legend>Uncontrolled</legend> | ||
{types.map(this.renderUncontrolled)} | ||
</fieldset> | ||
</form> | ||
<TestCase.ExpectedResult> | ||
The text field's cursor should not jump to the end. | ||
</TestCase.ExpectedResult> | ||
|
||
<InputTestCase type="email" defaultValue="" /> | ||
</TestCase> | ||
|
||
<TestCase title="Cursor when editing url inputs"> | ||
<TestCase.Steps> | ||
<li>Type "http://www.example.com"</li> | ||
<li>Select "www."</li> | ||
<li>Press backspace/delete</li> | ||
</TestCase.Steps> | ||
|
||
<TestCase.ExpectedResult> | ||
The text field's cursor should not jump to the end. | ||
</TestCase.ExpectedResult> | ||
|
||
<InputTestCase type="url" defaultValue="" /> | ||
</TestCase> | ||
|
||
<TestCase title="All inputs" description="General test of all inputs"> | ||
<InputTestCase type="text" defaultValue="Text" /> | ||
<InputTestCase type="email" defaultValue="[email protected]"/> | ||
<InputTestCase type="number" defaultValue={0} /> | ||
<InputTestCase type="url" defaultValue="http://example.com"/> | ||
<InputTestCase type="tel" defaultValue="555-555-5555"/> | ||
<InputTestCase type="color" defaultValue="#ff0000" /> | ||
<InputTestCase type="date" defaultValue="2017-01-01" /> | ||
<InputTestCase type="datetime-local" defaultValue="2017-01-01T01:00" /> | ||
<InputTestCase type="time" defaultValue="01:00" /> | ||
<InputTestCase type="month" defaultValue="2017-01" /> | ||
<InputTestCase type="week" defaultValue="2017-W01" /> | ||
<InputTestCase type="range" defaultValue={0.5} /> | ||
<InputTestCase type="password" defaultValue="" /> | ||
</TestCase> | ||
</FixtureSet> | ||
); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -209,8 +209,7 @@ var ReactDOMInput = { | |
// browsers typically do this as necessary, jsdom doesn't. | ||
node.value = '' + value; | ||
} | ||
// eslint-disable-next-line | ||
} else if (value != node.value) { | ||
} else if (node.value !== value) { | ||
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. shouldn't this be render(<input value={0}/>, el);
render(<input value={0}/>, el); // shouldn't reset value 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. Good question. I'll write up test case. 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. Good catch. Yes. I'll send out a PR shortly. 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. Done! #9806 |
||
// Cast `value` to a string to ensure the value is set correctly. While | ||
// browsers typically do this as necessary, jsdom doesn't. | ||
node.value = '' + value; | ||
|
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.
Might change to
{`The text field's value should not update.`}
to make syntax highlighters happy.