Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support custom icons without propType warnings, change name propType to string? #955

Closed
nolandg opened this issue Nov 29, 2016 · 3 comments
Labels

Comments

@nolandg
Copy link

nolandg commented Nov 29, 2016

With the whitelisting of the name property of <Icon>, we can no longer use custom icons without ugly warnings.

Previously, we could happily build a custom icon font using such awesome services as Fontello. Performance is important for a lot of reasons and lugging around the entire Font Awesome collection is not practical in a lot of cases (eg. complete rendering on first request with embedded octet stream icon font). Hence I point and click and build a custom icon pack and use that. It is tedious to try to align the names with the existing Semantic whitelist.

@levithomason says in #931 that we cannot add to the list of icons. Wondering what the rationale is for that? Can we change the <Icon name> propType to string to allow for custom icons again? Is there an awesome reason why we have to be so strict with the type?

@levithomason
Copy link
Member

levithomason commented Nov 29, 2016

@levithomason says in #931 that we cannot add to the list of icons. Wondering what the rationale is for that?

The names in the list of Icon names are SUI icon names. Since we officially support only SUI CSS we then only validate SUI Icon names. I think this makes sense.

Is there an awesome reason why we have to be so strict with the type?

Indeed, a great one IMO. One of the reasons that we have specific props for various things, like name, is so that we can validate them and offer really useful help as in #944.

Also, with a fixed list of possible values we're able to generate a better "component explorer" in the future. You can see the prototype here #427.

What to do then?

It is important to note here that the end result of all props is a string of classNames. Any className can be added to any component to work with all CSS. These render identical HTML:

<Icon color='blue' name='user' />
<Icon className='blue user' />

// => <i class="blue user icon"></i>

I would suggest using className='my-icon' for adding your own custom icon classes to the Icon element.


In the longer term, I could see somehow supporting different icon sets. Inspired by https://github.com/gorangajic/react-icons. This would fit in line with theming support and some config I've been considering. Some day perhaps...

@nolandg
Copy link
Author

nolandg commented Nov 29, 2016

you are shockingly fast with good answers.

@nolandg
Copy link
Author

nolandg commented Nov 29, 2016

For anyone else doing this, @levithomason's answer also works with the shorthand version of icons with a bit more markup:

<Icon className="my-custom-icon-name" />
<Button icon={{ className: 'my-custom-icon-name' }} />

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants