Skip to content
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.

Sign up for GitHub

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

Jump to bottom

CircularProgress sizing issue in a Dialog #11837

Closed
2 tasks done
mim-Armand opened this issue Jun 13, 2018 · 8 comments
Closed
2 tasks done

CircularProgress sizing issue in a Dialog #11837

mim-Armand opened this issue Jun 13, 2018 · 8 comments
Labels
bug 🐛 Something doesn't work component: dialog This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@mim-Armand
Copy link
Contributor

mim-Armand commented Jun 13, 2018 •
edited
Loading

  • This is a v1.x issue (v0.x is no longer maintained).
  • I have searched the issues of this repository and believe that this is not a duplicate.

Expected Behavior

The size prop should set the size of the element.

And it does but it doesn't take effect because the class .MuiDialogActions-action-1750 in a dialog sets the min-width to 64px, so anything less than 64px will have no effect.

Current Behavior

The container element can not reduce the size of the SVG child the animation however is selected applied to the parent element which is "rendered" smaller. resulting to a wobbly animation effect

Steps to Reproduce (for bugs)

Here is an example in the codesandbox for MU

  1. have an open dialog
  2. Put a <CircularProgress size={18}/> in the <DialogActions/> elemnt
  3. See the results.

Context

N/A

Your Environment

Tech Version
Material-UI v1.2.0
React v16.3.2
browser Chrome
etc N/A
@oliviertassinari
Copy link
Member

Please provide a full reproduction test case. This would help a lot 👷 .
A live example would be perfect. This codesandbox.io template may be a good starting point. Thank you!

@oliviertassinari oliviertassinari added the status: waiting for author Issue with insufficient information label Jun 13, 2018
@mim-Armand
Copy link
Contributor Author

Here you go
Thank you for the link.

@mim-Armand
Copy link
Contributor Author

@oliviertassinari I could not re-open the issue myself, could you please re-open it

@oliviertassinari oliviertassinari added good first issue Great for first contributions. Enable to learn the contribution process. component: dialog This is the name of the generic UI component, not the React module! and removed status: waiting for author Issue with insufficient information labels Jun 13, 2018
@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 13, 2018 •
edited
Loading

I have been browsing the specification. I can no longer find any reference to the min-width value for the Dialog. I think that we can remove the property in
https://github.com/mui-org/material-ui/blob/1a2ebf54bbb468ee3d1e31bb02e299f9309a89c4/packages/material-ui/src/DialogActions/DialogActions.js#L18
and reduce the default button min-width to 64:
https://github.com/mui-org/material-ui/blob/1a2ebf54bbb468ee3d1e31bb02e299f9309a89c4/packages/material-ui/src/Button/Button.js#L16
This should solve your problem.

@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Jun 13, 2018
@mim-Armand
Copy link
Contributor Author

mim-Armand commented Jun 13, 2018 •
edited
Loading

@oliviertassinari I can make these changes and make a PR if that's OK? that'd be a good start for me to contributing to this library as well..

Edit: On it! :)

@kirillkorneevcb

This comment has been minimized.

Sign in to view
@oliviertassinari
Copy link
Member

@mim-Armand Let's us know if you are doing some progress :).

@mim-Armand
Copy link
Contributor Author

Sorry for the delay! I've made the changes and all but got stuck on making the PR as wanted to make sure it's all perfect and couldn't figure out how to run the tests ( there were missing packages and multiple package.json files ) But I think I'm gonna make the PR and get your feedback if there were any issues, like it may go faster ;)
So I'll rebase to master again and make a PR shortly

mim-Armand pushed a commit to mim-Armand/material-ui that referenced this issue Jul 7, 2018
@davinci-team
attemting to fix Issue mui#11837 ( where the CircularProgress size we…
52674ac 
…re incorrect when rendered in a Dialog )
@mim-Armand mim-Armand mentioned this issue Jul 7, 2018
Merged
oliviertassinari pushed a commit to mim-Armand/material-ui that referenced this issue Jul 9, 2018
@davinci-team @oliviertassinari
attemting to fix Issue mui#11837 ( where the CircularProgress size we…
4bf3843 
…re incorrect when rendered in a Dialog )
oliviertassinari pushed a commit that referenced this issue Jul 9, 2018
@mim-Armand @oliviertassinari
[Dialog] Fix action width issue (#12081)
d205e2d 
* attemting to fix Issue #11837 ( where the CircularProgress size were incorrect when rendered in a Dialog )

* changint the max-with to the calculated amount instead of using theme.spacing.unit * 8

* remove dialog action min-width
acroyear pushed a commit to acroyear/material-ui that referenced this issue Jul 14, 2018
@mim-Armand
[Dialog] Fix action width issue (mui#12081)
926680b 
* attemting to fix Issue mui#11837 ( where the CircularProgress size were incorrect when rendered in a Dialog )

* changint the max-with to the calculated amount instead of using theme.spacing.unit * 8

* remove dialog action min-width
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug 🐛 Something doesn't work component: dialog This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mim-Armand @oliviertassinari @kirillkorneevcb