Skip to content

Conversation

@maxwondercorn
Copy link
Collaborator

Document changes to icon parameters and required icon component

@RobbieTheWagner
Copy link
Member

@maxwondercorn I'm not sure if we need to make these updates? We're using the same API as before, we're just now passing components instead of classes. If someone wanted to pass just the classes, they could and they could include their own CSS to make those classes add icons.

@maxwondercorn
Copy link
Collaborator Author

@rwwagner90 I see what you're saying about being able still use classes. A case of overthinking. I think we should be less font awesome specific. Something more generic such as "class names to use with custom CSS, web fonts, or ids to use with a custom icon component"

This is related to the incomplete upgrade guide. I call out using the included font awesome icons require implementing the fa-icon-wrapper or similar component, need to add your own icon packs to use other icons, etc.

I'll get back to this

@RobbieTheWagner
Copy link
Member

@maxwondercorn yes, I agree. Perhaps we should move all the FA deps to devDeps and update the docs to be more generic, but also show a new FA example.

@maxwondercorn
Copy link
Collaborator Author

@maxwondercorn yes, I agree. Perhaps we should move all the FA deps to devDeps and update the docs to be more generic, but also show a new FA example.

I agree as that looks like the original implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants