Code Monkey home page Code Monkey logo

Comments (95)

jackcallister avatar jackcallister commented on March 29, 2024 1

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.

from material-ui.

totty90 avatar totty90 commented on March 29, 2024

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)

from material-ui.

aranw avatar aranw commented on March 29, 2024

@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?

from material-ui.

totty90 avatar totty90 commented on March 29, 2024

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.

from material-ui.

robertknight avatar robertknight commented on March 29, 2024

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!

from material-ui.

totty90 avatar totty90 commented on March 29, 2024

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

from material-ui.

pspeter3 avatar pspeter3 commented on March 29, 2024

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?

from material-ui.

vjeux avatar vjeux commented on March 29, 2024

@pspeter3 do you have any benchmarks? :)

from material-ui.

pspeter3 avatar pspeter3 commented on March 29, 2024

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?

from material-ui.

totty90 avatar totty90 commented on March 29, 2024

@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

from material-ui.

hai-cea avatar hai-cea commented on March 29, 2024

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!

from material-ui.

mcwhittemore avatar mcwhittemore commented on March 29, 2024

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.

from material-ui.

jackcallister avatar jackcallister commented on March 29, 2024

@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.

from material-ui.

mcwhittemore avatar mcwhittemore commented on March 29, 2024

@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.

from material-ui.

jackcallister avatar jackcallister commented on March 29, 2024

@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.

from material-ui.

totty90 avatar totty90 commented on March 29, 2024

@mcwhittemore why do you need *:after?

from material-ui.

mcwhittemore avatar mcwhittemore commented on March 29, 2024

@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?

from material-ui.

totty90 avatar totty90 commented on March 29, 2024

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.

from material-ui.

mcwhittemore avatar mcwhittemore commented on March 29, 2024

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

from material-ui.

WRidder avatar WRidder commented on March 29, 2024

@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.

from material-ui.

hai-cea avatar hai-cea commented on March 29, 2024

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

from material-ui.

jackcallister avatar jackcallister commented on March 29, 2024

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.

from material-ui.

nigelsmith avatar nigelsmith commented on March 29, 2024

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.

from material-ui.

hai-cea avatar hai-cea commented on March 29, 2024

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.

from material-ui.

pspeter3 avatar pspeter3 commented on March 29, 2024

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

from material-ui.

hai-cea avatar hai-cea commented on March 29, 2024

@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

from material-ui.

cuongcua90 avatar cuongcua90 commented on March 29, 2024

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

from material-ui.

hai-cea avatar hai-cea commented on March 29, 2024

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?

from material-ui.

lucasjans avatar lucasjans commented on March 29, 2024

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.

from material-ui.

hai-cea avatar hai-cea commented on March 29, 2024

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?

from material-ui.

msikma avatar msikma commented on March 29, 2024

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.

from material-ui.

daniellewissc avatar daniellewissc commented on March 29, 2024

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.

from material-ui.

facundocabrera avatar facundocabrera commented on March 29, 2024

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!

from material-ui.

msikma avatar msikma commented on March 29, 2024

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.

from material-ui.

robertknight avatar robertknight commented on March 29, 2024

@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:

  • Modules, variables, calculations and mixins can all use existing tools that JS already has - CommonJS modules, JS variables, JS expressions rather than having to use a separate syntax and set of tools to manage them.
  • You can use existing JS tools to find out where a particular style is referenced. You can take this further with FlowType and TypeScript to get compile-time errors if you mistype a class name for example.
  • You can define the structure, appearance and logic in one place - which is a useful step towards making it easier to re-use components. Web Components solves this problem with HTML imports. Defining everything with one language is another approach.
  • Doing dynamic things with CSS is easier. Because both the static and dynamic styling is implemented in the same language, you can share constants etc. between them.

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 .

  • The theme + structure for the button is in one place, which is useful for development and re-use. Using an IDE you could easily find where all the particular elements of theme are referenced.
  • In the render() function I can build up the styling for the button depending on the props by referencing parts of the theme in a simple way without having to concatenate class-name strings.
  • When defining the theme, I can use JS to calculate certain colors using formulae specified by the material design spec (See colors.premultiplyColor())

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.

from material-ui.

facundocabrera avatar facundocabrera commented on March 29, 2024

