search

erikflowers / weather-icons

215 Weather Themed Icons and CSS
https://github.com/erikflowers/weather-icons
6.91k stars 851 forks source link

Add ability to override variables in SASS version #95

Closed Va1 closed 9 years ago

Va1 commented 9 years ago

Hey, guys!

I needed to override the path to fonts directory and found a variable in sources - $wi-path (here). I've tried to override it before importing the main SCSS file of the package but failed. Then I've looked carefully again and found out that it can not be overridden due to missing !default after its value. This applies to all those variables next to $wi-path. I can submit a PR.

erikflowers commented 9 years ago

This is specific to Sass and not Less? This is my first go with Sass and not less. I will look in to this, but also feel free to PR.

From: Valentyn notifications@github.com Reply: erikflowers/weather-icons reply@reply.github.com> Date: August 19, 2015 at 11:51:40 AM To: erikflowers/weather-icons weather-icons@noreply.github.com> Subject:  [weather-icons] Add ability to override variables in SASS version (#95)

Hey, guys!

I needed to override the path to fonts directory and found a variable in sources - $wi-path (here). I've tried to override it before importing the main SCSS file of the package but failed. Then I've looked carefully again and found out that it can not be overridden due to missing !default after its value. This applies to all those variables next to $wi-path. I can submit a PR.

— Reply to this email directly or view it on GitHub.

Va1 commented 9 years ago

Yes, this is specific to SASS. This is one of my first projects with SASS and I just ran into a fact that variables are loading in different way in it. In LESS you can do overrides anywhere, but in SASS you need to add !default to the variables that should be overridden before being declared. Some info.

Va1 commented 9 years ago

Also here, "Variable Handling" section.

Va1 commented 9 years ago

Looks nice! It would be also cool if you bump the package version so the fix will be available over package managers. Thank you.

erikflowers commented 9 years ago

Would that require adding a new tag and pointing the bower file there? I still don't quite grasp the package manager management.

Va1 commented 9 years ago

I guess so. And you need to update version (to 2.0.2, I think, as it is a fix).

Va1 commented 9 years ago

Good!