icons-pack / react-simple-icons

📦 This package provides the Simple Icons packaged as a set of React components.
MIT License
310 stars 19 forks source link

Generic icon component with type property. #110

Closed BlackBearFTW closed 3 years ago

BlackBearFTW commented 3 years ago

I would like to have a dynamic icon component that I can use for my card component, this way I can just pass an argument of type Icon to my card component and then use that parameter to load that icon.

For example: <Icon type={platformIcon} size={25} />

platformIcon would be a variable that holds an icon, like ReactJs, which then would become <Icon type="ReactJS" size="25" />

I currently see no way to have an icon component be dynamic, could this be added?

Edit: I know I could do this myself, but that would require me to import ALL the icons first

wootsbot commented 3 years ago

@BlackBearFTW I think this can help, I could send the feature to a future version, I will take it into account and work on it.

sachinraja commented 3 years ago

I don't get why this is necessary, though it would probably reduce the size of this library. You can conditionally render the icons.

wootsbot commented 3 years ago

I don't get why this is necessary, though it would probably reduce the size of this library. You conditionally render the icons.

I see it as a component that is used optionally, but a I have not seen its scope.

sachinraja commented 3 years ago

Oh I think I misunderstood the request. This would actually be a much better implementation of the library because it would only export one component, no generation necessary and you could dynamically plug in icons while keeping other attributes (like width). Currently I've been doing this with React.cloneNode, but this would be better. I can take this one @wootsbot if you assign it to me. Note that this would be a breaking change though.

Here's an example of what it would look like:

import githubIcon from 'simple-icons/icons/github'
import SIIcon from 'react-simple-icons'

const Component = () => <SIIcon icon={githubIcon} />
wootsbot commented 3 years ago

This would be nice to discuss as it radically changes the focus of the package.

sachinraja commented 3 years ago

This doesn't change the focus on the package, it only changes how it is used. What would you like to discuss though?

wootsbot commented 3 years ago

This doesn't change the focus on the package, it only changes how it is used. What would you like to discuss though?

I would like to know what you think if we keep the components and give the opportunity to have this new component. component import SIIcon from 'react-simple-icons' ?

we got the tree shake

sachinraja commented 3 years ago

That would be OK but I think a breaking change and a major version bump would be better. This package has a lot of files, I would like to avoid installing all of those if I only want one component.

emremutlu08 commented 3 years ago

If we have the same problem, I solve it like this

import * as Icons from '@icons-pack/react-simple-icons';

...

const TagName = Icons['ReactJs'];
...
<TagName className={classes.iconSize} />
wootsbot commented 3 years ago

@sachinraja perfect, we can opt for the option of having a single component