actum / gulp-dev-stack

Actum dev stack based on gulp
MIT License
11 stars 7 forks source link

SVG icons #101

Open janpanschab opened 8 years ago

janpanschab commented 8 years ago

Investigate better workflow for SVG icons. Symbols are OK, but you are not able to apply full power of styles on it. Inline SVGs are always better. Check this repo: https://github.com/iconic/SVGInjector

kettanaito commented 8 years ago

I want to add that today I've discovered that when you create several folders, which should compile into respective sprites, globbing the folders the way we do now does not provide a desired result. Sprites get overwritten, depending on the files in queue.

It may be that we need to parse folder first, divide into a simple Object with files as entries, and then pass each object to gulp.src().

Referenced in #104.

ronaldruzicka commented 7 years ago

I don't know if this is the issue with the styling, but if you get an SVG that has inline fill (or any other) attribute, you are not able to change it with CSS, e.g.:

<symbol id="icon"><path d="..." fill="#f0e"/></symbol>

But we could set up a rule inside gulp-svgmin, that would automatically remove attributes, that we would like to change in CSS, e.g.:

plugins: [{
    removeAttrs: {
        attrs: ['fill']
    },
    cleanupIDs: {
        prefix: `${prefix}-`,
        minify: true
    }
}]

FYI, the removeAttrs has to be before cleanupIDs(already in devstack), otherwise it doesn't work, don't know why :(

ronaldruzicka commented 7 years ago

I have another enhancement to the svg macro that we currently have. The problem now is, that we cannot change the size of the svg, because we don't have a modifier class to the <svg> element itself, only for <use> element:

{% macro svg(name, className, sprite = "icons") %}
    <svg class="icon">
        <use{% if className %} class="{{ className }}"{% endif %} xlink:href="/gfx/svg/sprites/{{ sprite }}.svg#{{ name }}" />
    </svg>
{% endmacro %}

But I propose to have two macro elements, one with <svg> and one with <use>:

{% macro template(class) %}
<svg class="icon {{ class }}">
    {{caller()}}
</svg>
{% endmacro %}

{% macro use(name, class="", sprite = "icons") %}
<use{% if class %} class="{{ class }}"{% endif %} xlink:href="/gfx/svg/sprites/{{ sprite }}.svg#{{ name }}" />
{% endmacro %}

Then we would include the svg elements like this:

{% import "../atoms/_svg.nunj" as svg %}

{% call svg.template('icon-form icon--xl') %}
    {{svg.use('icon-email', sprite='form')}}
{% endcall %}

What do you think? I currently use it on Walmark project.

kettanaito commented 7 years ago

@ronaldruzicka regarding fill issue. Remove attributes will not always fix the issue. The problem here is in the SVG source (we have had a hard time with proper SVG on Conrad, and it is doable). If icon is prepared poorly, when you remove fill and strokes it will break, and you will not be able to style it with CSS at all. So this is more a question for picking right icons and asking designers to export them properly (if you want, we can have a chat to share our experience with @zoltan-kovac on svg icons :) )

ronaldruzicka commented 7 years ago

Ok of course, we can talk about it :)

But anyway, regarding the svg macro, I would still propose this change, so that we can easily apply styles (modifier classes) also on the <svg> and manage e.g. the size of it.

vbulant commented 7 years ago

@ronaldruzicka two macros and their composition in templates seems too complicated to me. We won’t use them separately anyway and calling just {{ svg(…) }} is more straightforward.

What about having just

{% macro svg(id, className = "icon", useClassName = "", sprite = "icons") %}
<svg class="{{ className }}">
    <use class="{{ useClassName }}" xlink:href="/gfx/svg/sprites/{{ sprite }}.svg#{{ id }}" />
</svg>
{% endmacro %}

I appreciate that your approach avoids too many arguments in one macro but I think the added complexity isn’t worth it.

vbulant commented 7 years ago

After we finalize this macro tweak, we’re gonna close the issue as it’s rotting here for several months. Once someone investigates the matter on any new project, we’re gonna reevaluate it again.

ronaldruzicka commented 7 years ago

I just got used to nest the macros from the Sitecore Habitat Demo project, but this solution is also good. We just need to modify both svg and use and this works.

So we can implement this updated macro.

vbulant commented 7 years ago

OK, cool, let’s go for it.

We can benefit from nested macros as soon as we need something more composable (those that are going to be used both together and separately).

ronaldruzicka commented 7 years ago

Do we still have some unsolved issues with the svg elements? Or can we close this issue?