-
Notifications
You must be signed in to change notification settings - Fork 52
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
Custom progress indicator #13
Comments
I don't understand this request, what you have in mind? |
I was thinking of an specific use case I need and thought many more options could be offered: (Ignoring the text under the circle) In my example I need the text to be replaced by an icon, but I can see how different ui cases might required to place different items inside the circle: Images, Icons, Icons + number indicator etc... How do you feel about it? |
Thank you for clarifying. I think this is a great idea and I think it'd be useful. const Icon = <FaGithub/>;
// Passing of an icon component activates the "icon" mode, like in your example.
<Circle icon={ Icon } label="Women" textStyle={... /> This may add many more props to the API which has been slowly getting cluttered. We could address that potential issue by simplifying the entire API surface into a single configuration object that gets passed in as a prop. Just a thought. |
Yup definetly, that looks great. Also worried about the prop clutter. Although not sure how to feel about the object config. Any opinion on this @zzarcon ? |
Yeah I think we should stop adding more props and instead use the render children approach. That will make even possible to make the example from above (text under the circle included) <Circle progress={62,20} >
{(progress) => (
<div>
<SomeCustomIcon />
</div>
)}
</Circle> Not sure if it actually makes sense to pass progress back since is the user the one passing it down. |
It would be cool to be able to show progress with a custom children. For example including an icon + text, only icon, an image etc... maybe use render props for it?
The text was updated successfully, but these errors were encountered: