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-experimental/mdc-progress-spinner): fix noop animation #21359

Merged
merged 1 commit into from
Dec 17, 2020

Conversation

andrewseguin
Copy link
Contributor

Animations were still being shown because the selector :not(._mat-animation-noopable) by itself matched plenty of parent elements. This specifies the right parent element that should be checked.

Also, this forces the spinner to be a complete circle when there are no animations, rather than some odd looking broken spinner shape

@andrewseguin andrewseguin added the target: patch This PR is targeted for the next patch release label Dec 15, 2020
@google-cla google-cla bot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 15, 2020
@annieyw annieyw added the action: merge The PR is ready for merge by the caretaker label Dec 15, 2020
@include mdc-circular-progress-core-styles($query: animation);
}

// Render the indeterminate spinner as a complete circle when animations are off
._mat-animation-noopable .mdc-circular-progress__indeterminate-container circle {
Copy link
Member

Choose a reason for hiding this comment

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

I think you can target ._mat-animation-noopable .mdc-circular-progress__determinate-circle directly. The specificity shouldn't matter since you're using !important.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was hoping so, but they apply the styles directly to the element dynamically so there's no way to override it via CSS unless we use !important

Copy link
Member

Choose a reason for hiding this comment

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

I meant that changing the selector to the one I mentioned would reduce the specificity, but it won't matter if we keep the !important.

@mmalerba mmalerba merged commit 71b7b15 into angular:master Dec 17, 2020
mmalerba pushed a commit that referenced this pull request Dec 17, 2020
crisbeto added a commit to crisbeto/material2 that referenced this pull request Dec 18, 2020
…orking

Seems like a regression from angular#21359. We can't use `:not` to apply the animation
styles, because MDC's mixins target `.mdc-progress-spinner`, rather than `&`.

These changes use the same manual approach as other components to disable
the animations.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Dec 18, 2020
…ion not working

Seems like a regression from angular#21359. We can't use `:not` to apply the animation
styles, because MDC's mixins target `.mdc-progress-spinner`, rather than `&`.

These changes use the same manual approach as other components to disable
the animations.
annieyw pushed a commit to annieyw/components that referenced this pull request Dec 22, 2020
annieyw pushed a commit that referenced this pull request Jan 6, 2021
…ion not working (#21391)

Seems like a regression from #21359. We can't use `:not` to apply the animation
styles, because MDC's mixins target `.mdc-progress-spinner`, rather than `&`.

These changes use the same manual approach as other components to disable
the animations.
annieyw pushed a commit that referenced this pull request Jan 6, 2021
…ion not working (#21391)

Seems like a regression from #21359. We can't use `:not` to apply the animation
styles, because MDC's mixins target `.mdc-progress-spinner`, rather than `&`.

These changes use the same manual approach as other components to disable
the animations.

(cherry picked from commit 37a9c4e)
wagnermaciel pushed a commit to wagnermaciel/components that referenced this pull request Jan 14, 2021
wagnermaciel pushed a commit to wagnermaciel/components that referenced this pull request Jan 14, 2021
…ion not working (angular#21391)

Seems like a regression from angular#21359. We can't use `:not` to apply the animation
styles, because MDC's mixins target `.mdc-progress-spinner`, rather than `&`.

These changes use the same manual approach as other components to disable
the animations.
@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 Jan 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants