[Core] There should be a more sophisticated styling solution

[nathanmarks]

@callemall/material-ui please leave some input here when you can 👍

We need to decide on a styling solution for 0.16.0 that will help address long standing issues. Outside of performance issues, there are hacks and props all over the place to make up for the fact that we are missing out on some of the powerful features in CSS that cannot be used with inline styles -- pseudo classes, media queries (without matchmedia), etc.

From what I understand, the general consensus is that we want a JS style solution that has the capability to write styles to an actual stylesheet in the DOM.

Here are a few of the maintained solutions that do this:
https://github.com/rofrischmann/react-look
https://github.com/Khan/aphrodite
https://github.com/jsstyles/react-jss

Here are some points we need to consider when implementing the new solution (IMO):

1. Does it align with our goals? (lightly touched on above)
2. Implementing media queries that follow the high level breakpoints detailed in the spec that can be easily used in components with a stylesheet mixin (or whatever the implementation we use calls them). If we're overhauling the style implementation, it is the opportune moment to plant the seed for much better responsive UI support in this library. It would be even better if these tools are available in userland too 👍
3. Should we create layout helper components and/or mixins to help unify flexbox layout implementations across the lib?
4. Does theming needs to change to maximize best use of the new solution? While theming is one component of consistent styling, we should also look into creating variables for many of the common material styles such as global keylines/font sizes/spacing/margins/etc. I strongly recommend we improve our typography consistency by creating predefined type styles matching the material-ui typography guide, and try match up component elements like titles etc as best possible to these global type style variables as a default.
5. If using a library as large as react-look, try and see how we can import modules for a minimal build size. The full build size is 16kb gzipped which is fairly large. It would be great if we can minimize the impact on build size. I realized that 9kb of that is https://github.com/rofrischmann/inline-style-prefixer which we already use... 😄

[chrismcv]

Some further questions:
6) Will we remove or aim to remove xxxxStyle props and have users pass in an xxxClassName for overriding styles? Or will these be complimentary?
7) Will we allow override of styles via CSS and simplify our components interfaces. So if I want to override menuItemStyle in IconMenu, do I create a style = StyleSheet.create({ IconMenu: {MenuItem: { color:red }}) type override?

[nathanmarks]

@chrismcv My personal opinion is that we definitely need to leverage the solution to remove these super specific props we're seeing proposed: [TextField] add floatingLabelFocusStyle prop #4043

Before we know it, people are going to be asking for props for subcomponent styles for every possible focus/active/hover/whatever state.

I see so many issues that are "how to style XXXX", "can't style XXX inside YYYY", "add somethingInsideToTheLeftButSlightlyMiddleFocusHoverActiveStyle prop to XXX please"

[nathanmarks]

@chrismcv in 6. regarding the more general xxxxStyle props vs classNames, what do you think? with some of our components you're gonna need to be able to reach in one way or another (fortunately we'll be able to use :hover etc though!).

[nathanmarks]

@chrismcv I think that xxxxStyle props should go in the style property, not the className-ified style object.

[oliviertassinari]

@nathanmarks Thanks for bootstrapping the conversation. That's definitely one of the major issues we need to address for the next releases 💯 .

Outside of performance issues

For me, that's one of the main concern I have with the solution we will choose.
We don't have many benchmarks, be we can easy imagine that we lost many CPU cycles with our current approach. The CPU cycles are lost into memory allocations and garbage collection for our inline style object.

[nathanmarks]

@oliviertassinari

I am playing around with a couple of the JS style solutions at the moment. It's interesting but clear that it is still "early days" so to speak.

One thing I was thinking -- outside of dynamic style properties (for eg, if the component has a color prop), should we be changing the way we create the base style object?

It seems as though with the JS style libs, to get maximum performance you should use their StyleSheet.create() method once and have keys (css "classes") in that object for props/states such as open={true} rather than dynamically building the style object literal with a couple of conditional properties and passing it to the stylesheet factory method every render for every single style.

