Your CSS Sucks

outline.md

Your CSS sucks

Contents

• Styles aren't modular
• Confused content and container
• Not re-using styles
• Changing too much
• No naming convention
• Selectors are too strong
• JavaScript hooks are styled
• Not using enough classes
• Putting too much in a LESS/SASS file
• No logic behind pre-compiled structure
• Inception rule
• Trim the fat
• Not enough comments and white-space
• Using magic numbers

Sensible order

• Styles aren't modular
  • Confused content and container
  • No naming convention
  • Selectors are too strong
    • Selector strength calculation
  • Using magic numbers
  • Not re-using styles
    • Not using enough classes
• Changing too much
• JavaScript hooks are styled
• Putting too much in a LESS/SASS file
  • No logic behind pre-compiled structure
  • Inception rule
  • Trim the fat
  • Not enough comments and white-space

Styles aren't modular

Usually websites are built one page at a time. This is wrong, should be one module at a time (pages are just collections of modules).

Modules should be stand-alone. They shouldn't need to know about any other element on the page (No #sidebar .infobox stuff) and they should look the same wherever they're placed. This that are almost the same but slightly different are sub-modules (or possibly a different container).

Confused content and container

Styles use both content and container styles so can't be reused. Example:

<div class="widget"><!-- ... --></div>

.widget {
    background-color: red;            /* content */
    border:           2px solid blue; /* content */
    float:            left;           /* container */
    padding:          10px;           /* content */
    width:            25%;            /* container */
}

Better example:

<div class="box">
    <div class="widget"><!-- ... --></div>
</div>

.box {
    float: left;
    width: 25%;
}

.widget {
    background-color: red;
    border:           2px solid blue;
    padding:          10px;
}

... why is that better? How about this green widget:

<div clas="box">
    <div class="widget widget-green"><!-- ... --></div>
</div>

.widget-green {
    background-color: green;
}

... or this large widget?

<div class="box box-wide">
    <div class="widget"><!-- ... --></div>
</div>

.box-wide {
    width: 50%;
}

No naming convention

There should be some logic showing a part of a module and a modification of a module.

Personaly, I use:

• underscore for part: .module_part
• hyphen for modification: .module-variant

(which I'll use for the rest of this talk.)

Others use:

• one hyphen for part: .module-part
• two hyphens for modification: .module--variant

It doesn't matter which is used, so long as it's obvious and consistent.

What does Bootstrap use?

• one hyphen for part: .module-part
• one hyphen for modification: .module-variant

... this is one of the reasons I don't like Bootstrap.

I also go further and prefix some of my styles.

• b- = Background
• d- = Display classes
• h- = Heading
• hN = Heading level
• l- = Layout
• t- = Text
• u- = Utility
• x- = Surgical or helper class
• dy- = Dynamically generated
• is- = State
• js- = JavaScript hook

So all my modules must be 3 letters or more.

Selectors are too strong

What's wrong with this selector?

.module > li > a:hover {
    text-decoration: underline;
}

It only works on something like this:

<ul class="module">
    <li>
        <a href="#">text</a>
    </li>
</ul>

What happens if the markup gets updated?

<ul class="module">
    <li>
        <span>
            <a href="#">text</a>
        </span>
    </li>
</ul>
<!-- or -->
<div class="module">
    <a href="#">text</a>
</div>

The CSS must be updated:

.module > li > a:hover,
.module > li > span > a:hover,
.module > a:hover {
    text-decoration: underline;
}

Now we have 3 selectors when we only needed one. This is tempting, but highly inefficient:

.module a:hover {
    text-decoration: underline;
}

That will affect every link within .module (did we want that?) and it's slower to process. This is much better:

.module_link:hover {
    text-decoration: underline;
}

<ul class="module">
    <li>
        <a href="#" class="module_link">text</a>
    </li>
</ul>

<ul class="module">
    <li>
        <span>
            <a href="#" class="module_link">text</a>
        </span>
    </li>
</ul>

<div class="module">
    <a href="#" class="module_link">text</a>
</div>

See how the markup no longer matters?

While we're on the subject, only style classes, never IDs or attributes and never children. These are understandable:

#thing {
    /* ... */
}

input[type="text"] {
    /* ... */
}

.list > li {
    /* ... */
}

.table > tbody td {
    /* ... */
}

