elementalui / elemental

A flexible and beautiful UI framework for React.js
elemental-ui.com
MIT License
4.31k stars 216 forks source link

Better css in js #53

Open kof opened 9 years ago

kof commented 9 years ago

I think this project would benefit from styles written in js. I particular I am an author of jss and this is a shameless self promotion) There is also integration for react. It is used in production and I think its a more performant and feature reach solution for static styles than inline.

jossmac commented 9 years ago

Hi @kof,

We've been thinking the same thing. If you wouldn't mind, what are the benefits of JSS over something like Radium?

Also, would you consider submitting a PR with a reformatted component? I think one of the main obstacles is where/how to begin and knowing what sort of timeline to expect once we do

kof commented 9 years ago

Stylesheets are generated just once. Style element gets detached when its not used by any component. Inline styles should be used for dynamic things like animations. This way performance is easier to ensure.

kof commented 9 years ago

And yes I am happy to help with any questions and integrations, also I can take one component and make an example on it.

kof commented 9 years ago

I think its very strict forward. CSS Rules remain the same, its just a matter of format and using those.

Guria commented 8 years ago

@kof is there a use reference counter?

kof commented 8 years ago

yes react-jss is all about counting references and detaching/attaching styles.

zackify commented 8 years ago

I agree, in my components I include default inline styles and then if you specify a class name, you can overwrite the styles completely if you wish. I think that could work good for this too, although most people won't want to change the styles much on this stuff

jossmac commented 8 years ago

Aw shucks, thanks @zackify

jossmac commented 8 years ago

Okay, let's move forward with this. It's time to get away from LESS :)

@kof if the offer is still on the table, could you please submit a PR for a converted Button component?

kof commented 8 years ago

@jossmac yes!

kof commented 8 years ago

will do it in the next days

jossmac commented 8 years ago

That's fantastic! Thank you @kof - both @JedWatson and I really appreciate it

JedWatson commented 8 years ago

@kof very impressed, thanks a lot for getting involved. Joss and I are sold, let's make this happen :grinning:

I've got one question for #68 before we jump in:

How should we handle themeing? Most of the things set in the "theme" file should really be in a "theme" file; ideally users could set their own theme config (either extending or completely replacing the built-in styles) on either a page-wide or component basis.

Use cases include:

The more we can separate our styles and component functionality the better, I think, it'll make Elemental more useful as a framework.

material-ui implemented a context property to load the theme, which I think is a nice way to do it; we would default to a fallback when one wasn't specified.

I'm thinking something like this:

var myCustomTheme = Elemental.Theme.extend({
  brandPrimary: 'pink' // because who doesn't like pink
});

// this is responsible for setting the context, could also be done manually with contextType
<Elemental.Theme config={myCustomTheme}>
  <Elemental.Button type="primary">Pretty in Pink</Elemental.Button>
</Elemental.Theme>

// alternatively
<Elemental.Button type="primary" theme={myCustomTheme}>Pretty in Pink</Elemental.Button>

// or even?
<Elemental.Button type="primary" theme={{ primaryColor: 'pink' }}>Pretty in Pink</Elemental.Button>

Internally the Button component would look for context.elementalTheme, expecting something (a JSS sheet? config variables?).

I'm not sure how this meshes with useSheet; maybe a custom decorator for Elemental would do the trick? Basically looking for your thoughts on how we could achieve this, or whether you see a better way to implement themes with JSS.

jossmac commented 8 years ago

Ref #54 "Themes support"

JedWatson commented 8 years ago

Also ping @chantastic - you might be interested in this

JedWatson commented 8 years ago

As per #54 react-themeable might be a good way to do achieve this, ping @markdalgleish in case you've got any feedback :)

nkbt commented 8 years ago

@JedWatson, @jossmac, don't hurry =)

Do you want to try CSS-modules? Is it a general idea to completely remove any notion of css and go 100% js? Or there is some technical issue that does not allow this to happen and I miss it? Ping @geelen, @markdalgleish too.