Even though the stylesheet selectors get cached in the sense that they are written only once to the DOM (and in some libs only written to the DOM when needed for the render()), we're still wasting resources with some calculations + object creation + garbage collection as you mentioned above if we are creating a new style object each render just to have it thrown out.

[rvbyron]

Hmmm... I left a comment here yesterday (Sunday). Did it get deleted?

[nathanmarks]

@rvbyron I remember it clearly. I certainly didn't delete it though.

[oliviertassinari]

@rvbyron I do remember it too. I have no idea what happened.

[chrismcv]

@rvbyron - had it in my email, so back here in quoted form!

Well, I for one would like to see a number of things take place.

A) Yes, by all means get rid of using the element's style parameter at the library level. All of the components should be defined using className. Having the components use style destroys all of the power CSS offers.
B) It would be great to have classNames automatically attached to the root element of each component (e.g. The RaisedButton component would implicitly have className="mui-raised-button"on the component's root element). It would make styling a whole lot easier. That could even be configurable.
C) Get themeing out of the muiTheme.js files altogether. A themeing muiTheme.SCSS file would be much better in that it would allow any properties chosen by the theme creator to be applied and not just the ones specifically allowed by the component. Of course this would probably require that B be implemented and active.
D) React-look appears to be a good compromise as it converts JSON objects containing style properties into classNames. It makes things easier to override. However, as I said in C above, I would still like the themeing base to be created in SCSS. I am pretty open on which CSS assist package to use. But I would stay away from anything that wanted to use the element's style parameter over the className parameter.

[nathanmarks]

@chrismcv Thanks dude!

I think that what @rvbyron has to say is important because in some ways, JS styling is very much a paradigm shift from regular CSS. Most people are not using JS styling in their day job and it requires a different way of thinking + makes it harder for designers who may usually contribute to projects that aren't so proficient in JS.

It's important to consider all the angles here.

@oliviertassinari @chrismcv

One thing I realised about react-look is that you need to wrap all of your components in the look HoC. This seems pretty invasive, it even hijacks the render() function, stores super.render() in a const and performs style resolution operations that way. Some other solutions such as Khan/aphrodite just require a function wrapper around the styles.xxxx reference from Stylesheet.create() inside className={}.

Hijacking the render() function just for css seems over-engineered to me. It tells me that the React built in tooling/support for this sort of deeply integrated styles functionality is just not there, I'm not sure about the idea of a CSS helper HoC controlling the render for all of our components.

Thoughts?

[nastiatikk]

Hi Material-UI team!

Have been following you for a while and now, seeing this topic, I also wanted to contribute with some thoughts why moving from inline-styles to css might be a good idea. I have read a lot of discussions in this repo that cover the rendering performance, styling children/nested elements, the media queries, pseudo-elements, etc., so I'll focus on something else that I haven't met discussed yet, but which I think is also relevant. And it is styles organization. Here is what I mean:

• Some styles come from from the component defaults (which are set in the node folder and are not touchable)
• Some come from component parameters (for example Avatar component parameter size={40} sets width, height and line-height to 40px)
• When the component defaults are customized (the theme colours) the styles come from theme customization place
• When the the rest of component styles are modified the styles come from a component implementation, which is another part in the code

When it comes to adjust the styles there are at least 4 different locations (👆 ) to look at, which makes it hard to investigate issues.

When styles come with css you'd see the selector and know exactly where to fix. With inline-styles it takes time to understand where the particular property comes from, and where and what should be modified. And if developer modifies it in the wrong place (let's say in the styles area instead of size parameter, since nothing actually tells that size is the style) it will affect the whole width=height=line-height=40px block performance, or will lead to writing again width/height/line-height which will make size parameter inapplicable.

In addition it also makes it hard to investigate which of the parent elements have specific properties applied, because all you can see in the inspector is element.style.

When styles come with selectors (class names) it gives much more control over the styles structure than inline-styles.

Update: Forgot to mention the suggestion (which this topic is about):

/component
--component.js
--component.css (or sass that is converted to css when the page is rendering)

This structure will keep component in the scope, but will give more control over styling. And a BEM className convention will help to avoid unnecessary nesting:

.mui-component {}
.mui-component__child {}