... but these are better:

.thing {
    /* ... */
}

.input {
    /* ... */
}

.list_item {
    /* ... */
}

.table_cell {
    /* ... */
}

Of course, sometimes selectors stronger than a class are necessary:

.module:hover {
    /* ... */
}

.module.is-open {
    /* ... */
}

.module::after {
    /* ... */
}

... and there's nothing wrong with those. Just make them the exception rather than the rule and your styles will be much easier to use, re-use and modify.

It's also clearer what styles will affect the elements. Selector strength (specificity) is mitigated.

Selector strength calculation

To work out selector strength, break the selector into 4 parts:

• !important
• IDs
• classes (also attributes, pseudo-selectors and pseudo-elements)
• elements (not *)

In theory these are completely unrelated to one another, in practice, 256 of the previous are equivalent to 1 of the next (in some browsers).

Here's the specificity of those selectors again:

#thing              {} /* 0-1-0-0 */
input[type="text"]  {} /* 0-0-1-1 */
.list > li          {} /* 0-0-1-1 */
.table > tbody td   {} /* 0-0-1-2 */

.thing      {} /* 0-0-1-0 */
.input      {} /* 0-0-1-0 */
.list_item  {} /* 0-0-1-0 */
.table_cell {} /* 0-0-1-0 */

.module:hover   {} /* 0-0-2-0 */
.module.is-open {} /* 0-0-2-0 */
.module::after  {} /* 0-0-2-0 */

The universal selector * has a specificity of 0. It doesn't increase the selector strength but matches every element. Either use it on its own, or immediately after the direct descendant selector (>).

Inline styles (those in the style attribute) will override elements, classes and IDs - !important is the only thing that will override them.

Using magic numbers

When something doesn't align properly, certain numbers are used to correct it. These numbers, which fix it perfectly, are called "magic".

Example:

.module1 {
    width: 27px;
}

.module2 {
    padding-top: 3px;
}

It's not obvious where the numbers originated or when they need to be changed. Compare that to these examples:

/* LESS */
.module1 {
    width: (@line-height-computed * 2);
}

.module2 {
    line-height: @line-height-base;
}

