[Icon] Allow customizing the 'material-icons' base class name

[rart]

• I have followed (at least) the PR section of the contributing guide.

The Icon component always adds the material-icons class to the element; however, per the docs, you may use other icon fonts with this component so always having this base class doesn't make sense in all cases. Best case scenario, the class is ignored but if someone is mixing two icon fonts (material-icons + some other), this may cause issues.

Also, now that the different Material Icon variants can be used via icon font, it would be convenient to be able to swap out material-icons for material-icons-round, for example.

[mui-pr-bot]

Details of bundle changes

Generated by 🚫 dangerJS against 9ff4d23

[eps1lon]

I'll repeat here what I said numerous times. The Icon component is for usage with Material-UI which is why it is published form @material-ui/core.

If you need it for different purposes I'd recommend copying it and adjusting it for your own purposes.

[rart]

@eps1lon perhaps the docs referencing that it can be used with other fonts too is a source of confusion then?

Event if it is to be used with only material ui, material icons have the various base classes for the different icon variants (rounded, sharp, two-tone) so I felt it makes sense anyway.

From your comment, I understand I should just go ahead and close this PR as it won't be accepted?

[oliviertassinari]

The Icon component is for usage with Material-UI which is why it is published form @material-ui/core.

@eps1lon I don't follow the reasoning. Soon or later, developers need to add icons that aren't available in http://material.io/. We need a story to extend it. The value for using the component range between the integration with the theme, the extra features we apply, the right set of defaults. I think that the Icon should be able to host any font icons so is the SvgIcon to host any SVG icons. Material-UI !== Material Design. I think that Vuetify does it correctly: https://vuetifyjs.com/en/features/icons/.

updated to keep emphasize on material.io/icons with the two-tone

[mbrookes]

What a about taking the base class name from the className prop:

If the className contains two values, the first values overrides the base classname. So no API change, and more concise.

-<Icon baseClassName="fas" className="fa-plus-circle" />
+<Icon className="fas fa-plus-circle" />

[oliviertassinari]

@mbrookes The current proposal allows to leverage theme.components.MuiIcon.defaultProps.baseClassName="material-icons-two-tone". I think that it's important to avoid forcing the creation of a wrapper Icon component. In practice, I expect developers to almost never do:

<Icon baseClassName="fas" className="fa-plus-circle" />

At least, I wouldn't. I would configure the base class name once, abstract it, somehow.

<Icon className="fa-plus-circle" />

We might want to document this.

[mbrookes]

Good point. That seems worth adding to the docs.

[mbrookes]

👍

[eps1lon]

I don't follow the reasoning. Soon or later, developers need to add icons that aren't available in http://material.io.

Ok?

We need a story to extend it.

No? How did you make the logical jump?

[eps1lon]

And please don't dismiss my reviews. Unless you don't want to me review PRs

[oliviertassinari]

How did you make the logical jump?

@eps1lon Do you mean the logical jump between 1. developers need more icons than the one available in the filled theme of Material Design and 2. we need a story to extend it? 1. is the problem 2. is the solution.
So I assume the underlying question is: Why should we support anything else than the filled theme of Material Design?

I believe we already support it. Developers can apply the material-material-icons-two-tone class name manually for a different them of Material Design, or fas for the FA Solid theme. I think that the main value proposition of this pull request by @rart is a new API that makes it simpler to configure this globally with the theme and improved documentation for it.

Still why?

1. Integration with the theme, access to the system props, similar to why: https://chakra-ui.com/docs/components/icon#using-a-third-party-icon-library
2. Solve a few issues: aria-hidden for accessibility, notranslate, for Google trad, overflow hidden to reduce flash when loading the font.
3. Consistency, I think that it makes it easier to use two different sets of icons on the same page with a consistent DX and UX. You might need one icon from MD, another from your custom set of icons.