Which overall will help to implement, adjust and maintain Material-UI styles.

[rvbyron]

@chrismcv

Thanks for finding the email and placing it into the commentary!

[rvbyron]

Best of both worlds behavior approach?

What about a "behavior" concept of importing the styling technique that best suits your needs. An import of "StyleTheme" or "ClassTheme" could be used to select the behavior. StyleTheme could refer to the muiTheme object material-ui has today and make the component centric styling developers happy. Or, ClassTheme that would use an SCSS file that is built from the muiTheme.js object (synced) making the CSS centric developers happy. That would mean that all components would then need to accommodate {...theme.ComponentName} in the render elements.

To offer a very simplified example, a RoundButton component would have the render method like:

render() {
    return (<button {...theme.RoundButton} />);
}

For the StyleTheme behavior, theme.RoundButton would contain:

{ 
    style: {
        display: 'inline-block';
        borderRadius: '20px';
        width: '40px';
        height: '40px';
    }
}

For the ClassTheme behavior, theme.RoundButton would contain:

{ 
    className: 'mui-round-button'
}

A combination of the two could be retrieved as ClassStyleTheme behavior, theme.RoundButton would contain both:

{ 
    className: 'mui-round-button',
    style: {
        display: 'inline-block';
        borderRadius: '20px';
        width: '40px';
        height: '40px';
    }
}

The mui-theme.scss file would be generated from the muiTheme.js file and reflect in SCSS the exact properties the JS file contains. The camel casing of the JS names would be converted to more standard class names:

mui-round-button {
    display: inline-block;
    border-radius: 20px;
    width: 40px;
    height: 40px;
}

Any CSS customization to the MUI components would simply be identical classes loaded after the mui-theme.scss file was loaded, thereby overriding the mui-theme properties.

[nathanmarks]

@oliviertassinari

I think that using something like scss (+ css modules for namespacing/hashing) has a lot of benefits. Is this something you are 100% opposed to? I'm probably slightly biased because it is what I am accustomed to using at work, but a lot of people are in the same boat.

My main problem with the JS styling situation is it feels like all of the solutions are still a work in progress. There are not a lot of real world performance evaluations out there to look at, the libs we are looking at are unproven and I feel as though we are spending a lot of our time trying to figure out how to do styles in JS properly. The main goal of this lib is to implement the material design spec and I can't help but feel that this is hamstringing us at the moment. (we def need to sort the perf impact issues)

[alitaheri]

I think we have different concerns regarding this.

1. Where to put the static styles (easy, a css file can do)
2. How to switch some styles depending on input (className switch)
3. How to apply dynamic styles (inline styles can do that)

All seem good when you're writing an application, no one outside your code needs to change anything.

But with libraries things are different, all the above concerns are there, plus the following:

1. How to enable users to customize the static styles?
2. How to implement theme in a way that it's easy to switch, customize and isolated (different subtree different theme)?
3. How to avoid polluting the global namespace so we can play well with others?
4. How can the users override the inlined dynamic styles?
5. transformers! (rtl, prefixer, etc)

What we have right now somehow handles all those concerns with these caveats:

1. Performance!
2. Deeply nested components are hard to customized, as @nathanmarks said: somethingInsideToTheLeftButSlightlyMiddleFocusHoverActiveStyle
3. We can't utilize the power of some css features.

If we change our styling approach we'll have to do it all over again. If it comes to that we should be sure to answer all those before starting migration.

In my humble opinion, I think CSS and what ever that compiles to it are not parts of the future. I think we all agree that inline-styles made our lives a lot easier. Honestly, the only limitation I see with inline-styles is :hover!

We can solve those issues with doing a bit of design.

1. memoize the styles object, for components that have state, we can break down the state and put it in a smaller component so that the style object in it can be memoized based on the passed props that are the higher component's state. This is easily solvable!
2. break them down! why do we have a ListItem with a LOT of small functionalities embedded in it? we can break it down and make it composable, then we don't have to add monstrosities like somethingInsideToTheLeftButSlightlyMiddleFocusHoverActiveStyle
3. meh! all we get performance with is :hover I think we can live with that until browsers or react does something about it.