@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/

from material-ui.

msikma avatar msikma commented on March 29, 2024

@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?

from material-ui.

robertknight avatar robertknight commented on March 29, 2024

@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.

from material-ui.

msikma avatar msikma commented on March 29, 2024

Thanks for the explanation, sounds very good.

from material-ui.

ezmiller avatar ezmiller commented on March 29, 2024

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!

from material-ui.

TimothyKrell avatar TimothyKrell commented on March 29, 2024

@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>.

from material-ui.

vmakhaev avatar vmakhaev commented on March 29, 2024

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

from material-ui.

natew avatar natew commented on March 29, 2024

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!

from material-ui.

mjackson avatar mjackson commented on March 29, 2024

@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:

  • There is no global Theme object
  • An entire section of the UI can be themed just by changing the context in a single parent component (think theming an entire modal with one declaration)

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

from material-ui.

hackhat avatar hackhat commented on March 29, 2024

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.

from material-ui.

hackhat avatar hackhat commented on March 29, 2024

@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?

from material-ui.

natew avatar natew commented on March 29, 2024

@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: reapp/reapp-ui#29.

Sorry to threadjack, just saw an opportunity for some teamwork 👍

from material-ui.

hai-cea avatar hai-cea commented on March 29, 2024

@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.

from material-ui.

DylanPiercey avatar DylanPiercey commented on March 29, 2024

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

from material-ui.

mmrtnz avatar mmrtnz commented on March 29, 2024

@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:

  • Components will also be given a style prop to override the inline styles of a components root element. This will be carried out by the StylePropable mixin.
    • Some components will have additional props for styling internal component elements. Components without these additional props will not have access to override these styles. This is an anticipated breaking change after refactoring is complete. We'd love to hear your thoughts about this. Our reasoning was that overriding these styles break away from Material Design specifications.
  • For users still using Less, components will be given a className prop to their root element, so that users can add additional styling (this is the current purpose for our Classable mixin).
  • Another breaking change is the removal of all Less files including Less variables. This will change mui from being css-framework and set of react components to only being a set of react components. We believe that this shift is more appropriate as we continue to complete more Material Design components.

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?

from material-ui.

natew avatar natew commented on March 29, 2024

By the way we released themeing via context with reapp-ui. You do it like so:

const theme = {
  styles: {
    List: {
      self: { background: '#000', ... }
    }
  },
  animations: {},
  constants: {}
}

// top level component
export default React.createClass({
  render() {
    return (
      <Theme {...theme}>
        <RouteHandler />
      </Theme>
    );
  }
});

Check out the Theme file for how it sets the context:
https://github.com/reapp/reapp-ui/blob/4355556dee24407e5e7827df7a484944aeaf32cc/helpers/Theme.jsx

We also found it helpful to use constants as a first level layer so you can set all sorts of properties from isRetina to listBorderColor and then those constants are passed to styles when they are loaded through the helper. Usage like so:

import UI from 'reapp-ui';

UI.addConstants({
  borderColor: '#000'
});

UI.addStyles({
  List: c => ({
    firstItem: {
      border: `1px solid ${c.borderColor}`
    }
  })
})

// exports the theme object you need for the <Theme /> helper
export default UI.makeTheme()

Thought this may be helpful to look at as you guys dive into it.

from material-ui.

mmrtnz avatar mmrtnz commented on March 29, 2024

@natew I have been leaning towards this approach. Can you elaborate on the meaning of c? I'm not too familiar with ES6's fat arrow function. Is c the new constants parameter? Currently, inline-styles are overriden using React's shallow merge via our StylePropable mixin.

However, this method doesn't handle nested json objects unless they're explicitly passed. We've talked about using a recursive merge, but this will be expensive because every component would use the merge function multiple times. Seeing that we're already using ES6, this looks promising.

from material-ui.

bparadie avatar bparadie commented on March 29, 2024

I love the idea of refactoring material-ui's CSS into Javascript!

But I did run into some nasty problems when I tried to use inline styles in JS instead of LESS in one of my projects...

Safari: Ripple style bug on button components

The biggest problem was this one: #496
I could only work around the Safari problems by using grunt-autoprefixer
But autoprefixer only works for LESS/CSS. So at that point I had to abandon inline styles and rewrote my app to using LESS. (That was a bummer...)

