diff --git a/fixtures/dom/src/components/Header.js b/fixtures/dom/src/components/Header.js index 83f77eee31c20..5f717d8be12ee 100644 --- a/fixtures/dom/src/components/Header.js +++ b/fixtures/dom/src/components/Header.js @@ -44,6 +44,7 @@ const Header = React.createClass({ + diff --git a/fixtures/dom/src/components/fixtures/index.js b/fixtures/dom/src/components/fixtures/index.js index ea7cebbde8899..516e8ad3aee27 100644 --- a/fixtures/dom/src/components/fixtures/index.js +++ b/fixtures/dom/src/components/fixtures/index.js @@ -4,6 +4,7 @@ import TextInputFixtures from './text-inputs'; import SelectFixtures from './selects'; import TextAreaFixtures from './textareas'; import InputChangeEvents from './input-change-events'; +import NumberInputFixtures from './number-inputs/'; /** * A simple routing component that renders the appropriate @@ -22,6 +23,8 @@ const FixturesPage = React.createClass({ return ; case '/input-change-events': return ; + case '/number-inputs': + return ; default: return

Please select a test fixture.

; } diff --git a/fixtures/dom/src/components/fixtures/number-inputs/NumberTestCase.js b/fixtures/dom/src/components/fixtures/number-inputs/NumberTestCase.js new file mode 100644 index 0000000000000..2c072d478ebe4 --- /dev/null +++ b/fixtures/dom/src/components/fixtures/number-inputs/NumberTestCase.js @@ -0,0 +1,37 @@ +const React = window.React; + +import Fixture from '../../Fixture'; + +const NumberTestCase = React.createClass({ + getInitialState() { + return { value: '' }; + }, + onChange(event) { + const parsed = parseFloat(event.target.value, 10) + const value = isNaN(parsed) ? '' : parsed + + this.setState({ value }) + }, + render() { + return ( + +
{this.props.children}
+ +
+
+ Controlled + + Value: {JSON.stringify(this.state.value)} +
+ +
+ Uncontrolled + +
+
+
+ ); + }, +}); + +export default NumberTestCase; diff --git a/fixtures/dom/src/components/fixtures/number-inputs/index.js b/fixtures/dom/src/components/fixtures/number-inputs/index.js new file mode 100644 index 0000000000000..2c88c333edeb3 --- /dev/null +++ b/fixtures/dom/src/components/fixtures/number-inputs/index.js @@ -0,0 +1,167 @@ +const React = window.React; + +import FixtureSet from '../../FixtureSet'; +import TestCase from '../../TestCase'; +import NumberTestCase from './NumberTestCase'; + +const NumberInputs = React.createClass({ + render() { + return ( + + + +
  • Type "3.1"
  • +
  • Press backspace, eliminating the "1"
  • +
    + + + The field should read "3.", preserving the decimal place + + + + +

    + Notes: Chrome and Safari clear trailing + decimals on blur. React makes this concession so that the + value attribute remains in sync with the value property. +

    +
    + + + +
  • Type "0.01"
  • +
    + + + The field should read "0.01" + + + +
    + + + +
  • Type "2e"
  • +
  • Type 4, to read "2e4"
  • +
    + + + The field should read "2e4". The parsed value should read "20000" + + + +
    + + + +
  • Type "3.14"
  • +
  • Press "e", so that the input reads "3.14e"
  • +
    + + + The field should read "3.14e", the parsed value should be empty + + + +
    + + + +
  • Type "3.14"
  • +
  • Move the text cursor to after the decimal place
  • +
  • Press "e" twice, so that the value reads "3.ee14"
  • +
    + + + The field should read "3.ee14" + + + +
    + + + +
  • Type "3.0"
  • +
    + + + The field should read "3.0" + + + +
    + + + +
  • Type "300"
  • +
  • Move the cursor to after the "3"
  • +
  • Type "."
  • +
    + + + The field should read "3.00", not "3" + + +
    + + + +
  • Type "3"
  • +
  • Select the entire value"
  • +
  • Type '-' to replace '3' with '-'
  • +
    + + + The field should read "-", not be blank. + + +
    + + + +
  • Type "-"
  • +
  • Type '3'
  • +
    + + + The field should read "-3". + + +
    +
    + ); + }, +}); + +export default NumberInputs; diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 16aab7d56eaaf..ad888137c4114 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -837,6 +837,8 @@ src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js * should set className to empty string instead of null * should remove property properly for boolean properties * should remove property properly even with different name +* should update an empty attribute to zero +* should always assign the value attribute for non-inputs * should remove attributes for normal properties * should not remove attributes for special properties * should not leave all options selected when deleting multiple @@ -1409,6 +1411,10 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * should allow setting `value` to `objToString` * should not incur unnecessary DOM mutations * should properly control a value of number `0` +* should properly control 0.0 for a text input +* should properly control 0.0 for a number input +* should properly transition from an empty value to 0 +* should properly transition from 0 to an empty value * should have the correct target value * should not set a value for submit buttons unnecessarily * should control radio buttons @@ -1442,6 +1448,11 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * sets value properly with type coming later in props * does not raise a validation warning when it switches types * resets value of date/time input to fix bugs in iOS Safari +* always sets the attribute when values change on text inputs +* does not set the value attribute on number inputs if focused +* sets the value attribute on number inputs on blur +* an uncontrolled number input will not update the value attribute on blur +* an uncontrolled text input will not update the value attribute on blur src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js * should flatten children to a string diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js index f125280c5a12b..f0bbc0adcaef3 100644 --- a/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js +++ b/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js @@ -138,11 +138,8 @@ var ReactDOMInput = { ? props.checked : props.defaultChecked, initialValue: props.value != null ? props.value : defaultValue, + controlled: isControlled(props), }; - - if (__DEV__) { - node._wrapperState.controlled = isControlled(props); - } }, updateWrapper: function(element: Element, props: Object) { @@ -195,13 +192,24 @@ var ReactDOMInput = { var value = props.value; if (value != null) { - // Cast `value` to a string to ensure the value is set correctly. While - // browsers typically do this as necessary, jsdom doesn't. - var newValue = '' + value; + if (value === 0 && node.value === '') { + node.value = '0'; + // Note: IE9 reports a number inputs as 'text', so check props instead. + } else if (props.type === 'number') { + // Simulate `input.valueAsNumber`. IE9 does not support it + var valueAsNumber = parseFloat(node.value, 10) || 0; - // To avoid side effects (such as losing text selection), only set value if changed - if (newValue !== node.value) { - node.value = newValue; + // eslint-disable-next-line + if (value != valueAsNumber) { + // 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; + } + // eslint-disable-next-line + } else if (value != node.value) { + // 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; } } else { if (props.value == null && props.defaultValue != null) { diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index d7d5ddc80c012..ccd92eaf7c95c 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -210,6 +210,34 @@ var HTMLDOMPropertyConfig = { httpEquiv: 'http-equiv', }, DOMPropertyNames: {}, + DOMMutationMethods: { + value: function(node, value) { + if (value == null) { + return node.removeAttribute('value'); + } + + // Number inputs get special treatment due to some edge cases in + // Chrome. Let everything else assign the value attribute as normal. + // https://github.com/facebook/react/issues/7253#issuecomment-236074326 + if (node.type !== 'number' || node.hasAttribute('value') === false) { + node.setAttribute('value', '' + value); + } else if ( + node.validity && + !node.validity.badInput && + node.ownerDocument.activeElement !== node + ) { + // Don't assign an attribute if validation reports bad + // input. Chrome will clear the value. Additionally, don't + // operate on inputs that have focus, otherwise Chrome might + // strip off trailing decimal places and cause the user's + // cursor position to jump to the beginning of the input. + // + // In ReactDOMInput, we have an onBlur event that will trigger + // this function again when focus is lost. + node.setAttribute('value', '' + value); + } + }, + }, }; module.exports = HTMLDOMPropertyConfig; diff --git a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js index 840841542463f..5fcc92a825aab 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js @@ -296,6 +296,35 @@ describe('DOMPropertyOperations', () => { }); }); + describe('value mutation method', function() { + it('should update an empty attribute to zero', function() { + var stubNode = document.createElement('input'); + var stubInstance = {_debugID: 1}; + ReactDOMComponentTree.precacheNode(stubInstance, stubNode); + + stubNode.setAttribute('type', 'radio'); + + DOMPropertyOperations.setValueForProperty(stubNode, 'value', ''); + spyOn(stubNode, 'setAttribute'); + DOMPropertyOperations.setValueForProperty(stubNode, 'value', 0); + + expect(stubNode.setAttribute.calls.count()).toBe(1); + }); + + it('should always assign the value attribute for non-inputs', function() { + var stubNode = document.createElement('progress'); + var stubInstance = {_debugID: 1}; + ReactDOMComponentTree.precacheNode(stubInstance, stubNode); + + spyOn(stubNode, 'setAttribute'); + + DOMPropertyOperations.setValueForProperty(stubNode, 'value', 30); + DOMPropertyOperations.setValueForProperty(stubNode, 'value', '30'); + + expect(stubNode.setAttribute.calls.count()).toBe(2); + }); + }); + describe('deleteValueForProperty', () => { var stubNode; var stubInstance; diff --git a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js index 4a026bcbea57e..1b1d5b95c065a 100644 --- a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js +++ b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js @@ -258,6 +258,26 @@ function getTargetInstForInputOrChangeEvent(topLevelType, targetInst) { } } +function handleControlledInputBlur(inst, node) { + // TODO: In IE, inst is occasionally null. Why? + if (inst == null) { + return; + } + + // Fiber and ReactDOM keep wrapper state in separate places + let state = inst._wrapperState || node._wrapperState; + + if (!state || !state.controlled || node.type !== 'number') { + return; + } + + // If controlled, assign the value attribute to the current value on blur + let value = '' + node.value; + if (node.getAttribute('value') !== value) { + node.setAttribute('value', value); + } +} + /** * This plugin creates an `onChange` event that normalizes change events * across form elements. This event fires at a time when it's possible to @@ -316,6 +336,11 @@ var ChangeEventPlugin = { if (handleEventFunc) { handleEventFunc(topLevelType, targetNode, targetInst); } + + // When blurring, set the value attribute for number inputs + if (topLevelType === 'topBlur') { + handleControlledInputBlur(targetInst, targetNode); + } }, }; diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index c16b2f578a2b0..62df49a453262 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -423,6 +423,48 @@ describe('ReactDOMInput', () => { expect(node.value).toBe('0'); }); + it('should properly control 0.0 for a text input', () => { + var stub = ; + stub = ReactTestUtils.renderIntoDocument(stub); + var node = ReactDOM.findDOMNode(stub); + + node.value = '0.0'; + ReactTestUtils.Simulate.change(node, {target: {value: '0.0'}}); + expect(node.value).toBe('0.0'); + }); + + it('should properly control 0.0 for a number input', () => { + var stub = ; + stub = ReactTestUtils.renderIntoDocument(stub); + var node = ReactDOM.findDOMNode(stub); + + node.value = '0.0'; + ReactTestUtils.Simulate.change(node, {target: {value: '0.0'}}); + expect(node.value).toBe('0.0'); + }); + + it('should properly transition from an empty value to 0', function() { + var container = document.createElement('div'); + + ReactDOM.render(, container); + ReactDOM.render(, container); + + var node = container.firstChild; + + expect(node.value).toBe('0'); + }); + + it('should properly transition from 0 to an empty value', function() { + var container = document.createElement('div'); + + ReactDOM.render(, container); + ReactDOM.render(, container); + + var node = container.firstChild; + + expect(node.value).toBe(''); + }); + it('should have the correct target value', () => { var handled = false; var handler = function(event) { @@ -1074,4 +1116,88 @@ describe('ReactDOMInput', () => { 'node.setAttribute("checked", "")', ]); }); + + describe('assigning the value attribute on controlled inputs', function() { + function getTestInput() { + return React.createClass({ + getInitialState: function() { + return { + value: this.props.value == null ? '' : this.props.value, + }; + }, + onChange: function(event) { + this.setState({value: event.target.value}); + }, + render: function() { + var type = this.props.type; + var value = this.state.value; + + return ; + }, + }); + } + + it('always sets the attribute when values change on text inputs', function() { + var Input = getTestInput(); + var stub = ReactTestUtils.renderIntoDocument(); + var node = ReactDOM.findDOMNode(stub); + + ReactTestUtils.Simulate.change(node, {target: {value: '2'}}); + + expect(node.getAttribute('value')).toBe('2'); + }); + + it('does not set the value attribute on number inputs if focused', () => { + var Input = getTestInput(); + var stub = ReactTestUtils.renderIntoDocument( + , + ); + var node = ReactDOM.findDOMNode(stub); + + node.focus(); + + ReactTestUtils.Simulate.change(node, {target: {value: '2'}}); + + expect(node.getAttribute('value')).toBe('1'); + }); + + it('sets the value attribute on number inputs on blur', () => { + var Input = getTestInput(); + var stub = ReactTestUtils.renderIntoDocument( + , + ); + var node = ReactDOM.findDOMNode(stub); + + ReactTestUtils.Simulate.change(node, {target: {value: '2'}}); + ReactTestUtils.SimulateNative.blur(node); + + expect(node.getAttribute('value')).toBe('2'); + }); + + it('an uncontrolled number input will not update the value attribute on blur', () => { + var stub = ReactTestUtils.renderIntoDocument( + , + ); + var node = ReactDOM.findDOMNode(stub); + + node.value = 4; + + ReactTestUtils.SimulateNative.blur(node); + + expect(node.getAttribute('value')).toBe('1'); + }); + + it('an uncontrolled text input will not update the value attribute on blur', () => { + var stub = ReactTestUtils.renderIntoDocument( + , + ); + var node = ReactDOM.findDOMNode(stub); + + node.value = 4; + + ReactTestUtils.SimulateNative.blur(node); + + expect(node.getAttribute('value')).toBe('1'); + }); + }); }); diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index f1fc6a6068dea..2f30e43a26f5e 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -128,11 +128,8 @@ var ReactDOMInput = { : props.defaultChecked, initialValue: props.value != null ? props.value : defaultValue, listeners: null, + controlled: isControlled(props), }; - - if (__DEV__) { - inst._wrapperState.controlled = isControlled(props); - } }, updateWrapper: function(inst) { @@ -188,13 +185,24 @@ var ReactDOMInput = { var node = ReactDOMComponentTree.getNodeFromInstance(inst); var value = props.value; if (value != null) { - // Cast `value` to a string to ensure the value is set correctly. While - // browsers typically do this as necessary, jsdom doesn't. - var newValue = '' + value; + if (value === 0 && node.value === '') { + node.value = '0'; + // Note: IE9 reports a number inputs as 'text', so check props instead. + } else if (props.type === 'number') { + // Simulate `input.valueAsNumber`. IE9 does not support it + var valueAsNumber = parseFloat(node.value, 10) || 0; - // To avoid side effects (such as losing text selection), only set value if changed - if (newValue !== node.value) { - node.value = newValue; + // eslint-disable-next-line + if (value != valueAsNumber) { + // 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; + } + // eslint-disable-next-line + } else if (value != node.value) { + // 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; } } else { if (props.value == null && props.defaultValue != null) {