My point is, we already address a huge number of concerns with our current styling approach. It has limitations because we don't follow all the patterns around inline-styles. if we start breaking down our components and make them a lot more composable then we don't really have to worry about anything. take MeuItem I took that out and made it composable, you see how easy it is to use and how performance can be improved for it using pure rendering!

So instead of changing our approach and solving all these issues again let's invest the time improving what we already have. we don't need to support media queries, we can change our components in a way that's easy for others to implement responsiveness upon them. Hard to debug? if we break down our components then the inlined styles will be smaller and easier to read. debatable though! I mean all you need is a break point to see what participates in Object.assign to see where the styles are coming from.

That's of course my own opinion I'm very open to discussion.

[nathanmarks]

@alitaheri

You raise some great points. Thanks. I'm wondering if some sort of hybrid solution is a possibility. Seems that no matter what angle we take there are a bunch of issues 😆

I have time to work on this next week -- let's have a chat and see if we can come up with a clever idea.

I agree with trying to minimize the amount of change and having to re-solve already solved issues. There's 2 important goals here in my mind:

1. Performance
2. Developer experience (JS style libs help a lot here by enabling developers to use classes in their applications with ease)

Need to think about this more 😄 but as long as we manage to solve those 2 points I'll be happy with our solution.

[nathanmarks]

Another random note -- it would be great if we had an automated way to enforce certain code conventions and design patterns for inline/jsstyle/whatever across the lib.

[oliviertassinari]

Is this something you are 100% opposed to?

@nathanmarks I'm not 100% opposed to use SCSS (I'm using it at work). But from my point of view, it's a dead-end.
One of the main advantages of React is to abstract away the rendering specificity of browsers: the DOM.
As far as I know, the long term goal of the React Core Team is to provide an API to write cross-platform component. Actually, they are currently experimenting into this direction. SCSS won't help.

I completely agree with @alitaheri. We could start with those two steps:

• Use the getStyles pattern everywhere so we can more easily migrate later.
  Will we be able to use a codemod? Who knows.
• Breaking down each component as proposed by @alitaheri.

[nathanmarks]

@oliviertassinari I agree with the dead-end conclusion -- it's not the future. I brought it up largely to stimulate the conversation about how to make JS styles work for us. I knew some people around here had some good ideas surrounding the issues 😄 Wish FB were a tiny bit further along with their experiments!

I actually think the getStyles() pattern we currently employ has it's own problems. @alitaheri and I had a pretty good chat today about what to do to improve our JS style setup (including theming!). I'll reply back tomorrow with some more info once I get a chance to jot down some notes!

Next week I'm on vacation and I'm going to experiment with a solution to our current problems while keeping all styles JS based.

[hburrows]

Just read this post on Lessons Learned At React Amsterdam (https://medium.com/@kitze/lessons-learned-at-react-amsterdam-51f2006c4a59#.o12h794or) and one of presentations was about solutions for styling in React: http://slides.com/kof/javascript-style-sheets#/ A timely presentation for this discussion.

[hburrows]

One requirement that I keep thinking about and that I haven't seen explicitly stated (at least that I recall) is the need to support web-only vs. web and react native. If the intent is web-only (i.e. react native isn't a requirement) then solutions that leverage what browsers already efficiently and robustly support and people are very familiar would be a good thing. If supporting react native is a must then that opens up a different set of needs & requirements. Just something to think about and keep in mind as the technology choices, compromises, etc. are evaluated.

[nathanmarks]

@hburrows

Thanks for sharing that presentation!

@alitaheri

