Snugug / Aurora

Drupal HTML5 Base Theme
33 stars 14 forks source link

Extending clearfix results in duplication #28

Closed pascalduez closed 11 years ago

pascalduez commented 11 years ago

Not sure where's the best place to post this: Toolkit, Singularitygs, Aurora ?

Since Aurora is importing all of singularitygs and all of toolkit, clearfix @mixin and @extend only selectors are imported twice.

So if you want to extend clearfixin your styles you end up with twice the clearfix declarations. Either by using @extend %clearfix or @include clearfix(true).

Snugug commented 11 years ago

Indeed, that is true. I'll work on getting that worked out, although the best solution wont' be available until Existence Checks are available.

jslegers commented 11 years ago

For a clearfix without duplication, I wrote the following code.


@mixin css($rule, $value, $browsers:all, $quote:false) {
    @if $browsers == ie6 {
        @if $quote == true {
            #{$rule}: quote(#{$value});
        } @else {
            _#{$rule}: #{$value};
        }
    } @else if $browsers == ie6-7 {
        @if $quote == true {
            #{$rule}: quote(#{$value});
        } @else {
            *#{$rule}: #{$value};
        }
    } @else {
        @if $quote == true {
            #{$rule}: quote(#{$value});
        } @else {
            #{$rule}: #{$value};
        }
    }
}

@mixin clear($value, $browsers:all) {
    @include css(clear, $value, $browsers);
}

@mixin content($value, $browsers:all) {
    @include css(content, $value, $browsers, true);
}

@mixin display($value, $browsers:all) {
    @include css(display, $value, $browsers);
}

@mixin zoom($value, $browsers:all) {
    @include css(zoom, $value, $browsers);
}

@mixin zoom-hack() {
    @include zoom(1, ie6-7);
}

%display-block-hacked {
    @include display(block);
    @include zoom-hack();
}

%clear-both {
    clear : both;
}

%blank-as-table {
    @include display(table);
    @include content(" ");
}

%clearfix {
    &:before{
        @extend %blank-as-table;
    }
    & {
        @extend %display-block-hacked;
    }
    &:after {
        @extend %blank-as-table;
        @extend %clear-both;
    }
}

div {
    @extend %clearfix
}

main {
    @extend %clearfix
}

.fancyBox {
    @extend %clearfix
}

.imageBox {
    @extend %clearfix
}

OUTPUT :

div, main, .fancyBox, .imageBox {
  display: block;
  *zoom: 1;
}

div:after, main:after, .fancyBox:after, .imageBox:after {
  clear: both;
}

div:before, main:before, .fancyBox:before, .imageBox:before, div:after, main:after, .fancyBox:after, .imageBox:after {
  display: table;
  content: " ";
}
Snugug commented 11 years ago

The answer is much easier than that, and I will be providing an update to both Toolkit and Singularity to make this happen.

Snugug commented 11 years ago

Recent updated to Singularity and Toolkit should resolve this issue. Please update to Singularity 1.1.1 and Toolkit 1.3.5.

jslegers commented 11 years ago

The implementation I posted requires only Sass 3.2 to be installed. It requires neither Singularity nor Toolkit.

It can also be simplified to this :

%display-block-hacked {
    display: block;
    *zoom: 1;
}

%clear-both {
    clear : both;
}

%blank-as-table {
    display: table;
    content: " ";
}

%clearfix {
    &:before{
        @extend %blank-as-table;
    }
    & {
        @extend %display-block-hacked;
    }
    &:after {
        @extend %blank-as-table;
        @extend %clear-both;
    }
}
Snugug commented 11 years ago

This issue is coming from the interaction of Toolkit/Singularity. Silent selectors can be written within directives, so Toolkit supplies a fixed variable for whether its extends are available, Singularity provides a defaulted opposite version of that variable, and if the Toolkit versions are available, the Singularity versions won't be available, and you won't get duplicate extends.

jslegers commented 11 years ago

I didn't get that. My mistake...

pascalduez commented 11 years ago

I'm still having the issue with Aurora or Corona.

I put $toolkit-clearfix: true; in my variables file but it's not respected. In Aurora or Corona variables are imported after singularity and toolkit. If I put that variable before the import it works. I guess the same is valid for $toolkit-box-sizing: true;

A bit related to #27

Snugug commented 11 years ago

@pascalduez Did you update to the new versions of Toolkit and Singularity? If you have (and have updated your Gemfile to properly reflect the new versions), you should not need to add the $toolkit-clearfix variable in.

Additionally, in both Aurora and Polaris, the legacy support variables are stored in style.scss before any imports happen, making them available.

pascalduez commented 11 years ago

Yes, I've upgraded to the latest version of both toolkit and singularity. (1.3.5 and 1.1.1)

If you put those variables before importing toolkit/singularity it works. If you put them after, does not work.

Basically I wanted to put those variables into the variables partial which is imported after.

Weird, I need to investigate more. Basically the $toolkit-clearfix var in Toolkit should override the one in Singularity right ?

pascalduez commented 11 years ago

I guess it's the way it works in Sass, or am I missing something ?

Works

$color: red !default;
@mixin foo {
  color: $color;
}
$color: blue;
.bar {
  @include foo;
}
// result
.bar {
  color: blue;
}

Does not works

$color: red !default;
.foo {
  color: $color;
}
$color: blue;
.bar {
  @extend foo;
}
// result
.foo {
  color: red;
}