Now it's obvious what the numbers are and when they'll be changed (since we're using a pre-processor, updating the variables will update these modules, too). It's also obvious what the modules are trying to do.

Numbers that are not magic:

• 100% (also 50%, 33.3333%, 25% etc. - use them for containers)
• 1em (same as the letter)
• a variable (easy to update, especially if properly calculated)
• 0 (easy to understand)

Feel free to use them.

Not re-using styles

Because content and container are confused, nothing can be re-used. You need to start recognising repeated design patterns and write those in CSS. Repeated patterns are things like:

• Grid layout (setting widths and positions).
• An image next to text (.media).
• White text on a blue background.
• Lists without bullet points.
• Boxes with the first element on the left and the second is on the right.

There are no hard-and-fast rules about this, it will change in every design. Identifying them allows them to be coded and re-used frequently.

Not using enough classes

Elements can have multiple classes:

<div class="one two three"><!-- perfectly valid --></div>

This means we can have modules and module variants defined on the element.

<div class="module module-variant"><!-- ... --></div>

We can also add additional classes.

<div class="module module-variant d-hide-xs"><!-- ... --></div>

And we should group the classes to make that easier to read:

<div class="[ module module-variant ] d-hide-xs"><!-- ... --></div>
<div class="module module-variant / d-hide-xs"><!-- ... --></div>
<div class="module module-variant | d-hide-xs"><!-- ... --></div>

Doesn't add anything to the styles, but can make it easier to read the markup. Prioritise readability over stream-lined code and remember - "tidy" code is a myth.

Changing too much

When changing something in a media query or a module variant, only define the things that actually change.

.module {
    float: left;
    width: 100%;
}

@media (min-width: 500px) {
    .module {
        float: left; /* Unnecessary, it hasn't changed */
        width: 50%;
    }    
}

A better example:

.module {
    float: left;
    width: 100%;
}

@media (min-width: 500px) {
    .module {
        width: 50%;
    }    
}

This is the same for CSS transitions - the transition state should only have the changes defined.

.module {
    color:      black;
    padding:    10px;
    transition: 0.3s ease-in-out; /* transition everything */
}

.module.is-active {
    color: blue;
}

JavaScript hooks are styled

How many times have you done suff like this?

<a href="#" class="module_link">text</a>

.module_link {
    font-weight: bold;
}

$(function () {
    $('.module_link').on('click', function (e) {
        // ...
    });
});

Now if I need that functionality anywhere, I have to use the module_link class even if the element is part of module or shouldn't be bold. A better solution: prefix all JavaScript hooks and never style them:

<a href="#" class="module_link js-something-useful">text</a>

.module_link {
    font-weight: bold;
}

$(function () {
    $('.js-something-useful').on('click', function (e) {
        // ...
    });
});

Now the functionality can be added to anything without worrying about the styles. As a rule-of-thumb, I'd recommend a naming convention:

.js-module-identifier

• js JavaScript-hook prefix so we know not to style it.
• module JavaScript module or script name, so we know which script is being used.
• identifier Identifier for the script.

A few examples:

• js-basket-add
• js-search-trigger
• js-tab-show4

Easy, huh?

Putting too much in a LESS/SASS file

When you start writing a module, it should be in its own file. This keeps it to an easy-to-find location, makes it simple to swap it out and re-inforces the idea that it's stand-alone.

Please stop writing custom.less.

No logic behind pre-compiled structure

Without planning, you'll always trip over selectors and have to increase selector strength to make your styles work.

A rough layout should be:

1. Variables
2. Mixins
3. Base
4. Modules
5. Helpers

But I tend to go further

1. Variables
2. Mixins
3. Base
4. Utility
5. Layout
6. Modules
7. Helpers
8. Global states

The important thing that you're planning where things should go.

Start with variables then mixins - internal things. Then move onto base styles (a, body etc.) Then modules (grids, header etc.) Put helper classes (display:none; or float:left; etc.) right at the end.

Inception rule

Don't go more than 3 levels deep, it's too easy to get lost.

/* LESS - looks simple, right? */
.module { /* 1 */
    > li,
    > div { /* 2 */
        > a,
        > span { /* 3 */
            > span,
            > em { /* 4 :( */
                display: inline;
            }
        }
    }
}

/* CSS - 8 selectors! */
.module > li > a > span,
.module > li > a > em,
.module > li > span > span,
.module > li > span > em,
.module > div > a > span,
.module > div > a > em,
.module > div > span > span,
.module > div > span > em {
    display: inline;
}

Trim the fat

Ever seen this in your LESS/SASS?

/* LESS */
.module {
    a {
        //color:   red;
        //display: inline;
    }
}

It's a nested selector that does nothing. If you're removing it, remove it all.

/* LESS */
//.module {
//    a {
//        color:   red;
//        display: inline;
//    }
//}

Simple, huh?

I've also seen this in CSS. Never do this - the .module is completely unnecessary:

.module {}
    .module_part1 { color: red; }
    .module_part2 { float: left; }

Unused selectors cause the browser to parse the selector and find the elements but not apply any styles to them. It's a degree of work to absolutely no benefit.

Remember, Internet Explorer has a selector limit: maximum of 4095 selectors, maximum of 31 style sheets (126,945 selectors absolute maximum). This also includes any third-party style sheets (such as Google Fonts or Thickbox etc.)

Not enough comments and white-space

Comments and white-space get automatically removed during minification (which you should be doing before serving the style sheet) so use them as much as you can in the source files.

Here's how I white-space:

/* LESS */
.selector {

    .mixin();

    display:     inline;
    font-weight: bold;

    &:hover {
        background-color: yellow;
        color:            red;
        text-decoration:  underline;
    }

}

Here's how I comment:

/*
# Selector

Information about the "selector" module like what it does and why it exists.
Maybe some information about layout or utility dependencies.

Example markup:

    <div class="selector">
        <!-- ... -->
    </div>

Explanation about sub-classes and how they're different:

    <div class="selector selector-variant">
        <!-- ... -->
    </div>

## Selector part

If this part is significant enough, it gets broken into a new section. This
would include markup examples and some kick-ass explaination about why this
isn't simply another module.

 */

I use StyleDocco as my CSS documenter. Find one you like and use it. Liberally.

Always code as if the person who ends up maintaining your code is a violent psychopath who knows where you live. Source

Not helpful at all. Better:

Always code like it's being maintained by someone new at this who's hungry to learn.