I'd be keen to implement the same with css-modules to compare

JedWatson commented 8 years ago

@nkbt one of the biggest upsides with "css in js" - or, "styles that are packaged with your javascript" is the flexibility of styling and ease of use for consumers; remembering that Elemental will be used in all sorts of different environments.

In particular, moving away from LESS will be a real win here; it means somebody can just use an Elemental component and all the styles and theme support is "batteries included"; you don't even necessarily need a build step, and there's no SASS / LESS / etc. war to worry about.

I have a lot of love for css-modules, but as far as I know it works best in a fairly opinionated Webpack-based development workflow at the moment, which I don't think is viable for a framework like Elemental.

Would be great to see an implementation of the above with css-modules that proves otherwise, and meets the following:

nkbt commented 8 years ago

@JedWatson css-modules work pretty good now without webpack, and support universal js usage too. Do you want to completely avoid adding CSS file by the end-user? I mean, would it be okay if it is something like:

<link rel="stylesheet" href="//cdn/elemental-base.css">
<script type="text/javascript" charset="utf-8" src="//cdn/elemental.js"></script>

No Webpack or build steps involved.

Give me some days to give it a shot.

andreypopp commented 8 years ago

There's another css-in-js solution developed by me which has theming support included — https://github.com/prometheusresearch/react-stylesheet if you are interested to compare.

The gist of the idea:

An example if using react-stylesheet to produce a themed version of an autocomplete component — https://github.com/prometheusresearch/react-autocomplete/blob/master/src/themed/Bootstrap.js

Also there's https://github.com/andreypopp/rethemeable which provides an API closer to what @JedWatson is described, but tbh I don't think that theme override with contexts is a useful thing. People usually know what theme to use beforehand and can just import needed set of components instead of setting it dynamically.

markdalgleish commented 8 years ago

@JedWatson I actually made react-themeable partly due to our conversations at CampJS. I think it's a perfect fit for something like this, and I'm happy to help however I can :)

kof commented 8 years ago
  1. @JedWatson theming done right is a bit more complex than a direct port of the current styles. Currently styles are a bit messed up, things which should be part of a theme are not. Also there should be a spec first: which kind of declarations are theme. For e.g. should margins, paddings, border sizes be a theme?

I would prefer to separate conversion from less to something else and theme into 2 steps. Otherwise discussions will blow up.

Lets do first step first, do you agree?

  1. @markdalgleish I like your react-themeable approach too!
  2. Regarding css-modules, I have the following issues:
    • it converts css classes to namespaces, its weird .... in js you do it more strict forward ... for me its just a hack
    • it outputs pure css at the end, in js I have access to all constants and rules, I can use constants for calculations and rules to insert them inline and modify if needed on the fly.
    • I don't see much of a benefit writing styles in css language just because there are some quotes less to be written.
    • vendor prefixing can't be done dynamically at runtime, you still load the whole burden of them.
    • as you convert to pure css, you generate much more code than you actually have, just because we don't have real code reuse in pure css.
    • I don't like the direction which css language is targeting. It adds more and more dynamic features we already have in js, but in some awkward and limited way.

In fact in jss we use the good parts of css - the rules declarations. You get rid (mostly) of selectors which has been designed for documents not for apps. But jss still generates a stylesheet which is static and cached. It generates just once and it will be only attached/detached when component is going to mount (react-jss takes care of refs counting and mounting/unmounting)

markdalgleish commented 8 years ago

Well, even though I'm biased towards CSS Modules, the great thing about react-themeable is that it supports JSS, Radium, etc too. CSS Modules doesn't get any extra support :)

mattapperson commented 8 years ago

Less about tech more about API. I like how belle does it.

JedWatson commented 8 years ago

@markdalgleish :grinning:

I suspected it might be - thanks for the support. Appreciate your input as we work through this; if there's something in css-modules we're missing and it would fit well with the goals I've described, do shout out. Don't want to dismiss the work you guys are doing over a lack of experience with it.