Pseudo styles and media selectors are not supported

Some CSS features like all of the pseudo styles and media selectors are not supported by react's inline styles. In the beginning I ended up using LESS for the missing pieces and inline styles for the rest.

Auto-appending "px" to numbers

There are a bunch of properties where react inline styles decide to append px to a number. For example order: 1 becomes order: 1px, which is nonsense.
Having to deal with unwanted px seems to be a common problem, see facebook/react#3499

Poor minification support

I have to use Google's Closure Compiler with ADVANCED_MODE for my projects. When I tried to use react inline styles I ended up externing at least these properties:

/**
* @type {*}
*/
var __REACT_DEVTOOLS_GLOBAL_HOOK__;

/**
* @type {*}
*/
var Symbol;

/**
* @type {*}
*/
var MSApp;

In addition I had to use strings for property names like backgroundSize and float.
For example, instead of using

var iconStyle = {
  backgroundSize: "300px 400px",
  float: "left"
};

I had to use strings instead in order to be able to compile with Google's Closure Compiler in ADVANCED_MODE

var iconStyle = {
  'backgroundSize': "300px 400px",
  'float': "left"
};

But other than those problems everything was fine!
Don't get me wrong: I think inline styles is the right way to go. There are just a few bumps along the road. I am hopeful that all of those problems will be addressed by Facebook, that is, @vjeux.

from material-ui.

hai-cea avatar hai-cea commented on March 29, 2024

Thanks to everyone's input, we were able to complete the less refactor to inline styles yesterday! The current master contains a prerelease of v0.8.0 for you guys to check out, and we'd love any feedback that you may have.

You can also run the docs site locally to check out the updated documentation. Here are some highlights on what we were able to achieve:

  1. All component styles are now done inline. This not only allowed us to be more explicit about the styles that go into each component, but also made each component much more portable. Since there aren't any external less/css dependency, you should be able to just require each component that you need. Also, styles from the MUI library won't conflict with styles from your own app.
  2. Customization can be achieved on 2 levels. You can override theme variables that will affect all children components using theme context object. Also, each component accepts a style prop which you can use to implement finer grain customization.
  3. Automatic vendor prefixing is done through feature detection using a custom, light weight build of Modernizr. Prefixing is only done when needed and be best part? It's automatically done for your within each component.

I'd like to have this bake in master for a week or 2 before releasing it. During this time, we'll work on putting together example code and converting some of the docs site styles to use in-line styles.

Thanks again for everyone's input and patience. We're really looking forward to releasing this and getting back to addressing issues and adding new components. :)

from material-ui.

ukabu avatar ukabu commented on March 29, 2024

Do we need to do something beside just using the components? I'm trying it out and I get a TypeError: Cannot read property 'component' of undefined in the components getStyles method, whenever this method access this.context.theme.component...

Note that I'm trying to package it as a meteor package (essentially using browserify to bundle all the components) and I may do something wrong.

Looks like we have to instantiate a ThemeManager and set it as a theme childContext on an component in which we use material-ui components.

from material-ui.

mmrtnz avatar mmrtnz commented on March 29, 2024

EDIT: theme has been renamed to muiTheme

Hi @ukabu, ThemeManager only needs to be instantiated at the outer most component of your app. It will then passed to all its children and grandchildren. You can read more about React's context feature here and here.

Include this code in your outermost component:

If you're using ES6

var React = require('react');
var mui = require('mui');
var ThemeManager = new mui.Styles.ThemeManager();
class OuterMostParentComponent extends React.Component {
  // Important!
  getChildContext() { 
    return {
      muiTheme: ThemeManager.getCurrentTheme()
    };
  }
};
// Important!
OuterMostParentComponent.childContextTypes = {
  muiTheme: React.PropTypes.object
};
module.exports = OuterMostParentCompone

If you're using ES5

var React = require('react');
var mui = require('mui');
var ThemeManager = new mui.Styles.ThemeManager();
var OuterMostParentComponent = React.createClass ({
  // Important!
  childContextTypes: {
    muiTheme: React.PropTypes.object
  },
  // Important!
  getChildContext: function() { 
    return {
      muiTheme: ThemeManager.getCurrentTheme()
    };
  }
});
module.exports = OuterMostParentComponent

