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

add a new mode that combines the view and the symbol mode. #81

Open
wants to merge 4 commits into
base: master
from

Conversation

4 participants
@flobacher

flobacher commented May 7, 2015

through the use of 'use'-tags, filesize increases only very little

Florian Bacher
add a new mode that combines the view and the symbol mode. through th…
…e use of 'use'-tags, filesize increases only very little
@coveralls

This comment has been minimized.

coveralls commented May 7, 2015

Coverage Status

Coverage remained the same at 80.34% when pulling 8d7e598 on flobacher:master into e3764cb on jkphl:master.

@flobacher

This comment has been minimized.

flobacher commented May 7, 2015

maybe the name of the new mode should be changed to combined, or something else, but the idea is to be able to reference symbols via use tags in the html and to be able to use the same spritesheet via the help of the generated positions (view mode) in css for e.g. in an ::after selector.
i am really looking forward to hear your feedback.

@coveralls

This comment has been minimized.

coveralls commented May 7, 2015

Coverage Status

Coverage remained the same at 80.34% when pulling 25f4a45 on flobacher:master into e3764cb on jkphl:master.

@jkphl

This comment has been minimized.

Owner

jkphl commented May 7, 2015

Hi @flobacher,

thanks for these suggestions. Due to my current workload (and some travels the next days) I'll probably not be able to review this before mid of next week. Thanks for your patience!

Cheers, Joschi

@jkphl jkphl self-assigned this May 7, 2015

@jkphl jkphl added the enhancement label May 7, 2015

@coveralls

This comment has been minimized.

coveralls commented May 7, 2015

Coverage Status

Coverage remained the same at 80.34% when pulling 8239fa6 on flobacher:master into e3764cb on jkphl:master.

@flobacher

This comment has been minimized.

flobacher commented May 7, 2015

Hi Joschi,

no need to hurry. just think such a combined spritesheet is realy handy.
u can reference symbols via their id in use tags.
and in css you can use the background-positions calculated from the viewmode.
and since it is all combined in one file, it will only be downloaded once.
this way it is also possible to generate a png fallback from the spritesheet,
which was not possible via the symbol mode alone.

hope you like it!

Cheers, Flo

Zitat von Joschi Kuphal notifications@github.com:

Hi @flobacher,

thanks for these suggestions. Due to my current workload (and some
travels the next days) I'll probably not be able to review this
before mid of next week. Thanks for your patience!

Cheers, Joschi


Reply to this email directly or view it on GitHub:
#81 (comment)

@coveralls

This comment has been minimized.

coveralls commented May 7, 2015

Coverage Status

Coverage remained the same at 80.34% when pulling a94985a on flobacher:master into e3764cb on jkphl:master.

@flobacher

This comment has been minimized.

flobacher commented May 7, 2015

I just renamed the mode to universal, because its universal nature (symbols, views and positions all in one spritesheet) reminded me of the universal module definition pattern for js modules. I hope you think as well, that this is a better name for this mode.
cheers, flo

@jkphl

This comment has been minimized.

Owner

jkphl commented Jun 9, 2015

Hi @flobacher,

first of all sorry for not being more responsive! It really took me until now to find the time to thoroughly review your suggestions.

