HiveBrain v1.2.0
Get Started
← Back to all entries
patterncssMinor

SASS Code Structure / Readability

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codereadabilitystructuresass

Problem

I'm trying out a new approach to SASS stylesheets, which I'm hoping will make them more organizined, maintainable, and readable.

I often feel like there is a thin line between code that is well structured and code that is entirely convoluted. I would appreciate any thoughts as to which side of the line this code falls.

I don't want to tell you too much more about what these styles are intended to produce -- my hope is that the code will explain this for itself. Also note that this is part of a larger project, so don't worry about missing dependencies, etc.

Questions for review

  • How would you make this code easier to read/maintain?



  • Can you understand what these styles are trying to produce?



  • Is the purpose of the mixins/placeholders clear?



File structure:

theme/sass/partials/
    widget/
        collapsable/
            _appendicon.scss
            _closeall.scss
            _toggleswitch.scss
        collapsable.scss
        collapsablered.scss
    _button.scss


collapsable.scss

/**
 * Collapsable widget.
 *
 * The widget has "open" and "closed" states.
 * The widget has a Toggle Switch, which is visible in
 * both open and closed states.
 *    All other content is hidden in the closed state.
*/
@mixin setOpenState {
  &,
  &.state-open {
    @content;
  }
}

@mixin setClosedState {
  &.state-closed {
    @content;
  }
}

@mixin setToggleSwitchStyles {
  &>h1:first-child, .collapseableToggle {
    @content;
  }
}

@import "collapsable/closeall";
@import "collapsable/appendicon";
@import "collapsable/toggleswitch";

%collapsable {
  @include setOpenState {
    @include setToggleSwitchStyles {
      @extend %toggleSwitch;
    }
  }

  @include setClosedState {
    @extend %closeAllExceptToggle;

    @include setToggleSwitchStyles {
      @extend %toggleSwitchClosed;
    }
  }
}


collapsablered.scss

```
@import "collapsable";
@import "../button";

%collapsableRed {
@extend %collapsable;

@include setOpenState {
@include setToggl

Solution

Overall, your naming conventions are pretty good. I don't feel like I need to go look at mixins themselves to figure out what their purpose is. The extensive use of extends does concern me, since it can lead to larger CSS rather than smaller like you might expect (see: Mixin, @extend or (silent) class?).

Your %textOnDarkBg and %textOnRedBg extend classes might be redundant. If you're not already using Compass, you might want to take a look at it. It offers a function as well as a mixin for setting a good contrasting color against your desired background color (see: http://compass-style.org/reference/compass/utilities/color/contrast/). Highly useful if your project is intended to be themed.

Generally speaking, using colors for class names isn't very clear unless the content is about color (eg. a color wheel or a rainbow). What is red for? Is it for errors? Or maybe a call to action? The same thing goes for dark. Using inverted or closed might be better choices. If the site's design is already dark, a dark button probably doesn't make much sense.

Your code only allows the user to have exactly 2 colors (default and red), which seems more limited than it needs to be. You could easily make it very flexible by making use of lists (or maps, which will be in the next version of Sass). Here's an example from my own project:

//      name     dark    light
$dialog-help:    #2E3192 #B9C2E1 !default; // purple
$dialog-info:    #005FB4 #BDE5F8 !default; // blue
$dialog-success: #6F7D03 #DFE5B0 !default; // green
$dialog-warning: #A0410D #EFBBA0 !default; // orange
$dialog-error:   #C41616 #F8AAAA !default; // red

$dialog-attributes:
    ( help    nth($dialog-help, 1) nth($dialog-help, 2)
    , info    nth($dialog-info, 1) nth($dialog-info, 2)
    , success nth($dialog-success, 1) nth($dialog-success, 2)
    , warning nth($dialog-warning, 1) nth($dialog-warning, 2)
    , error   nth($dialog-error, 1) nth($dialog-error, 2)
    ) !default;

@each $a in $dialog-attributes {
    $name: nth($a, 1);
    $color: nth($a, 2);
    $bg: nth($a, 3);

    %dialog-colors.#{$name} {
        color: $color;
        background-color: $bg;
    }

    %dialog-colors-inverted.#{$name} {
        color: $bg;
        background-color: $color;
    }

    %badge-colors.#{$name} {
        background-color: $color;
        color: $background-color;
    }

    %button-colors.#{$name} {
        @include button($base: $bg) {
            @include button-text($color, inset);

            @include button-states;
        }
    }

    %button-colors-inverted.#{$name} {
        @include button($base: $color) {
            @include button-text($bg, inset);

            @include button-states;
        }
    }

    %button-colors-faded.#{$name} {
        @include button($base: fade($bg, 10%)) {
            color: #CCC;

            @include button-states;
        }
    }
}


In case you're wondering why I'm using multiple classes, I've setup a short demo: http://sassmeister.com/gist/7792677

Code Snippets

//      name     dark    light
$dialog-help:    #2E3192 #B9C2E1 !default; // purple
$dialog-info:    #005FB4 #BDE5F8 !default; // blue
$dialog-success: #6F7D03 #DFE5B0 !default; // green
$dialog-warning: #A0410D #EFBBA0 !default; // orange
$dialog-error:   #C41616 #F8AAAA !default; // red

$dialog-attributes:
    ( help    nth($dialog-help, 1) nth($dialog-help, 2)
    , info    nth($dialog-info, 1) nth($dialog-info, 2)
    , success nth($dialog-success, 1) nth($dialog-success, 2)
    , warning nth($dialog-warning, 1) nth($dialog-warning, 2)
    , error   nth($dialog-error, 1) nth($dialog-error, 2)
    ) !default;

@each $a in $dialog-attributes {
    $name: nth($a, 1);
    $color: nth($a, 2);
    $bg: nth($a, 3);

    %dialog-colors.#{$name} {
        color: $color;
        background-color: $bg;
    }

    %dialog-colors-inverted.#{$name} {
        color: $bg;
        background-color: $color;
    }

    %badge-colors.#{$name} {
        background-color: $color;
        color: $background-color;
    }

    %button-colors.#{$name} {
        @include button($base: $bg) {
            @include button-text($color, inset);

            @include button-states;
        }
    }

    %button-colors-inverted.#{$name} {
        @include button($base: $color) {
            @include button-text($bg, inset);

            @include button-states;
        }
    }

    %button-colors-faded.#{$name} {
        @include button($base: fade($bg, 10%)) {
            color: #CCC;

            @include button-states;
        }
    }
}

Context

StackExchange Code Review Q#36647, answer score: 4

Revisions (0)

No revisions yet.