[ux-icons] fill attribute override

[Nayte91]

Hello,

I continue to play with ux-icons, and there is a weird behavior:

1. create or open a svg,
2. put in its svg tag a fill="#fff" attribute,
3. in your html, put something like <div style="color: red;">{{ ux_icon('mysvg') }}</div>,
4. clean your cache,
5. check out that your icon is red; The fill attribute inside svg tag is discarded by currentColor config,
6. modify your function to add an attribute: <div style="color: red;">{{ ux_icon('mysvg', {fill: '#00f'}) }}</div>,
7. clean your cache,
8. check that your icon is blue; This fill attribute takes precedence.

When you render this, your svg is filled by red, not by your white #fff. Thing is that the ux_icon component put the default fill="currentColor" attribute even if there is already a fill attribute in the file.

Maybe it is not intended, or counter-intuitive, and the function should check if fill exists before writing one? Because is a given project, there is some icons that you want to be color swappable, and that can be a majority that deserves a default fill="currentColor" config for it, but also have few icons that must stay in a given color and not move from it (at least not as easily as just color: whatever). For example brand icons, action buttons, ...

Current workaround is to put the fill attribute inside the svg tag, for example on a path tag or wrap the tags with a g tag. Or to give the color control to CSS 😄

[smnandre]

In the meanwhile, not sure, but maybe one of fill: revert; or fill: revert-layer; could help you

[Nayte91]

Hello and thank your for your suggestion,

I think I don't understand where you image putting fill revert or revert layer? Because whatever you put in svg tag as a fill value, it will be overridden by currentColor from config.

I also edited my post to add another interesting case, points 6, 7 & 8.

Not sur if this behavior is intended, I feel like not but I may miss a usecase; I may change this, and I suppose the attributes are computed in this method, but as I'm not good enough to do it fast, I can do this next week only.

But I wonder here some code should check if attribute already exists, then not override it from conf (but do it from forced attribute, like {{ ux_icon('mysvg', {attribute: 'this is forced'}) }}).

[smnandre]

Oh sorry i have not been explicit enough :)

This is a bug, and i'll try to fix it as soon as possible, sorry for that :)

Until then, i think you may prevent the currentColor attribute to have an effect if you target the desired SVG ?

svg.brand-icon {
    /** one of those lines _may_ work */ 
    fill: revert;  
    fill: revert-layer;
}

Probably with some !important or scope there i guess

[smnandre]

After some thinking.. i'm a bit hesitant there :|

If i defined "width: 24px" in the configuration, i expect my icons to be 24px, whatever the original attribute was.

And it's how the documentation explain it to work (in theory no attribute should be on SVG icons).

