I have the PR!

he-man

We recently implemented a stronger, more active, pull request culture at CustomInk, as part of our deploy process. As you may already know, a pull request is a feature on Github that broadcasts to others that you would like to have your changes merged into an upstream project. [note: Turns out it's built into git itself http://git-scm.com/docs/git-request-pull]. We find that it's great for learning what other engineering teams are working on. It's also a great opportunity to share knowledge.

Case in point, this blog post is the direct result of that process. I was tasked with the simple job of updating a close button with something cleaner. Since we are no longer supporting Internet Explorer 8, why not do it with CSS and eliminate the need for an image file? Here's how it evolved:

First Iteration

Give an anchor some dimensions, a border-radius of 50% so it's round, and then position it absolutely in the corner of the box. Position the × icon inside the anchor and done!

ship it squirrel

HTML

<a class="removeItem">
  <p class="closer">&times;</p>
</a>

CSS

.removeItem {
  position: absolute;
  left: 3px;
  top: 3px;
  cursor: pointer;
  border-radius: 50%;
  background-color: red;
  border: 1px solid red;
  width: 18px;
  height: 18px;
  .closer {
    display: block;
    position: relative;
    top: -17px;
    left: 4px;
    color: white;
    font-size: 16px;
    font-weight: 700;
  }
}

See it on SassMeister

Simple enough - push changes and make a pull request so it can be deployed. Or at least I thought. Constructive advice in the pull request revealed that there's at least one way to do this more cleanly. Here's the second iteration:

Second Iteration

The second iteration took the content out of the span by way of the CSS content property (which uses the ::before or ::after pseudo-elements). We also replaced the &times; with \00d7 because of how the symbol gets generated in CSS vs HTML. These pseudo-elements are handy because as their names suggest, they allow you to insert content before or after another HTML element.

It's just a small taste of what can be done with dynamic content. [note: I hear there's an upcoming CustomInk blog post on with amazing examples of pseudo-generated content so keep your eyes peeled.]

<a class="removeItem">
  <p class="closer"></p>
</a>
.removeItem {
  position: absolute;
  left: 3px;
  top: 3px;
  cursor: pointer;
  border-radius: 50%;
  background-color: red;
  border: 1px solid red;
  width: 18px;
  height: 18px;
  .closer::before {
    display: block;
    position: relative;
    content: '\00d7';
    top: -17px;
    left: 4px;
    color: white;
    font-size: 16px;
    font-weight: 700;
  }
}

See it on SassMeister

Third Iteration

Pushing this new revision up to Github and requesting feedback led to the third and final version which didn't even use the inner p element (duh!). A ::before pseudo-element did the trick.

<a class="removeItem"></a>
.removeItem {
  position: absolute;
  left: 3px;
  top: 3px;
  cursor: pointer;
  border-radius: 50%;
  background-color: red;
  border: 1px solid red;
  width: 18px;
  height: 18px;
  &::before {
    display: block;
    position: relative;
    content: '\00d7';
    top: 0;
    left: 4px;
    color: white;
    font-size: 16px;
    font-weight: 700;
  }
}

See it on SassMeister

Ending Notes

I hope this blog post encourages your team to start using pull requests not only for merging branches but also as a tool for sharing code and starting conversations. The experience was meaningful for me because my grasshopper knowledge of CSS had me assuming I knew the only way to implement the code. Instead it turned into a great opportunity to level up my styling work.

Thoughtbot has excellent resources for developers and the guidelines for code reviews are no exception.


by Kalimar Maia