From e6f40dc2d25ed76e2bc356396515666128a3abea Mon Sep 17 00:00:00 2001 From: Linmic Date: Thu, 20 Aug 2015 00:58:08 +0800 Subject: [PATCH 1/4] fixed the issue that inline-styles updating could lose style properties with new test cases --- src/renderers/dom/shared/ReactDOMComponent.js | 5 ++-- .../__tests__/ReactDOMComponent-test.js | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index 4301b3438e511..d4f6932c36dae 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -884,10 +884,9 @@ ReactDOMComponent.Mixin = { styleUpdates[styleName] = ''; } } - // Update styles that changed since `lastProp`. + // Update styles that changed and keep the unchanged ones since `lastProp`. for (styleName in nextProp) { - if (nextProp.hasOwnProperty(styleName) && - lastProp[styleName] !== nextProp[styleName]) { + if (nextProp.hasOwnProperty(styleName)) { styleUpdates = styleUpdates || {}; styleUpdates[styleName] = nextProp[styleName]; } diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js index 55bb5c574896f..223692759fa3c 100644 --- a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js +++ b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js @@ -100,6 +100,32 @@ describe('ReactDOMComponent', function() { expect(stubStyle.display).toEqual('block'); expect(stubStyle.fontFamily).toEqual('Helvetica'); expect(stubStyle.lineHeight).toEqual('0.5'); + + var node = ReactTestUtils.renderIntoDocument(
rhinoceros
); + + var oldStyle = { + 'background': 'url(http://abc.com/a.jpg) no-repeat center', + 'backgroundSize': 'cover', + 'backgroundColor': 'red', + }; + + node.setProps({ style: oldStyle }); + + expect(node.getDOMNode().style.background).toBe(oldStyle.background); + expect(node.getDOMNode().style.backgroundSize).toBe('cover'); + expect(node.getDOMNode().style.backgroundColor).toBe('red'); + + var newStyle = { + 'background': 'url(http://abc.com/b.jpg) no-repeat center', + 'backgroundSize': 'cover', + 'backgroundColor': 'red', + }; + + node.setProps({ style: newStyle }); + + expect(node.getDOMNode().style.background).toBe(newStyle.background); + expect(node.getDOMNode().style.backgroundSize).toBe('cover'); + expect(node.getDOMNode().style.backgroundColor).toBe('red'); ReactDOM.render(
, container); expect(stubStyle.display).toBe(''); From bd618189c16e41169fdd0b76df979676792b9a02 Mon Sep 17 00:00:00 2001 From: Linmic Date: Thu, 20 Aug 2015 15:39:47 +0800 Subject: [PATCH 2/4] set all styles regardless of they changed or not if either or has shorthand properties --- src/renderers/dom/shared/CSSProperty.js | 42 +++++++++++++++++++ src/renderers/dom/shared/ReactDOMComponent.js | 23 +++++++--- 2 files changed, 60 insertions(+), 5 deletions(-) diff --git a/src/renderers/dom/shared/CSSProperty.js b/src/renderers/dom/shared/CSSProperty.js index 143b641cf9fd1..791d80a332f8c 100644 --- a/src/renderers/dom/shared/CSSProperty.js +++ b/src/renderers/dom/shared/CSSProperty.js @@ -120,9 +120,51 @@ var shorthandPropertyExpansions = { }, }; +var shorthandProperties = [ + 'background', + 'font', + 'margin', + 'border', + 'borderTop', + 'borderRight', + 'borderBottom', + 'borderLeft', + 'borderWidth', + 'borderColor', + 'borderStyle', + 'transition', + 'WebkitTransition', + 'MozTransition', + 'OTransition', + 'msTransition', + 'transition', + 'WebkitTransform', + 'MozTransform', + 'OTransform', + 'msTransform', + 'transform', + 'padding', + 'listStyle', + 'borderRadius', +]; + +/** + * @param {object} style object to be examined if it contains shorthand property + */ +function hasShorthandProperty(styles) { + for(var styleName in styles) { + if(shorthandProperties.indexOf(styleName)) { + return true; + } + } + + return false; +} + var CSSProperty = { isUnitlessNumber: isUnitlessNumber, shorthandPropertyExpansions: shorthandPropertyExpansions, + hasShorthandProperty: hasShorthandProperty, }; module.exports = CSSProperty; diff --git a/src/renderers/dom/shared/ReactDOMComponent.js b/src/renderers/dom/shared/ReactDOMComponent.js index d4f6932c36dae..fff57a332eb78 100644 --- a/src/renderers/dom/shared/ReactDOMComponent.js +++ b/src/renderers/dom/shared/ReactDOMComponent.js @@ -15,6 +15,7 @@ 'use strict'; var AutoFocusUtils = require('AutoFocusUtils'); +var CSSProperty = require('CSSProperty'); var CSSPropertyOperations = require('CSSPropertyOperations'); var DOMProperty = require('DOMProperty'); var DOMPropertyOperations = require('DOMPropertyOperations'); @@ -884,11 +885,23 @@ ReactDOMComponent.Mixin = { styleUpdates[styleName] = ''; } } - // Update styles that changed and keep the unchanged ones since `lastProp`. - for (styleName in nextProp) { - if (nextProp.hasOwnProperty(styleName)) { - styleUpdates = styleUpdates || {}; - styleUpdates[styleName] = nextProp[styleName]; + // Update only styles that changed since `lastProp` + // Unless either `nextProp` or `lastProp` has shorthand properties + if (!CSSProperty.hasShorthandProperty(lastProp) && + !CSSProperty.hasShorthandProperty(nextProp)) { + for (styleName in nextProp) { + if (nextProp.hasOwnProperty(styleName) && + lastProp[styleName] !== nextProp[styleName]) { + styleUpdates = styleUpdates || {}; + styleUpdates[styleName] = nextProp[styleName]; + } + } + } else { + for (styleName in nextProp) { + if (nextProp.hasOwnProperty(styleName)) { + styleUpdates = styleUpdates || {}; + styleUpdates[styleName] = nextProp[styleName]; + } } } } else { From c827fd6714502c956821d3f190e229b71f4052c6 Mon Sep 17 00:00:00 2001 From: Linmic Date: Thu, 20 Aug 2015 15:43:38 +0800 Subject: [PATCH 3/4] lint up --- src/renderers/dom/shared/CSSProperty.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/renderers/dom/shared/CSSProperty.js b/src/renderers/dom/shared/CSSProperty.js index 791d80a332f8c..721cd7103a0bf 100644 --- a/src/renderers/dom/shared/CSSProperty.js +++ b/src/renderers/dom/shared/CSSProperty.js @@ -152,8 +152,8 @@ var shorthandProperties = [ * @param {object} style object to be examined if it contains shorthand property */ function hasShorthandProperty(styles) { - for(var styleName in styles) { - if(shorthandProperties.indexOf(styleName)) { + for (var styleName in styles) { + if (shorthandProperties.indexOf(styleName)) { return true; } } From 11848dde8cc798b4973e72bb1547f7b750650fef Mon Sep 17 00:00:00 2001 From: Linmic Date: Thu, 20 Aug 2015 22:22:01 +0800 Subject: [PATCH 4/4] fixed typo --- src/renderers/dom/shared/CSSProperty.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/renderers/dom/shared/CSSProperty.js b/src/renderers/dom/shared/CSSProperty.js index 721cd7103a0bf..2495006663eba 100644 --- a/src/renderers/dom/shared/CSSProperty.js +++ b/src/renderers/dom/shared/CSSProperty.js @@ -153,7 +153,7 @@ var shorthandProperties = [ */ function hasShorthandProperty(styles) { for (var styleName in styles) { - if (shorthandProperties.indexOf(styleName)) { + if (shorthandProperties.indexOf(styleName) > -1) { return true; } }