So maybe what is missing is some flag to either force override or keep original (what i'd find more usefull in practice)....

... but i'm not sure we should go there for now.

What do you think ?

[Nayte91]

Hello @smnandre,

That's a good point! And I'm partially sold to it. Nevertheless, let me explain some counter arguments to it :smile:

• You took as an argument one of the most critical attribute for a svg; Not sure if any class name, or title, or id, or any other attribute that I can think of would fit as an argument here?
• Sidenote: If I enforce a class: foo in my icons.yml, maybe it can be a good addition to add the class name to the "current array of classes" at the attribute carries multiple values (example: my svg has a class="foo", my config has a default class: "bar", I assume my rendered svg will have class="foo bar")?
• You can always override svg's given height or width with a ux_icon('mySvg', {width: '24px'}) second parameter. You are not TOTALLY stuck by how your svg is coded,
• This behavior is easy to document, understand, and work around (previous point) as it is.
• Your last proposition, with a configuration for "override existing: true/false (default to false :laughing:)" can be a good point! Seems quite easy to add, with very few ramifications, basically it only affects IconRenderer::renderIcon() if I can read code correctly?
• Good luck to find a real world use case where you will fill your favicon, header brand logo action pictograms in every situations and background image with a single 24px width! Point is that I'm not sure that enforcing a size is a good use of the feature, unless there is a currentSize value (ohhh boy I miss this one so much, who do we need to send a RFC to for that?).
• As a follow up to the previous point, note that the default config for a default attribute is a very convenient "choose your color" value; going into conf.yml and enforcing all my icons as pink would be a weird use of this option, and I'm not sure anyone would have code this "default attributes" feature if currentColor would have not existed?
• I'm always annoyed when I see the number of svgs that enforce size (height & width) in their core; That's just the opposite of what a scalable, vectorial shaped icon is and can do. What you need is a canvas where you draw, and the viewBox attribute serves this purpose. Here's an example below of a minimal functional svg that I used when we spoke together:

<svg viewBox="0 0 240 267" xmlns="http://www.w3.org/2000/svg">
    <path
        fill="#FF7324"
        d="M237.969 58.1199C168.331 76.284 106.561 108.419 56.9547 162.626C69.4662 176.402 83.775 180.628 100.754
        183.852C137.221 145.596 181.923 121.297 232.259 106.546C244.627 102.916 238.387 69.4299 237.969 58.1199Z"
    />
    <path
        fill="#FF7324"
        d="M54.5035 60.3423C2.38617 90.9409 -15.4728 158.733 14.614 211.728C30.1498 239.089 55.4372 257.166 83.4303
        264.036C167.897 284.728 238.485 193.114 235.499 112.728C187.509 127.22 145.8 150.942 111.675 185.803C211.933
        203.374 121.567 275.268 57.3366 231.12C11.2647 199.452 5.88849 140.034 32.0682 95.5783C48.1521 68.2625 70.6671
        57.8372 97.822 46.0573C82.9969 47.5999 68.2419 52.2729 54.5035 60.3423Z"
    />
    <path
        fill="#FF7324"
        d="M237.618 1.00075C145.086 30.0503 45.0931 56.3388 39.3281 117.471C38.047 126.555 39.9305 136.468 46.1479
        147.415C48.1397 150.918 50.1729 154.052 52.2571 156.875C99.7789 102.133 160.504 69.6353 232.647
        50.9333C245.123 47.6991 238.039 12.3075 237.618 1.00075Z"
    />
</svg>

Note that is a brand logo from the challonge.com website (I'm not related to it), that I "minimized" to put in my collection. ViewPort is mandatory, height & width aren't. Not how I have to fill with Orange in each path for now :wink: . Point is that you can use it as a favicon, brand logo on your header, background image or action pictogram with this base. That's exactly why SVGs are powerful. I put this argument as the last one because I will not by myself make every svgs creator removes their height & width properties, so it's basically just some wishfull thinking.

I'm pretty happy with my arguments and I managed to switch back my opinion about this configuration point, so I'm please to share them with you. :blush:

[smnandre]

You took as an argument one of the most critical attribute for a svg; Not sure if any class name, or title, or id, or any other attribute that I can think of would fit as an argument here?

I have plenty of those :)

data-iconset="lucide" class="icon-lucide" hidden style="--foo: #45261" stroke="none"

Sidenote: If I enforce a class: foo in my icons.yml, maybe it can be a good addition to add the class name to the "current array of classes" at the attribute carries multiple values (example: my svg has a class="foo", my config has a default class: "bar", I assume my rendered svg will have class="foo bar")?

Let's keep that in mind for the future yes

You can always override svg's given height or width with a ux_icon('mySvg', {width: '24px'}) second parameter. You are not TOTALLY stuck by how your svg is coded,

I worked on this bundle to not be forced to pass arguments :) I want to use configuration and not using hard-coded values dispatched in multiple templates.

This behavior is easy to document, understand, and work around (previous point) as it is.

Yes but it's not the goal of the feature :)

Your last proposition, with a configuration for "override existing: true/false (default to false 😆)" can be a good point! Seems quite easy to add, with very few ramifications, basically it only affects IconRenderer::renderIcon() if I can read code correctly?

Yes that would be the idea... let's keep that in mind, that may rejoin the class idea

Point is that I'm not sure that enforcing a size is a good use of the feature, unless there is a currentSize value (ohhh boy I miss this one so much, who do we need to send a RFC to for that?).

I can change them in CSS / pass arguments in the rare case the 24/24 model will not work The width and height are there to create a ratio for the viewmodel / performances in rendering.

Good luck to find a real world use case where you will fill your favicon, header brand logo action pictograms in every situations and background image with a single 24px width

Oh there are many cases ;) And currentColor has the advantage to be easy to change in CSS.

After..... you also can change the configuration and not inject currentColor to all your icons :)

As a follow up to the previous point, note that the default config for a default attribute is a very convenient "choose your color" value; going into conf.yml and enforcing all my icons as pink would be a weird use of this option, and I'm not sure anyone would have code this "default attributes" feature if currentColor would have not existed?

I think they would ... and if they would not, they would set something else instead :) And again, nothing is required there ;)

'm always annoyed when I see the number of svgs that enforce size (height & width) in their core; That's just the opposite of what a scalable, vectorial shaped icon is and can do. What you need is a canvas where you draw, and the viewBox attribute serves this purpose. Here's an example below of a minimal functional svg that I used when we spoke together:

See my previous answer... it's not to give a size but a ratio :)

Point is that you can use it as a favicon, brand logo on your header, background image or action pictogram with this base.

How do you handle darkmode ? Negative rendering ?

In reality there are many usages / needs out there... We set as default configuration this value, because we think it's a good practice / DX.. and it can be removed :)

So i'm very happy to have this very interesting discussion with you..... ... but sadly i'm not yet convinced :)

.. let's see what other think :)

[smnandre]

Poke #2156