Skip to content

Commit

Permalink
Fix styles to prevent subtle diffing bugs, add support for vendor pre…
Browse files Browse the repository at this point in the history
…fixing
  • Loading branch information
jimfb authored and jim committed May 4, 2016
1 parent c9504d9 commit c4463a1
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 23 deletions.
5 changes: 5 additions & 0 deletions src/isomorphic/React.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

'use strict';

var CSSPropertyOperations = require('CSSPropertyOperations');
var ReactChildren = require('ReactChildren');
var ReactComponent = require('ReactComponent');
var ReactClass = require('ReactClass');
Expand Down Expand Up @@ -62,6 +63,10 @@ var React = {
only: onlyChild,
},

CSS: {
multi: CSSPropertyOperations.multi,
},

Component: ReactComponent,

createElement: createElement,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,9 @@ describe('ReactNativeOperationHistoryDevtool', () => {
if (ReactDOMFeatureFlags.useCreateElement) {
assertHistoryMatches([{
instanceID: inst._debugID,
type: 'update styles',
type: 'update attribute',
payload: {
color: 'red',
backgroundColor: 'yellow',
style: 'color:red;background-color:yellow;',
},
}, {
instanceID: inst._debugID,
Expand Down Expand Up @@ -130,20 +129,20 @@ describe('ReactNativeOperationHistoryDevtool', () => {

assertHistoryMatches([{
instanceID: inst._debugID,
type: 'update styles',
payload: { color: 'red' },
type: 'update attribute',
payload: { style: 'color:red;' },
}, {
instanceID: inst._debugID,
type: 'update styles',
payload: { color: 'blue', backgroundColor: 'yellow' },
type: 'update attribute',
payload: { style: 'color:blue;background-color:yellow;' },
}, {
instanceID: inst._debugID,
type: 'update styles',
payload: { color: '', backgroundColor: 'green' },
type: 'update attribute',
payload: { style: 'background-color:green;' },
}, {
instanceID: inst._debugID,
type: 'update styles',
payload: { backgroundColor: '' },
type: 'update attribute',
payload: { style: null },
}]);
});

Expand All @@ -164,10 +163,9 @@ describe('ReactNativeOperationHistoryDevtool', () => {

assertHistoryMatches([{
instanceID: inst._debugID,
type: 'update styles',
type: 'update attribute',
payload: {
color: 'red',
backgroundColor: 'yellow',
style: 'color:red;background-color:yellow;',
},
}]);
});
Expand Down
29 changes: 23 additions & 6 deletions src/renderers/dom/shared/CSSPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,13 +172,26 @@ var CSSPropertyOperations = {
continue;
}
var styleValue = styles[styleName];
if (__DEV__) {
warnValidStyle(styleName, styleValue, component);
}
var processedStyleName = processStyleName(styleName);
if (styleValue != null) {
serialized += processStyleName(styleName) + ':';
serialized +=
dangerousStyleValue(styleName, styleValue, component) + ';';
if (styleValue.__isMultiCssValue) {
for (var i = 0; i < styleValue.length; i++) {
var elem = styleValue[i];
if (__DEV__) {
warnValidStyle(styleName, elem, component);
}
serialized += processedStyleName + ':';
serialized +=
dangerousStyleValue(styleName, elem, component) + ';';
}
} else {
if (__DEV__) {
warnValidStyle(styleName, styleValue, component);
}
serialized += processedStyleName + ':';
serialized +=
dangerousStyleValue(styleName, styleValue, component) + ';';
}
}
}
return serialized || null;
Expand Down Expand Up @@ -236,6 +249,10 @@ var CSSPropertyOperations = {
}
},

multi: function() {
arguments.__isMultiCssValue = true;
return arguments;
},
};

ReactPerf.measureMethods(CSSPropertyOperations, 'CSSPropertyOperations', {
Expand Down
11 changes: 8 additions & 3 deletions src/renderers/dom/shared/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ var ReactPerf = require('ReactPerf');
var ReactServerRenderingTransaction = require('ReactServerRenderingTransaction');

var emptyFunction = require('emptyFunction');
var emptyObject = require('emptyObject');
var escapeTextContentForBrowser = require('escapeTextContentForBrowser');
var invariant = require('invariant');
var isEventSupported = require('isEventSupported');
Expand Down Expand Up @@ -992,10 +993,14 @@ ReactDOMComponent.Mixin = {
}
}
if (styleUpdates) {
CSSPropertyOperations.setValueForStyles(
var nextStyles = emptyObject;
if (nextProps != null && nextProps[STYLE] != null) {
nextStyles = nextProps[STYLE];
}
DOMPropertyOperations.setValueForAttribute(
getNode(this),
styleUpdates,
this
'style',
CSSPropertyOperations.createMarkupForStyles(nextStyles, this)
);
}
},
Expand Down
19 changes: 19 additions & 0 deletions src/renderers/dom/shared/__tests__/CSSPropertyOperations-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,25 @@ describe('CSSPropertyOperations', function() {
);
});

it('should allow array of vendor prefixed values', function() {
var styles = {
padding: 0,
margin: 0,
listStyle: 'none',
display: React.CSS.multi('-webkit-box', '-moz-box', '-ms-flexbox', '-webkit-flex'),
WebkitFlexFlow: 'row wrap',
justifyContent: 'space-around',
};

var markup = ReactDOMServer.renderToString(
<div style={styles} />
);

expect(markup).toContain('-ms-flexbox');
expect(markup).toContain('-webkit-flex');
expect(markup).toContain('-webkit-box');
});

it('should warn about style having a trailing semicolon', function() {
var Comp = React.createClass({
displayName: 'Comp',
Expand Down

0 comments on commit c4463a1

Please sign in to comment.