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

default sass vars + DOMContentLoaded "bugfix" by strajk #17

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

pouipouidesign
Copy link

Hi,

First, thanks for the really cool work :)
My modest "contribution", not very used to pull requests... but I track your plugin with bower and fear I'll forget I modified your file, update it and loose my mods...

  • I have a separate variables.scss file for sass init and declaring variables in it didn't seem to replace the defaults in your css/_breakpoints.scss file so I checked it: it was missing " !default" in vars declarations, so I added them and voilà, an even greater plugin :)
  • Since I found that @Strajk's branch was a bit ahead and didn't break anything (and the fix seemed useful), I thought it'd be smarter to include it in the pull request instead of starting from an earlier version? ;
  • Upped the version in bower.json.

Hope this is useful to you, and thanks again for the great work!

Doesn't seem to be breaking anything, so let's incorporate it.
adding " !default" to each variable declaration otherwise you can't easily override them in your own sass variables file
@Strajk
Copy link

Strajk commented Oct 20, 2014

@pouipouidesign 👍

@@ -21,7 +21,7 @@ $mq-sync:
usn-large-medium $usn-large-medium,
usn-large $usn-large,
usn-x-large $usn-x-large
;
!default;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this `!default~ have to be split across usn-large, usn-large-medium, et al? Or does just putting it here apply it to all?

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

3 participants