selaux / node-sprite-generator

Generates image sprites and their spritesheets (css, stylus, sass or less) from sets of images. Supports retina sprites. Provides express middleware and grunt task.
MIT License
192 stars 39 forks source link

Using additional mixins in LESS #34

Closed alberto-bottarini closed 9 years ago

alberto-bottarini commented 9 years ago

When i generate my LESS file I get some - i hope so - useful variables @generali-sprites-logo-x: "0"; @generali-sprites-logo-y: "0"; @generali-sprites-logo-width: "113px"; @generali-sprites-logo-height: "91px";

Why these are in string format? In this way I cannot use them for calculations. If I would like to calculate, for example, margin based on image width or height, I have to use image-size native LESS function despite having already the size inside my sprite LESS file.

Why not convert them in number and use interpolation in sprite mixin?

daltones commented 9 years ago

Good point. I made another template myself.

Actually that LESS template is not working at all. Those syntaxes to get array itens are wrong.

selaux commented 9 years ago

@alberto-bottarini: True, that was a bit short sighted by me. It basically stems from me, wanting to keep all the css-preprocessors as similar as possible with the least possible hassle. I will have to look into how all the css-prerocessors handle units before fixing this.

@daltones: I know it's annoying if you want to use a software and it's not working, but if you knew that the less template is broken and needed to write your own, why don't you contribute parts of your template back or open an issue about it? Just mentioning it in a sentence in another issue micht not be the best way to bring that to my attention.

selaux commented 9 years ago

@alberto-bottarini, @daltones : Can you try out the fix-stylesheet-variables-being-strings branch and check wether the the variables (and array indexing) work as expected in this branch? If so, I will merge and release 0.8.1 asap. :beers:

daltones commented 9 years ago

@selaux, you're right. I'm sorry if my reply seems like rude. Actually my intention was to contribute later, I was working at that time.

I will write a fix today.

alberto-bottarini commented 9 years ago

I did some test with LESS and now it seems more usable. I can now use all the auto-generated variables also in other contexts.

I'll wait for 0.8.1 :+1:

selaux commented 9 years ago

@alberto-bottarini: Cool, I will merge this then

@daltones: No offense taken :wink:, it's just that I only test all the stylesheet generators by using a online converter (i use http://less2css.org/ for less) and there it worked. To get to know that one of them does not work at all was kind of a surprise (or shock) to me. Is using extract(@sprite, 1); with a 1-based index as in the PR (https://github.com/selaux/node-sprite-generator/pull/35) the way to go?

selaux commented 9 years ago

Closed with https://github.com/selaux/node-sprite-generator/pull/35.