mui / material-ui

Material UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
91.86k stars 31.57k forks source link

Refactor CSS into Javascript #30

Closed aranw closed 8 years ago

aranw commented 9 years ago

Move component CSS into Javascript to remove the need for adding CSS/Less files to projects.

Suggest comes from seeing this slideshow https://speakerdeck.com/vjeux/react-css-in-js from @vjeux

totty90 commented 9 years ago

I like that, but I don't know how it works in real world. Imagine having a lot of elements, every element will have a custom style. It will make the dom huge. Isn't this a performance problem? (I've made a similar UI entirely in react.js with style and everything worked ok, but was a small test project)

aranw commented 9 years ago

@totty90 yeah I did wonder this, but from the slides the issue at Facebook from sounds of it was that all the CSS files and CSS classes were a performance issue.

Maybe @vjeux could shed come light on this and its the suggested solution in that slideshow.

Maybe for some of the smaller components this might be a better solution?

Edit: Another thing I was unsure of was how to deal with media queries and responsiveness?

totty90 commented 9 years ago

Already asked him to show me a real world app using this technique. This solves one of the best problems: a component came in a single file. I can share a file with you and you get my component rendered exactly as mine without any setup from your part. No css problems.

robertknight commented 9 years ago

There is another project which aims to provide a good way to integrate CSS-in-JS and React - https://github.com/js-next/react-style and also some initial work on Material UI components using them - https://github.com/SanderSpies/react-material

This project has more traction at the moment and a larger number of components implemented, but react-material does have the advantage that components work standalone without having to bundle CSS files. You also get all the benefits outlined in @vjeux's presentation from it.

It would be great if the two projects could be combined!

totty90 commented 9 years ago

The other project is a lot worst. The only part interesting is the css in js.

pspeter3 commented 9 years ago

I like the concept of having JS and CSS be coupled but inline styles are bad for browser performance and implementing CSP. Maybe use WebPack if you're concerned?

jackcallister commented 9 years ago

Webpack seems to be the only "reasonable" way to import styles. Have a require('component.css'); - but then you end up having a dependency for your component. They must use Webpack too! I don't like that solution. One should be able to share a component easily just install, use and profit.

The current approach here looks user friendly. It's not perfect, but copying a couple of import lines is quite simple. Although I would suggest that using Less once again locks users into that preprocessor...

It's tough making things simple! Anybody got any suggestions? This is much wider than just this project. The entire react/polymer/component community needs a solution.

P.S - coupling CSS inside your JS looks like a bad idea. For one you end up writing a pseudo CSS with camel cased properties and stringified values and you are cognitively mixing concerns making it harder to reason about your code. I've never had issues writing CSS. Yes, it can be hard and things can go wrong (global namespace, unintended consequences etc) - but shifting that to JS is NOT going to fix the issue. Writing better CSS is.

vjeux commented 9 years ago

@pspeter3 do you have any benchmarks? :)

pspeter3 commented 9 years ago

Mainly these two JsPerfs, http://jsperf.com/class-vs-inline-styles/2 and http://jsperf.com/classes-vs-inline-styles. However http://jsperf.com/inline-style-vs-css-class/9 does seem to imply the opposite. Bad for performance was an overstatement. I'm sure you can also cause bad performance with stylesheets by using poor selectors. Do you have any advice on implementing Content Security Policy when using inline styles?

totty90 commented 9 years ago

@jarsbe

P.S - coupling CSS inside your JS looks like a bad idea. For one you end up writing a pseudo CSS with camel cased properties and stringified values and you are cognitively mixing concerns making it harder to reason about your code. I've never had issues writing CSS. Yes, it can be hard and things can go wrong (global namespace, unintended consequences etc) - but shifting that to JS is NOT going to fix the issue. Writing better CSS is.

I've written a lot of CSS and CSS into JS. The first one is easier but in big projects get messy the other one I think (because I've only built a small app) would work better in a bigger app. Try to program a js app with everything global + merging stuff, that's hell!!! (Or I've always done it wrong...)

Here is a test I've done with react.js http://jsperf.com/react-class-name-vs-inline-css2

hai-cea commented 9 years ago

Yes, I wish there was an easy answer for this. :) In the end, I think there are merits to both approaches.

My biggest concern with including styles inside the JS components has to do with code maintenance. For example, what happens to the CSS that's needed for cross browser resets? Do those styles go into the components as well? Also, we get a lot conveniences from using LESS like mixins, variables, etc that we would have somehow replace and abstract inside JS.

With that said, we've namespaced all of the CSS classes that are used in this project with "mui" and we try to write object oriented CSS. In the future, there probably should be some process that will let users pick and choose which components to download into a custom build.

Thanks for the issue. Really good points brought up is discussion!

mcwhittemore commented 9 years ago

What really makes the style of a component less a part of that component than its structure (html) and functionality (js)? It seems to me that we've come to mistake a method to mitigate one problem (maintainability of code) as an inherently wise design pattern and have forgotten that it was really a patch. Arguably React is more elegant solution to the maintainability of code problem than dividing structure, functionality and style into their own languages. It has its own problems (speed in the css case), but if we come to agree that this reunited way of writing components is good we can start trying to solve the speed problem.

It should be noted that I personally think that this is one of the failures of the web components spec. Where it groups different languages into one module, React challenges the very idea of html and css.

jackcallister commented 9 years ago

@mcwhittemore Yes, the style is no less a part of the component than the HTML or JS. Together they combine to create one component. This does not necessitate that the component should be written as one 'unit', that's just how it should be distributed.

React tightly couples the structure and functionality. It forces you into this pattern and I've found this to be simple and logical. I can easily understand the code I'm looking at, my brain is not overloaded.

On the other hand CSS inside a JS file makes the code harder to understand for me. Descriptions of visual styling do not fit well next to functionality and structure. I'm not simply annoyed by it I am confused and slowed down since I have to process multiple concerns. It also tightly couples the styling with the component which makes it harder for other authors to write their own styles.

I want to distribute my component as a single bundle. Lego block development is the dream! It sounds like we are all on the same page with that end goal. Inline CSS sounds like a good fix but I do not believe it is the right fix.

mcwhittemore commented 9 years ago

@jarsbe 100% agree that reading style in the component is not clear enough. Simply finding where the style is created can be difficult at times. I've been using a style.json file where its defaults and permanents are merged with this.props.style does a bunch for clarity.

{
  "defaults": {
    "backgroundColor": "#efefef"
  },
 "permanents": {
    "width": "100%"
  }
}

Another argument against doing the styling in JS vs CSS is that the style attribute does not have the same powers as a CSS selector. For instance I don't think the *:after in the current less is possible to implement.

jackcallister commented 9 years ago

@mcwhittemore trying to get a psuedo element was exactly what made me go, "huh? oh yeah that doesn't exist in this context...". Yes, extraction into another file does help. I must take a look at the system on bespoke.js for styling new themes. I've heard that implements CSS in JS but am weary.

totty90 commented 9 years ago

@mcwhittemore why do you need *:after?

mcwhittemore commented 9 years ago

@totty90 Its currently part of the CSS. There might be ways to solve this problem, but it is not a simple "convert your css to js" situation. Do you have a solution for the :after or :before selector that I'm just not thinking of?

totty90 commented 9 years ago

IMO :after and :before are kind of hacks and I never needed them, I want to know what you need them for to give you a solution/opinion.

mcwhittemore commented 9 years ago

@totty90 I'll mention you in another issue on another project so not to clutter this one.

WRidder commented 9 years ago

