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
Added a new retina-image mixin for quick retina background-images #157
Conversation
@mixin retina-image($filename, $image-width, $image-height, $extension: png, $retina-filename: null, $asset-pipeline: false) { | ||
background-image: url($filename + "." + $extension); | ||
height: $image-height; | ||
width: $image-width; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we want to set the height
and width
properties?
The current mixin forces the elements height and width, which may be different from the image-dimensions and background-size.
A user might want to do:
background-image: url(...);
background-repeat: no-repeat;
background-size: 60px 50px;
height: 300px;
width: 300px;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. What about an optional background-size
instead?
background-size: $image-width $image-height;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to change the mixin to accept a $background-size
variable rather than $image-width, $image-height
.
This way you can pass anything to the background-size property: contain
cover
or 50px 100px
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also removing the height and width from this mixin.
I tend to use background images in combination with |
We are starting to step into a lot of different use cases I think. You have basic retina-image needs, which would be an icon which makes use of the Thoughts on maybe breaking things up? A |
New PR with changes, see here: #174 |
Pertaining to #136