We may want to look at the lib mentioned in that presentation (here's a demo). It could save us a lot of work. I was looking at it previously but there were a few things I didn't like (such as the requirement of wrapping every single component in a proprietary HoC). However, there is some discussion about some of that happening here . He mentions that implementing changes like this would allow for HoC-free use. I do prefer that method/design for the sheet writing (also seen in aphrodite).

Alternatively, we could always create our own react bindings for jss that can work with mixout.

The lib author also said this, which falls in line with my own thoughts on the matter 👍 :

However you need to understand that not everything makes sense to be style sheets. For the most dynamic changes you should use inline styles.

[alitaheri]

@nathanmarks The implementation is rather very small and very easy to mixout 😁
https://github.com/jsstyles/react-jss/blob/master/src/index.js
I think this library can help us out 👍

[jrop]

For myself, I appreciate the way Material-UI does the styling as opposed to something like react-toolbox, and here's why: In one of our projects, we have the ability for the user to select dynamic themes:

Because the user might select a color pair we cannot determine ahead of time, whatever theming scheme we use must be highly dynamic. With , changes to the theme can happen live, b/c all the styles are generated inline: all I have to do is give it primary/accent colors.

In react-toolbox et. al., this seems to be more of a chore from what I can see.

[DominikGuzei]

@jrop yeah in this very special case you need dynamic JSS but for many applications that's not an important requirement and the current story of customizing (not even speaking about UI testing) material-ui is simply not acceptable. We are building a highly visual and designed app that cannot just use the default material-ui look, as we mostly need/want the behavior parts of it.

[rosskevin]

@DominikGuzei you can customize next branch using jss. This is not what you see on the current release.

Folks, I feel like we are beating a dead horse here. The next branch has has chosen a different (much improved) path with theming and allows for overrides. It is not using inline styles any more and provides the flexibility I have seen requested. For others that want more customizability, they can use an HOC over the next components and write their own stylesheets.

@nathanmarks, since this decision has been made (as far as I can tell, and works quite well 👍 ), I suggest you write a summary and close/lock this issue so those that find it understand the way forward.

[oliviertassinari]

if one could have a Wiki page about the project goals and philosophy (written by the core maintainers) with a strong remark on why Javascript styling is so ingrained into the framework.

@fgarcia I have given a presentation at a local event in Paris earlier this week regarding the evolution of the styling approach used by Material-UI. You might be interested having a look at the slides and the notes: A Journey toward better style. I focus on the Challenges, Problems and Solutions.

the user to select dynamic themes

The static style generation aspect of CSS-Modules has quite some consequences.
Injecting style at runtime is less cost efficient but allow us to:

• Gives us more flexibility with the style implementation. E.g. The CircularProgress has a fixed thickness linked to how the CSS keyframe animation is implemented. With dynamic style, we can add a thickness property so users can change the value dynamically. ⚠️ not to be abused.
• On large scale app, it allows to inject only the needed styles for the displayed component on the screen. Reducing the amount of worked needed by the browser to bootstrap the page. Hence, faster time to first paint after interaction. E.g. inlining critical CSS with SSR.
• Allow users to use dynamic theme and variation of components. E.g. changing the color of a button with all the color variations needed.

[jrop]

@oliviertassinari That is awesome to hear. I may have a non-standard point-of-view regarding CSS modules, but I don't think they are worth all the fuss. I was excited to hear of JSS through material-ui for this reason. It seems much more inline with the philosophy of JavaScript than CSS modules.

[DominikGuzei]

@oliviertassinari thanks for the summary of benefits. It's definitely interesting when you look at these aspects. The downside of JSS and it's ecosystem (also postcss) at the moment is that all that is bleeding edge and you cannot rely on the well established tool chain and knowledge base like with simple css-modules + SASS. But that's the price you pay for innovation, it just depends if you have certain requirements / are willing to pay the price of bugs and the less documented path.

[kof]

JSS has been created prior to css-modules. CSS-modules has become popular faster because it solved a problem for a majority users without changing the syntax too radically.

[kof]

That being said, you are using react, which is also kinda "new" technology. If you want SASS, maybe you should also pick vanilajs or backbonejs)

[DominikGuzei]

@kof React is a very different story as it is used by hundreds of thousands of devs and is managed by Facebook (it won't go away over night), so it might be "new" but very solid (haven't run into many issues with it so far).

SASS is one of the most established css preprocessor and can be used in any project – it's very solid and brings a lot of value to the table. So i would only choose something like JSS if absolutely necessary (e.g: because of dynamic themes) but dislike to throw out years of experience / frameworks / knowledge base etc. in a commercial software project with several devs. So i would even consider building only the dynamic theme aspect in JSS and the rest with sass / css-modules.