@hai-cea Keeping a SMACSS workflow in mind (https://smacss.com/book/categorizing) a good approach may be to keep the css for the Base, Layout and (maybe) Theme categories in the good ol' css files. Module and State css seems like a good candidate for me to do JS style.

hai-cea commented 9 years ago

@WRidder Thanks for the link - that makes a lot of sense.

jackcallister commented 9 years ago

A random thought that I didn't know where to put... How do native components implement styling? I envision that react components should be as similar in nature to native components. It might be a good place to look for ideas.

nigelsmith commented 9 years ago

I'm very sceptical of this approach of placing CSS within JS - the presentation itself talks about the issue of associating styles with components as if there hasn't been an entire, multi-year long, effort to do just that with the Shadow DOM standard. Now clearly React doesn't use that mechanism at present but I wouldn't be surprised if it did in time.

That standard works because it provides a way for a component creator to associate some minimal set of basic styles with their component that can then be overridden by a designer reaching down into the shadow DOM. It's far from clear how a designer would override styles set purely within Javascript if they weren't themselves using the components with react directly.

hai-cea commented 9 years ago

I just got back from the react conf and thank you so much to @vjeux for organizing it. I was skeptical with this approach as well, but I think it does have its advantages. This project has some challenges that I'd love some help solving if we went with this approach.

  1. We'd like these components to be themeable. They would have some default styles, but developers should be able to change the look to suit their needs. Currently, they can do this by overriding certain variables in LESS. How could we offer the same capabilities using js styles?
  2. A lot of times, components will generate nested children. How do we let developers customize styles on nested children. My instinct is that we should only expose style props on those children that would be customizable? But would this tightly couple our styles with a component's dom structure? What I mean is, if we end up changing a component's dom structure, would we end up changing it's style props as well?

Thanks for all of your help and feedback so far - I'm looking forward to what everyone has to say.

pspeter3 commented 9 years ago

Was there a video about this idea at the React conference?

hai-cea commented 9 years ago

@vjeux Touched on it a little in his React Native talk - http://conf.reactjs.com/schedule.html#react-native

He also told me about a talk he gave at NationJS, but I don't know if there's audio? http://blog.vjeux.com/2014/javascript/react-css-in-js-nationjs.html

cuongcua90 commented 9 years ago

Hi @hai-cea, My team has very small experiment on the theming problem. My team solution is material ui library expose a function to set the theme: https://github.com/agencyrevolution/material-ui/blob/css-in-js-experiment/src/index.js

When we use material-ui we call this function with the custom define property. And material-ui will call update of this function. You can see at: https://github.com/agencyrevolution/material-ui/blob/css-in-js-experiment/src/js/scaffold-styles.js

and the example we use at these files: https://github.com/agencyrevolution/material-ui/blob/css-in-js-experiment/src/js/raised-button.jsx https://github.com/agencyrevolution/material-ui/blob/css-in-js-experiment/example/src/app/custom-styles.js https://github.com/agencyrevolution/material-ui/blob/css-in-js-experiment/example/src/app/components/main.jsx

hai-cea commented 9 years ago

Thanks @cuongcua90 - I had a similar idea as yours.

There would be a theme object that will take care of holding all of the variables defined in custom-variables.less. Devs can override these variables on initialization of their app with a helper method like this:

//Override Theme Variables
Theme.set({
  textColor: 'red'
});

To provide more granular control of styles, each component will take in style props that will be merged with default styles:

//merge styles that are passed in
var styles = this.mergePropStyles(defaultStyles, this.props.style);

Also, ran across some other challenges in my test:

  1. How would we handle vendor prefixes?
  2. What about media queries?
lucasjans commented 9 years ago

hey @cuongcua90 and @hai-cea - what if we leveraged another framework to do this? I haven't read much about it, but appears that http://jhudson8.github.io/fancydocs/index.html#project/jhudson8/react-css-builder?focus=outline might be a good fit.

It supports mixins and variables.

hai-cea commented 9 years ago

Gents,

I've created a css-in-js branch to work on this issue and have a prototype in the example folder.

Developers have 2 points for customization. First is to override default theme variables. Variables will include things like component colors, font faces, etc. https://github.com/callemall/material-ui/blob/css-in-js/example/src/app/app.jsx#L16

For more granular control, developers can also override default component styles through props: https://github.com/callemall/material-ui/blob/css-in-js/example/src/app/components/main.jsx#L10

Here's an example of how we would implement a component on the MUI side: https://github.com/callemall/material-ui/blob/css-in-js/src/js/svg-icons/svg-icon.jsx

You can see that we'll take care of merging in theme vars, style props, and also auto prefixing based on feature detection.

Any feedback on this pattern?

msikma commented 9 years ago

Moving all the CSS into a JS-based system would be nice, and would solve #316. I'm not sure how much is known in terms of maintainability, and some common styles apply to all elements (e.g. the reset and the stuff in less/core/), so I'm not sure what's planned for how to get those styles into people's applications. Some open questions, but definitely very interesting.

daniellewissc commented 9 years ago

we could create some sort of classical inheritence system for creating style objects. that way for common styles there can just be an inheritable BaseStyle object.

facundocabrera commented 9 years ago

I'm curious to understand the real problem you are trying to solve mixing CSS into JS.

1- Make building easier because we don't be worry about "Where is the CSS I need for this?" 2- Theming form JavaScript? 3- Override styles?

These problems exists since the beginning, and in my case everything was solved by having a simple folder structure plus a "page.(css|less|sass)" with @import rules, all relative to the file.

Could someone details/explain/share why mix CSS into JS is a good idea?

Thanks for your time!

msikma commented 9 years ago

I'm not the author of the package, but creating styles dynamically with JS is something that's becoming more popular lately.

There are a number of problems with CSS in general. CSS is compiled with a number of static variables, such as (for example) the size of your grid, the color of your main type, et cetera. You can't change them afterwards, except if you manually single out the elements affected and set custom styles that override the compiled CSS.

Putting CSS in JS means it becomes fully dynamic, and you can change any of the underlying variables at will. It also means (specifically in the case of React) that you get to keep the relevant styles inside the component.

I'm not 100% sold on it, though. It also means you don't have styling until the JS finishes loading. Not a major deal in the case of a single-page app, but something that might warrant some thought in the future. (Or perhaps server-side rendering already deals with this in an efficient way? I'm also hoping to hear from someone who has more experience actually doing this.) Having a separate CSS file also means the browser can start loading the styles in parallel.

robertknight commented 9 years ago

@dendril - The presentation that @vjeux gave describes the problems with CSS at scale. Some of the problems that using JS to author CSS solves include:

As an example, I have an implementation of a Material Design style button using TypeScript + inline styles here: https://github.com/robertknight/passcards/blob/master/webui/controls/button.ts .

There is a trade-off here which is the usual trade-off of declarative vs. imperative approaches to specifying things. The imperative language (JS) gives you lots of power. The syntax is more verbose and the flexibility is open to abuse though. For complex components with lots of dynamic styling, the balance leans in JS' favor. For static styling which is easily expressed in CSS, not so much. I wouldn't use this approach for document-like content.

@msikma - Specifying styles in JS doesn't have to mean using inline styles or waiting for your app to load. You can use JS just as a language for authoring the styles in that compiles down to JS - basically an alternative to SASS or LESS. Combine this with server-side rendering and the actual output that the browser gets can look pretty much like CSS and that works nicely with browser dev tools.

facundocabrera commented 9 years ago

@robertknight Thanks a lot for your answer, you just added enough information to fully understand the idea.

I prefer the evolution of SASS/LESS instead of add more JavaScript to the show, at the end, if you want to reuse mechanisms, we can improve LESS which is coded in JavaScript, and reuse as much as we want from JavaScript, but seems it's easy to just design a new API and start mixing everything in one file.

Hope we find out a better solution, because losing the declarative idea of CSS wont be a good idea, and less when we are making everything declarative instead of imperative. \o/

msikma commented 9 years ago

@msikma - Specifying styles in JS doesn't have to mean using inline styles or waiting for your app to load.

Is it possible to declare styles globally (by dynamically constructing a style element)? I'd figure so, since that's essential.

How do you avoid having to wait for the JS to load, though?

robertknight commented 9 years ago

@msikma - Yes. In the example I linked to, all the styling in the 'theme' var which is defined at the top of the file is compiled to CSS at build time. In the render() function, there are calls to style.mixin(theme.someElement, otherProps) which returns an object with a className property that is the usual space-separated list of class names. If you ran React on the server, you'd get a normal HTML string with elements that have 'class' and 'style' attributes that can be rendered on the client without running any code.

@dendril - The alternative to declarative CSS doesn't have to be imperative JS. The JS can be written in a functional/declarative way - all the static styling can be a simple Javascript object. If you need to reference the same constant (eg. A Color or font size) in the dynamic styling and the static styling, that becomes easy to do.

msikma commented 9 years ago

Thanks for the explanation, sounds very good.

ezmiller commented 9 years ago

I'm just coming into this thread, but am interested because I'm trying to develop a project with material-ui. I had switched to the SASS version, having discovered that there is no grid system implemented in material-ui and then realized that the SASS version is not in sync. So I'm quite curious to know what direction this project will take in the future. It seems as though there so many tradeoffs with putting the css into the components themselves that it's not a clear choice. Intuitively, I find the idea a bit dangerous because it's such a radical departure form heretofore established web dev practices. On the other hand, so is React!

TimothyKrell commented 9 years ago

@hai-cea Have you considered the JSS library? I have been using it for a new React project I have been working on, and so far I really like it. What's nice is you have the option to declare global class names if you need to on rare occasion. Pretty much anything you can do in css you can do in JSS since it is creating it's own namespaced classes and adding them to a <style> tag in the <head>.

vmakhaev commented 9 years ago

There is new project about React styles for your consideration. Here is a link: Radium

natew commented 9 years ago

Just saw this thread and wish I had ran across it earlier. We are doing CSS in JS on an iOS theme included in reapp-ui. It was in development for a few months before release.

Actually @hai-cea I landed on a very similar system as yours.

  1. Load "constants" (think: colors, feature detection). We actually do two levels of constants, first is feature detection and colors, second is constants for specific components.
  2. Load styles for components. These actually have a consistent structure. Objects with keys that are "ref" => "stylesObj".
  3. Load animations.

Here's how you load a theme all together with the three elements. And here's how you load a custom theme mixing and matching the base theme with your own variables to "theme" it.

This has some really awesome benefits:

  1. Users don't have to load any extra styles they don't need.
  2. Components have consistent names and style declarations.
  3. Everything is themeable at every level.
  4. Easy to add themes for other platforms.

We're using react-style for all of this and it's worked really well. Along with a Styled mixin that lets us do really awesome declarative adding of styles. That, and the Tappable mixin also gives Focus/Active states. Both of those will be extracted in short time.

There's actually a bit more to it all that I'd like to write out in official docs soon.

Our next step is actually to build a material UI theme in Reapp though, and I've had this project bookmarked for a while to come back to. Wondering if we couldn't work together on it!

mjackson commented 9 years ago

@hai-cea Your Theme object would actually fit in really nicely with React's context feature. In case you're not familiar with it, context is basically props that automatically get passed down your component hierarchy. At any given level in the hierarchy, components can query their context for some value.

In your case, I'd recommend setting a theme variable on the context. It would work something like this:

var App = React.createClass({
  childContextTypes: {
    theme: React.PropTypes.instanceOf(Theme)
  },
  getChildContext() {
    return {
      theme: CustomTheme
    };
  },
  // ...

Then, in your <Button> component you could do

var Button = React.createClass({
  contextTypes: {
    theme: React.PropTypes.instanceOf(Theme)
  },
  getTheme() {
    return this.context.theme || DefaultTheme;
  },
  render() {
    return <button style={this.getTheme().getButtonStyle()}>...</button>;
  }
});

Things I like about this approach:

Anyway, awesome work you guys are doing here. I hope this helps :)

hackhat commented 9 years ago

Hy! I've created a library for writing css into react views, take a lookhttps://github.com/hackhat/smart-css

What it does: Write CSS in JavaScript with namespaces, proper names, no conflicts and expected results for react.js.

hackhat commented 9 years ago

@mjackson I really like your approach of sending the theme context down the hierarchy, but this part

this.getTheme().getButtonStyles()

Is not pretty scalable because the theme should know about the style. I think the theme should be a name, a color palette and some spacing definitions, but Not a complete css style. This would enable really customizable and scalable components and apps. What do you think?

natew commented 9 years ago

@mjackson I like the context Idea. We use a decorator which does basically the same thing.

@hackhat Yep, if you do your constants right you can load as many as you want so you can layer them. Start with "base" for colors, then have "components" for specific, etc. This is how we can allow "themes" to even work from material to iOS to Window (in the future).

Either way, good luck with it all! I think we're heading in the same direction but thats a good thing. If you'd be interested in merging, get in touch. Since we've build out a really big set of stuff for iOS and you Android and we've explored pretty heavily into CSS/JS stuff, could be useful. Either way I've added some docs here on contributing to that: https://github.com/reapp/reapp-ui/issues/29.

Sorry to threadjack, just saw an opportunity for some teamwork :+1:

hai-cea commented 9 years ago

@natew Glad to hear about your project. It looks like we're on the right track. :)

@mjackson Thanks for telling me about React's context feature - I'd never used it before, but heard it mentioned at the react conference and also saw it used in React Router. Seems like a great solution - I'll definitely try it out.

We've been making pretty good progress in moving styles into js. I think one of the biggest advantages that I'm seeing is that we're being more explicit about the styles of each component. So far, we haven't felt a need to pull in a library like JSS, Radium, or React Styles. Although I can be persuaded otherwise.

I'm looking forward to completing this architecture change so that we can get back to hardening up our existing components.

DylanPiercey commented 9 years ago

Will this change make it difficult to override the default styles?

mmrtnz commented 9 years ago

@DylanPiercey great question. As @hai-cea mentioned, we haven't felt the need to use any styling-related libraries. All component styles will be defined inline, so there will no longer be any Less files (aside from those in the docs site).

This is currently our approach to override styles:

One last thing: We will also provide a Theme file using react's context feature as suggested by @mjackson for added customization.

What are everyone's thoughts on this approach?