Comments on SASS from CSS for a SASS beginner

Question

I've leaped into SASS and am loving it. I'm asking for comments here on whether my SASS is well-formatted.

Here's the original CSS based on Zurb's Foundation Pagination styles, but adapted for use with Laravel's built-in pagination:

div.pagination ul { display: block; height: 24px; margin-left: -5px; }
div.pagination ul li { float: left; display: block; height: 24px; color: #999; font-size: 14px; margin-left: 5px; }
div.pagination ul li a { display: block; padding: 1px 7px 1px; color: #555; font-weight: normal}
div.pagination ul li:hover a,
div.pagination li a:focus { background: $bgltblue; }
div.pagination ul li.disabled a { cursor: default; color: #999; }
div.pagination ul li.disabled:hover a,
div.pagination li.disabled a:focus { background: transparent; }
div.pagination ul li.active a { background: $headblue; color: white; font-weight: normal; cursor: default; }
div.pagination ul li.active a:hover,
div.pagination li.active a:focus { background: $headblue; }

Here's my manual conversion to SASS:

div.pagination {
 ul {
 display: block; height: 24px; margin-left: -5px;
 li {
 float: left; display: block; height: 24px; color: #999; font-size: 14px; margin-left: 5px;
 a {
 display: block; padding: 1px 7px 1px; color: #555; font-weight: normal;
 }
 &:hover a {
 background: $bgltblue;
 }
 &.disabled {
 a {
 cursor: default; color: #999;
 }
 &:hover a {
 background: transparent;
 }
 }
 &.active {
 a {
 background: $headblue; color: white; font-weight: normal; cursor: default;
 &:hover {
 background: $headblue;
 }
 }
 }
 }
 }
 li {
 a:focus {
 background: $bgltblue;
 }
 &.disabled {
 a:focus {
 background: transparent;
 }
 }
 &.active {
 a:focus {
 background: $headblue;
 }
 }
 }
}

I'd like to know if you think I have reduced it correctly, and where I might make improvements. I plan to replace the colours with vars already.

• cimmanon: Doesn't Foundation already use Sass?

Answer 1

Some comments on your style of selecting:

• div.pagination is overqualified. A container is almost always going to be a div, so you just can write .pagination
• In many cases, there is no use for an extra container. You might as well apply the pagination class to the list itself: <ul class="pagination">

• You also have a lot of redundancy in your selectors because you always use ul and li in the chains. This is not necessary and could be made easier:

  .pagination ul {}
  /* li's can only appear in lists, no need to add it to the selector */
  .pagination li {}
  /* The only links inside your pagination are usually in relation to the pagination */
  .pagination a {}
  

• If you have lists inside your pagination that need separate styling, use separate classes or place them outside of the actual pagination

• Don't overqualify the .active and .disabled classes as well. They will end up being on the list items anyway

Conclusion:

You nest far too deep. Avoid nesting selectors deeper than 2–3 levels. Otherwise you get bloated CSS like this.

Compare my SCSS to yours:

.pagination {
 display: block;
 height: 24px;
 margin-left: -5px;
 li {
 float: left;
 display: block;
 height: 24px;
 color: #999;
 font-size: 14px;
 margin-left: 5px;
 &:hover a {
 background: $bgltblue;
 }
 &.disabled {
 a {
 cursor: default;
 color: #999;
 &:hover,
 &:focus {
 background: transparent;
 }
 }
 }
 &.active {
 a {
 background: $headblue;
 color: white;
 font-weight: normal;
 cursor: default;
 &:hover,
 &:focus {
 background: $headblue;
 }
 }
 }
 }
 a {
 display: block;
 padding: 1px 7px 1px;
 color: #555;
 font-weight: normal;
 &:focus {
 background: $bgltblue;
 }
 }
}