[kof]

I understand this way of thinking, but also I consider it as contra-productive and blame it for slowing down innovation and the industry.

[delijah]

Why u just don't support both? Styles written in JavaScript AND styles written in css-modules? If i have a look at https://github.com/callemall/material-ui/blob/next/src/Button/Button.js for example, it would be quite easy to use class-names passed in by a prop (https://github.com/javivelasco/react-css-themr) instead of using:

const classes = this.context.styleManager.render(styleSheet);

Isn't it?

It doesn't look like the discussion about using JSS or CSS is finding an end (yet?). To me it looks like a fifty/fifty thing at the moment.

[mgcrea]

Regarding SASS, react-toolbox are currently migrating away from it in favor of a full postCSS solution, I personally think postCSS (with sassLike plugins) is a better pick, more powerful without build deps.

[estaub]

Styling in all JS, and in React in particular, is obviously still in a rapid state of evolution. So whatever you do, I'd try to avoid being opinionated - just fit well with with a broad set of current usage.

Consider just adding a *className prop wherever there's a *style prop. Period.

Full disclosure: I'm a happy Aphrodite user.

[ligaz]

Adding a className will still require !important hacks because the inline styles (from style property) takes precedence.

[estaub]

@ligaz Of course, you're right. Aphrodite adds !important to everything by default, so perhaps my thinking about a "broad set of usage" is badly skewed.

[rosskevin]

@ligaz @estaub - next is already coded without style and allows users of the components to override by providing className (and multiple class names to the various parts of complex components).

The styling paradigm setup on the next branch by the maintainers is using JSS. It is a pleasure to use and easy to override on an individual or a theme basis.

This horse is dead. Let's stop beating it. @oliviertassinari I suggest you close and lock this issue with a statement of direction on next.

[giuseppepaul]

Hi @rosskevin - can you point me at any examples at all of how this is working in next?

I'm currently looking at frameworks for a new project, I'd love to use Material UI but we'd need the changes that you've mentioned are in next. I've added the next branch to a test project and added a simple component and I'm seeing an inspector full of inline styles, so I'm guessing I'm doing something wrong?

Any further details would be really helpful!

Thanks

[rosskevin]

@giuseppepaul next is not published on npm because it is pre-alpha and is not yet complete. You can clone https://github.com/callemall/material-ui.git#next and run the docs/site to see that there are no more inline styles. It also has a reasonable amount of demos.

I am publishing my own private version of next (so I can control the updates) and am using this in a soon to be production application.

[oliviertassinari]

This horse is dead. Let's stop beating it. @oliviertassinari I suggest you close and lock this issue with a statement of direction on next.

That horse isn't fully dead. But definitely in a tight spot 🔫 .
I agree, let's close this issue.

A big thanks to @nathanmarks for his hard work on that story 👏 !
I have gathered more information on the road ahead with #5718.

[ilan-schemoul]

Hello,
IconButton have a CSS3 transition property for ripple effect. In the complex interaction of the doc of Card@next, you have added a transition to transformation for add a transition when the icon reverses.
If I do the same on my app, one transition (ripple or reversion) overrides the other (an element cannot have two properties otherwise one overrides the other one).
However your chevron icon has successfully two transitions applied to it. Because in a someway you merge transitions provided by IconButton and the one defined in your code. How do you achieve that ?
Some other stuffs are obscure for me and I cannot find related doc. theme has a lot of methods/properties as breakpoints (not used here) transitions (used here). Is there already doc about theme property of what MuiThemeProvider.createDefaultContext() returns as it seems useful especially that it's used extensively inside your code.
this.context.styleManager.render(styleSheet) ? What ?
You also use jss-theme-reactor which I still don't understand the point compared to react-jss.

[oliviertassinari]

@NitroBAY This thread is over. Please ask you question on the right channel.if you notice a bug or want the documentation to be improved, open an issue. Otherwise you can use StackOverflow of gitter.im.