-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
[Joy UI] Add CircularProgress
component
#33869
Conversation
@mui/joy: parsed: +2.00% , gzip: +1.83% |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sweet, looking awesome, Benny! A question: we mention determinate and indeterminate in the introduction-how would I go about it to controlling the component so I have one or the other? Is that done through a prop?
Also, maybe we should document size
and color
(even though they're on the Playground there already) just for the sake of consistency?! Other components have those sections too.
Yes, through a prop called
I agree. Will do that! |
I dig the idea of a boolean |
Done! Can you suggest some ideas for demos? |
@danilo-leal @hbjORbj Does it make sense to not follow the Material Design look and feel? Here is a simpler version using div + border: Screen.Recording.2565-08-15.at.12.12.48.movhttps://mui.com/joy-ui/react-link/#common-examples VariantLet's add the global variant to CircularProgress. I think it is useful for composition: <Button startIcon={<CircularProgress variant="soft" />}>
ChildrenI think it should be possible to place a child center by default. It is quite a common use case. https://www.pinterest.com/pin/480337116518704364/ Please add a demo for this as well. CSS variablesFor whatever design the CircularProgress looks like, it should contain these generic variables:
Please add a CSS variables demo. |
2d52619
to
0e1a7c8
Compare
Hey, Jun @siriwatknp , I addressed all your feedback except one: I am not sure how to go about implementing |
@hbjORbj Looks like my proposed solution, using a border, would not work for |
@hbjORbj Here is a working POC using SVG https://codesandbox.io/s/poc-circular-progress-3dffpd?file=/index.js |
b7fa715
to
c63fd9a
Compare
Sweet, thanks @hbjORbj, it looks good now! I see we're centralizing them using specific values for each CircularProgress size. Have you verified if that works out if you change the icon's size? |
I verified that it works out if we change the component (circular progress)'s size, but not the icon's size. Just confirmed that it breaks with varying the icon's size. Hmm, I am not sure how to best about this. Any idea @siriwatknp Jun? |
FYI, I am trying alternative way, will share soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking great now - awesome work you two @hbjORbj & @siriwatknp as always!
Brilliant Jun 🤩🤩🤩 Thanks!!! |
docs/data/joy/components/circular-progress/circular-progress.md
Outdated
Show resolved
Hide resolved
Co-authored-by: danilo leal <[email protected]> Signed-off-by: Siriwat K <[email protected]>
Sorry for commenting after merge, I don't have time these days to follow progress on GitHub, but happened to spot this while looking for something else. A few thoughts (I haven't read the discussion here if they've already been covered):
|
No problem!
Agree, that's a better
Yeah, the swatch should follow the variant. Will fix it in a separate PR.
Good suggestion but I think it will be a customization example, not a built-in style. Will try to add it soon.
Yeah, I think we should start the |
There is a growing list of components we should create in MUI Base :) Let's set priorities on the next product meeting. |
Done:
color
,size
,variant: 'indeterminate' | 'determinate'
, etc; To be more discussed with Jun and DaniloUPDATE:
Preview: https://deploy-preview-33869--material-ui.netlify.app/joy-ui/react-circular-progress/