icons-pack / react-simple-icons

📦 This package provides the Simple Icons packaged as a set of React components.
MIT License
320 stars 19 forks source link

[RFC] Adding a default classname to all icons #188

Closed mwskwong closed 1 year ago

mwskwong commented 1 year ago

There are use cases where I wish to apply global styles to all icons. e.g. when integrating it with a UI component library, such that I can make use of the design tokens provided by the library to style the icons. For this, it will be much simpler if all the icons by default have a class name applied.

I believe it can be easily achieved at https://github.com/icons-pack/react-simple-icons/blob/main/src/base.tsx#L20.

e.g.

<svg
  // rest of the props...
  className={`react-simple-icons ${className}`}
  {...others}
>
LitoMore commented 1 year ago

I think you should apply your styles to its parent elements. The className might not be helpful for everyone.

mwskwong commented 1 year ago

It is indeed not helpful for everyone, but I can see why some other icon libraries are doing it. Some examples:

Simply applying styles to the parent elements will not work because

  1. The "parent element" has to be global for it to cover all icons, e.g. html or body
  2. I can do something like this:
    body {selector} {
    color: var(--Icon-color);
    margin: var(--Icon-margin);
    fontSize: var(--Icon-fontSize, 20px);
    width: 1em;
    height: 1em;
    }

    Now the key question is, what selector should I use? Simple svg will be too broad, while a class name makes perfect sense here.

LitoMore commented 1 year ago

I mean:

<div className="your-list">
    <SiReact className="si si-react" />
    <SiNpm className="si si-npm" />
</div>

Selectors can be written in many ways:

.your-list {}
.your-list svg {}
.your-list .si {}
.si {}
.si-react {}

It allows you to pass in the className to an icon. This is easy enough.

mwskwong commented 1 year ago

I mean:

<div className="your-list">
  <SiReact className="si si-react" />
  <SiNpm className="si si-npm" />
</div>

Selectors can be written in many ways:

.your-list {}
.your-list svg {}
.your-list .si {}
.si {}
.si-react {}

It allows you to pass in the className to an icon. This is easy enough.

Yep. Except we have to add si to every single icon we use manually. And that becomes a problem. For (1), it's getting repetitive. (2), the class name could have been a constant, while it is being used as a variable, and we have to remind ourselves to add this class name whenever we use simple icons.