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.

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

fix(material/progress-spinner): progress-spinners with diameter less than or equal to 10 #20193

Closed

Conversation

AlexLangdon
Copy link
Contributor

@AlexLangdon AlexLangdon commented Aug 4, 2020

As @paulferaud pointed out in #19469, using BASE_STROKE_WIDTH when calculating the spinner circle radius is causing issues, such as the progress spinner becoming invisible if the diameter is less than or equal to 10.

One solution is to change the radius calculation for diameters less than 11 and subtract a fraction of the diameter instead of BASE_STROKE_WIDTH.

This also maintains the current behaviour for any diameters above 10, so shouldn’t cause breaking screenshot tests.

Fixes #19469

…than or equal to 10 do not show

Fixes a bug in the Angular Material ‘progress-spinner’ component where progress-spinners
with a diameter less than or equal to 10 do not show. This is because the spinner radius
calculation is based on subtracting a constant stroke width of 10 from the diameter,
resulting in a radius of 0 or less for small diameters. For these cases a fraction of the
diameter can be subtracted instead of the constant stroke width.

Fixes angular#19469
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Aug 4, 2020
return (this.diameter - BASE_STROKE_WIDTH) / 2;
// In the general case adjust the diameter by the base stroke width.
// For small diameters adjust by a diameter percentage so that the radius is greater than 0.
return Math.max(this.diameter - BASE_STROKE_WIDTH, this.diameter * 0.09) / 2;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The smallest currently supported diameter is 11, which gives a circle radius of ½ or approximately (0.09*11)/2. So for diameters <= 10 I maintain the same circle radius ratio by using (0.09 * diameter)/2, instead of subtracting a BASE_STROKE_WIDTH of 10.

@andrewseguin
Copy link
Contributor

@AlexLangdon Sorry this review took so long - this change looks okay to me. Would you like to rebase and we can see about getting it landed?

@andrewseguin andrewseguin added target: patch This PR is targeted for the next patch release needs: clarification The issue does not contain enough information for the team to determine if it is a real bug labels Mar 24, 2022
@andrewseguin
Copy link
Contributor

Closing due to inactivity - sorry that we didn't get a chance to finish this when sent. Feel free to reopen a rebased PR if you'd still like to land this

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators May 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs: clarification The issue does not contain enough information for the team to determine if it is a real bug target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(MatProgressSpinner): Small Spinners with diameter <= 10 don’t show
3 participants