More documentation about how to use Themes can be seen if you run the docs locally and go to #/customization/themes

from material-ui.

ukabu avatar ukabu commented on March 29, 2024

Thanks. It's working now 👍

from material-ui.

ukabu avatar ukabu commented on March 29, 2024

Now that all the styles are inline, we still need to have a reset css. It seems that there is no css or less files in the src/ tree. Do you plan on adding one?

from material-ui.

bparadie avatar bparadie commented on March 29, 2024

@hai-cea @mmrtnz Congratulations! Do you have any recipes for code that relies on CSS Pseudo-classes? I'll miss :hover for sure, see reactjs/react-future#13 . @vjeux recommends using onMouseOver but that's a pain. Does 0.8.0 come with some handy replacement code for :hover?

from material-ui.

vjeux avatar vjeux commented on March 29, 2024

@bparadie I changed my mind on this. I think I want the following to work:

<div
  style={{color: 'blue'}} 
  hoverStyle={{color: 'red'}}
/>

from material-ui.

JedWatson avatar JedWatson commented on March 29, 2024

@vjeux are you talking at the React framework level or would that be a thing projects would implement individually? because at the framework level that would be exciting...

It begs for some variations though like pressedStyle focusedStyle disabledStyle (for inputs) etc.

Also, congratulations to everyone who worked on this, big achievement 😄

I was wondering though might it be better to use a more specific context key for the theme? e.g muiTheme - then this wouldn't conflict if there were to be another library on the page that also wants to use that context key, or if I wanted to use context.theme in my app for my own thing.

from material-ui.

bparadie avatar bparadie commented on March 29, 2024

@vjeux I like that!

from material-ui.

troutowicz avatar troutowicz commented on March 29, 2024

@bparadie

This is a util that I have been experimenting with to make hover styling a little less annoying...

'use strict';

const React = require('react');

const withHoverStyle = (Component, compType) => {
  class ComponentWithHover extends React.Component {
    _handleMouseOver() {
      const theme = this.context.theme.component[compType];

      React.findDOMNode(this).style.backgroundColor = theme.hoverColor;
    }

    _handleMouseOut() {
      const theme = this.context.theme.component[compType];

      React.findDOMNode(this).style.backgroundColor = theme.color;
    }

    render() {
      const hoverProps = {
        onMouseOver: this._handleMouseOver.bind(this),
        onMouseOut: this._handleMouseOut.bind(this)
      };

      return React.createElement(Component, Object.assign({}, this.props, hoverProps));
    }
  }

  ComponentWithHover.contextTypes = {
    theme: React.PropTypes.object
  };

  return ComponentWithHover;
};

module.exports = withHoverStyle;

https://github.com/troutowicz/geoshare/blob/css-in-js/app/utils/withHoverStyle.js

Can be used like this:

'use strict';

const React = require('react');
const FlatButton = require('material-ui/lib/flat-button');

const withHoverStyle = require('../utils/withHoverStyle');

class AuthButton extends React.Component {
  render() {
    return <FlatButton {...this.props} />;
  }
}

AuthButton.propTypes = {
  label: React.PropTypes.string,
  onClick: React.PropTypes.func
};

AuthButton.defaultProps = {
  label: 'Login',
  onClick() {}
};

module.exports = withHoverStyle(AuthButton, 'flatButton');

https://github.com/troutowicz/geoshare/blob/css-in-js/app/components/AuthButton.jsx

from material-ui.

vjeux avatar vjeux commented on March 29, 2024

I want it at the React level and also at the dom level :) Unclear when i'll the time to implement it

from material-ui.

bparadie avatar bparadie commented on March 29, 2024

@troutowicz Thanks for sharing!

from material-ui.

hackhat avatar hackhat commented on March 29, 2024

Hy!
I've seen you are talking about css in js for a while. Here is a CSS in JS lib comparison https://github.com/hackhat/css-in-js-comparison take a look. Already reviews 3 libs, might be interesting to see the different approaches to this problem.

Also take a look at https://github.com/hackhat/smart-css, you don't need of onMouseOver to add ":hover"

from material-ui.

JedWatson avatar JedWatson commented on March 29, 2024

Did anyone have feedback on my idea to use a different context key for theme? e.g muiTheme

@vjeux if you pointed me in the right direction to implement that in React I'd be happy to have a go, don't want to hijack this thread though so let's discuss elsewhere :)

from material-ui.

anyong avatar anyong commented on March 29, 2024

I hope this is being fully tested with other react modules that depend on context, because I've so far come across at least one library that clobbers context from other libraries - flocks.js. The author there is working on it, but please make sure that material-ui is not going to clobber react-router or any other modules that use contexts. Great idea though, happy to see inline styles and hope it will make our lives easier for using material ui 👍

from material-ui.

hai-cea avatar hai-cea commented on March 29, 2024

@JedWatson Yes, I think it's a great idea. We'll definitely do that.

from material-ui.

JedWatson avatar JedWatson commented on March 29, 2024

@hai-cea awesome, glad to hear it 😄

@anyong / @vjeux / anyone - is there any documentation available on how context behaves when there are multiple libraries using it? i.e how do we (as library authors) make sure we play nicely? I haven't done a lot of testing against how defining context at different levels works.

from material-ui.

mmrtnz avatar mmrtnz commented on March 29, 2024

@bparadie I would like to add a mixin or class like @troutowicz's example for pseudo selectors in the near future, but for now you'll have to use events like onMouseOver. Although I agree that it can be a pain.

@JedWatson That's a great idea, I'll be making an update shortly.

@ukabu There currently are not any plans for readding a resets.css file. We decided to have Material-UI move away from being a css framework to just a set of React components. However, the resets.css can be found online if it is needed.

from material-ui.

ukabu avatar ukabu commented on March 29, 2024

@mmrtnz I understand, but as it is right now, some components won't display properly with the user agent styles. Example: if I don't include any css, h1 have top margins (user agent style), which prevent the AppBar title to be aligned properly.

from material-ui.

mmrtnz avatar mmrtnz commented on March 29, 2024

@ukabu I see what you mean, thanks for pointing this out. Components whose styles depend on resets.css and other related support css files have been redefined inline. This appears to be an example that I have missed during refactoring. If more issues are found related to the need for resets.css or any other stylesheet we can add them back, but not before trying to solve the issue at a component level first.

from material-ui.

wmertens avatar wmertens commented on March 29, 2024

For hover etc I opened #684.

from material-ui.

hai-cea avatar hai-cea commented on March 29, 2024

Thanks to everyone for this awesome discussion. We were able to release this feature in v0.8.

from material-ui.

pablofierro avatar pablofierro commented on March 29, 2024

I was looking forward to using this library but seeing everything uses inline styles was a huge turn down, I do not understand why you would do this, I read the slides on why it was done but it makes no sense, why merge appearance, views and logic into one single thing, separation of concerns people, that's one of the most solid statements for software engineering.

from material-ui.

wmertens avatar wmertens commented on March 29, 2024

Pablo, these concerns are not separate. If you want things to look
differently you can use the theme manager, a true separation of concerns.
For the rest, what components should do and their html and css are closely
coupled, and separating that out just makes things more complex.

On Sun, Aug 23, 2015, 05:35 Pablo Fierro [email protected] wrote:

I was looking forward to using this library but seeing everything uses
inline styles was a huge turn down, I do not understand why you would do
this, I read the slides on why it was done but it makes no sense, why merge
appearance, views and logic into one single thing, separation of concerns
people, that's one of the most solid statements for software engineering.


Reply to this email directly or view it on GitHub
#30 (comment)
.

Wout.
(typed on mobile, excuse terseness)

from material-ui.

grrowl avatar grrowl commented on March 29, 2024

I agree with @pablofierro — inline styles can't be cached by the browser and we lose fairly basic CSS functionality like :hover, sibling selectors or any notion of cascading styles.

It's a compromise to go with inline styles to enable static client-side-only apps and avoid dependencies on pre-processing toolchains (webpack, etc), but it lets down larger projects with the extra capability (isomorphic apps most prominently). I'd love the option using external stylesheets or use of CSS Modules support but it'd screw over the simpler cases.

from material-ui.

Kagami avatar Kagami commented on March 29, 2024

@grrowl cascades are considered bad practice in large stylesheet codebases anyway. See e.g. BEM approach.

from material-ui.

