search

material-components / material-components-web

Modular and customizable Material Design UI components for the web
https://material.io/develop/web
MIT License
17.14k stars 2.15k forks source link

Library missing general concept of `mdc-icon` #4633

Open jamesmfriedman opened 5 years ago

jamesmfriedman commented 5 years ago

There are many places where icons are used in mdc-web. The 'idea' of an icon is all over the place, but the implementation is scattered. This makes it hard for consumers to do things at runtime that can globally impact icons.

Here are some real world examples...

<i class="mdc-top-app-bar__action-item"></i>

<button class="mdc-icon-button">
    <i class="mdc-icon-button__icon">favorite_border</i>
    <i class="mdc-icon-button__icon mdc-icon-button__icon--on">favorite</i>
</button>

<i class="mdc-list-item__graphic"></i>
<i class="mdc-list-item__meta"></i>

<i class="mdc-text-field__leading-icon"></i>

<i class="mdc-chip__leading-icon"></i>

These are all icons, but only in concept. There is no global selector I could make to target all of them. Here is my proposed, extremely simple fix.

<i class="mdc-top-app-bar__action-item mdc-icon"></i>

<button class="mdc-icon-button">
    <i class="mdc-icon-button__icon mdc-icon">favorite_border</i>
    <i class="mdc-icon-button__icon mdc-icon-button__icon--on mdc-icon">favorite</i>
</button>

<i class="mdc-list-item__graphic mdc-icon"></i>
<i class="mdc-list-item__meta mdc-icon"></i>

<i class="mdc-text-field__leading-icon mdc-icon"></i>

<i class="mdc-chip__leading-icon mdc-icon"></i>

This would allow you to inherit a lot of behavior across all icons, and also give consumers some more control over them.

Let me be clear, I am NOT advocating for it to have a foundation, or any javascript behavior at all. Just shared styles with a deterministic class name.

For reference, this is RMWC's custom implemenation of the above proposed that adds in icon sizing from the spec as well. https://github.com/jamesmfriedman/rmwc/blob/master/src/icon/icon.css

Thanks!

UPDATE:

The reason this came up, and an actual issue to fix with it. Icons can be either svg or font icons and should respect color either way (it doesn't currently in all cases). Proposed fix.

.mdc-icon {
 color: whatever-color;
 fill: currentColor; /** Would fix color issues with SVGs */
}
moog16 commented 5 years ago

We've come to the consensus that mdc-icon should be a separate component that other components would use (ie. select, text-field, icon-button, etc.). This is a feature request and needs to be vetted before implementing.

jamesmfriedman commented 5 years ago

Great. I would encourage you to check out my reference implantation since I can confirm it works the way you would expect in all places in the api.