Skip to content
This repository has been archived by the owner on Nov 9, 2017. It is now read-only.

Use of Sass's @extend directive inside media queries leads to sadness. #6

Open
mysterycommand opened this issue Sep 12, 2011 · 4 comments

Comments

@mysterycommand
Copy link

Hi, I just wanted to point out that in the Sass version of the grid, the @extend directive is used inside "IE fixes" and the min-width: 40em media query. Because of the way Sass handles @extend this results in the .huge declaration being converted to:

.huge, .ie h1, h1 {
  /* 42px / 48px  */
  font-size: 2.625em;
  line-height: 1.143em;
}

… which I don't think is what's intended. Less has a slightly simpler mixin syntax, but I think this is what you want:

@mixin huge {
    /* 42px / 48px  */
    font-size: #{42 / $em}em;
    line-height: ($line *2) / 42  * 1em;
}
.huge { @include huge; }

… then, in the IE-specific, and media query declarations, you'd use the same, @include huge; … this produces a CSS document that looks more like the GGS.css that's included in the source.

@jonikorpi
Copy link
Owner

Thanks for pointing this out!

I don't currently use SASS myself, so I can't test this out properly. I'll make a note to convert all the "LESS mixins" into proper SCSS mixins when I get the chance to do so.

@frankpf
Copy link

frankpf commented Sep 23, 2011

Mixins lead to duplicated code easily, while @extend directives do it all in a DRY approach. It's better for performance, file size and maintenance.

The compiled CSS will be different, but I think it would be better if GGS used the best features of each preprocessor.

Example:

SASS with mixins (@include and @mixin):

/* Source */
@mixin huge {
    font-size: 100px;
}

.ie h1 {
     @include huge;
}


/* Compiled CSS */
huge {
    font-size: 100px;
}

.ie h1 {
    font-size: 100px;
}

SASS with @extend directives:

/* Source */
.huge {
    font-size: 100px;
}

.ie h1 {
    @extend .huge
}


/* Compiled CSS */
.huge, .ie h1 {
    font-size: 100px
}

As you can see, in this example the filesize is almost insignificant, but in real CSS files mixins compile to a lot bigger and unperformant files.

@mysterycommand
Copy link
Author

Yeah, using mixing produces more code … more CSS declarations, that's true. I was simply pointing out that this in Less:

.huge {
    /* 42px / 48px */
    font-size: 42 / @em;
    line-height: (@line*2) / 42 * 1em;
}
/* ... */
.ie h1 {
    .huge();
    margin: (48/42*1em) 0 (24/42*1em);
}

… is equivalent to this in Sass:

@mixin huge {
    /* 42px / 48px  */
    font-size: #{42 / $em}em;
    line-height: ($line *2) / 42  * 1em;
}
/* ... */
.ie h1 {
    @include huge;
    margin: (48/42*1em) 0 (24/42*1em);
}

… I know mixins produce more code, but they also produce different code than @extend, and in this particular case, using @extend .huge; inside a media query pulls the h1 declaration out of the media query … which could have unexpected/unwanted consequences.

@xtalx
Copy link

xtalx commented Oct 3, 2011

With sass you don't HAVE to compile mixins to CSS. The sass implementation of GGS should convert all of the classes like .huge, .massive, .wrapper etc. to mixins in a partial. If you aren't going to use those classes in your markup there is no need to define them as classes.

A simple example would be to create a directory called "partials" add a file called _type-presets.sass (or scss) where you define the type preset mixins.

=huge 
    /* 42px / 48px  */
    font-size: #{42 / $em}em
    line-height: ($line *2) / 42  * 1em
...

Then you can @import "partials/type-presets"

and use

#MyOwnMarkup
    +huge

Then your actual CSS file won't have a bunch of unused classes.

Maybe it's out of scope for this project or repo but the current GGS.scss file, although helpful and appreciated, isn't that much of a value add as it doesn't really leverage sass/scss as much as it could. It would be nice to have something like the
compass-less-plugin for GGS. I will try to get the ball rolling on that and do it without making it a compass plugin (just sass/scss).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants