Code review guidelines for CSS, JS and HTML

Question

I have been asked to create guidelines for review CSS, JS and HTML. I know there are coding guidelines for JS but I don't know any about HTML and CSS. To review JS I will certainly follow those guidelines and mention them. But what about CSS and HTML? Apart from logical errors and indentation issues, are there any specific things I need to check when I review markup and or CSS?

Answer 1

Some things to look for:

• Is structured information identified using the appropriate HTML tags? H1-H6 for headings, UL/OL and LI for lists, etc?
• Are no legacy HTML tags (<b>, <i>, <center>, <font>) used?
• Does the site use the least amount of markup possible?
• Is style information externalized to CSS files?
• Is all Javascript externalized? including event handlers?
• Do CSS class names refer to the function in the page (img-caption), and not form (bold-red) or content (pink-elephant)?
• Are images in the appropriate format (PNG or JPEG, depending on the type)?
• Have minimized versions of Javascript libraries been used?
• Optionally, have all locally-developed Javascript and CSS files been minimized?
• Does the HTML / CSS validate?
• Has YSlow (or similar) been used to check/optimize performance?
• (mostly) [SEO] Is the site accessible with Javascript turned off?
• [SEO] Is the most relevant content found at the top of the HTML?

Answer 2

One of important elements of a good style in HTML is progressive enhancement. This means that HTML layout will render good even without CSS or JavaScript. Then, once JS/CSS are processed, it will look better (e.g. old-style HTML <select> dropdown will become animated).

This also has to do with non-intrusive markup. Instead of <font style="color:red;font-size:16pt">Hello</font> one would use <div class="red-colored-big-fond">Hello</div>.

Same with JavaScript. Instead of <button onclick="javascript:alert('a');">Clickme</button> one would specify a button class/ID and target it from JavaScript. It also makes your markup code easier to maintain.

Answer 3

Validate that the least amount of markup is used to accomplish the task. Make sure markup is semantic as well, don't use when a tag does the same exact thing and gives more information. Validate that spans aren't being made block elements and vice-versa.

Additionally, alex brings up a great point in a roundabout way. Make sure nobody uses classnames like "red-colored-big-font", because approximately 20 seconds after it's deployed for real somebody is going to change it to a small blue font. I have actually seen this CSS:

.arial12pt { font-family: Verdana; font-size: 8pt; }

Everything in your markup should be describing what the page is, not what it looks like. I definitely agree about progressive enhancement, but again in a roundabout way. The page has no requirements to look anywhere close to the same with CSS as without it. Don't try to make them look the same, because you'll end up back in table-land and spacer.gif.

Answer 4

As far as HTML goes, I always make a point to have hierarchies and indentation in my files. For example, if I have a bunch of divs:

<div id="content">
   <div id="post">
     <div class="title">
       Blah Blah Title
     </div>
   </div>
</div>

I suppose that's fairly obvious to most who create layouts and templates, but more often than not I just see garbled HTML that has no structural hierarchy whatsoever, making it difficult to read for another person. I guess coming from a more CS background, this is something that sticks in my mind. The same goes for CSS as well. Let's say you're styling a div:

#whatever{
    background-image: url('blah.gif');
    color: #FFF000;
}

Indentation makes it a lot easier to read through quickly when you're used to another language mixed in like PHP/Ruby/Whatever. Again, it depends on how you work best, but when others read my HTML, I like to make it really organized :).

Also, as said above, it is always a great idea to name your CSS classes and ids relevant names to your layout, especially when it gets hairy (much like naming variables and methods in other languages). Something else to watch out for is the dreaded "guess and check" of margins, paddings and other alignment issues. Something I often try to avoid is putting negative numbers in my margins and paddings. It can get confusing if you did not make the layout yourself and if you want to come back to it later and modify it, you might have to overhaul it. In my opinion it is always a good idea to not try anything hokey or "kludgy" in CSS, even if it looks nice; there's usually a better way to do it, even if you have to restructure your CSS!

Answer 5

For Javascript I would always want it to validate with JSLint, i can think of so many bugs that JSLint catches that not using it is just crazy.

Answer 6

I came across this standards document that I rather liked. I'll also echo what was said about progressive enhancement. In general, if someone else writes the HTML/CSS, you should be able to look at it later down the line and be pleasantly surprised how not-bad the markup is and should be able to make simple style tweaks easily and efficiently.