thoughtbot / bourbon

A Lightweight Sass Tool Set
https://www.bourbon.io/
MIT License
9.09k stars 878 forks source link

Moving a few things to placeholders #362

Closed KittyGiraudel closed 10 years ago

KittyGiraudel commented 10 years ago

I think some addons could (should) be moved to placeholders to minimize resulting output. More informations: http://www.sitepoint.com/sass-mixin-placeholder/.

%clearfix {
  &:after {
    content: '';
    display: table;
    clear: both;
  }
}

If you don't want to break the API (which I can understand), or simply want to keep everything working with mixins, here is what you could do:

@mixin clearfix {
  @extend %clearfix;
}

Same for ellipsis and hide-text.

kaishin commented 10 years ago

@HugoGiraudel Until media queries start handling global placeholder selectors (which might never happen, since it's a philosophical stance), I don't think we should use placeholders in any of our libraries. Or if we do, we should make them secondary/optional.

The current way of supporting global placeholder selectors within media queries (maps + at-root) is too hacky and might have a lot of side effects to be used in Bourbon.

KittyGiraudel commented 10 years ago

I have yet to come across a case where you need to clear floats only at a certain breakpoint and not the others.

That being said, Gzip does some amazing job crushing down repeated strings so I suppose using mixins is not that bad if we only look at final file size. I've opened a discussion on Gist a while back about this.

kaishin commented 10 years ago

@HugoGiraudel I don't think that's true and even if it was, we can't say that for the other mixins.

What we can do, is creating placeholder selectors on Bitters instead. At least users would be free to remove or edit them. @kylefiedler / @plapier what do you think?

KittyGiraudel commented 10 years ago

@HugoGiraudel I don't think that's true and even if it was, we can't say that for the other mixins.

What are you refering to? The fact I don't think we usually clear floats at a specific breakpoint, or the thing about Gzip?

kaishin commented 10 years ago

This:

The fact I don't think we usually clear floats at a specific breakpoint

Clearing floats is a layout-related thing, and that's the number 1 target for media queries.

KittyGiraudel commented 10 years ago

Then I suppose I might never have encountered the case. Although I can get your point: the fact placeholders can't be extended cross media scopes is a shame.

What about adding both mixins and placeholders?

kaishin commented 10 years ago

@HugoGiraudel Yes, either that or we move placeholders to http://bitters.bourbon.io

KittyGiraudel commented 10 years ago

I didn't know about Bitters. That sounds like a reasonable idea.

kylefiedler commented 10 years ago

@HugoGiraudel I'm open to adding the placeholders to bitters but what is the benefit of having both mixin and placeholder? Personally, I would think it confusing to take over a code base to find both @extend %clearfix; and @include clearfix; in it.

Why don't you just do something like this at the beginning of your stylesheets?

%clearfix {
  @include clearfix;
}
kaishin commented 10 years ago

@kylefiedler Agreed. But the point is that if we end up adding those on every new project, then maybe having them in bitters would be the right thing to do.

kylefiedler commented 10 years ago

@kaishin I think my point before was that I wouldn't even do that

KittyGiraudel commented 10 years ago

Why don't you just do something like this at the beginning of your stylesheets?

Because the point of a framework is I don't have to do this kind of things.

kaishin commented 10 years ago

@kylefiedler I think some of these mixins should be used as extends, as a good practice. I agree with @HugoGiraudel that %clearfix and %hide-text are good candidates for this.

kylefiedler commented 10 years ago

Moved this conversation over to Bitters since it seems these would make more sense there.

https://github.com/thoughtbot/bitters/issues/42