drdk / grunt-dr-svg-sprites

Grunt plugin to create SVG sprites with PNG fallbacks at needed sizes
113 stars 19 forks source link

Customizing CSS output #9

Closed kReEsTaL closed 10 years ago

kReEsTaL commented 10 years ago

Hi!

First, thank you for this plugin, which is a true time-saver! :+1:

However I have a few suggestions to generate a better and simpler CSS output:

1- Add an option to use .no-svg rather than .svg: at the moment, you force browsers that support SVG to download both the PNG and the SVG files, which is a pity. Moreover, if JS is down for some reason, the PNG will load and not the SVG. That's why it'd be useful to have the choice to have the following CSS output:

.no-svg .icon { background-image: url(path/to/my-sprites-small.png); background-size: 20px 20px; }

.svg .icon { background-image: url(path/to/my-sprites.svg); }

rather than this one:

.icon { background-image: url(path/to/my-sprites.png); background-size: 20px 20px; }

.svg .icon { background-image: url(path/to/my-sprites.svg); }

2- Remove the suffix from the CSS class names if there is only one size defined in Gruntfile.js. Indeed, the suffix is useless in this case: we just need the .icon class. Same comment about the PNG file name: my-sprites.png is enough, we don't need my-sprites-small.png (for example).

3- Since we usually need the PNG fallback file to be half the size of the SVG file, it would be easy if the plugin would generate a PNG file that would simply be half the size of the SVG sprite. In this case there would be no need to add an option for the fallback's size.

4- Add an option to add, or not, the width and height of each CSS class. At the moment, they are automatically added, but sometimes you may not need them.

Thanks a lot for considering!

phloe commented 10 years ago
  1. On my todo-list :)
  2. Makes plenty sense.
  3. Hmmm, I don't quite follow this.You need the displayed size of the PNG sprite to be half that of the displayed SVG sprite? Or is it because of CSS-pixels vs native pixels? - This wouldn't be necessary - I think - as SVG are rendered in native pixels.
  4. Can do. But wouldn't that also require some more sprite-layouts? Like diagonal?
kReEsTaL commented 10 years ago

Hi Rasmus,

Thanks for your quick reply!

3- I'm not talking about the displayed size of the PNG sprite, but simply about the size of the generated PNG: I'm suggesting that the PNG sprites' width would be equal to half the width of the SVG sprites, and same thing for their respective heights. But that's because I'm used to have SVG files that are twice the size of the PNG fallback, so indeed it may probably not be needed by anyone but me! ^^

4- Yes, probably, however at the moment I don't need this (even though it'd be great if you could implement it one day, of course). I was just thinking that width and height defined in pixels might be an issue on websites coded in rem or em for example (which is the case on most of the websites I work on).

phloe commented 10 years ago

... And then I actually did something about it.

  1. .no-svg/.svg can now be handled via options.cssPngPrefix and options.cssSvgPrefix respectively.
  2. options.prefix has been optional for a while (I think) - but size tags are now omitted if options.sizes is unused. If you need to define a single size - but don't want a size tag - it's possible to just use an empty string as the name: sizes: {"": 26}
  3. SVG sprites are built directly from the SVG elements (no scaling involved in that step). The PNG fallbacks are made by scaling the SVG sprite and taking a screensht in PhantomJS. In CSS background-size scales the SVG sprite to the same size of the equivalent PNG sprite. This essentially means that you can get what you want (if I'm interpreting your comments correctly) with options like; ... sizes: {medium: 26}, refSize: 52, ... (stating the size is half that of refSize).
  4. Element sizes can be omitted via (the rather verbose) options.cssIncludeElementSizes. Alternatively you could customise stylesheet output by writing your own template via options.template. rem is supported through options.cssUnit and options.cssBaseFontSize.