Allow style object to accept array of values

[brainkim]

This allows components to set multiple values for a single css rule. The main use case is to set vendor prefix styles and allow for broader css compatibility. Example:

var style = {
  cursor: ['-webkit-grab', '-moz-grab', 'grab'],
  backgroundColor: ['rgb(200, 54, 54)', 'rgba(200, 54, 54, 0.5)']
};

React.renderComponent(<div style={style}>Grabbable!</div>, mountNode);

When the component initially renders, all styles will be set through the inline markup string. When the component updates its style, the last valid style in the array will be set.
This change does not handle the edge case where you pass in a singleton array of an empty string, in the sense that it improperly unsets shorthand rules in IE8. I think the css stuff is due for a refactoring anyways though.

Something to think about: Should react manage css vendor prefixes automatically?

Other things I did:

• Added actual unit tests for CSSPropertyOperations.setValueForStyles
• Fix a bug where setting a style with an empty string will render markup for that rule.

Edit: Added backgroundColor to example.

[syranide]

@brainkim Is this actually useful for anything besides cursor: grab? I don't know of any other CSS property that has vendor prefixes inside of them. If that's the case, then this specific issue could be solved by just defining a custom CSS class and use that instead.

However, it seems like vendor prefixes overall may need considering for React.

Edit: Hmm, this is actually useful for CSS3 backgrounds too.

[brainkim]

Off the top of my head there's also background: linear-gradient and fallback CSS3 colors (hex, rgb, rgba).
On Dec 27, 2013 4:27 AM, "syranide" [email protected] wrote:

@brainkim Is this actually useful for anything besides cursor: grab? I
don't know of any other CSS property that has vendor prefixes inside of
them. If that's the case, then this specific issue could be solved by just
defining a custom CSS class and use that instead.

However, it seems like vendor prefixes overall may need looking over in
React.

—
Reply to this email directly or view it on GitHub.

[benjamn]

What about things like var style = { margin: [10, 20] }? This would expand those to margin:10px;margin:20px, when the intent was probably to produce margin:10px 20px;.

[brainkim]

@benjamn Grah! I hadn't thought of that. That could be surprising behavior. As it currently stands though, you would do var style = { margin: '10px 20px' };, right? One thought I had while looking through the css source code was that it doesn't really make sense why React automatically appends 'px' to some numbers in the first place. I think it adds a lot of complexity for little payoff (I didn't even know React did this). The larger concern for me is that we are currently unable to specify a rule multiple times with the style object despite the fact that it's a common thing to do in css to provide greater cross-browser support.

On Fri, Dec 27, 2013 at 9:18 AM, Ben Newman [email protected]:

What about things like var style = { margin: [10, 20] }? This would
expand those to margin:10px;margin:20px, when the intent was probably to
produce margin:10px 20px;.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/723#issuecomment-31261719
.

[chenglou]

Yeah the px addition is funny. It's been pretty handy though.
Would your problem be solved with something like margin: interpolate('%spx %spx', 10, 20);?

[vjeux]

I feel like those use cases can easily be addressed by a small wrapper that has feature detection.

var style = {
  cursor: grabCursorWithPrefix(),
  backgroundColor: rgbaWithFallback(200, 54, 54, 0.5)
};

But, if you really need to write them using fallback, you can just make a CSS rule for them and add use a class.

[petehunt]

Regarding vendor prefixing: we talked about doing vendor prefixing in React a while ago but decided it was a lot of bytes and complexity for little payout.

[brainkim]

@vjeux I like this suggestion. I just looked it up and CSS.supports is a thing (not supported in IE/Safari but shimmable). But at the same time I feel like using an array to specify fallbacks conveys intent much more clearly than delegating to a function, especially considering the fact that the style object maps perfectly to a style string otherwise. I know there's always css classes, but sometimes I feel like a css rule is essential to a component, as in the cursor: grab case. I dunno. You're the judge, jury, and executioner. 💪

@petehunt I agree. Who actually wants to keep track of vendor prefixes?

[syranide]

Problem with vendor-prefixes is also that some of the time they are just that because they aren't 100% compatible. Given proper CSS.supports-shims of which it seems they exist (https://github.com/termi/CSS.supports/, and it's also light-weight), as well as vendor-prefixing-utility-functions, that sounds like the best route, as the application can then intelligently decide what to do when certain things aren't supported. It's not like style should replace stylesheets.

[fabiomcosta]

Maybe we should modify the invariant check (

react/src/core/ReactDOMComponent.js

Lines 57 to 61 in d914522

	invariant(
	props.style == null || typeof props.style === 'object',
	'The `style` prop expects a mapping from style properties to values, ' +
	'not a string.'
	);

) that expects the style value to be an object and accept it to also be a string. In the documentation it could be suggested to always use an object as the style value and point out that sometimes (like this case) you'll need it to be a string and you can do that.

[brainkim]

@syranide I don't understand your argument. The only reason vendor prefixes exist is because fallback is such a rock-solid strategy for the browser. I can't think of a single situation where using CSS.supports would add any "intelligence" over my solution. I guarantee you that >95% of fallback functions will be

if (this style works) {
  use this style
} else if (that style works) {
  use that style
} else {
  use this other style
}

and these fallback functions will be full of suck because we're now asking the DOM a series of questions for every render. We could always memoize the fallback function, but then the burden of evidence is on you to prove that requiring a memoized function and a browser shim whenever you want inline css fallback is better than simply setting a rule multiple times with an array.

I sincerely do not know why we're not jumping at this opportunity to reduce cyclomatic complexity in our applications, except perhaps a misbegotten sense of propriety derived from archaic "best practices." The thing I like about React is that it doesn't insist that we need the holy trinity of html, css, and javascript to separate concerns, and we get a lot of cool things like react-styles as a result. I argue that the style object should be able to do anything inline styles can do, and fallback is a small but important part of that.

In short, this pull request embodies the very essence of React.js and the United States of America is the greatest country in the world. I'm going to go stick my head in a bucket of water. Thank you.

[brainkim]

@fabiomcosta I think that invariant exists because React will actually merge style objects when using this.transferPropsTo. In other words if you have a component like:

