feature: Ability to customize icons; avoid large icon dependency #6
Conversation
|
@audiolion @mmgolden |
baleeds
changed the title
|
The pull request looks good overall, I like the new icon abstraction. Will need to pull it and test locally. The code formatting is now a bit inconsistent throughout the project, but I was planning to add Prettier to the project anyway so that shouldn't matter much. Will try to merge in the next couple of days. |
|
RE: Tests - ignore them, they don't do anything at the moment. I'll figure something out later. |
|
Sounds good. Yeah I noticed the formatting, definitely muddles the review process, sorry about that. I think prettier would be a very nice addition. |
Perhaps a separate PR is created and merged first that applies prettier and adds a prettier config so that when this PR is rebased it will remove all formatting changes? |
|
Just merged #7, can you please rebase your changes on top of that? |
Bump react to ensure support for context Add styleguidist to ensure consistent version when building
|
It worked! Much easier to see what I changed now. It was a gnarly merge resolution though, so there is possibly some broken things. I tested everything I could think of manually in the styleguide. |
| export interface ConfigValue { | ||
| icons: typeof icons; | ||
| Icon: typeof Icon; | ||
| } | ||
|
|
||
| export const ConfigContext = React.createContext<ConfigValue>({ Icon, icons }); |
There was a problem hiding this comment.
We should probably break this context down into smaller parts once we add hooks.
…_yarn/reduxjs/toolkit-1.9.2 Bump @reduxjs/toolkit from 1.9.1 to 1.9.2
This PR will:
Points of contention:
IconUtilsruns every now and then, but its not the least efficient thing ever#5