From 8a565146d54a4fad17456363cde5997d651779a5 Mon Sep 17 00:00:00 2001 From: Christopher Chedeau Date: Tue, 16 Sep 2014 08:51:27 -0700 Subject: [PATCH] [React] Add support for arrays with holes in style attribute Summary: See the style proposal on react-future: https://github.com/reactjs/react-future/blob/master/04%20-%20Layout/Inline%20Style%20Extension.md The goal is to enable writing CSS purely in JavaScript. We've been using this for a couple of internal projects and it's proven to be a very expressive tool. Note that this diff doesn't bring `StyleSheet.create` nor `StylePropType`. I'm keeping it simple by just allowing arbitrary arrays, objects and falsy values. Test Plan: ``` grunt test ``` Reviewers: jwalke zpao sema CC: tomocchino --- src/browser/ui/ReactDOMComponent.js | 6 +- .../ui/__tests__/ReactDOMComponent-test.js | 15 +++ src/browser/ui/__tests__/flattenStyle-test.js | 112 ++++++++++++++++++ src/browser/ui/flattenStyle.js | 50 ++++++++ 4 files changed, 180 insertions(+), 3 deletions(-) create mode 100644 src/browser/ui/__tests__/flattenStyle-test.js create mode 100644 src/browser/ui/flattenStyle.js diff --git a/src/browser/ui/ReactDOMComponent.js b/src/browser/ui/ReactDOMComponent.js index 276be2e93b84..a4f1a695a907 100644 --- a/src/browser/ui/ReactDOMComponent.js +++ b/src/browser/ui/ReactDOMComponent.js @@ -33,7 +33,7 @@ var escapeTextForBrowser = require('escapeTextForBrowser'); var invariant = require('invariant'); var isEventSupported = require('isEventSupported'); var keyOf = require('keyOf'); -var merge = require('merge'); +var flattenStyle = require('flattenStyle'); var mixInto = require('mixInto'); var monitorCodeUse = require('monitorCodeUse'); @@ -163,7 +163,7 @@ ReactDOMComponent.Mixin = { } else { if (propKey === STYLE) { if (propValue) { - propValue = props.style = merge(props.style); + propValue = props.style = flattenStyle(props.style); } propValue = CSSPropertyOperations.createMarkupForStyles(propValue); } @@ -312,7 +312,7 @@ ReactDOMComponent.Mixin = { } if (propKey === STYLE) { if (nextProp) { - nextProp = nextProps.style = merge(nextProp); + nextProp = nextProps.style = flattenStyle(nextProp); } if (lastProp) { // Unset styles on `lastProp` but not on `nextProp`. diff --git a/src/browser/ui/__tests__/ReactDOMComponent-test.js b/src/browser/ui/__tests__/ReactDOMComponent-test.js index ef971bbac611..8ddb010cacc8 100644 --- a/src/browser/ui/__tests__/ReactDOMComponent-test.js +++ b/src/browser/ui/__tests__/ReactDOMComponent-test.js @@ -104,6 +104,21 @@ describe('ReactDOMComponent', function() { expect(stubStyle.lineHeight).toBe(''); }); + it("should update styles with nested arrays and holes", function() { + var style = [ + null, undefined, + { display: 'block', color: 'red' }, + false, '', + [ { color: 'blue', fontFamily: 'Arial' } ], + [ [], false ], + ]; + var stub = ReactTestUtils.renderIntoDocument(
); + var stubStyle = stub.getDOMNode().style; + expect(stubStyle.display).toEqual('block'); + expect(stubStyle.color).toEqual('blue'); + expect(stubStyle.fontFamily).toEqual('Arial'); + }); + it("should update styles if initially null", function() { var styles = null; var stub = ReactTestUtils.renderIntoDocument(
); diff --git a/src/browser/ui/__tests__/flattenStyle-test.js b/src/browser/ui/__tests__/flattenStyle-test.js new file mode 100644 index 000000000000..6b7ccf54c07d --- /dev/null +++ b/src/browser/ui/__tests__/flattenStyle-test.js @@ -0,0 +1,112 @@ +/** + * Copyright 2013-2014 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * @jsx React.DOM + * @emails react-core + */ + +"use strict"; + +require('mock-modules').dontMock('flattenStyle'); + +var flattenStyle = require('flattenStyle'); + +describe('StyleSheet', () => { + var moduleA = { + elementA: { + styleA: 'moduleA/elementA/styleA', + styleB: 'moduleA/elementA/styleB' + }, + elementB: { + styleB: 'moduleA/elementB/styleB' + } + }; + + it('should not allocate an object when there is no style', () => { + var nullStyle = flattenStyle(null); + var nullStyleAgain = flattenStyle(null); + + expect(nullStyle).toBe(undefined); + expect(nullStyleAgain).toBe(undefined); + }); + + it('should allocate an object when there is a style', () => { + var style = {a: 'b'}; + var nullStyle = flattenStyle(style); + + expect(nullStyle).not.toBe(style); + }); + + it('should allocate an object when there is a single class', () => { + var singleStyle = flattenStyle(moduleA.elementA); + var singleStyleAgain = flattenStyle(moduleA.elementA); + + expect(singleStyle).not.toBe(singleStyleAgain); + expect(singleStyle).toEqual({ + styleA: 'moduleA/elementA/styleA', + styleB: 'moduleA/elementA/styleB' + }); + }); + + it('should merge single class and style properly', () => { + var style = {styleA: 'overrideA', styleC: 'overrideC'}; + var arrayStyle = flattenStyle([moduleA.elementA, style]); + + expect(arrayStyle).toEqual({ + styleA: 'overrideA', + styleB: 'moduleA/elementA/styleB', + styleC: 'overrideC' + }); + }); + + it('should merge multiple classes', () => { + var AthenB = flattenStyle([moduleA.elementA, moduleA.elementB]); + var BthenA = flattenStyle([moduleA.elementB, moduleA.elementA]); + + expect(AthenB).toEqual({ + styleA: 'moduleA/elementA/styleA', + styleB: 'moduleA/elementB/styleB', + }); + expect(BthenA).toEqual({ + styleA: 'moduleA/elementA/styleA', + styleB: 'moduleA/elementA/styleB', + }); + }); + + it('should merge multiple classes with style', () => { + var style = {styleA: 'overrideA'}; + var AthenB = flattenStyle([moduleA.elementA, moduleA.elementB, style]); + var BthenA = flattenStyle([moduleA.elementB, moduleA.elementA, style]); + + expect(AthenB).toEqual({ + styleA: 'overrideA', + styleB: 'moduleA/elementB/styleB', + }); + expect(BthenA).toEqual({ + styleA: 'overrideA', + styleB: 'moduleA/elementA/styleB', + }); + }); + + it('should flatten recursively', () => { + var style = [{styleA: 'newA', styleB: 'newB'}, {styleA: 'newA2'}]; + var AthenB = flattenStyle([moduleA.elementA, moduleA.elementB, style]); + + expect(AthenB).toEqual({ + styleA: 'newA2', + styleB: 'newB', + }); + }); +}); diff --git a/src/browser/ui/flattenStyle.js b/src/browser/ui/flattenStyle.js new file mode 100644 index 000000000000..7c7477b8ef4a --- /dev/null +++ b/src/browser/ui/flattenStyle.js @@ -0,0 +1,50 @@ +/** + * Copyright 2013-2014 Facebook, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * @providesModule flattenStyle + */ + +"use strict"; + +var merge = require('merge'); +var mergeInto = require('mergeInto'); + +/** + * flattenStyle is a merge function that allows nested arrays with holes. + * + * For example, + * flattenStyle([ {color: 'red'}, null, [ {fontWeight: 'bold'} ] ]) + * -> {color: 'red', fontWeight: 'bold'} + */ +function flattenStyle(style) { + if (!style) { + return undefined; + } + + if (!Array.isArray(style)) { + return merge(style); + } + + var result = {}; + for (var i = 0; i < style.length; ++i) { + var computedStyle = flattenStyle(style[i]); + if (computedStyle) { + mergeInto(result, computedStyle); + } + } + return result; +} + +module.exports = flattenStyle;