mozilla / addons

☂ Umbrella repository for Mozilla Addons ✨
Other
125 stars 41 forks source link

Document some simple guidelines for our approach to CSS #9848

Closed muffinresearch closed 7 years ago

muffinresearch commented 8 years ago

A few times the question of whether we should adopt some kind of methodology for CSS has come up. If we go this route here's some likely goals of adopting anything along these lines:

There are lots of methodologies that exist e.g. (SMACSS, BEM and suitcss). Most look to provide some structure to classnames to aid consistent code modularization.

We don't have to opt for any one specific approach necessarily, but whatever we decide would be good to document how we are going to build our component classnames (or if we are wholesale adopting something that exists we can just point to their docs instead).

Bonus points for stylelint rules to help maintain whatever the end result is.

kumar303 commented 8 years ago

The only concrete one I can think of right now is that we should enforce all component class names to be prefixed with the component name. Sadly, I don't see an existing eslint rule for this but it probably wouldn't be too hard to write one.

muffinresearch commented 8 years ago

Are we going to make a distinction between descendants and modifiers?

kumar303 commented 8 years ago

I personally don't really see the point in making a distinction between descendants and modifiers. The rationale listed in SuitCSS doesn't seem compelling. The only compelling point is specificity but I think we already gain that by prefixing component classes with the component name.

muffinresearch commented 8 years ago

I personally don't really see the point in making a distinction between descendants and modifiers. The rationale listed in SuitCSS doesn't seem compelling. The only compelling point is specificity but I think we already gain that by prefixing component classes with the component name.

Yep, that seems reasonable to me; the simpler the better.

kumar303 commented 8 years ago

On a closer look at SuitCSS I see it tries to standardize shared css classes with the u- (for utility) prefix. I ignored this section before because they called one of their classes u-floatleft which sounded gross 🔥 We might need a way to prefix shared classes but we could look at it more on a case by case basis.

mstriemer commented 7 years ago

I used the modifier in the updated SearchInput component because I think it makes things clearer. @tofumatt mentioned this in https://github.com/mozilla/addons-frontend/pull/1269#discussion_r84780256.

In that component there is an input SearchInput-input and when it is focused or has text in it the root component gets a SearchInput--text class added to it to move the icon to the side and hide the placeholder.

I'm not sure avoiding the distinction between descendants and modifiers actually makes things "simpler". This makes me think of Rich Hickey talks where he always points out that simple and easy are two different things. I think it really makes it more complex since using the same format for descendants and modifiers leaves the distinction up to the reader to discover.

As for the utility class prefix, on its own it feels silly but if every other class starts with MyComponent I think it makes sense to explicitly state that this is a utility class and the component name wasn't forgotten. We use visually-hidden in several places, it would make sense for that to be u-visually-hidden.

I'm not sure if the u- prefix would make adding lint rules for this possible or if it's just too complicated but that might be an added bonus.

My vote would be to follow the suitcss guidelines as they have written them.

tofumatt commented 7 years ago

After using the component prefixes I'm totally on board with utility ones too.

muffinresearch commented 7 years ago

@tofumatt to document