pablofierro avatar pablofierro commented on March 29, 2024

@Kagami considered bad practice by whom ? If you break your stylesheets into reusable modules(similar to the react philosophy) this becomes highly maintainable. Even React's webpage mentions it is the View for your components and not the appearance, this decision of forcing everything to be inline is just plain ridiculous.

from material-ui.

rozzzly avatar rozzzly commented on March 29, 2024

This needs to be changed. Inlining certainly has it's benefits in some use cases, but the resulting markup is:

  • bloated -- a ton of bandwidth is gonna be used up just to send down the style. In every request. For every element node... Just a quick estimate--I don't care to spend any time doing the metrics--here, but it would not be unreasonable to assert that the resulting markup could be 5x LARGER for some run of the mill app. And what if you have an app with many, many DOM nodes? And how does that affect memory overhead and performance? This brings in a whole host of implications that we shouldn't have to deal with.
  • ugly -- have fun reading what it generates, its about as enjoyable as reading raw/unformatted minified css.
  • and stupid -- all the way back to the CSS 1 spec, the WW3C included classes for a reason. This approach styling throws the flexibility/rapid prototyping/functionality the community has gained by creating LESS/SCSS/etc. etc. out of the window. You can already see the wheel being reinvented; scroll up though this thread and look at all the comments suggesting different libraries/whatever for extracting the css from javascript.

But wait! There's more:

  • No out of the box IDE support
  • No psuedoclass support

Guys: this is something jquery can do. Why do we have to dredge this up to the surface? moreover force it by default?

a fun case-in-point

Open Chrome Developer Tools
Try making some style edits--change the background color or something--to a page that has several mui components of the same type, e.g. the <MenuItem ...>'s in a navigation menu.

it only changes that component. What about React Devtools? nope same thing.

I don't mean to be a dick--although I most assuredly sound like one, but as @pablofierro said, this is ridiculous. This functionality may be cool, the lack of css dependencies is very appealing. But ripping out the .css doesn't make sense.

from material-ui.

rozzzly avatar rozzzly commented on March 29, 2024

I should add; I'd be more than happy to create a PR. I just want to hear what the community has to say about reimplementing this; hopefully come to some kind on consensus on how to go about this.

from material-ui.

oliviertassinari avatar oliviertassinari commented on March 29, 2024

What do you suggest to do instead? For my use case (Cordova app) the inlined approach is relay great.

from material-ui.

DylanPiercey avatar DylanPiercey commented on March 29, 2024

@oliviertassinari it seems like the biggest benifit of inline styles is that the project does not need to have an external style sheet as a dependency.

If this is the case, I would recommend reverting back to classes (as it seems many want that and it seems to make things simpler) and instead exposing all of the CSS via a string.

/*styles.css for people who are fine with a separate file*/
.my-styles {...}
// styles.js (expose as "material-ui/theme" or something)

var css = { __html: "injected styles.css content" };

// alternatively we could expose a component.
module.exports = (
    <style dangerouslySetInnerHTMLIfYouAreSureItIsWhatYouWantToDo={ css }/>
);

This way people can use the theme directly with JavaScript by:

theme = require("material-ui/theme");

MyJSX = (
    <div>{ theme }</div>
);

Pros:

  • its just css
  • potentially multiple themes without it all being bundled
  • can be imported through js, css pre processor or manually in html.

Cons:

  • a bunch of time was spent making all styles inline.

from material-ui.

smontlouis avatar smontlouis commented on March 29, 2024

@rozzzly +1111111000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000

With MUI -> http://recordit.co/9mBknPLZUb
Without MUI -> http://recordit.co/6MkzWhQiBt

from material-ui.

oliviertassinari avatar oliviertassinari commented on March 29, 2024

Well @rozzzly I disagree.

  • bloated. Sure, we use more bandwidth to send the style when the user arrives in the site.
    But latency and not bandwidth, is the performance bottleneck for most websites. for more detail.
    So, I think that sending the style inlined lower down the first render time. And this is what really matters for users. You don't have to wait the RTT (round-trip time) to have the style.
  • ugly. Yeap, it's not easy to read. Why do you need to read it?
  • and stupid. Facebook explained their motivations for using inlined style. It's pretty convincing. The link

from material-ui.

