Dealing with bad front end code

I've worked on a lot of websites and seen a lot of front end code. Some good, some bad. Some... really bad.

More often than I'd like, I encounter the same patterns of badness. Here's a few examples of the kind of things that some front end developers do that drive me to despair. Warning: this is going to get technical.

By way of a disclaimer before I start wading into this, if you read this and think I'm pointing out some obvious things here, I am. The sad fact is that I haven't sat down and come up with this list myself - I have genuinely seen all of these things done.

Also, while some of these things seem small, if left unchecked they can produce a site that is near impossible to maintain.

Let's dive in.

Unnecessary style attributes

.wrapper {
    display:block;
    width:100%;
}

Two examples of the same thing to start off with - putting in rules that are unnecessary because of another rule. Firstly, an element that is set to display:block will automatically have a width of 100% (unless flexbox is involved), so there's no reason to explicitly define that (unless of course another rule that this one inherits from already defined a width).

.block {
    float:left;
    display:block;
}

Secondly, a floated element is already a block level element, so there's no need to define that it should be.

Overly specific selectors

input.btn {
    background:#CCff99;
    color:#FFFFFF;
}

This one's about code maintainability. If a button class is defined, I want the freedom to be able to apply that class to not only an input element, but also a link and a button. Good CSS shouldn't be limited like this.

Page specific styles

.well {
    background:orange;
}
body.page_homepage .well {
    background:green;
}

Another example of inflexibility. Maybe the design for your site has a recurring element that looks the same on every page apart from one. No problem - define an additional class that can be applied to that element. Don't set a page specific rule that prevents certain elements from appearing on certain pages. Remember that the site you build today will not necessarily be the site the client wants tomorrow.

Not using percentages

.sidebarnav {
    float:left;
    width:163px;
    margin-right:24px;
}

I see this less these days since the rise of responsive design, but it can still be a problem. Layout widths defined with percentages mean that if a width changes further up the DOM everything below it will either expand or contract to fit. Layout widths defined with pixels won't.

Overly long selectors

.pagewrapper .wrapper .block .tile .form .listwrapper .listitem .para .btn {
    float:right;
}

There are several things wrong with this example but I want to focus on the most obvious one - constraining this element to only this highly specific position within the DOM. A good front end developer can build a site that looks like the design. A really good front end developer can allow for possible changes in the markup and write their CSS to allow for flexibility.

Overly long selectors are an increasing problem now that we have CSS preprocessors like SASS and Less - it's far too easy to just nest everything.

IDs for styling

#sharelinks {
    border:solid 2px silver;
}

I still have debates with people over the validity of using IDs for styling, so I'm going to take this opportunity to make my case. An ID in CSS has higher precedence than almost anything else, so using it breaks the inheritence structure, and that way leads madness.

Some people think it's okay to use an ID to style an element that they know will be unique. My response is that there is no such thing as a unique element in the lifetime of a site. Like I said earlier, a really good front end developer anticipates future changes. Using IDs in styling does not.

If you're still unconvinced, there are plenty of more in depth articles on this issue. Here's a few.

Overly long selectors involving IDs

.pagewrapper .wrapper .block .tile #newsletter > .listwrapper .listitem .para .btn + .next {
    float:left;
}

A combination of the previous two problems. First, it's really not necessary. Secondly, everything before the ID is unnecessary, because the ID is unique and overrides most other classes. Tip: if you find yourself having to write CSS this specific, the rest of your code is really too complicated.

Use of !important

.pagewrapper.productpage .wrapper .block {
    margin-right:20px !important;
}

The !important rule seems like a handy get out if you find yourself stuck in a nest of complex rules, but all it really shows is that the CSS is badly structured. Once you start down this dark path, forever will it dominate your code.

The only time you should ever need to use !important is if you're working with code that can't be directly manipulated (for example, elements styled inline by third-party Javascript).

Styles that assume a very specific markup structure and don't use classes

.pagewrapper > section main ul > li p span a {
    font-size: 120%;
}

Here's a surprisingly common one. This rule relies upon a very specific markup structure to work. "Yes," comes the response, "that's what I wanted". The problem with this is, once again, inflexibility.

I usually encounter this kind of CSS accidentally, when I add a new element to someone else's site and find it has styles associated with it even though I've not written any. Then I either have to override those styles (by writing unnecessary code) or go back and find where this is supposed to apply and change it (almost impossible in any large site).

Inconsistent naming conventions

.PageWrapper .mainWRAP .list_wrapper .list-item {
    display:inline-block;
}

This one is more of a personal preference than something that will break your site later. Choose a naming convention and stick with it. It's far easier to remember.

Overly specific naming conventions

.greenbutton {
    background:green;
}

There can't be anything wrong with this, can there? Again this is about allowing for future changes. If the site gets slightly redesigned so this element is blue, you're not going to want to go back through all the markup and rename the class. Then suppose the blue button gets redesigned to be green... it can get messy quick. Call it something more generic, like .primarybutton.

Poor class reuse

.productpage .btn {
    background:silver;
    font-size:1.1em;
}

.basketpage .btn {
    background:orange;
    font-size:1.1em;
}

.homepage .btn {
    background:orange;
    font-size:1.5em;
}

Here we have three buttons with broadly similar styles that duplicate code and also have some of the other problems I've already been over. Wouldn't this be better like this?

.btn {
    background:silver;
    font-size:1.1em;
}

.btn.basket {
    background:orange;
}

.btn.large {
    font-size:1.5em;
}

Using existing classes the wrong way

.productpage .col3 {
    box-shadow:0 5px 5px #333333;
    background:#DDDDDD;
}

This is an example of defining a class for one purpose, then retrofitting it in one specific place for another. The .col3 class in this instance is a grid layout class, but some visual styles have been added to it in this context. If you've written a class for one purpose, don't confuse it later by using it for something else - just write another class. That way if the markup changes, the site won't break; the visual styles can be easily moved onto another element.

Excessiveness

.pub_image .cask_img {
    display: inline-block;
    z-index: 999999999999999999;
}

This is an unedited example from a real site I was asked to work on.

Just... what

div[class^="span"] > *:last-child:not(.video) {
    margin-bottom: 0;
    padding-bottom: 0;
}

To finish, another unedited example that I found recently, written this year. The problem here isn't just that there was no comment or documentation to explain this, or that it accidentally clashed with and broke another part of the site - it's that the approach taken could have been so much simpler had the author simply written a new, once only class and applied it where it was needed.

The end

Hopefully this walk through CSS has inspired you to write better code, or to think more about the flexibility and future maintainability of your front end. Remember - someone else will work on your site after you've built it. Try not to make their life more difficult than it needs to be.

Related

This article is tagged with