atom / github

:octocat: Git and GitHub integration for Atom
https://github.atom.io
MIT License
1.12k stars 393 forks source link

CSS naming convention #92

Closed simurai closed 8 years ago

simurai commented 8 years ago

Because this packages is still somewhat young, might be worth to define some naming convention. Well, at least for the CSS classes. If it works out well, we could also start using it for other packages, like atom/github.

simurai commented 8 years ago

So here a proposal. Itโ€™s a mix of BEM/SUIT and Chainable Modifiers, but customized for Atom's use case.

Example

Below the commit box as a possible example:

<div class='git-CommitBox'>
    <div class='git-CommitBox-editor'></div>
    <footer class='git-CommitBox-footer'>
      <button class='git-CommitBox-button'>Commit to sm-branch</button>
      <div class='git-CommitBox-counter is-warning'>50</div>
    </footer>
</div>

And when styled in Less:

.git {
  &-CommitBox {

    &-editor {}

    &-footer {}

    &-button {}

    &-counter {
      background: red;

      &.is-warning {
        color: black;
      }
    }
  }
}

Breakdown

Here another example:

<button class='git-CommitBox-commitButton is-primary is-disabled'>

And now let's break it down into all the different parts.

<button class='namespace-ComponentName-childElement is-modifier is-state'>

namespace

git-CommitBox

Every class starts with a namespace, in this case it's the package name. Since packages are unique (well, at least for all the packages published on atom.io), it avoids style conflicts. Using a namespace like this makes sure that no styles leak out. And even more importantly that no styles leak in.

ComponentName

git-CommitBox

Components use PascalCase. This makes it easy to spot them. Especially if package names use dashes. For example in settings-view-List, it's easy to see that the component is List and not view.

childElement

git-CommitBox-commitButton

Elements that are part of a component use camelCase and are appended to the component's class name.

is-modifier/is-state

git-CommitBox-commitButton is-primary is-disabled

Modifier and states are prefixed with a short verb, like is- or has-. Since these class names probably get used in many different places, it should never be styled stand-alone and always be a chained selector.

.is-primary {
  // nope
}
.git-CommitBox-commitButton.is-primary {
  // yep
}

If multiple words are needed, camelCase is used: is-firstItem, has-collapsedItems.

More awesome guidelines

Benefits

Concerns

Class names get quite long

Only in the DOM. During authoring class names can be split into different parts and glued together with &.

.git-CommitBox {
  &-editor {
    // styles
  }
}

will output as

.git-CommitBox-editor {
  // styles
}

I don't like nesting selectors

The whole selector (or just different parts) can of course also be written out as a whole. Nesting just makes sure selectors are grouped together.

.git-CommitBox {}

.git-CommitBox-editor {}

.git-CommitBox-footer {}
joshaber commented 8 years ago

This looks great. I'm happy to defer to whatever you think is best :+1:

broccolini commented 8 years ago

Hello @simurai! Just wanted to draw your attention to the documentation the design systems team have been putting together: https://github.com/github/design/blob/styleguide-principles/docs/code/principles.md - We're discussing some very similar things.

I am a fan of approaches like BEM, and reference SUITCSS a fair bit (as well as BASSCSS) - all of them have approaches to scoping styles so they don't leak all over the place - so I'm really happy to discover this conversation happening on another GitHub product.

I don't want to push our approach on the Atom team, but it might be good for the two teams to discuss and see if we can work with the same approach so that it's easier for designers and developers to be able to work on both GitHub and Atom.

We have been having bi-weekly design systems team meetings, and the next one is tomorrow. I'll add you to the invite and if that time works for you we could discuss it then? If not, I'm happy to find another time when we can all meet.

simurai commented 8 years ago

:wave: @broccolini

see if we can work with the same approach so that it's easier for designers and developers to be able to work on both GitHub and Atom.

That would awesome. I'll take a look at the "Styleguide principles". The :point_up: above conventions are a bit customized for Atom's use-case. Would be interesting to see if both could be aligned. The environments are bit different. Like sending CSS over the wire (.com) vs. reading from disk (Atom). Or be able to change markup and styles at any time (.com) vs once a selector is out it needs to be supported till the next major version (of Atom).

If not, I'm happy to find another time when we can all meet.