iclanzan avatar iclanzan commented on March 29, 2024

I’ve been using inline styles exclusively for more than 6 months and it has been a liberating experience. Styles have become more manageable, flexible and with far less surprises compared to CSS.

from material-ui.

adamscybot avatar adamscybot commented on March 29, 2024

I personally agree with @rozzzly who details the issues well. This just isn't a good idea at scale. CSS works. We're reinventing the wheel just so its more "portable". This insane change boils down to fixing one problem -- including css. This would take 1 line of explanation in the readme. Not to mention, even rookie developers are well versed in including CSS in HTML or with their build tool. This isn't new people.

Instead we're rebuilding styling for web pages in JS. It just seems silly. Absolutely basic styling features like pseudo elements? Gone. SASS/LESS compilation? Gone.

from material-ui.

mpseidel avatar mpseidel commented on March 29, 2024

Hey everyone. I started using material-ui in a side project and have been following the discussion here and here.

I wanted to add a few points to the mix:

Does this make sence for smaller applications?
I have written a lot of smaller internal web apps for businesses using bootstrap and the like. I have never felt much pain when having to include some more component-css alongside its markup and js. I'ts a one time effort. Conflicts were very seldom an issue and easy to resolve most of the time.

But I defenitely feel pain looking at the DOM in my devtools now. Even seeing the basic tree structure is much much harder with all the inline styles. And this pain is not a one time thing - its coming back everytime I open up devtools.

Are we alienating CSS-savvy designers here with this approach?
With all the styles embedded in JavaScript it seems hard for designers that are used to draft and edit working in CSS files. Aren't they now forced to learn JavaScript, ES6, transpilers, buildtools etc and are much more likely to break stuff?

Maybe thats not a bad thing - I am all for cross functional teams and I haven't worked with "component designers" yet, but I want to make sure designers get considered in this discussion as well.

Also:

+1 for separation of concerns - styling and ui logic have to be separated in my book
+1 for considering lack of IDE-Support

I absolutely agree that there are problems with CSS especially at scale. I hope this discussion leads to a better place for all of us in this regard.

from material-ui.

wmertens avatar wmertens commented on March 29, 2024

Well, I too found the talk convincing but in practice using mostly css with
module-based renaming and maybe some inline styling is what is easiest to
work with imho.

On Tue, Nov 24, 2015, 9:49 AM Owen Campbell-Moore [email protected]
wrote:

For what it's worth, use of inline styles was a large reason I became a
huge fan of MUI. I found the talk by Facebook extremely convincing and
personally feel that style and structure do not make sense to be separated
when building UIs.


Reply to this email directly or view it on GitHub
#30 (comment)
.

Wout.
(typed on mobile, excuse terseness)

from material-ui.

justjacksonn avatar justjacksonn commented on March 29, 2024

Interesting points @mpseidel. I however think as more and more frameworks move towards components, it seems to me keeping the JS and CSS together as part of the component makes more sense than having a more global css include that covers a range of components, pages, etc. However, the concept of theming components has me scratching my head.. I don't know CSS much, so maybe there is an easy way for components to pick up theming styles to overwrite their default styles so that component writers can have their components fit in with an overall theme style? If there is I'd like to understand that more.

from material-ui.

akaRem avatar akaRem commented on March 29, 2024

There is a big discussion here. And It's hard to guess overall direction.
Can anyone estimate the chances that styles will be returned to their place?

I don't use material-ui yet. And from my "fresh" point of view, I see these problems which beware me of using this project in future:

  • presentation and logic layers are messed
  • performance degradation due to inline styles
  • hard to add own styles
  • Ugly unreadable DOM

I don't see any benefits from inline styles.

It would be better to reorganize less/sass/css in more readable and maintainable way than try to reinvent styling from scratch.

from material-ui.

oliviertassinari avatar oliviertassinari commented on March 29, 2024

This discussion is pretty much finished.
Please follow #684.

from material-ui.

akaRem avatar akaRem commented on March 29, 2024

@oliviertassinari .. it means no way back to CSS?

from material-ui.

oliviertassinari avatar oliviertassinari commented on March 29, 2024

It means that we are exploring options to improve the current style approach (CSS Modules, Radium, Looks, react-themeable).

from material-ui.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.