v5-beta FR customization friendly tool bar icons (zoom icon)

[amkdev]

currently the zoom icon changes it's state by hiding a svg segment via class

this makes it very hard to use an own svg - or better: two svgs - one for each state (zoom in / out).

If buttons have more than one state, it should be possible to set separate icons for each state.

BTW: adding svg's to javascript code is really uggly. this should all be done via css and external files or a font.

[dimsemenov]

v5 does not prevent you from using an external svg or a font, simply set a default value to an empty string zoomSVG:'' or whatever markup you wish.

pswp--zoomed-in class is added to the root element of PhotoSwipe when current image is zoomed.

BTW: adding svg's to javascript code is really uggly. this should all be done via css and external files or a font.

I find it way more ugly to include font or write svg in css. If you look at how exactly default SVG are added - it's a very tiny opportunity cost.

[amkdev]

OK ... thought twice about. Right, I could also define an additional html element with it's own class as "zoomSVG" ... but why you name the option SVG than?

Of course I know that there is a zoomed-in class, but using this feels like hacking to get a solution. Would you write this into the v5 documentation? ;) This is not customization friendly.

Fonts are hard to edit but a nice way to load a bunch of vector stuff at once. The XML data in CSS is of course just as disadvantageous as within JavaScript.

Separate design from code was never a bad idea ... and using xml to describe vector data does not mean that svg should be understood as code imho. ;)

[dimsemenov]

why you name the option SVG than?

Because its primary usage is SVG or an empty string (if the developer wants to style the icon just via CSS).

And most modern icon libraries such as FontAwesome or Octicons have an API to get SVG string, so you can just import the icons that you want.

I'm not sure what are you suggesting. If you want PhotoSwipe to use an external SVG, that's not something that I would do. External SVG prevents dynamically adjusting color or shadow, and icons would appear with delay if not preloaded.

[amkdev]

That's all ok - I realize I was thinking a little incomplete yesterday. ;)

But what I recommend is to add a state class to buttons tag instead of working with the container class.

<button class="pswp__button pswp__button--zoom pswp__button-off" type="button" title="Zoom (z)">
<svg aria-hidden="true" class="pswp__icn" viewBox="0 0 32 32" width="32" height="32">
    <use class="pswp__icn-shadow" xlink:href="#pswp__icn-zoom"></use>
    <path d="M17.426 19.926a6 6 0 1 1 1.5-1.5L23 22.5 21.5 24l-4.074-4.074z" id="pswp__icn-zoom"></path>
    <path fill="currentColor" class="pswp__zoom-icn-bar-h" d="M11 16v-2h6v2z"></path>
    <path fill="currentColor" class="pswp__zoom-icn-bar-v" d="M13 12h2v6h-2z"></path>
</svg>
</button>

.pswp__button--zoom.pswp__button-off .pswp__zoom-icn-bar-v {
 display:none;
}