SASS Code Structure / Readability

Question

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

1. How would you make this code easier to read/maintain?
2. Can you understand what these styles are trying to produce?
3. 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 setToggleSwitchStyles {
 @extend %buttonWithRedBg;
 }
 }
 @include setClosedState {
 @include setToggleSwitchStyles {
 @extend %buttonWithDarkBg;
 }
 }
}

collapsable/_closeall.scss

%closeAllChildren {
 * {
 display: none;
 }
}
%closeAllExceptToggle {
 @extend %closeAllChildren;
 @include setToggleSwitchStyles {
 display: block;
 .icon-sprite {
 display: inline-block;
 }
 }
}

collapsable/_appendicon.scss

@import "compass/utilities/general/clearfix";
@import "../../icon";
@mixin appendIcon {
 @include pie-clearfix;
 .icon-sprite {
 margin-right: 5px;
 vertical-align: -3px;
 }
 &:after {
 content: '';
 position: relative;
 top: 2px;
 float: right;
 @content;
 }
}
%withCloseIcon {
 @include appendIcon {
 @extend .icon-close; // defined in _icon.scss
 }
}
%withOpenIcon {
 @include appendIcon {
 @extend .icon-rChevronDk; // defined in _icon.scss
 top: 1px;
 }
}

collapsable/_toggleswitch.scss

%toggleSwitch {
 cursor: pointer;
 @extend %withCloseIcon;
}
%toggleSwitchClosed {
 @extend %toggleSwitch;
 @extend %withOpenIcon;
}

partials/_button.scss

@import "typography";
%buttonWithRedBg {
 @extend %textOnRedBg; // defined in _typography.scss
 cursor: pointer;
 &:hover {
 background-color: $redDk;
 }
 &:active {
 background-color: $black;
 }
}
%buttonWithDarkBg {
 @extend %textOnDarkBg; // defined in _typography.scss
 cursor: pointer;
 &:hover {
 background-color: #000;
 }
 &:active {
 background-color: $redDk;
 }
}

• cimmanon: There appears to be a CSS error in your icon-sprite class. You have vertical-align: -3px, but the vertical-align property only takes specific values like top/bottom/middle/text-top/etc. Also, is this code intended to be portable (Compass extension)?
• edan: As I said, this is a component of a larger project, not a portable extension -- though I am concerned with modularity and reusability. Also, I'm looking for comments on readability/structure/etc., not syntax errors -- though vertical-align does take px arguments, for the record.

Answer 1

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

• edan: I'm glad the structure and use of the mixins is easy to grok. The use of @extend vs @mixin is still a bit of a mystery to me, though I've been trying to use @extend to denote a type of style (noun), and @include to do something to a style (verb). I think you're right about the color naming. I split out collapsableRed because I wanted to make it easy to extend the base collapsable widget for different themes. It would probably be better to use a named theme instead of a color name (eg myAwesomeThemeCollapsable). I wouldn't use .42pxWideHeader instead of .sidebarHeader
• edan: And yes, I am using Compass. The %textOnWhateverColorBg runs into the same problem as %collapsableWhateverColor -- I shouldn't be naming a class with a specific style value. I'll have to think of a better name. Maybe %textColorsActive and %textColorsInactive would be more descriptive.