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: prefix ($whirl-) for all variables, added !default #10

Merged
merged 1 commit into from Sep 22, 2016
Merged

SASS: prefix ($whirl-) for all variables, added !default #10

merged 1 commit into from Sep 22, 2016

Conversation

stefanmeschke
Copy link
Contributor

Hi jh3y,

first of all I would like to thank you for these easy to use spinners and loading animations.

I've made some changes regarding the sass version. I've added !default to all variables to override these before importing one or more files into our sccs file. The prefix whirl- for all variables is to avoid other variable changes from other libs.

Please let me know what you think.

Greetings,
Stefan

@jh3y
Copy link
Owner

jh3y commented Sep 22, 2016

Hey @stefanmeschke

Thanks for the feedback 😄

I'm actually looking to rewrite whirl entirely very soon with a whole bunch of different spinners and animations I've been putting together here and there. I may be moving over to using stylus as it's my favored preprocessor. However, I do understand that it may be pretty necessary to keep a sass version going.

The changes you've proposed actually make a lot of sense to me. The !default flag is a good fit and prefixing will stop any interference from other variable declarations as you've mentioned 😄

Happy to merge this in 👍

@jh3y jh3y merged commit c6c568a into jh3y:master Sep 22, 2016
@stefanmeschke
Copy link
Contributor Author

Good morning @jh3y

thank you!

Cheers

@jh3y
Copy link
Owner

jh3y commented Sep 23, 2016

No problem @stefanmeschke 😄

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

Successfully merging this pull request may close these issues.

None yet

2 participants