Published: Mon 12 Oct 2020

Refactoring Some Old Code

I was working on a 5-year old codebase today. Here's a tiny section of CSS that I think I tidied up quite nicely.

Today's task was not code refactoring, but sometimes when you look at a legacy codebase, you just can't help yourself.

Here's a small section of code that I came across. The purpose of this code was to get 4 "Hero Blocks" and align them 2x2 on small screens and 1x4 on large screens.

.hero-block__content {
  margin: 0;
  position: relative;
  display: inline-block;
  float: left;
  text-align: center;
  border-bottom: 3px solid $colour-secondary;
  @include susy-media(0 850px) {
    width: 48%;
    margin-right: 1.33%;
    margin-bottom: 2%;
    &:nth-child(1) {
      @include first
      margin-top: -20px;
    }
    &:nth-child(2) {
      @include last;
      margin-top: -20px;
    }
    &:nth-child(3) {
      @include first;
      margin-top: 10px;
    }
    &:nth-child(4) {
      @include last;
      margin-top: 10px;
    }
    &:nth-child(5) {
      @include first;
      margin-top: 10px;
    }
  }
  @include susy-media(850px) {
    width: 24%;
    margin-right: 1.33%;
    margin-bottom: 2%;
    &:nth-child(2n-1) {
      @include first;
    }
    &:last-child {
      @include last;
    }
  }
}

 

What's wrong with the above? Here's a few items:

 

  1. We shouldn't need to load the whole of Suzy just to add a media query and some layout
  2. float: left; is not going to work, because we already have display: inline-block; set.
  3. If the float did work, it would take the content out of the DOM flow
  4. We don't like magic numbers like 1.33% for spacing
  5. We need to add a mixin for first or last to each item in our list, and would need to continue doing this if we added more items. This is not scalable.

 

After refactoring (I know, that wasn't the task, but I don't consider it wasted budget as it'll make working on this codebase easier for the future) here's what I came up with:

.hero-block {
  display: flex;
}
  
.hero-block__content {
  position: relative;
  width: calc(50% - 1rem);
  margin: 0 .5rem 1rem;
  text-align: center;
  border-bottom: 3px solid $colour-secondary;
}
@media screen and (min-width: 850px) {
  .hero-block__content {
    width: calc(25% - 1rem);
  }
}

 

Comments

Add new comment

Restricted HTML

  • Allowed HTML tags: <a href hreflang> <em> <strong> <cite> <blockquote cite> <code> <ul type> <ol start type> <li> <dl> <dt> <dd> <h2 id> <h3 id> <h4 id> <h5 id> <h6 id>
  • Lines and paragraphs break automatically.
  • Web page addresses and email addresses turn into links automatically.