patterncssMinor
SASS Code Structure / Readability
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
File structure:
collapsable.scss
collapsablered.scss
```
@import "collapsable";
@import "../button";
%collapsableRed {
@extend %collapsable;
@include setOpenState {
@include setToggl
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.scsscollapsable.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
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:
In case you're wondering why I'm using multiple classes, I've setup a short demo: http://sassmeister.com/gist/7792677
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.