I'm in the Japanese time-zone. Sometime around HQ's afternoon/evening would work best for me. I'll also be at the upcoming Mini Summit where we can have a session about it.

zeke commented 8 years ago

@simurai this proposal looks good to me! I think it borrows the right concepts from BEM without requiring the unsightly double-underscore delimiter and such.

simurai commented 8 years ago

@broccolini + @github/design-systems I tried to look a bit more into the main differences between the naming convention above :point_up: and https://github.com/github/design/blob/styleguide-principles/docs/code/principles.md. Here some of the main points that stood out:

Multiple words

For example when seeing the class .tabs-bar-group, it isn't always easy to guess what is the Block and what is the Element? Using camelCase helps to see what belongs together. A .tabsBar-group must be a child element of the .tabsBar component.

Atom's convention actually uses PascalCase for the main component to make it stand out even more. So that when a namespace is used, it shows where the component starts. e.g. tabs-TabsBar-group.

I guess to differentiate like that isn't so important if you pick commonly known class names. But in Atom's case, there might be more exotic packages and components where the name isn't that common.

Utilities

Since there are many 3rd party themes for Atom, it makes it harder to use utility classes. That's because themes can only override existing classes, but not change the markup. For example .rounded, would make less sense if a theme doesn't want to have rounded corners.

Instead of utility classes, themes define variables. Like @border-radius: 5px; that then get used everywhere. This results it larger CSS files, but maybe not a big concern for Atom because the CSS can be read straight from disk and doesn't have to be downloaded over a network.

Modifiers

In Atom's convention there is no difference between a modifier and a state. Often itโ€™s hard to classify what is a modifier and what is a state. For example a button that initially is a "primary" button, might change to an "error" button later. So you could say that "primary" and "error" are states. It might help to know which classes are the default and got added on initial load and use modifiers for those. But for something that constantly changes or the state gets restored when opening Atom, it maybe matters less what the initial state used to be.

One downside to use the SMACSS style .Button.is-primary {} for modifiers is that it increases specificity. So thatโ€™s something that still needs to be tested in practice and see if it has a negative impact.

Conclusion

So yeah.. I think for Atom's use-case, having as few classes as possible, might be the better approach. Basically the opposite of Atomic/BASSCSS/OOCSS. As mentioned above, themes and user styles can't make changes to the markup, so they need to rely on overriding. To make that easier, Atom would define just a single class as an "identifier", together with a few modifier/states. It's like a public API. Once a class gets added, it shouldn't change anymore until the next major version of Atom. Which could take 1-2 years.

Does that make sense? :grimacing:

broccolini commented 8 years ago

Thanks @simurai for explaining how Atom's use-cases in detail! ๐Ÿ’–

Multiple words

I'm not opposed to github.com CSS using PascalCase too. I don't think we have as strong as need for it but I do think it makes the markup easier to scan and identify component vs non-components, and if it helped keep things be a little more consistent between the two systems then perhaps it's worth doing. I'd like to discuss with the rest of the @github/design-systems team first though and see how they feel about that.

Utilities

๐Ÿ‘ Totally understand why utilities won't work for you. Do you think there is any value in keeping Atom variables consistent with github.com utility class names?

Modifiers

a "primary" button, might change to an "error" button later. So you could say that "primary" and "error" are states.

I'd argue that primary is a property and not a state. I think is-error is a fairly common state, however that's not how we are handling button modifiers on github.com. There are many use cases for something like a red button that are not an error, the most obvious being a button that deletes something.

It might help to know which classes are the default and got added on initial load and use modifiers for those. But for something that constantly changes or the state gets restored when opening Atom, it maybe matters less what the initial state used to be.

I'm finding this a little harder to follow tbh. This might because I don't fully understand how Atom works with CSS though ๐Ÿ˜ , so maybe better to discuss this on video ๐Ÿ‘–

I'll try and find a spot on the calendars that works so we can discuss in person.

simurai commented 8 years ago

I'll try and find a spot on the calendars that works so we can discuss in person.

Sounds good. :+1:

simurai commented 8 years ago

Let's move this to https://github.com/atom/design/blob/master/specs/css-naming-convention.md so we can close this issue.

Then once the GitHub package is public, we can open a new PR in https://github.com/atom/design-decisions to finalize this proposal.