Although there are some parts that I would solve a little different, I really like your approach. In fact I wonder if we couldn't even take this a little further. Let me explain:

  1. The css mode is equivalent to your <symbol>s in combination with the <use> elements..
  2. The view mode is an extension to the css mode adding the <view> elements.
  3. The defs, symbol and stack modes are mutually exclusive, but could probably all be extended by the <use> plus <view> features (not sure about the stack mode, but I'll try).

So why not generally configure a sprite by combining "flags" like this (hope you understand the EBNF like try of an explanation):

Sprite                    = UniversalSprite | BackgroundSprite ;
UniversalSprite           = LibrarySprite , { LibraryBackgroundSprite } ;
LibrarySprite             = DefsSprite | SymbolSprite | StackSprite ;
BackgroundSprite          = CssSprite, { ViewSprite } ;
LibraryBackgroundSprite   = UseLibrarySprite, { ViewSprite } ;

In words:

  1. First you have to decide wether you want one of the "library based" sprites (symbol, defs or stack) or just a -"simple" CSS sprite for use as background image (css / view).
  2. In case of the latter you have to decide whether you want the <view> elements or not (IMHO the only reason against them could be to save some bytes ...).
  3. If you go for the library based sprite, you may additionally enable the background capabilities (including or excluding the <view> elements).

This could elmininate the need for the different modes altogether and make the configuration more like a flag based process. Maybe that's a lot simpler than the current way of configuring. What do you think? Looking forward to your thoughts!

Cheers,
Joschi

@jkphl

This comment has been minimized.

jkphl commented on 25f4a45 Jun 9, 2015

Just to understand this: Where would these "namespaced styles" be defined? Outside the SVG file itself (i.e. contained in the embedding HTML document or an external CSS)? (Wouldn't probably work cross-browser, but anyway ...). Thanks!

This comment has been minimized.

Owner

flobacher replied Jun 12, 2015

you are right, if they would be defined in an external stylesheet styling svgs that are embedded in html via is only possible in firefox.
But it is possible to have style blocks inside the svg. If you have lots of svgs exported from illustrator, they will all have the same css-class-names so if you combine them in a single spritesheet, they will influence each other which is not desirable at all... therefor I preprocess the individual files (with the awesome extract-svg-styles node_module) and prefix their css-classes inside their inline style definitions with a classname created from their filename and put this class on the root svg element in the single svg file. upon spritesheet generation, one could just copy this class or take the name like in the approach above, since it is the same anyway, but i just did find it easier, than getting the class attribute of the shape. i hope it is more or less understandable..

This comment has been minimized.

jkphl replied Jun 17, 2015

Yep, totally understandable. Sounds like this pretty much intersects with my idea of adding support for "class namespacing". That would even eliminate the need to use additional tools (however, I'll have a look at extract-svg-styles — thanks for the recommendation!).

This comment has been minimized.

Owner

flobacher replied Jun 19, 2015

yes, I think its the same idea! the great stuff about extract-svg-styles is, that it gives you the option, to output the style blocks into external stylesheets and to remove them from the svg, which is the much better solution, since you will have the style-definition only once per symbol and not (if left in the style block inside the svg) per symbol instance. I use this together with svg-injector-2, because in my point of view the only feasible option to style svgs with css at the moment is to inline them into the html.

@flobacher

This comment has been minimized.

flobacher commented Jun 12, 2015

Hi @jkphl,
thanks for taking the time to get back on me.. I am sure, you can do a much better way of integrating that new mode into your code than me! And yes, I like your flag based approach of combining the diffrent modes into a single file very much! Just one comment.. I think at Point 3 of your explanation I would just add for completeness, to add those bg capabilities via tags inside the spritesheet, so that the shapes don't have to be duplicated.. and the , just like the just adds a couple of bytes, so that should not hurt anyone.. I would really love to see that in svg-sprite =)

@jkphl

This comment has been minimized.

Owner

jkphl commented Jun 24, 2015

Hey @flobacher,
just to let you know: I'm planning a major overhaul of svg-sprite somewhere around August, mostly focusing on usability and ease of configuration. I'll try to combine all this then.
Cheers,
Joschi

@flobacher

This comment has been minimized.

flobacher commented Jun 24, 2015

Great!!!

Let me know, if you need any help!

Cheers,
Flo

Zitat von Joschi Kuphal notifications@github.com:

Hey @flobacher,
just to let you know: I'm planning a major overhaul of svg-sprite
somewhere around August, mostly focusing on usability and ease of
configuration. I'll try to combine all this then.
Cheers,
Joschi


Reply to this email directly or view it on GitHub:
#81 (comment)

@jkphl jkphl referenced this pull request May 24, 2016

Closed

Combine symbols and views #55

@jkphl jkphl added this to the 2.0.0 Release milestone Nov 29, 2016

@stikoo

This comment has been minimized.

stikoo commented Feb 10, 2017

This is exactly the functionality I'm after. I'm currently moving to a symbol workflow but would like to retain the ability to use the CSS method as well. Will just have to have 2 output modes for now :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment