12
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Doesn't Foundation already use Sass? \$\endgroup\$ – cimmanon Sep 5 '13 at 20:33
12
\$\begingroup\$

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;
        }
    }
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.