@nkbt can you give an example of how theming would be handled? would we be keeping the LESS build process with css-modules in your vision for it?

@mattapperson I think one of the limiting factors of Belle's theming system is that themes are effectively global. There's no way to be specific about it; you're changing all components, or setting styles on a specific component (unless I've missed something) - this is also a limitation of our current LESS-based variable system.

Aside from that Belle has a very similar to the API I describe above where you can extend an Elemental theme.

Can you be more specific about what exactly you like?

@kof agreed that we should tackle the two tasks separately; the conversion would, as you've said, be much simpler if it's just a straight port of the existing LESS to JS. We should really have a separate discussion to design the API, but I think it's good to get an idea of how it would (or could) work in the context of choosing the new way we'll be implementing styles.

Although it's helped us get this far, @jossmac and I are very aware that the way we've used LESS has some real drawbacks; probably the reason it's continued this long is wanting to hold off and make the right switch when we do. There will be a lot of knock-on effects with the choice extending to react-select, KeystoneJS and more.

For the purposes of this discussion, so as long as we can say "implementing theme API x could work like this with y" then I'm happy :)

@andreypopp using context for themes wasn't so much intended as an override, but as a way of setting the theme for an application. material-ui implements themes the same way. You'd wrap some very high level component (or provide your own context) and all the components would, by default, inherit it. It avoids setting theme variables as globals (or maybe more accurately a global singleton) and allows component-specific overrides with props.

I'll look into react-stylesheet in more detail; Radium also has a good overview of packages here: https://github.com/FormidableLabs/radium/tree/master/docs/comparison

We really appreciate everyone's time and involvement with this - what a great community to be part of.

chrisui commented 8 years ago

Hey all, Just weighing in with my 2 cents!

I think the two most important things mentioned here so far are @mattapperson with "Less about tech more about API" and @JedWatson not wanting to pidgeon hole the framework into a single solution.

With this in mind and for the flexibility and ease-of-access for developers then supporting both styling via CSS class names AND inline styles is required. Using JSS, radium, css-modules or some other solution is an implementation detail on the consumer side which should all be considered but not specifically written for within the framework itself.

The react 3rd-party component ecosystem is already fragmented enough in how you customize styles, control rendering and even manage data to the point it's often the case that quickly writing your own component with what you need is usually less of a hassle in the long term.

Disclaimer: I don't use elemental ui but am very interested in resolving component interopability issues in the near future. Elemental is clearly a popular library so it would be great to flagship some best practices!

The best direction I can see to go in, imo, is to look at react-themeable as mentioned ( @markdalgleish ). It provides you with the per-component control you need for any solution.

Composability

I don't want to say completely avoid using context to pass down styles but I would highlight it will not really work in all cases. Firstly there are the update issues (see https://github.com/facebook/react/issues/2517) and then there's the fact that putting everything into your single point of context is just like making a big global variable which is pretty much what all the new CSS/CSS-in-JS solutions are trying to fix.

In React we should be encouraging composition and explicitness over anything else. If you have a framework which exports component X you should simply be able to create a new app component which composes that component with whatever functionality you require (eg. in this case styles) and the api which suits your environment rather than the framework relying on forced implicit context being present.

Coming up with a pattern of composition at the component level before worrying about any global level configuration is key to building a flexible and scalable api.

nkbt commented 8 years ago

@JedWatson, I will hack some css-modules today then. I am pretty sure they now pretty perfectly fit the task with minimum API changes and virtually zero added complexity.

chantastic commented 8 years ago

@JedWatson thanks for looping me in. looks like you've already got a crack team to get this rolling :)

+1 for react-themeable +1 for JSS

With the objectives listed, I'd lean toward JSS. CSS Modules is fantastic (for reals, I love it) but I don't see what this particular project would benefit from CSS Modules; @kof did a bang up job of stating why JS is likely a better fit.

