“Is it CSS? Yes? Next.” - Tips for CSS code reviews

Hi! I’m David and I love CSS but... do you? -Suspense music- Jokes aside, let me ask you one more question.

How many times have you been in that situation when you are reviewing code, happily leaving comments and suggestions, wondering how your partner came up with such an awesome solution for something and all of a sudden, bang, a CSS file comes up next and you simply scroll until the next non-CSS file? Or maybe you just look at it, without as much focus as you would put in reviewing Javascript, Java, Ruby, etc.?

If I have to be honest, I’ve been there… and I’m sure we’ve all been there at some point.

In this post, I’m hoping to give you some tips you can use to either start reviewing CSS files in other people’s code, or start having more confidence while reviewing CSS or start looking for more specific stuff.

These tips are just tips, you can take them or leave them. There are no musts. If you have tips that I’ve missed or forgot, feel free to add them in the comments section! I would like to learn more tips, take that for granted!

Check for !important

This is kind of an easy one. If I were you and I see an !important, I would ask the author what they tried to achieve. 95% of the time you shouldn’t be using it, but in some specific cases, like utility classes, it’s needed.

Check for “Known colors”

If there are variables (CSS or SASS) that represent known colors but in the review you find a hex that is one of those colors, changing it to the variable might be a good suggestion. On the other hand, if the color does not have a variable, but it’s supposed to be a “known” one, the a good suggestion would be to make it a variable.

Are the class names following the project conventions?

There are some conventions that can be used for CSS class names, like BEM. If you are aware that one of them is being used in the project’s CSS, you can check if it is being followed in the code you’re reviewing. Besides that, you could also check if the class name is using the correct case that other classes are using.

Do the class names make sense?

Is the class name intention-revealing? Does it have a good name according to the context where it’s being used? Names are important, that’s it.

Are the properties being used correctly/have valid values?

Checking if a property has a valid value can be an easy thing to do, but is it being used correctly? Properties might not always work according to the context. e.g. height in an inline element does nothing. Here you can find a nice reference to check!

Note: These things might not be obvious at first, but we’re learning while reviewing!

Are there properties that could be better by using their shorthanded versions?

CSS defines a lot of properties. A lot. But some of them can be simplified by writing their shorthanded version. For example, padding-top: 0; padding-bottom: 0; padding-left: 10px; padding-right: 10px; could be written as padding: 0 10px;. The same concept applies to margin or border.

Hardcoded sizes

Sometimes we do want a specific amount of pixels, sometimes we don’t. But we should check if the units we’re using allow us to keep the responsiveness of the page. Units like em, rem, %, vh and vw are better for responsiveness than a fixed amount of pixels.

Are all the devices taken care of? (If it applies)

Some pages have different styles depending on the screen size that it’s being viewed on. And forgetting a media query to apply those styles can lead to some issues for a user who is using a device with a certain screen.

Does the page look like it should?

Not much to add here. This is not a CSS-specific thing, but since we’re talking about visual things, checking that the page that is being affected by the code you’re reviewing is looking as expected is a nice thing to do. One cool thing to add here is that if someone else designed the mockups, you could review the page with that person to check that every single thing is covered.

Is specificity going to be an issue in the future?

Are rules using way too many selectors and being too specific? Or are they being too vague? This one might not be an easy one to review, but it’s worth taking some time trying to analyze if the selectors will make future changes more complicated. CSS specificity exists, and knowing about it will let you understand why sometimes your styles won’t apply.

Use your browser debugger to understand changes that you don’t get at first glance

It’s always good to have it open and see how things are working together and which property does what.

"z-index"es

Sometimes, in order to show an element above the other, z-index is used. Most of the time, this is the correct way to go, but in some cases changing the order of the HTML elements is enough. (Psst, psst! Since we’re already here, if you sometime happen to wonder why z-index is not working as you would like, this might be the reason!)

General Review Mindset

There are two more things that I personally like to do that are related to reviewing other people’s code, and specifically reviewing CSS, but I wouldn’t consider them “tips”:

Opportunity to improve both technical and communication skills

Regarding improving communication skills, thoughts like

• “Oh, I didn’t know you could do that!”
• “How should I express this suggestion without sounding harsh?”
• “What a nice way to solve this”
• “Hm, I wonder if I can get an explanation of why this was done in that way”
• “This is not doing what the ticket says it should”

are common ones I usually have while reviewing, and writing them in order to not hurt the other person or let them know they did an awesome job is something that we all sometimes struggle with.

Regarding improving technical skills, while reviewing, we often see ways to solve things that we haven’t thought about, so that’s something we can learn from and add a new tool into our toolbox!

Reviewing imagining

Read the code, understand as much as you can of it and try to picture it in your head. Then see how it actually looks like and try to find what you got wrong in your mental picture. This might be a good exercise to practice what you know about the different CSS properties!

And that’s all I got! Hopefully this can be helpful in your next review and, as I said before, don’t hesitate to leave some of your tips in the comments! Cheerios!