@samitouri / QOS-React-2 / commits / 93fc188afb
Fork 0

The library for web and native user interfaces.

style prop improvements

Some improvements to how style={{x:y}} is handled in React: * ignores null styles, rather than setting them. Codez: var highlighted = false; <div style={{color: highlighted ? 'red' : null}} /> Before: <div style="color:;"></div> After: <div></div> Respects that 0 has no units.

CommitSyncScript committed Jun 4, 2013 at 12:15 UTC 93fc188afb016b44f0b4e464621bfca721979cc5
4 files changed +57 -13
src/domUtils/CSSProperty.js
+3 -4
@@ -19,20 +19,19 @@
19 "use strict";
20
21 /**
22 - * CSS properties for which we do not append "px".
22 + * CSS properties which accept numbers but are not in units of "px".
23 */
24 -var isNumber = {
24 +var isUnitlessNumber = {
25 fillOpacity: true,
26 fontWeight: true,
27 opacity: true,
28 orphans: true,
29 - textDecoration: true,
29 zIndex: true,
30 zoom: true
31 };
32
33 var CSSProperty = {
35 - isNumber: isNumber
34 + isUnitlessNumber: isUnitlessNumber
35 };
36
37 module.exports = CSSProperty;
src/domUtils/CSSPropertyOperations.js
+3 -3
@@ -42,7 +42,7 @@ var CSSPropertyOperations = {
42 * Undefined values are ignored so that declarative programming is easier.
43 *
44 * @param {object} styles
45 - * @return {string}
45 + * @return {?string}
46 */
47 createMarkupForStyles: function(styles) {
48 var serialized = '';
@@ -51,12 +51,12 @@ var CSSPropertyOperations = {
51 continue;
52 }
53 var styleValue = styles[styleName];
54 - if (typeof styleValue !== 'undefined') {
54 + if (styleValue != null) {
55 serialized += processStyleName(styleName) + ':';
56 serialized += dangerousStyleValue(styleName, styleValue) + ';';
57 }
58 }
59 - return serialized;
59 + return serialized || null;
60 },
61
62 /**
src/domUtils/__tests__/CSSPropertyOperations-test.js
+40 -1
@@ -19,6 +19,8 @@
19
20 "use strict";
21
22 +var React = require('React');
23 +
24 describe('CSSPropertyOperations', function() {
25 var CSSPropertyOperations;
26
@@ -41,12 +43,49 @@ describe('CSSPropertyOperations', function() {
43 })).toBe('display:none;');
44 });
45
46 + it('should ignore null styles', function() {
47 + expect(CSSPropertyOperations.createMarkupForStyles({
48 + backgroundColor: null,
49 + display: 'none'
50 + })).toBe('display:none;');
51 + });
52 +
53 + it('should return null for no styles', function() {
54 + expect(CSSPropertyOperations.createMarkupForStyles({
55 + backgroundColor: null,
56 + display: null
57 + })).toBe(null);
58 + });
59 +
60 it('should automatically append `px` to relevant styles', function() {
61 expect(CSSPropertyOperations.createMarkupForStyles({
62 + left: 0,
63 margin: 16,
64 opacity: 0.5,
65 padding: '4px'
49 - })).toBe('margin:16px;opacity:0.5;padding:4px;');
66 + })).toBe('left:0;margin:16px;opacity:0.5;padding:4px;');
67 + });
68 +
69 + it('should set style attribute when styles exist', function() {
70 + var styles = {
71 + backgroundColor: '#000',
72 + display: 'none'
73 + };
74 + var div = <div style={styles} />;
75 + var root = document.createElement('div');
76 + React.renderComponent(div, root);
77 + expect(/style=".*"/.test(root.innerHTML)).toBe(true);
78 + });
79 +
80 + it('should not set style attribute when no styles exist', function() {
81 + var styles = {
82 + backgroundColor: null,
83 + display: null
84 + };
85 + var div = <div style={styles} />;
86 + var root = document.createElement('div');
87 + React.renderComponent(div, root);
88 + expect(/style=".*"/.test(root.innerHTML)).toBe(false);
89 });
90
91 });
src/domUtils/dangerousStyleValue.js
+11 -5
@@ -23,7 +23,8 @@ var CSSProperty = require('CSSProperty');
23
24 /**
25 * Convert a value into the proper css writable value. The `styleName` name
26 - * name should be logical (no hyphens), as specified in `CSSProperty.isNumber`.
26 + * name should be logical (no hyphens), as specified
27 + * in `CSSProperty.isUnitlessNumber`.
28 *
29 * @param {string} styleName CSS property name such as `topMargin`.
30 * @param {*} value CSS property value such as `10px`.
@@ -39,13 +40,18 @@ function dangerousStyleValue(styleName, value) {
40 // This is not an XSS hole but instead a potential CSS injection issue
41 // which has lead to a greater discussion about how we're going to
42 // trust URLs moving forward. See #2115901
42 - if (value === null || value === false || value === true || value === '') {
43 +
44 + var isEmpty = value == null || typeof value === 'boolean' || value === '';
45 + if (isEmpty) {
46 return '';
47 }
45 - if (isNaN(value)) {
46 - return !value ? '' : '' + value;
48 +
49 + var isNonNumeric = isNaN(value);
50 + if (isNonNumeric || value === 0 || CSSProperty.isUnitlessNumber[styleName]) {
51 + return '' + value; // cast to string
52 }
48 - return CSSProperty.isNumber[styleName] ? '' + value : (value + 'px');
53 +
54 + return value + 'px';
55 }
56
57 module.exports = dangerousStyleValue;