If there's a shootout in the works, I think Card or Modal might be the most interesting targets.

nkbt commented 8 years ago

I've spent couple of hours tonight, here are my thoughts.

@JedWatson I've tried to squeeze css-modules into current build process and it does not look trivial to me at all unfortunately. All the building is basically hidden under react-component-gulp-tasks and bunch of other helpers. Reviewing this build stack and patching it to support css-modules looks like reasonably complex path.

I feel like I am spoiled with webpack, loaders and modularity too much and completely missed that there are still complex frontend build processes :trollface:.

I also went through less structure and I see why you are not feeling awkward with JSS syntax and all these & things.

I am not done yet though, and going to try some more ideas around how to make css-modules happen without breaking current build process. That is an interesting task itself. For some complex styles that are tightly coupled with all vars and mixins it would be a difficult journey. Maybe @markdalgleish can have a quick look and give a hint if there is an easy way? Well, the fairly easy one would be to replace gulp with wepback + loaders, but that is a different story :)

kof commented 8 years ago

I also went through less structure and I see why you are not feeling awkward with JSS syntax and all these & things.

This is not part of jss core but rather a plugin for a reason. Basically nested selectors is something jss tries to avoid and this plugin gives us the ability to do so only for rare cases where its absolutely needed or for easier migration.

nkbt commented 8 years ago

@kof I know that, nesting is evil. We have absolutely flat structure with css-modules now everywhere except some 3rd-party modules which are restyled with :global selector.

Since JSS does not require to change a thing in a build process (just a JS after all) - it is its great advantage for this project.

nkbt commented 8 years ago

Hm, I guess it is possible to use https://github.com/css-modules/css-modules-require-hook to avoid dealing with build process.

DenisIzmaylov commented 8 years ago

Anyway we have to think about end-users. I meant Elemental UI could be used by Web Designer which are already known about HTML, CSS and how it works. It would be difficult to read/write JSS and try to integrate it to projects which probably already use a lot of custom CSS rules and CSS frameworks like Bootstrap. I think if we want to populate Elemental UI we have look at Bootstrap as a most popular CSS framework and its UX i.e. using CSS and CSS-preprocessor like LESS/SASS. If we want limit Elemental UI audience to a little part of web-developers, ok, we could use JSS-like libraries.

kof commented 8 years ago

Users of elemental are not required to use jss. If they want to be ugly - they can, by defining themes in pure css and passing classes around.

kof commented 8 years ago

Actually this is one of benefits using jss vs. inline styles. You can overwrite inline styles only by inline styles. But stylesheets can be overwritten by other classes and inline styles.

DenisIzmaylov commented 8 years ago

Why we would not use 'patched' CSS to use themes? Like a Bootstrap. If we look at development process it's more easy - web designer, junior developer and a lot of other people could write & update CSS (either LESS/SASS) because a lot of documents and resolved issues there.

LorbusChris commented 8 years ago

just for completeness I'm linking this here: https://github.com/postcss/postcss Has anyone heard of it or used it? Seems rather big (used by Google and Twitter) and I was wondering wether this might be a fit to whats required here...

nkbt commented 8 years ago

@LorbusChris css-modules are based on post-css

LorbusChris commented 8 years ago

@nkbt Thanks :+1:

DenisIzmaylov commented 8 years ago

Bootstrap 5 will be in PostCSS :) https://twitter.com/mdo/status/591364406816079873

markdalgleish commented 8 years ago

What about making regular old global CSS the default since it's the least opinionated choice? I think that would be more inclusive of the different project configurations in the wild. It also requires zero code, so doesn't add any overhead to the size of the library.

You can still allow overrides via react-themeable, so if they think JSS is the best choice, it's easy to swap it out.

That's probably the way I'd go in any open source components I make.

kof commented 8 years ago

@markdalgleish lol, I thought you have already enough of global names clashes as you created css-modules.

