patterncssModerate

Comments on SASS from CSS for a SASS beginner

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

cssbeginnerforfromcommentssass

Problem

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 i

Solution

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: `




- 
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;
        }
    }
}

Code Snippets

.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 {}

.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;
        }
    }
}

Context

StackExchange Code Review Q#24716, answer score: 12

Revisions (0)

No revisions yet.