From 0c8f935c5b942e46e01626e089278145235d640b Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Tue, 26 Jul 2016 20:51:50 -0400 Subject: [PATCH 01/31] Only re-assign defaultValue if it is different --- src/renderers/dom/stack/client/wrappers/ReactDOMInput.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index f1fc6a6068dea..a743985833457 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -207,7 +207,6 @@ var ReactDOMInput = { // // https://github.com/facebook/react/issues/7253 if (node.defaultValue !== '' + props.defaultValue) { - node.defaultValue = '' + props.defaultValue; } } if (props.checked == null && props.defaultChecked != null) { From 8654886c204514551497df05fa530dff53c02b4f Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Thu, 28 Jul 2016 17:56:36 -0400 Subject: [PATCH 02/31] Do not set value if it is the same --- src/renderers/dom/shared/DOMPropertyOperations.js | 4 ++++ src/renderers/dom/stack/client/wrappers/ReactDOMInput.js | 4 +--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index be65bdbf1b739..9f093cc60978e 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -160,6 +160,10 @@ var DOMPropertyOperations = { (propertyInfo.hasOverloadedBooleanValue && value === true) ) { node.setAttribute(attributeName, ''); + } else if (attributeName === 'value') { + if (node.value != value) { + node.value = '' + value; + } } else { node.setAttribute(attributeName, '' + value); } diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index a743985833457..b760d2d27edff 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -207,11 +207,9 @@ var ReactDOMInput = { // // https://github.com/facebook/react/issues/7253 if (node.defaultValue !== '' + props.defaultValue) { + node.defaultChecked = !!props.defaultChecked; } } - if (props.checked == null && props.defaultChecked != null) { - node.defaultChecked = !!props.defaultChecked; - } } }, From 0addf2ba915ca16536645520b1557eb40bb35d2e Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Thu, 28 Jul 2016 21:06:03 -0400 Subject: [PATCH 03/31] Properly cover defaultValue --- src/renderers/dom/shared/DOMPropertyOperations.js | 4 ++-- src/renderers/dom/stack/client/wrappers/ReactDOMInput.js | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index 9f093cc60978e..286de4be12e00 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -161,8 +161,8 @@ var DOMPropertyOperations = { ) { node.setAttribute(attributeName, ''); } else if (attributeName === 'value') { - if (node.value != value) { - node.value = '' + value; + if (node.value !== value) { + node.setAttribute(attributeName, '' + value); } } else { node.setAttribute(attributeName, '' + value); diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index b760d2d27edff..3c836fb6c0cd3 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -207,9 +207,12 @@ var ReactDOMInput = { // // https://github.com/facebook/react/issues/7253 if (node.defaultValue !== '' + props.defaultValue) { - node.defaultChecked = !!props.defaultChecked; + node.defaultValue = !!props.defaultValue; } } + if (props.checked == null && props.defaultChecked != null) { + node.defaultChecked = !!props.defaultChecked; + } } }, From 1845151751bc741b4c0813902eeba4bbc271610a Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Thu, 28 Jul 2016 21:40:52 -0400 Subject: [PATCH 04/31] Use coercion to be smart about value assignment --- src/renderers/dom/shared/DOMPropertyOperations.js | 4 ++-- .../dom/stack/client/wrappers/ReactDOMInput.js | 10 ++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index 286de4be12e00..7bee38c6a4d35 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -160,8 +160,8 @@ var DOMPropertyOperations = { (propertyInfo.hasOverloadedBooleanValue && value === true) ) { node.setAttribute(attributeName, ''); - } else if (attributeName === 'value') { - if (node.value !== value) { + } else if (attributeName === 'value' && node.hasAttribute('value')) { + if (node.value != value) { node.setAttribute(attributeName, '' + value); } } else { diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index 3c836fb6c0cd3..e4b6daed00d27 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -188,13 +188,11 @@ 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; - // To avoid side effects (such as losing text selection), only set value if changed - if (newValue !== node.value) { - node.value = newValue; + 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) { From 1e7855969e2d71e22b8fe68baa6fab0c69112b89 Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Mon, 1 Aug 2016 07:29:06 -0400 Subject: [PATCH 05/31] Add explanation of loose type checks in value assignment. --- src/renderers/dom/shared/DOMPropertyOperations.js | 4 +++- src/renderers/dom/stack/client/wrappers/ReactDOMInput.js | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index 7bee38c6a4d35..bcc6c909148f6 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -161,7 +161,9 @@ var DOMPropertyOperations = { ) { node.setAttribute(attributeName, ''); } else if (attributeName === 'value' && node.hasAttribute('value')) { - if (node.value != value) { + // Use loose coercion to prevent replacement on comparisons like + // '3e1' == 30 in Chrome (~52). + if (node.value != value) { // eslint-disable-line node.setAttribute(attributeName, '' + value); } } else { diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index e4b6daed00d27..a0be9eec3affb 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -188,8 +188,9 @@ var ReactDOMInput = { var node = ReactDOMComponentTree.getNodeFromInstance(inst); var value = props.value; if (value != null) { - // To avoid side effects (such as losing text selection), only set value if changed - if (value != node.value) { + // Use loose coercion to prevent replacement on comparisons like + // '3e1' == 30 in Chrome (~52). + if (value != node.value) { // eslint-disable-line // 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; From 8ceb1f4defdc9cdef28e4abda306eaad9619b9d5 Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Mon, 1 Aug 2016 07:54:51 -0400 Subject: [PATCH 06/31] Add test coverage for setAttribute update. --- .../shared/__tests__/DOMPropertyOperations-test.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js index 840841542463f..84ebfca69cd57 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js @@ -294,6 +294,19 @@ describe('DOMPropertyOperations', () => { // some browsers) expect(stubNode.className).toBe(''); }); + + it('should not update numeric values when the input.value is loosely the same', function() { + stubNode.setAttribute('type', 'number'); + stubNode.setAttribute('value', '3e1'); + // Keep in sync. The comparison occurs between setAttribute and value. + stubNode.value = '3e1'; + + spyOn(stubNode, 'setAttribute'); + + DOMPropertyOperations.setValueForProperty(stubNode, 'value', 30); + + expect(stubNode.setAttribute.calls.count()).toBe(0); + }); }); describe('deleteValueForProperty', () => { From c56f548eff37408bf0158e0f9e363baa08bfa95f Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Mon, 5 Sep 2016 09:39:03 -0400 Subject: [PATCH 07/31] Only apply loose value check to text inputs --- .../dom/shared/DOMPropertyOperations.js | 6 ---- .../dom/shared/HTMLDOMPropertyConfig.js | 17 +++++++++ .../__tests__/DOMPropertyOperations-test.js | 35 +++++++++++++++++++ 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/renderers/dom/shared/DOMPropertyOperations.js b/src/renderers/dom/shared/DOMPropertyOperations.js index bcc6c909148f6..be65bdbf1b739 100644 --- a/src/renderers/dom/shared/DOMPropertyOperations.js +++ b/src/renderers/dom/shared/DOMPropertyOperations.js @@ -160,12 +160,6 @@ var DOMPropertyOperations = { (propertyInfo.hasOverloadedBooleanValue && value === true) ) { node.setAttribute(attributeName, ''); - } else if (attributeName === 'value' && node.hasAttribute('value')) { - // Use loose coercion to prevent replacement on comparisons like - // '3e1' == 30 in Chrome (~52). - if (node.value != value) { // eslint-disable-line - node.setAttribute(attributeName, '' + value); - } } else { node.setAttribute(attributeName, '' + value); } diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index d7d5ddc80c012..71ecec19c4046 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -210,6 +210,23 @@ var HTMLDOMPropertyConfig = { httpEquiv: 'http-equiv', }, DOMPropertyNames: {}, + DOMMutationMethods: { + value: function (node, value) { + if (value == null) { + return node.removeAttribute('value'); + } + + if (node.hasAttribute('value')) { + // Use loose coercion to prevent replacement on comparisons like + // '3e1' == 30 in Chrome (~52). + if (node.value == value) { // eslint-disable-line + return; + } + } + + 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 84ebfca69cd57..a83cb46a57509 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js @@ -295,7 +295,14 @@ describe('DOMPropertyOperations', () => { expect(stubNode.className).toBe(''); }); + }); + + describe('value mutation method', function() { it('should not update numeric values when the input.value is loosely the same', function() { + var stubNode = document.createElement('input'); + var stubInstance = {_debugID: 1}; + ReactDOMComponentTree.precacheNode(stubInstance, stubNode); + stubNode.setAttribute('type', 'number'); stubNode.setAttribute('value', '3e1'); // Keep in sync. The comparison occurs between setAttribute and value. @@ -307,6 +314,34 @@ describe('DOMPropertyOperations', () => { expect(stubNode.setAttribute.calls.count()).toBe(0); }); + + 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', () => { From 2af7e153a6b59c333f8158c0fedd31f07569e3c2 Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Mon, 5 Sep 2016 10:12:19 -0400 Subject: [PATCH 08/31] Fix case where empty switches to zero --- src/renderers/dom/shared/HTMLDOMPropertyConfig.js | 13 +++++++++---- .../shared/__tests__/DOMPropertyOperations-test.js | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index 71ecec19c4046..0dadbd6333daf 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -211,22 +211,27 @@ var HTMLDOMPropertyConfig = { }, DOMPropertyNames: {}, DOMMutationMethods: { - value: function (node, value) { + value: function(node, value) { if (value == null) { return node.removeAttribute('value'); } if (node.hasAttribute('value')) { + if (value === 0) { + // Since we use loose type checking below, zero is + // falsy, so we need to manually cover it + value = '0'; + } // Use loose coercion to prevent replacement on comparisons like // '3e1' == 30 in Chrome (~52). if (node.value == value) { // eslint-disable-line - return; + return false; } } 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 a83cb46a57509..acef256a0aa6c 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js @@ -320,7 +320,7 @@ describe('DOMPropertyOperations', () => { var stubInstance = {_debugID: 1}; ReactDOMComponentTree.precacheNode(stubInstance, stubNode); - stubNode.setAttribute('type', 'radio') + stubNode.setAttribute('type', 'radio'); DOMPropertyOperations.setValueForProperty(stubNode, 'value', ''); spyOn(stubNode, 'setAttribute'); From a00150e8d685d289cf49316da3530baa7fd3416e Mon Sep 17 00:00:00 2001 From: Nate Hunzaker Date: Mon, 5 Sep 2016 10:37:47 -0400 Subject: [PATCH 09/31] Handle zero case in controlled input --- .../shared/wrappers/__tests__/ReactDOMInput-test.js | 11 +++++++++++ .../dom/stack/client/wrappers/ReactDOMInput.js | 5 +++++ 2 files changed, 16 insertions(+) diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index c16b2f578a2b0..6e13328571ea9 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -423,6 +423,17 @@ describe('ReactDOMInput', () => { expect(node.value).toBe('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 have the correct target value', () => { var handled = false; var handler = function(event) { diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index a0be9eec3affb..71a968362dac4 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -188,6 +188,11 @@ var ReactDOMInput = { var node = ReactDOMComponentTree.getNodeFromInstance(inst); var value = props.value; if (value != null) { + if (value === 0) { + // Since we use loose type checking below, zero is + // falsy, so we need to manually cover it + value = '0'; + } // Use loose coercion to prevent replacement on comparisons like // '3e1' == 30 in Chrome (~52). if (value != node.value) { // eslint-disable-line From 6e8c818befda60331059be343c3b5972457dcd11 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Tue, 18 Oct 2016 19:31:53 -0400 Subject: [PATCH 10/31] Correct mistake with default value assignment after rebase --- src/renderers/dom/stack/client/wrappers/ReactDOMInput.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index 71a968362dac4..9125c96d25fc6 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -211,7 +211,7 @@ var ReactDOMInput = { // // https://github.com/facebook/react/issues/7253 if (node.defaultValue !== '' + props.defaultValue) { - node.defaultValue = !!props.defaultValue; + node.defaultValue = '' + props.defaultValue; } } if (props.checked == null && props.defaultChecked != null) { From 60eef17dac76f62b19991d62305be2df45177f3c Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Wed, 19 Oct 2016 23:05:40 -0400 Subject: [PATCH 11/31] Do not assign bad input to number input --- .../dom/shared/HTMLDOMPropertyConfig.js | 17 ++++------------- .../__tests__/DOMPropertyOperations-test.js | 17 ----------------- 2 files changed, 4 insertions(+), 30 deletions(-) diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index 0dadbd6333daf..a54ec53e0b5d5 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -216,20 +216,11 @@ var HTMLDOMPropertyConfig = { return node.removeAttribute('value'); } - if (node.hasAttribute('value')) { - if (value === 0) { - // Since we use loose type checking below, zero is - // falsy, so we need to manually cover it - value = '0'; - } - // Use loose coercion to prevent replacement on comparisons like - // '3e1' == 30 in Chrome (~52). - if (node.value == value) { // eslint-disable-line - return false; - } + // Don't update number inputs inputs if they have bad input. + // Chrome clears bad input and drops trailing decimal places + if (node.type !== 'number' || node.validity.badInput === false) { + node.setAttribute('value', '' + value); } - - node.setAttribute('value', '' + value); }, }, }; diff --git a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js index acef256a0aa6c..0ae91d7d91cc5 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js @@ -298,23 +298,6 @@ describe('DOMPropertyOperations', () => { }); describe('value mutation method', function() { - it('should not update numeric values when the input.value is loosely the same', function() { - var stubNode = document.createElement('input'); - var stubInstance = {_debugID: 1}; - ReactDOMComponentTree.precacheNode(stubInstance, stubNode); - - stubNode.setAttribute('type', 'number'); - stubNode.setAttribute('value', '3e1'); - // Keep in sync. The comparison occurs between setAttribute and value. - stubNode.value = '3e1'; - - spyOn(stubNode, 'setAttribute'); - - DOMPropertyOperations.setValueForProperty(stubNode, 'value', 30); - - expect(stubNode.setAttribute.calls.count()).toBe(0); - }); - it('should update an empty attribute to zero', function() { var stubNode = document.createElement('input'); var stubInstance = {_debugID: 1}; From 7bd5551b2febb71b0154c2a51971e60c20aa6783 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Wed, 19 Oct 2016 23:39:23 -0400 Subject: [PATCH 12/31] Only trigger number input value attribute updates on blur --- .../dom/shared/HTMLDOMPropertyConfig.js | 19 ++++++- .../stack/client/wrappers/ReactDOMInput.js | 57 ++++++++++++------- 2 files changed, 52 insertions(+), 24 deletions(-) diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index a54ec53e0b5d5..f84719bc1c8fd 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -216,9 +216,22 @@ var HTMLDOMPropertyConfig = { return node.removeAttribute('value'); } - // Don't update number inputs inputs if they have bad input. - // Chrome clears bad input and drops trailing decimal places - if (node.type !== 'number' || node.validity.badInput === false) { + // 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.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); } }, diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index 9125c96d25fc6..367a8edd7297b 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -49,27 +49,24 @@ var ReactDOMInput = { var value = props.value; var checked = props.checked; - var hostProps = Object.assign( - { - // Make sure we set .type before any other properties (setting .value - // before .type means .value is lost in IE11 and below) - type: undefined, - // Make sure we set .step before .value (setting .value before .step - // means .value is rounded on mount, based upon step precision) - step: undefined, - // Make sure we set .min & .max before .value (to ensure proper order - // in corner cases such as min or max deriving from value, e.g. Issue #7170) - min: undefined, - max: undefined, - }, - props, - { - defaultChecked: undefined, - defaultValue: undefined, - value: value != null ? value : inst._wrapperState.initialValue, - checked: checked != null ? checked : inst._wrapperState.initialChecked, - }, - ); + var hostProps = Object.assign({ + // Make sure we set .type before any other properties (setting .value + // before .type means .value is lost in IE11 and below) + type: undefined, + // Make sure we set .step before .value (setting .value before .step + // means .value is rounded on mount, based upon step precision) + step: undefined, + // Make sure we set .min & .max before .value (to ensure proper order + // in corner cases such as min or max deriving from value, e.g. Issue #7170) + min: undefined, + max: undefined, + }, props, { + defaultChecked: undefined, + defaultValue: undefined, + value: value != null ? value : inst._wrapperState.initialValue, + checked: checked != null ? checked : inst._wrapperState.initialChecked, + onBlurCapture: inst._wrapperState.onBlur, + }); return hostProps; }, @@ -128,6 +125,7 @@ var ReactDOMInput = { : props.defaultChecked, initialValue: props.value != null ? props.value : defaultValue, listeners: null, + onBlur: _handleBlur.bind(inst), }; if (__DEV__) { @@ -280,6 +278,23 @@ var ReactDOMInput = { }, }; +function _handleBlur(event) { + var props = this._currentElement.props; + var value = LinkedValueUtils.getValue(props); + + if (value != null) { + DOMPropertyOperations.setValueForProperty( + ReactDOMComponentTree.getNodeFromInstance(this), + 'value', + value + ); + } + + if (props.onBlur) { + return props.onBlur.call(undefined, event); + } +} + function updateNamedCousins(thisInstance, props) { var name = props.name; if (props.type === 'radio' && name != null) { From 36459b9a873f5901852a2a36e4f27c1058c4a7f4 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Sat, 19 Nov 2016 12:07:23 -0500 Subject: [PATCH 13/31] Remove reference to LinkedValueUtils --- src/renderers/dom/stack/client/wrappers/ReactDOMInput.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index 367a8edd7297b..40e1dafee468b 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -280,7 +280,7 @@ var ReactDOMInput = { function _handleBlur(event) { var props = this._currentElement.props; - var value = LinkedValueUtils.getValue(props); + var value = props.value if (value != null) { DOMPropertyOperations.setValueForProperty( From b642d924f9e9b4fe94a85ce3d89f2451a12c8906 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Sun, 20 Nov 2016 16:40:24 -0500 Subject: [PATCH 14/31] Record new fiber tests --- scripts/fiber/tests-passing.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 16aab7d56eaaf..41907b1f5a5f4 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,7 @@ 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 transition from an empty value to 0 * should have the correct target value * should not set a value for submit buttons unnecessarily * should control radio buttons From 99e87b8a951533e2c676842a4a69d40d03ef5a20 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Wed, 14 Dec 2016 18:36:33 -0500 Subject: [PATCH 15/31] Add tests for blurred number input behavior --- .../wrappers/__tests__/ReactDOMInput-test.js | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index 6e13328571ea9..57432f0b2c5bd 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -1085,4 +1085,61 @@ 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); + + node.focus(); + + ReactTestUtils.Simulate.change(node, { target: { value: '2'} }); + ReactTestUtils.SimulateNative.blur(node) + + expect(node.getAttribute('value')).toBe('2'); + }); + }); + }); From b2085f45be847eac7994a84ff087ac9a2eef1ddd Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Wed, 14 Dec 2016 19:28:00 -0500 Subject: [PATCH 16/31] Replace onBlur wrapper with rule in ChangeEventPlugin --- .../dom/shared/HTMLDOMPropertyConfig.js | 2 +- .../shared/eventPlugins/ChangeEventPlugin.js | 5 +++++ .../wrappers/__tests__/ReactDOMInput-test.js | 16 +++++++--------- .../stack/client/wrappers/ReactDOMInput.js | 19 ------------------- 4 files changed, 13 insertions(+), 29 deletions(-) diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index f84719bc1c8fd..9bd22fd905a26 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -219,7 +219,7 @@ var HTMLDOMPropertyConfig = { // 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') { + if (node.type !== 'number' || node.hasAttribute('value') === false) { node.setAttribute('value', '' + value); } else if (node.validity && !node.validity.badInput && diff --git a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js index 4a026bcbea57e..a9abc70c1d0bd 100644 --- a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js +++ b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js @@ -316,6 +316,11 @@ var ChangeEventPlugin = { if (handleEventFunc) { handleEventFunc(topLevelType, targetNode, targetInst); } + + // When blurring, set the value attribute for number inputs + if (topLevelType === 'topBlur') { + targetNode.setAttribute('value', targetNode.value); + } }, }; diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index 57432f0b2c5bd..05803841eaf4e 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -1091,7 +1091,7 @@ describe('ReactDOMInput', () => { return React.createClass({ getInitialState: function() { return { - value: this.props.value == null ? '' : this.props.value + value: this.props.value == null ? '' : this.props.value, }; }, onChange: function(event) { @@ -1102,22 +1102,22 @@ describe('ReactDOMInput', () => { var value = this.state.value; return (); - } + }, }); } it('always sets the attribute when values change on text inputs', function() { - var Input = getTestInput() + 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 Input = getTestInput(); var stub = ReactTestUtils.renderIntoDocument(); var node = ReactDOM.findDOMNode(stub); @@ -1129,14 +1129,12 @@ describe('ReactDOMInput', () => { }); it('sets the value attribute on number inputs on blur', () => { - var Input = getTestInput() + var Input = getTestInput(); var stub = ReactTestUtils.renderIntoDocument(); var node = ReactDOM.findDOMNode(stub); - node.focus(); - ReactTestUtils.Simulate.change(node, { target: { value: '2'} }); - ReactTestUtils.SimulateNative.blur(node) + ReactTestUtils.SimulateNative.blur(node); expect(node.getAttribute('value')).toBe('2'); }); diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index 40e1dafee468b..5a22750d154f2 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -65,7 +65,6 @@ var ReactDOMInput = { defaultValue: undefined, value: value != null ? value : inst._wrapperState.initialValue, checked: checked != null ? checked : inst._wrapperState.initialChecked, - onBlurCapture: inst._wrapperState.onBlur, }); return hostProps; @@ -125,7 +124,6 @@ var ReactDOMInput = { : props.defaultChecked, initialValue: props.value != null ? props.value : defaultValue, listeners: null, - onBlur: _handleBlur.bind(inst), }; if (__DEV__) { @@ -278,23 +276,6 @@ var ReactDOMInput = { }, }; -function _handleBlur(event) { - var props = this._currentElement.props; - var value = props.value - - if (value != null) { - DOMPropertyOperations.setValueForProperty( - ReactDOMComponentTree.getNodeFromInstance(this), - 'value', - value - ); - } - - if (props.onBlur) { - return props.onBlur.call(undefined, event); - } -} - function updateNamedCousins(thisInstance, props) { var name = props.name; if (props.type === 'radio' && name != null) { From b0bc9d48f66a1f373a0726b493c8168fbcf3687e Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Wed, 14 Dec 2016 19:34:33 -0500 Subject: [PATCH 17/31] Sift down to only number inputs --- src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js index a9abc70c1d0bd..e537bc5b929c1 100644 --- a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js +++ b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js @@ -318,7 +318,7 @@ var ChangeEventPlugin = { } // When blurring, set the value attribute for number inputs - if (topLevelType === 'topBlur') { + if (topLevelType === 'topBlur' && targetNode.type === 'number') { targetNode.setAttribute('value', targetNode.value); } }, From 4fc8c2ea67f66254375d7a9fd9539d44039bb0b1 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Wed, 14 Dec 2016 19:37:56 -0500 Subject: [PATCH 18/31] Re-record fiber tests --- scripts/fiber/tests-passing.txt | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index 41907b1f5a5f4..d857ce8fd8361 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1445,6 +1445,9 @@ 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 src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js * should flatten children to a string From d410290f4e82e22c9cdf1f479f4fb6e278322bfe Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Wed, 14 Dec 2016 19:57:54 -0500 Subject: [PATCH 19/31] Add test case for updating attribute on uncontrolled inputs. Make related correction --- .../dom/shared/eventPlugins/ChangeEventPlugin.js | 14 ++++++++++++-- .../wrappers/__tests__/ReactDOMInput-test.js | 11 +++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js index e537bc5b929c1..193b29a642ec6 100644 --- a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js +++ b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js @@ -258,6 +258,16 @@ function getTargetInstForInputOrChangeEvent(topLevelType, targetInst) { } } +function handleControlledInputBlur(inst, node) { + if (node.type !== 'number') { + return; + } + + if (inst._currentElement && inst._currentElement.props.hasOwnProperty('value')) { + node.setAttribute('value', '' + node.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 @@ -318,8 +328,8 @@ var ChangeEventPlugin = { } // When blurring, set the value attribute for number inputs - if (topLevelType === 'topBlur' && targetNode.type === 'number') { - targetNode.setAttribute('value', targetNode.value); + 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 05803841eaf4e..b38b85711c5a4 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -1138,6 +1138,17 @@ describe('ReactDOMInput', () => { expect(node.getAttribute('value')).toBe('2'); }); + + it('an uncontrolled 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'); + }); }); }); From 1e696124ebeba259f47b33d3a64f6a1c05816d24 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Wed, 14 Dec 2016 20:36:48 -0500 Subject: [PATCH 20/31] Handle uncontrolled inputs, integrate fiber --- scripts/fiber/tests-passing.txt | 2 ++ .../dom/fiber/wrappers/ReactDOMFiberInput.js | 5 +---- .../dom/shared/eventPlugins/ChangeEventPlugin.js | 10 ++++++---- .../shared/wrappers/__tests__/ReactDOMInput-test.js | 13 ++++++++++++- .../dom/stack/client/wrappers/ReactDOMInput.js | 5 +---- 5 files changed, 22 insertions(+), 13 deletions(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index d857ce8fd8361..b2952e9c47d27 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1448,6 +1448,8 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js * 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..4baf7a94be487 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) { diff --git a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js index 193b29a642ec6..0017021066247 100644 --- a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js +++ b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js @@ -259,13 +259,15 @@ function getTargetInstForInputOrChangeEvent(topLevelType, targetInst) { } function handleControlledInputBlur(inst, node) { - if (node.type !== 'number') { + // Fiber and ReactDOM keep wrapper state in separate places + let state = inst._wrapperState || node._wrapperState; + + if (!state || node.type !== 'number' || !state.controlled) { return; } - if (inst._currentElement && inst._currentElement.props.hasOwnProperty('value')) { - node.setAttribute('value', '' + node.value); - } + // If controlled, assign the value attribute to the current value on blur + node.setAttribute('value', '' + node.value); } /** diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index b38b85711c5a4..f934affb9ae1d 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -1139,7 +1139,7 @@ describe('ReactDOMInput', () => { expect(node.getAttribute('value')).toBe('2'); }); - it('an uncontrolled input will not update the value attribute on blur', () => { + it('an uncontrolled number input will not update the value attribute on blur', () => { var stub = ReactTestUtils.renderIntoDocument(); var node = ReactDOM.findDOMNode(stub); @@ -1149,6 +1149,17 @@ describe('ReactDOMInput', () => { 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 5a22750d154f2..d3320599e51b0 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -124,11 +124,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) { From d1812026a1c0958d7f3709b949a66365f4c669a5 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Wed, 14 Dec 2016 20:42:36 -0500 Subject: [PATCH 21/31] Reorder boolean to mitigate DOM checks --- src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js | 2 +- src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js | 2 +- src/renderers/dom/stack/client/wrappers/ReactDOMInput.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js index 4baf7a94be487..6c7a170f6d641 100644 --- a/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js +++ b/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js @@ -138,7 +138,7 @@ var ReactDOMInput = { ? props.checked : props.defaultChecked, initialValue: props.value != null ? props.value : defaultValue, - controlled: isControlled(props) + controlled: isControlled(props), }; }, diff --git a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js index 0017021066247..96d1a31347a15 100644 --- a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js +++ b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js @@ -262,7 +262,7 @@ function handleControlledInputBlur(inst, node) { // Fiber and ReactDOM keep wrapper state in separate places let state = inst._wrapperState || node._wrapperState; - if (!state || node.type !== 'number' || !state.controlled) { + if (!state || !state.controlled || node.type !== 'number') { return; } diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index d3320599e51b0..a0feb670d89ee 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -124,7 +124,7 @@ var ReactDOMInput = { : props.defaultChecked, initialValue: props.value != null ? props.value : defaultValue, listeners: null, - controlled: isControlled(props) + controlled: isControlled(props), }; }, From 9beadf0dc65e2858cb90d729dda9dd05ad95c7bc Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Thu, 22 Dec 2016 11:21:33 -0500 Subject: [PATCH 22/31] Only assign value if it is different --- src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js index 96d1a31347a15..33e41b315cea1 100644 --- a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js +++ b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js @@ -267,7 +267,10 @@ function handleControlledInputBlur(inst, node) { } // If controlled, assign the value attribute to the current value on blur - node.setAttribute('value', '' + node.value); + let value = '' + node.value; + if (node.getAttribute('value') !== value) { + node.setAttribute('value', value); + } } /** From aa30507a6d67f1e22c969f1b23c48a8904f849e1 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Sat, 21 Jan 2017 10:12:04 -0500 Subject: [PATCH 23/31] Add number input browser test fixtures During the course of the number input fix, we uncovered many edge cases. This commit adds browser test fixtures for each of those instances. --- fixtures/dom/src/components/Header.js | 1 + fixtures/dom/src/components/fixtures/index.js | 3 + .../fixtures/number-inputs/index.js | 133 ++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100644 fixtures/dom/src/components/fixtures/number-inputs/index.js 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/index.js b/fixtures/dom/src/components/fixtures/number-inputs/index.js new file mode 100644 index 0000000000000..25a55617b285e --- /dev/null +++ b/fixtures/dom/src/components/fixtures/number-inputs/index.js @@ -0,0 +1,133 @@ +const React = window.React; + +const TestCase = React.createClass({ + getInitialState() { + return { value: '' }; + }, + onChange(event) { + this.setState({ value: event.target.value }); + }, + render() { + return ( +
+
{this.props.children}
+ +
+
+ Controlled + + Value: {JSON.stringify(this.state.value)} +
+ +
+ Uncontrolled + +
+
+
+ ); + }, +}); + +const NumberInputs = React.createClass({ + render() { + return ( +
+

Number inputs

+

+ Number inputs inconsistently assign and report the value + property depending on the browser. +

+ + +

+ The decimal place should not be lost when backspacing from + "3.1" to "3."? +

+ +
    +
  1. Type "3.1"
    2. +
  2. Press backspace, eliminating the "1"
    3. +
  3. The field should read "3.", preserving the decimal place
    4. +
+

+ 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. +

+ + + +

+ Supports decimal precision greater than 2 places +

+ +
    +
  1. Type "0.01"
    2. +
  2. The field should read "0.01"
    3. +
+

+ 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. +

+ + + +

+ Pressing "e" at the end of a number does not reset the value +

+
    +
  1. Type "3.14"
    2. +
  2. Press "e", so that the value reads "3.14e"
    3. +
  3. The field should read "3.14e"
    4. +
+

+ Notes: IE does not allow bad input typed into the end of the number. + This makes it impossible to type "3.14e". +

+ + + +

+ Pressing "ee" in the middle of a number does not clear the display value +

+
    +
  1. Type "3.14"
    2. +
  2. Move the text cursor to after the decimal place
    3. +
  3. Press "e" twice, so that the value reads "3.ee14"
    4. +
  4. The field should read "3.ee14"
    5. +
+

+ Notes: IE does not allow bad input typed into the middle of the number. + This makes it impossible to type "3.ee14". +

+ + + +

+ Typing "3.0" preserves the trailing zero +

+
    +
  1. Type "3.0"
    2. +
  2. The field should read "3.0"
    3. +
+ + + +

+ Inserting a decimal in to "300" maintains the trailing zeroes +

+
    +
  1. Type "300"
    2. +
  2. Move the cursor to after the "3"
    3. +
  3. Type "."
    4. +
  4. The field should read "3.00", not "3"
    5. +
+ +
+ ); + }, +}); + +export default NumberInputs; From c44a87a8e47e4d59a0e8881b0ffe108e9623c0fe Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Sat, 21 Jan 2017 11:00:44 -0500 Subject: [PATCH 24/31] Address edge case preventing number precision lower than 1 place 0.0 coerces to 0, however they are not the same value when doing string comparision. This prevented controlled number inputs from inputing the characters `0.00`. Also adds test cases. --- .../fixtures/number-inputs/index.js | 10 ++++----- .../wrappers/__tests__/ReactDOMInput-test.js | 21 +++++++++++++++++++ .../stack/client/wrappers/ReactDOMInput.js | 13 +++++------- 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/fixtures/dom/src/components/fixtures/number-inputs/index.js b/fixtures/dom/src/components/fixtures/number-inputs/index.js index 25a55617b285e..f25e6807f8a1f 100644 --- a/fixtures/dom/src/components/fixtures/number-inputs/index.js +++ b/fixtures/dom/src/components/fixtures/number-inputs/index.js @@ -5,7 +5,10 @@ const TestCase = React.createClass({ return { value: '' }; }, onChange(event) { - this.setState({ value: event.target.value }); + const parsed = parseFloat(event.target.value, 10) + const value = isNaN(parsed) ? '' : parsed + + this.setState({ value: value }) }, render() { return ( @@ -66,11 +69,6 @@ const NumberInputs = React.createClass({
  • Type "0.01"
  • The field should read "0.01"
    • -

    - 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. -

    diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index f934affb9ae1d..e4e6d415443ba 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -423,6 +423,16 @@ describe('ReactDOMInput', () => { expect(node.value).toBe('0'); }); + it('should properly control a value of number `0.0`', () => { + 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'); @@ -434,6 +444,17 @@ describe('ReactDOMInput', () => { 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) { diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index a0feb670d89ee..a6549344d736b 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -181,14 +181,11 @@ var ReactDOMInput = { var node = ReactDOMComponentTree.getNodeFromInstance(inst); var value = props.value; if (value != null) { - if (value === 0) { - // Since we use loose type checking below, zero is - // falsy, so we need to manually cover it - value = '0'; - } - // Use loose coercion to prevent replacement on comparisons like - // '3e1' == 30 in Chrome (~52). - if (value != node.value) { // eslint-disable-line + if (value === 0 && node.value === '') { + node.value = '0'; + } else if (value != node.value) { // eslint-disable-line + // Use loose coercion to prevent replacement on comparisons like + // '3e1' == 30 in Chrome (~52). // 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; From d0441cc5403c6fd93bd58502797fac04a941c067 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Sat, 21 Jan 2017 12:06:39 -0500 Subject: [PATCH 25/31] Accommodate lack of IE9 number input support IE9 does not support number inputs. Number inputs in IE9 fallback to traditional text inputs. This means that accessing `input.value` will report the raw text, rather than parsing a numeric value. This commit makes the ReactDOMInput wrapper check to see if the `type` prop has been configured to `"number"`. In those cases, it will perform a comparison based upon `parseFloat` instead of the raw input value. --- .../components/fixtures/number-inputs/index.js | 16 +++++++++++++++- .../wrappers/__tests__/ReactDOMInput-test.js | 12 +++++++++++- .../dom/stack/client/wrappers/ReactDOMInput.js | 12 ++++++++++-- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/fixtures/dom/src/components/fixtures/number-inputs/index.js b/fixtures/dom/src/components/fixtures/number-inputs/index.js index f25e6807f8a1f..f7434897257fc 100644 --- a/fixtures/dom/src/components/fixtures/number-inputs/index.js +++ b/fixtures/dom/src/components/fixtures/number-inputs/index.js @@ -8,7 +8,7 @@ const TestCase = React.createClass({ const parsed = parseFloat(event.target.value, 10) const value = isNaN(parsed) ? '' : parsed - this.setState({ value: value }) + this.setState({ value }) }, render() { return ( @@ -71,6 +71,20 @@ const NumberInputs = React.createClass({ + +

    + Supports exponent form ("2e4") +

    + +
      +
    1. Type "2e"
      2. +
    2. The field should read "2e"
      3. +
    3. Type 4, to read "2e4"
      4. +
    4. The field should read "2e4"
      5. +
    5. The value should read "20000"
      6. +
    +     +

    Pressing "e" at the end of a number does not reset the value diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index e4e6d415443ba..bb97d55233d77 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -423,7 +423,7 @@ describe('ReactDOMInput', () => { expect(node.value).toBe('0'); }); - it('should properly control a value of number `0.0`', () => { + it('should properly control 0.0 for a text input', () => { var stub = ; stub = ReactTestUtils.renderIntoDocument(stub); var node = ReactDOM.findDOMNode(stub); @@ -433,6 +433,16 @@ describe('ReactDOMInput', () => { 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'); diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js index a6549344d736b..f4559efb614d4 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -183,9 +183,17 @@ var ReactDOMInput = { if (value != null) { 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; + + if (value != valueAsNumber) { // eslint-disable-line + // 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 (value != node.value) { // eslint-disable-line - // Use loose coercion to prevent replacement on comparisons like - // '3e1' == 30 in Chrome (~52). // 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; From 38a583f8d44d6723f24905ab6e2dfc68072bbebe Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Sat, 21 Jan 2017 12:13:54 -0500 Subject: [PATCH 26/31] Remove footnotes about IE exponent issues With the recent IE9 fix, IE properly inserts `e` when it produces an invalid number. --- .../dom/src/components/fixtures/number-inputs/index.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/fixtures/dom/src/components/fixtures/number-inputs/index.js b/fixtures/dom/src/components/fixtures/number-inputs/index.js index f7434897257fc..1b5d38b3ba77f 100644 --- a/fixtures/dom/src/components/fixtures/number-inputs/index.js +++ b/fixtures/dom/src/components/fixtures/number-inputs/index.js @@ -94,10 +94,6 @@ const NumberInputs = React.createClass({
  • Press "e", so that the value reads "3.14e"
  • The field should read "3.14e"
    • -

    - Notes: IE does not allow bad input typed into the end of the number. - This makes it impossible to type "3.14e". -

    @@ -110,10 +106,6 @@ const NumberInputs = React.createClass({
  • Press "e" twice, so that the value reads "3.ee14"
  • The field should read "3.ee14"
    • -

    - Notes: IE does not allow bad input typed into the middle of the number. - This makes it impossible to type "3.ee14". -

    From 71d7f4529f9a09e8df7a72f97aa91d23650b3783 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Sat, 21 Jan 2017 12:21:52 -0500 Subject: [PATCH 27/31] Address exception in IE9/10 ChangeEventPlugin blur event On blur, inputs have their values assigned. This is so that number inputs do not conduct unexpected behavior in Chrome/Safari. Unfortunately, there are cases where the target instance might be undefined in IE9/10, raising an exception. --- src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js index 33e41b315cea1..1b1d5b95c065a 100644 --- a/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js +++ b/src/renderers/dom/shared/eventPlugins/ChangeEventPlugin.js @@ -259,6 +259,11 @@ 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; From 6fe3372cfccd64b1bf69593e699964aa193171f7 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Sat, 21 Jan 2017 12:37:17 -0500 Subject: [PATCH 28/31] Migrate over ReactDOMInput.js number input fixes to Fiber Also re-record tests --- scripts/fiber/tests-passing.txt | 3 +++ .../dom/fiber/wrappers/ReactDOMFiberInput.js | 21 +++++++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index b2952e9c47d27..ad888137c4114 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1411,7 +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 diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js index 6c7a170f6d641..4dc97759466e4 100644 --- a/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js +++ b/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js @@ -192,13 +192,22 @@ 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; + if (value != valueAsNumber) { // eslint-disable-line + // 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 (value != node.value) { // eslint-disable-line + // 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) { From d7f9b4e6fd4ed14bf5741c9cfb60a585981b079d Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Thu, 2 Mar 2017 20:25:35 -0500 Subject: [PATCH 29/31] Update number fixtures to use latest components --- .../fixtures/number-inputs/NumberTestCase.js | 37 ++++ .../fixtures/number-inputs/index.js | 190 +++++++++--------- 2 files changed, 132 insertions(+), 95 deletions(-) create mode 100644 fixtures/dom/src/components/fixtures/number-inputs/NumberTestCase.js 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 index 1b5d38b3ba77f..3f6f008346fdd 100644 --- a/fixtures/dom/src/components/fixtures/number-inputs/index.js +++ b/fixtures/dom/src/components/fixtures/number-inputs/index.js @@ -1,58 +1,33 @@ const React = window.React; -const TestCase = 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 - -
    -
    -
    - ); - }, -}); +import Fixture from '../../Fixture'; +import FixtureSet from '../../FixtureSet'; +import TestCase from '../../TestCase'; +import NumberTestCase from './NumberTestCase'; const NumberInputs = React.createClass({ render() { return ( -
    -

    Number inputs

    -

    - Number inputs inconsistently assign and report the value - property depending on the browser. -

    - - -

    - The decimal place should not be lost when backspacing from - "3.1" to "3."? -

    - -
      + + +
    1. Type "3.1"
    2. Press backspace, eliminating the "1"
      3. -
    3. The field should read "3.", preserving the decimal place
      4. -
    + + + + 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 @@ -60,76 +35,101 @@ const NumberInputs = React.createClass({

    - -

    - Supports decimal precision greater than 2 places -

    - -
      + +
    1. Type "0.01"
      2. -
    2. The field should read "0.01"
      3. -
    -     + + + + The field should read "0.01" + - -

    - Supports exponent form ("2e4") -

    + +     -
      + +
    1. Type "2e"
      2. -
    2. The field should read "2e"
    3. Type 4, to read "2e4"
      4. -
    4. The field should read "2e4"
      5. -
    5. The value should read "20000"
      6. -
    + + + + The field should read "2e4". The parsed value should read "20000" + + + - -

    - Pressing "e" at the end of a number does not reset the value -

    -
      + +
    1. Type "3.14"
      2. -
    2. Press "e", so that the value reads "3.14e"
      3. -
    3. The field should read "3.14e"
      4. -
    +
  • Press "e", so that the input reads "3.14e"
    • + + + + The field should read "3.14e", the parsed value should be empty + + +     - -

    - Pressing "ee" in the middle of a number does not clear the display value -

    -
      + +
    1. Type "3.14"
    2. Move the text cursor to after the decimal place
    3. Press "e" twice, so that the value reads "3.ee14"
      4. -
    4. The field should read "3.ee14"
      5. -
    + + + + The field should read "3.ee14" + + +     - -

    - Typing "3.0" preserves the trailing zero -

    -
      + +
    1. Type "3.0"
      2. -
    2. The field should read "3.0"
      3. -
    + + + + The field should read "3.0" + + +     - -

    - Inserting a decimal in to "300" maintains the trailing zeroes -

    -
      + +
    1. Type "300"
    2. Move the cursor to after the "3"
    3. Type "."
      4. -
    4. The field should read "3.00", not "3"
      5. -
    + + + + The field should read "3.00", not "3" + +     - + ); }, }); From 4a3eecae0a6ce18e0abca64c9c8ff181e1315476 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Sat, 25 Mar 2017 10:15:47 -0400 Subject: [PATCH 30/31] Add number input test case for dashes and negative numbers --- .../fixtures/number-inputs/index.js | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/fixtures/dom/src/components/fixtures/number-inputs/index.js b/fixtures/dom/src/components/fixtures/number-inputs/index.js index 3f6f008346fdd..87e651785c508 100644 --- a/fixtures/dom/src/components/fixtures/number-inputs/index.js +++ b/fixtures/dom/src/components/fixtures/number-inputs/index.js @@ -1,6 +1,5 @@ const React = window.React; -import Fixture from '../../Fixture'; import FixtureSet from '../../FixtureSet'; import TestCase from '../../TestCase'; import NumberTestCase from './NumberTestCase'; @@ -129,6 +128,36 @@ const NumberInputs = React.createClass({
    + + + +
  • Type "3"
    • +
  • Type '-'
    • +     + + + The field should read "3-", not "3" + + +     + + + +
  • Type "-"
    • +
  • Type '3'
    • +     + + + The field should read "-3". + + +     ); }, From 7cb979ff30831808231ce79c6d605bfc40212db4 Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Sat, 25 Mar 2017 10:44:49 -0400 Subject: [PATCH 31/31] Replace trailing dash test case with replace with dash Also run prettier --- .../fixtures/number-inputs/index.js | 9 ++-- .../dom/fiber/wrappers/ReactDOMFiberInput.js | 6 ++- .../dom/shared/HTMLDOMPropertyConfig.js | 8 ++-- .../__tests__/DOMPropertyOperations-test.js | 2 - .../wrappers/__tests__/ReactDOMInput-test.js | 31 ++++++++----- .../stack/client/wrappers/ReactDOMInput.js | 44 +++++++++++-------- 6 files changed, 58 insertions(+), 42 deletions(-) diff --git a/fixtures/dom/src/components/fixtures/number-inputs/index.js b/fixtures/dom/src/components/fixtures/number-inputs/index.js index 87e651785c508..2c88c333edeb3 100644 --- a/fixtures/dom/src/components/fixtures/number-inputs/index.js +++ b/fixtures/dom/src/components/fixtures/number-inputs/index.js @@ -130,16 +130,17 @@ const NumberInputs = React.createClass({
  • Type "3"
    • -
  • Type '-'
    • +
  • Select the entire value"
    • +
  • Type '-' to replace '3' with '-'
    •  - The field should read "3-", not "3" + The field should read "-", not be blank.     diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js index 4dc97759466e4..f0bbc0adcaef3 100644 --- a/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js +++ b/src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js @@ -199,12 +199,14 @@ var ReactDOMInput = { // Simulate `input.valueAsNumber`. IE9 does not support it var valueAsNumber = parseFloat(node.value, 10) || 0; - if (value != valueAsNumber) { // eslint-disable-line + // 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; } - } else if (value != node.value) { // eslint-disable-line + // 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; diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js index 9bd22fd905a26..ccd92eaf7c95c 100644 --- a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js +++ b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js @@ -221,9 +221,11 @@ var HTMLDOMPropertyConfig = { // 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) { + } 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 diff --git a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js index 0ae91d7d91cc5..5fcc92a825aab 100644 --- a/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js +++ b/src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js @@ -294,7 +294,6 @@ describe('DOMPropertyOperations', () => { // some browsers) expect(stubNode.className).toBe(''); }); - }); describe('value mutation method', function() { @@ -324,7 +323,6 @@ describe('DOMPropertyOperations', () => { expect(stubNode.setAttribute.calls.count()).toBe(2); }); - }); describe('deleteValueForProperty', () => { diff --git a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js index bb97d55233d77..62df49a453262 100644 --- a/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js @@ -429,7 +429,7 @@ describe('ReactDOMInput', () => { var node = ReactDOM.findDOMNode(stub); node.value = '0.0'; - ReactTestUtils.Simulate.change(node, { target: { value: '0.0' }}); + ReactTestUtils.Simulate.change(node, {target: {value: '0.0'}}); expect(node.value).toBe('0.0'); }); @@ -439,7 +439,7 @@ describe('ReactDOMInput', () => { var node = ReactDOM.findDOMNode(stub); node.value = '0.0'; - ReactTestUtils.Simulate.change(node, { target: { value: '0.0' }}); + ReactTestUtils.Simulate.change(node, {target: {value: '0.0'}}); expect(node.value).toBe('0.0'); }); @@ -1126,13 +1126,13 @@ describe('ReactDOMInput', () => { }; }, onChange: function(event) { - this.setState({ value: event.target.value }); + this.setState({value: event.target.value}); }, render: function() { var type = this.props.type; var value = this.state.value; - return (); + return ; }, }); } @@ -1142,36 +1142,42 @@ describe('ReactDOMInput', () => { var stub = ReactTestUtils.renderIntoDocument(); var node = ReactDOM.findDOMNode(stub); - ReactTestUtils.Simulate.change(node, { target: { value: '2'} }); + 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 stub = ReactTestUtils.renderIntoDocument( + , + ); var node = ReactDOM.findDOMNode(stub); node.focus(); - ReactTestUtils.Simulate.change(node, { target: { value: '2'} }); + 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 stub = ReactTestUtils.renderIntoDocument( + , + ); var node = ReactDOM.findDOMNode(stub); - ReactTestUtils.Simulate.change(node, { target: { value: '2'} }); + 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 stub = ReactTestUtils.renderIntoDocument( + , + ); var node = ReactDOM.findDOMNode(stub); node.value = 4; @@ -1182,7 +1188,9 @@ describe('ReactDOMInput', () => { }); it('an uncontrolled text input will not update the value attribute on blur', () => { - var stub = ReactTestUtils.renderIntoDocument(); + var stub = ReactTestUtils.renderIntoDocument( + , + ); var node = ReactDOM.findDOMNode(stub); node.value = 4; @@ -1192,5 +1200,4 @@ describe('ReactDOMInput', () => { 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 f4559efb614d4..2f30e43a26f5e 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMInput.js @@ -49,23 +49,27 @@ var ReactDOMInput = { var value = props.value; var checked = props.checked; - var hostProps = Object.assign({ - // Make sure we set .type before any other properties (setting .value - // before .type means .value is lost in IE11 and below) - type: undefined, - // Make sure we set .step before .value (setting .value before .step - // means .value is rounded on mount, based upon step precision) - step: undefined, - // Make sure we set .min & .max before .value (to ensure proper order - // in corner cases such as min or max deriving from value, e.g. Issue #7170) - min: undefined, - max: undefined, - }, props, { - defaultChecked: undefined, - defaultValue: undefined, - value: value != null ? value : inst._wrapperState.initialValue, - checked: checked != null ? checked : inst._wrapperState.initialChecked, - }); + var hostProps = Object.assign( + { + // Make sure we set .type before any other properties (setting .value + // before .type means .value is lost in IE11 and below) + type: undefined, + // Make sure we set .step before .value (setting .value before .step + // means .value is rounded on mount, based upon step precision) + step: undefined, + // Make sure we set .min & .max before .value (to ensure proper order + // in corner cases such as min or max deriving from value, e.g. Issue #7170) + min: undefined, + max: undefined, + }, + props, + { + defaultChecked: undefined, + defaultValue: undefined, + value: value != null ? value : inst._wrapperState.initialValue, + checked: checked != null ? checked : inst._wrapperState.initialChecked, + }, + ); return hostProps; }, @@ -188,12 +192,14 @@ var ReactDOMInput = { // Simulate `input.valueAsNumber`. IE9 does not support it var valueAsNumber = parseFloat(node.value, 10) || 0; - if (value != valueAsNumber) { // eslint-disable-line + // 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; } - } else if (value != node.value) { // eslint-disable-line + // 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;