RocketChat / Rocket.Chat

The communications platform that puts data protection first.
https://rocket.chat/
Other
40.38k stars 10.49k forks source link

Add cssstats to our CI and restrict the number of acceptable selectors, colours, etc. #4091

Open engelgabriel opened 8 years ago

engelgabriel commented 8 years ago

https://www.npmjs.com/package/cssstats

Thanks @timkinnane

timkinnane commented 8 years ago

I've tried setting up the command line tool, but I'm a node noob and can't get it to output anything. Help? https://github.com/timkinnane/rocket.chat/tree/feature/css-stats

In the meantime I pulled the local dev version of the online tool and ran some reports (attached). The one from my color-fixes branch shows some improvements. These are the data points I think worth focusing on...

Data Before After Notes
Size 551K 546K still way too big!
Unique Colors 331 314 not amazing
Unique Background Colors 126 91 notable
Unique Font Sizes 125 125 waaaay too many
Unique Z Indices 31 29 prone to cause display bugs
Media Queries 12 12 needs consolidation to 4 or 5

There's obviously lots of plugins and packages inserting their own css/less where only a small portion is actually used. It might be worth reviewing to see what can be kept out of build and just inherit the default styles.

CSS Stats v0.37.0-color-fixes.pdf CSS Stats v0.37.1.pdf

pajasevi commented 8 years ago

Just a question @timkinnane, do you think that media queries need consolidation? IMHO when you use media queries with component approach, you style that component so it has it's own breakpoints which is better. Components are unique (mostly) and trying to stuff them into 4-6 breakpoints will have negative effects.

Anyway other stuff is right, 331 unique colors is waaaay too many 😞

timkinnane commented 8 years ago

My concern is that there's too many variants of a view, which is inviting visual bugs and janky behaviour as components try to work within an unpredictable layout. It's really not a big concern though, just thinking with things like breakpoints and z-indexes it might help to have some named global standards for contributors so they don't have to think about where it might a component might fit with everything else.

I really like include-media for ease-of-use but its only sass.

pajasevi commented 8 years ago

Assuming that e.g. everything under 768px is phone and everything under 1024px is tablet etc. is wrong and really does not help. These universal breakpoints are useful for frameworks or for layout stuff, but not for individual components.

There are components that really need one breakpoint and there are those that need five. And it's not wrong. But as with everything - problem is documentation 😅

engelgabriel commented 7 years ago

See improvements after removing the CodeMirror package from atmosphere

https://github.com/RocketChat/Rocket.Chat/issues/5399

engelgabriel commented 7 years ago

To track http://cssstats.com/stats?url=http%3A%2F%2Fdemo.rocket.chat&ua=Browser%20Default

timkinnane commented 7 years ago

56 Unique Colors

🎉 🎉 🎉