asotog
2/14/2018 - 2:20 PM

Frontend Code Review Checklist

HTML

HTML5 provides us with lots of semantic

// not so BAD 
<div class="nav">
...
</div>

// but BETTER 
<nav>
...
</nav>

// not so BAD 
<div class="footer">
...
</div>

// but BETTER 
<footer>
...
</footer>

CSS

Try to use relative sizes for images to prevent them from accidentally overflowing the container.

// BAD 
.content-image {
   width: 600px;
}
// GOOD
.content-image {
  max-width: 100%;
  width: 600px;
}

// GOOD or rely on image natural size but only set the max
.content-image {
  max-width: 100%;
}

While the semicolon is technically a separator in CSS, always treat it as a terminator.

/* bad */
div {
  color: red
}

/* good */
div {
  color: red;
}

Don't duplicate style declarations that can be inherited

/* bad */
div h1, div p {
  font-weight: bold;
  text-shadow: 0 1px 0 #fff;
}

/* good */
div {
  font-weight: bold;
  text-shadow: 0 1px 0 #fff;
}

If need to use a browser hack, document it please

// BAD
@media screen and (-ms-high-contrast: active), (-ms-high-contrast: none) {
    .selector { property:value; }
}

// GOOD
/* this targets only IE 10 and 11 */
@media screen and (-ms-high-contrast: active), (-ms-high-contrast: none) {
    .selector { property:value; }
}

Write code using multiple lines and spaces

// BAD
.list-item-title, .list-item-date, .list-item-autor { color: #FFF; }

// GOOD
.list-item-title,
.list-item-date,
.list-item-autor { 
  color: #FFF; 
}

Reduce the complexity of your selectors

// BAD
.base .list-item div p a { text-decoration: underline }

// GOOD
.list-item-summary a {
  text-decoration: underline 
}

Javascript

Avoid query same DOM element multiple times if possible

BAD
// first do something
...
document.querySelector('.contact-form')
...

// another do something
...
document.querySelector('.contact-form')
...

// last do something
...
document.querySelector('.contact-form')
...

GOOD
var contactForm = document.querySelector('.contact-form');


// first do something  with form element
...
contactForm
...

// another do something with form element
...
contactForm
...

// last do something  with form element
...
contactForm
...

Avoid over exposing globally the js methods, variables, objects ,etc...

//BAD
// main.js
var formErrors = [];

var submitContactForm() {
  ...
};

var contactForm = document.querySelector('form');

if (contactForm) {
  ...
}
...

// GOOD
// main.js

(function() {
  var formErrors = [];
  
  var submitContactForm() {
    ...
  };
  
  var contactForm = document.querySelector('form');
  
  if (contactForm) {
    ...
  }
...
})();

Keep the state located in one place of your file

// BAD
search-results.js

ln 1 => var keyword = '';


ln 50 => var pageSize = 10;


ln 80 => var selectedFilters = ...;

ln 90 => var requestingResults = false 

ln 100 => var offset = 0;

// GOOD
search-results.js

ln 1 => var keyword = '';


ln 2 => var pageSize = 10;


ln 3 => var selectedFilters = ...;

ln 4 => var offset = 0;

ln 5 => var requestingResults = false 

// GOOD, better :P
search-results.js

ln 1 => 
var state = {
  keyword: '',
  pageSize: 10,
  selectedFilters: ..,
  offset: 0,
  requestingResults: false
};

Avoid nesting 2 or more callback levels

//  BAD
doSomethingAsync1(param1, function(someArgs) {

  doSomethingAsync2(function() {
      doSomethingAsync3(function() {
    
      
      });
  });
});


// GOOD

function doSomethingAsync3(someArgs) {
    ...
}

function doSomethingAsync2(someArgs) {
...
  doSomethingAsync3(function() {
  
    ...
  });
}

doSomethingAsync1(param1, doSomethingAsync2);

// GOOD, even better, Promise approach
doSomethingAsync1(param1)
  .then(doSomethingAsync2)
  .then(doSomethingAsync3)
  .then(function(someArgs) {
    // finally chain ended
    // do last stuff here
  }).catch ...

Try to make externally configurable values, such as api endpoints, parameters, attributes, properties, strings/language

// BAD

/* products.js */
const fetchProducts = _ => {
  return fetch('/api1/products.json', ...);
}

// GOOD

/* page */
<script>
window.Config = {
  api: {
    products: '/api1/products.json'
  },
  strings: {
    ...
  },
  settings: {
    ...
  }
}
</script>


/* products.js */
const config = window.Config;

const fetchProducts = _ => {
  return fetch(config.api.products, ...);
}