Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

_sass files could all be in extendable placeholders. #12

Closed
AhoyLemon opened this issue Mar 25, 2016 · 10 comments
Closed

_sass files could all be in extendable placeholders. #12

AhoyLemon opened this issue Mar 25, 2016 · 10 comments

Comments

@AhoyLemon
Copy link

For the sass structure, do you think it might be helpful to rewrite the library using placeholders instead of classes? Ex:

%hamburger-slider-r {
  .hamburger-inner { }
}

That could help in a situation where somebody is getting the whole package but only want to use one of the transitions, and don't want to inflate the code with the ones they don't use.

@AhoyLemon AhoyLemon changed the title _sass files could all be in @extends. _sass files could all be in extendable placeholders. Mar 25, 2016
@jonsuh
Copy link
Owner

jonsuh commented Mar 25, 2016

Hey @AhoyLemon, I had initially thought of that but instead broke out each type as its own partial. If someone is using the Sass setup and does not want to include 1 or more styles, they can easily go to _sass/hamburgers/hamburgers.scss and comment out the types they don’t want:

// Types (Remove or comment out what you don’t need)
// ==================================================
@import "types/3dx";
@import "types/3dx-r";
@import "types/3dy";
@import "types/3dy-r";
@import "types/arrow";
@import "types/arrow-r";
@import "types/arrowalt";
@import "types/arrowalt-r";
@import "types/boring";
@import "types/collapse";
@import "types/collapse-r";
@import "types/elastic";
@import "types/elastic-r";
@import "types/emphatic";
@import "types/emphatic-r";
@import "types/slider";
@import "types/slider-r";
@import "types/spring";
@import "types/spring-r";
@import "types/stand";
@import "types/stand-r";
@import "types/spin";
@import "types/spin-r";
@import "types/squeeze";
@import "types/vortex";
@import "types/vortex-r";

Aside from that, I’m not sure if there’s added value of Extends as opposed to what’s currently in place, but all ears in the event I’m missing something and there’s a compelling reason to change it.

@christianmagill
Copy link

Commenting out source files of a package isn't the best scenario when using package managers such as Bower. It's expected that packages remain intact, and as such, the components themselves are often not committed with the projects they are included in.

@jonsuh
Copy link
Owner

jonsuh commented Mar 28, 2016

@christianmagill Valid and great point! +@AhoyLemon Instead of extends, I came up with an alternate solution. I added another settings variable, $hamburger-types (a Sass list), that you can set to only include the types you want:

$hamburger-types (3dx, 3dy);

When you don’t set it, it includes all of the types by default. Here’s the commit: e914391

@christianmagill
Copy link

Looks great! Thanks

@resknow
Copy link

resknow commented Mar 31, 2016

Just tried to compile Hamburgers using Sass and I get the following:

Import directives may not be used within control directives or mixins. AFAIK Sass doesn't allow conditional imports.

Compiles beautifully when I remove the @if statements :)

@jonsuh
Copy link
Owner

jonsuh commented Apr 4, 2016

@resknow Hm. Curious, with what are you compiling the Sass with? And what version? Seems like it’s libsass related and the issue could be fixed in a latter version sass/libsass#1724

@resknow
Copy link

resknow commented Apr 4, 2016

I'm using Ruby Sass v3.4.20 (Selective Steve).

@jonsuh
Copy link
Owner

jonsuh commented Apr 4, 2016

@resknow Good to know! Apparently later versions of libsass allows @import in @if but Ruby Sass does not. I pushed a new release that should fix the issue https://github.com/jonsuh/hamburgers/releases/tag/v0.5.0 Would you mind testing it to confirm that it works or doesn’t?

@resknow
Copy link

resknow commented Apr 5, 2016

@jonsuh Thanks! Works perfectly now.

@jonsuh
Copy link
Owner

jonsuh commented Apr 5, 2016

Awesome! Gonna safely assume this issue is good to close

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

No branches or pull requests

3 participants