var RedDiv = React.createClass({
  render: function() {
    return this.transferPropsTo(<div style={{backgroundColor: 'red', outline: 'black solid 1px'}}>{this.props.children}</div>);
  }
}

and then call it like so:

React.renderComponent(<RedDiv style={{outline: 'blue dotted 1px'}}>Hello world!</RedDiv>, mountNode)

the resulting div will have a style object of {backgroundColor: 'red', outline: 'blue dotted 1px'}. Or at least that's the behavior I've seen while using React.

[syranide]

@brainkim I don't count as official opinion on React, but it seems dangerous to rely on style for all CSS. The DOM is expensive and injecting tons of inline CSS is not going to be cheap. As for intelligent functions I don't so much mean writing functions for selectively generating styles, but using helpers like @vjeux suggested.

The problem in my opinion with vendor-prefixes is that they are for a reason, either because it's based on a non-final standard, it's not based on any standard at all or because there genuinely are differences. Opacity doesn't work the same across all browsers in all cases. So by forcing use of prefixed versions we make some things easier, but we also prevent some options.

As in, feature -webkit-x used to work like this, but then the standard changed and x now works like this, however you can no longer separate them because we vendor-prefixed them automatically. If you want, just push all of this through a function like cx / classSet that does vendor-prefixing for you. Problem solved, React remains lean and unopinionated, you get all the benefits while being able to workaround it when necessary.

[brainkim]

@syranide i need this pull request. pls. my children will starve without it.

[syranide]

@brainkim As I alluded to above, I'm not part of the team.

[brainkim]

I understand. No worries!

On Fri, Dec 27, 2013 at 5:09 PM, Andreas Svensson
[email protected]:

@brainkim https://github.com/brainkim As I alluded to above, I'm not
part of the team.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/723#issuecomment-31282375
.

[vjeux]

Given that's a very broad API for a very narrow use case and there work-arounds available, I'm going to close this pull request. Thanks a lot for submitting it, it's always good to stimulate a healthy debate on the API!

[brainkim]

KK. Happy new year!

On Tue, Dec 31, 2013 at 3:15 PM, Christopher Chedeau <
[email protected]> wrote:

Closed #723 #723.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/723
.

[mccutchen]

@vjeux: Given that's a very broad API for a very narrow use case and there work-arounds available, I'm going to close this pull request.

I'm sorry to resurrect this pull request, but I'm not sure what the available workarounds are for the specific problem I'm encountering:

I'm rendering an item with a linear gradient background image that's based on the dominant colors extracted from the item's image, which means I need to specify the background image rule inline on the item (rather than, say, specifying the image in a class defined in a stylesheet). And in order for this to work across browsers, I need to specify a separate background-image rule for each vendor-prefixed version of the linear-gradient value I want to use. E.g.:

background-image: -webkit-linear-gradient(left, #900 0, #009 100%);
background-image: -moz-linear-gradient(left, #900 0, #009 100%);
background-image: linear-gradient(left, #900 0, #009 100%);

What's the best way to handle this situation given the constraints around the style property in React? Do I need to try to do browser detection to pick the one rule that I'm allowed to use?

[syranide]

@mccutchen Instead of adding all of them, detect which one is the correct and apply only that. It will be faster too.

[zertosh]

I think this is worth revisiting. I was reading @vjeux's React: CSS in JS, and in trying to implement it, I immediately ran into issues with flexbox (the display property has vendor prefixed values). Doing feature detection outside of React to generate the object that will be passed as style is perfectly reasonable for client-side rendering. But if you're doing server-side rendering then that's not an option unless you do crazy UA matching.

I quickly hacked this together in CSSPropertyOperations so that it could handle arrays for values such that something like {display: ['-webkit-flex', '-moz-box', '-ms-flexbox', 'flex'] got treated as style="display:-webkit-flex;display:-moz-box;..." with good results (so far).

diff --git a/src/browser/ui/dom/CSSPropertyOperations.js b/src/browser/ui/dom/CSSPropertyOperations.js
index 3417378d..900fc699 100644
--- a/src/browser/ui/dom/CSSPropertyOperations.js
+++ b/src/browser/ui/dom/CSSPropertyOperations.js
@@ -78,10 +78,15 @@ var CSSPropertyOperations = {
           warnHyphenatedStyleName(styleName);
         }
       }
-      var styleValue = styles[styleName];
-      if (styleValue != null) {
-        serialized += processStyleName(styleName) + ':';
-        serialized += dangerousStyleValue(styleName, styleValue) + ';';
+      var styleValues = styles[styleName];
+      if (styleValues != null) {
+        if (!Array.isArray(styleValues)) {
+          styleValues = [styleValues];
+        }
+        for (var i = 0; i < styleValues.length; i++) {
+          serialized += processStyleName(styleName) + ':';
+          serialized += dangerousStyleValue(styleName, styleValues[i]) + ';';
+        }
       }
     }
     return serialized || null;
@@ -105,22 +110,28 @@ var CSSPropertyOperations = {
           warnHyphenatedStyleName(styleName);
         }
       }
-      var styleValue = dangerousStyleValue(styleName, styles[styleName]);
       if (styleName === 'float') {
         styleName = styleFloatAccessor;
       }
-      if (styleValue) {
-        style[styleName] = styleValue;
-      } else {
-        var expansion = CSSProperty.shorthandPropertyExpansions[styleName];
-        if (expansion) {
-          // Shorthand property that IE8 won't like unsetting, so unset each
-          // component to placate it
-          for (var individualStyleName in expansion) {
-            style[individualStyleName] = '';
-          }
+      var styleValues = styles[styleName];
+      if (!Array.isArray(styleValues)) {
+        styleValues = [styleValues];
+      }
+      for (var i = 0; i < styleValues.length; i++) {
+        var styleValue = dangerousStyleValue(styleName, styleValues[i]);
+        if (styleValue) {
+          style[styleName] = styleValue;
         } else {
-          style[styleName] = '';
+          var expansion = CSSProperty.shorthandPropertyExpansions[styleName];
+          if (expansion) {
+            // Shorthand property that IE8 won't like unsetting, so unset each
+            // component to placate it
+            for (var individualStyleName in expansion) {
+              style[individualStyleName] = '';
+            }
+          } else {
+            style[styleName] = '';
+          }
         }
       }
     }

(This can be optimized a bit so you don't create arrays for every prop.)

I don't think React should be in the business of determining what is/isn't a valid prefix - but it should respect wanting to set different values for the same css property. I'm not going to defend the sanity of this, but it is undeniably a pattern that's widely followed in the webdev community.

To get a feel for what properties someone might want to do this for, just look at autoprefixer. Look for the classes that extend Value: https://github.com/postcss/autoprefixer-core/tree/master/lib/hacks.

UPDATE: lol hadn't looked at the PR's code since it was so old - just quickly looked at the comments - hooray for reinventing the wheel!

[Zinggi]

I don't understand why this pull request was closed, as it fixes an actual problem with inline styles. This would make inline styles more flexible.

Anyway, anyone trying to inline-style a gradient or something similar, here's a very hacky workaround that works (without you needing to check for available features as syranide suggested)

var style = {
    background: "linear-gradient(to right, #5c8484 0%,#ea4f07 100%); background: -webkit-linear-gradient(left, #5c8484 0%,#ea4f07 100%)"
};

So just:

• add a ;
• repeat the attribute, e.g. background:
• add the -webkit-linear-gradient(...)
• Profit!

[gaearon]

For gradients, you can just use a helper like this:

'use strict';

var GRADIENT_VARIANTS = {
  'LinearGradient': 'linear-gradient',
  'WebkitLinearGradient': '-webkit-linear-gradient',
  'WebkitGradient': '-webkit-gradient',
  'MozLinearGradient': '-moz-linear-gradient',
  'msLinearGradient': '-ms-linear-gradient'
};

var test = function (topColor, bottomColor) {
  var testEl = document.createElement('div'),
      style = testEl.style;

  for (var gradientName in GRADIENT_VARIANTS) {
    var prefixedString = GRADIENT_VARIANTS[gradientName],
        gradient = '',
        templateString = '';

    style.backgroundImage = '';

    if (gradientName === 'LinearGradient') {
      gradient = prefixedString + '(to bottom,' + topColor + ',' + bottomColor + ')';
      templateString = prefixedString + '(to bottom, %topColor%, %bottomColor%)';
    } else if (gradientName === 'WebkitGradient') {
      gradient = prefixedString + '(linear,' + topColor + ',' + bottomColor + ')';
      templateString = prefixedString + '(linear, %topColor%, %bottomColor%)';
    } else {
      gradient = prefixedString + '(top,' + topColor + ',' + bottomColor + ')';
      templateString = prefixedString + '(top, %topColor%, %bottomColor%)';
    }

    style.backgroundImage = gradient;

    if (style.backgroundImage.length) {
      test.templateString = templateString;
      return gradient;
    }
  }
};

var gradient = function (topColor, bottomColor) {
  if (!test.templateString) {
    test(topColor, bottomColor);
  }

  return test.templateString.replace('%topColor%', topColor).replace('%bottomColor%', bottomColor);
};

module.exports = gradient;

It's not very flexible (just top to bottom) but you can modify it as you see fit..

[super-cache-money]

Over a year ago @vjeux closed this because it was a

very narrow use case and there work-arounds available

The main work around advocated was to dynamically add only the relevant prefix in the browser.

With the rising popularity of inline styles and isomorphic apps, this workaround doesn't cut it anymore.

You can't only dynamically prefix in the browser, since that will cause an inconsistency between the server and client rendered HTML, which breaks isomorphism.

For example, it's needed in the biggest(?) library of components: mui/material-ui#748

#popitopen

[brainkim]

Has history vindicated me yet? 😛

[cezary]

What's the best way to prevent invalid checksum errors on a universal app when a third-party component is doing browser detection? Fallbacks are used in the browser, but not on the server. The only option I see is to fork the component and extract styles into a stylesheet.