kof commented 8 years ago

Especially considered this ui components could be used in any dirty environment, robustness is important.

andreypopp commented 8 years ago

What about making regular old global CSS the default since it's the least opinionated choice? I think that would be more inclusive of the different project configurations in the wild. It also requires zero code, so doesn't add any overhead to the size of the library.

I think this is a good idea. But it can be implemented at the "distribution level" not the "source level”.

What I mean is that you can have styles maintained as CSS modules (or JSS stylesheets) in the repo. Then there can be a distribution built where a single CSS file is provided which is compiled from source format (CSS modules or JSS, though I’m not sure if JSS supports extraction).

nkbt commented 8 years ago

Whatever the case here (jss, css modules, etc) I would strongly consider keeping original classnames in place. So end-users can override styling by old-good global css rules. It is especially important considering backward compatibility with existing code using elemental

NogsMPLS commented 8 years ago

I really really like CSS Modules/local scope css. proper linting, easy for anyone to jump in and do CSS, and is easily cache-able.

That being said, I agree that anyone should be able to come in and change whatever they want and use whatever tech they want. I like react-themeable 's approach of being 'tech agnostic' in the components. Should theoretically allow people to rip out and put in whatever styles they want.

kof commented 8 years ago

@JedWatson due to lots of discussions I am not sure what's the current state here. To sum up what I think i wrote an article: https://medium.com/@oleg008/jss-is-css-d7d41400b635

Major points of confusion I see here is:

Let me know If I should continue guide you guys through jss and finish the button implementation.

NogsMPLS commented 8 years ago

Does JSS actually export a CSS file(s)?

My biggest issue with many of the inline css javascript stuff, is that the "css" isn't cache-able. Which you seem to list as a negative:

It outputs pure css during the build process server side and allows to serve static CSS to the client.

That's actually a big benefit for performance.

Also if it doesn't export to CSS, then when you get to media queries - are page resizes looking at offSetWidth and things like that? Sounds like a LOT of repaints and way less performant.

vendor prefixing can be handled via multiple css file builds and user agent lookup, it's a non-issue and pretty common practice.

You say it removes bloat to use jss, but really I feel like you're just 'moving the bloat' to the main app javascript - which will change more frequently than a css file, again getting back to caching issue.

But all that is just me and I am admittedly somewhat ignorant of all the nuances of the solutions currently available. They are all viable solutions given specific situations. Seeing as how most of these issues are irrelevant in the grand scheme of things though, and this being a framework that is meant to be used in any environment, something like react-themeable or something truly 'universal' (which I'll admit that css-modules might not fit, because of webpack) still seems like the most prudent path.

kof commented 8 years ago

@NogsMPLS

My biggest issue with many of the inline css javascript stuff, is that the "css" isn't cache-able.

if you are talking about browser files cache then javascript is cached the same way CSS is.

Also if it doesn't export to CSS, then when you get to media queries - are page resizes looking at offSetWidth and things like that?

no, it generates a style element and media queries work exactly the same way

'moving the bloat' to the main app

no, you are free to load your jss styles in separate files or whatever you want to separate stuff.

I am admittedly somewhat ignorant

yes you are ... do more, talk less ...

NogsMPLS commented 8 years ago
  1. the javascript app files in any given application will be changing more than the css files, thus breaking cache more often than the separate css file
  2. cool, that's awesome! Glad that that's been solved.
  3. Sure, but you listed the 'style.something' as a negative, when you'd effectively be using the same syntax with jss when/if you separate the file. Also, my bloat comment was more in reference to the css/javascript not being separate files after compile thus bringing the filesize of your main app.js up because of css 'bloat' in the js, and 'breaking cache' again on the whole app when you only change logic and/or just a style.
  4. ...okay...

edit: again, there are a lot of viable solutions. i just wanted to ask a few questions and continue discussion. My main point is that I think something that is universal and - in my opinion - preferably can output to a css file at the end of the day, is optimal.