Icon Button svg fill comment

[Deufel]

Should the icon button SVG fill set to current color?

I am using Lucid Icons and im not sure that the fill = current collor is always the desired look, for an example here is the mailbox icon

But if i comment that out, I get the icon as it is intended to look (more or less)

button-variants.css

    /* Icon */
    &:where(:has(svg), &.icon-only) {
        gap: 1ex;

        & svg {
            /* fill: currentColor; */
        }
    }

I can live with this if that is just the nature of the some icons vs lucid but just wanted to check that this is intended. Thanks

[felix-bohlin]

I think this is one of those cases where (by design), you'd have to modify the code to fit your needs. Icons are quirky! 😄 The way to circumvent that on a library level would be to:

1. provide all the icons myself (but that would result in lock-in)
2. add fixes for every type of icon pack we find along the way
3. leave it open for people to modify it to their needs in their respective projects

I'm obviously in camp #3, but if there's something inherent to ex solid icons vs outlined icons specifically, I would like to do some fix for that.

Either way, I'll look into Lucid icons and how they do it, thanks for taking the time @Deufel ❤️

Do you have any thoughts on this?

[Deufel]

Thanks for the quick response! Yeah I would say that 1 and 2 are not the best way to go nor maintainable. Im a bit new at all this stuff so I am usually unsure if the issue I am having is by design or if it is my poor implementation. I think that this boils down to stroke based icons vs fill based icons.

Maybe something like the following would allow the library to be more general purpose with out over committing to support every icon under the sun.

/* Icon */
    &:where(:has(svg), &.icon-only) {
        gap: 1ex;

        & svg:not([fill="none"]) {
            fill: currentColor;
        }

        & svg[stroke] {
            stroke: currentColor;
        }
    }

With Change:

With Out (original):

the log-in icon is a bit distorted because the fill is changed from none to currentColor

[felix-bohlin]

Great stuff! I could implement your fix, but I'd be happy to give you contributor credits to this repo so if you want to open a PR you're more than welcome to do it @Deufel :)

[Deufel]

Glad to help! i just submitted PR. I think I have the correct stuff filled out; but double check as it's the first public PR I'v submitted :/

[felix-bohlin]

Merged! Thanks for the help! ❤️