Skip to content
This repository has been archived by the owner on Aug 19, 2022. It is now read-only.

Vendor prefix #128

Merged
merged 6 commits into from
May 7, 2015
Merged

Vendor prefix #128

merged 6 commits into from
May 7, 2015

Conversation

ianobermiller
Copy link
Contributor

Reopening #109 after deleting base wrap-api branch :(

Add some simple prefixing capability, based on css-vendor. Don't use css-vendor directly, though, to avoid going back and forth between dash-case and camelCase. Also, be smarter than css-vendor about valid values that the browser rewrites.

While this approach works quite well, it falls down a bit for values that require complex prefixing. PrefixFree is much more comprehensive. For example, it will prefix values inside of transition, rewrite linear-gradient, and more.

Comment from @alexlande:

I really want Radium to handle vendor prefixing, and there's obviously a huge demand for it given #11, but I'm not sure about implementing browser-only prefixing.

On the one hand, we already have several features that are browser-only at the moment, like media queries + pseudo-selectors. On the other, I desperately want to get both of those features working for server-side rendering.

I think where I'm landing on it is-- if we can come up with a plan to add isomorphic autoprefixing in a timely manner, we should do it. If not, I guess it can be one more thing on the list for when we fully support server-rendering.

@ianobermiller
Copy link
Contributor Author

One way to do isomorphic prefixing would be to detect the browser on the server via user agent and do the prefixing using a known list or tool, like autoprefixer.

@ianobermiller
Copy link
Contributor Author

We should probably also include support for the old-style flexbox syntax: https://css-tricks.com/using-flexbox/

@alexlande
Copy link
Contributor

One way to do isomorphic prefixing would be to detect the browser on the server via user agent and do the prefixing using a known list or tool, like autoprefixer.

Won't handle static site generation, but that's not a particularly common use case I guess. I'm good with this for now. Can re-evaluate later if it's a problem.

@ianobermiller
Copy link
Contributor Author

When you say, "good with this", do you mean the current approach or the proposed approach? Should we merge this PR?

@alexlande
Copy link
Contributor

Sorry yeah, feel free to merge this. 👍

ianobermiller added a commit that referenced this pull request May 7, 2015
@ianobermiller ianobermiller merged commit 66dbd7a into master May 7, 2015
var prefix = function (style, mode /* 'css' or 'js' */) {
mode = mode || 'js';
var newStyle = {};
Object.keys(style).forEach(function (property) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unnecessary since it runs in the browser. You can test all prefixes and once you found one that works, it becomes the prefix for the rest of the session.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I'm not following you. What part is unnecessary? The code determines the prefix at the top of the file, and caches the result for individual properties/values.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh apologies, I was completely misreading this. Need more coffee 💩

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hah no problem. Thanks much for taking a look!

@wenbing
Copy link

wenbing commented Jun 4, 2015

"Won't handle static site generation"
It should handle static site generation.

@wmertens
Copy link

wmertens commented Jun 4, 2015

@wenbing handling static site generation means adding 2MB of databases and code. Just post-process the generated HTML with https://github.com/postcss/autoprefixer on the server.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants