Skip to content

fix(progress-bar/progress-spinner): prevent animations when in a noopanimations module context #10881

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

Merged
merged 1 commit into from
May 4, 2018
Merged

fix(progress-bar/progress-spinner): prevent animations when in a noopanimations module context #10881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions src/lib/core/style/_noop-animation.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// If the mat-animation-noop class is present on the components root element,
// prevent non css animations from running.
// NOTE: Currently this mixin should only be used with components that do not
// have any projected content.
@mixin _noop-animation() {
Copy link
Member

Choose a reason for hiding this comment

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

_mat-noop-animation

Copy link
Contributor

Choose a reason for hiding this comment

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

can we call it noopable? it's not nooping the animation, it's just making it noopable in case you add the noop class

// @at-root is used to steps outside of the hierarchy of the scss rules. This is
// done to allow a class to be added to be added to base of the scss nesting
// context.
// For example:
// .my-root {
// .my-subclass {
// @include _noop-animation()
// }
// }
// results in:
// ._mat-animation-noopable.my-root .my-subclass { ... }
@at-root ._mat-animation-noopable#{&} {
transition: none;
animation: none;
}
}
11 changes: 11 additions & 0 deletions src/lib/progress-bar/progress-bar.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
@import '../core/style/variables';
@import '../core/style/vendor-prefixes';
@import '../core/style/noop-animation';

$mat-progress-bar-height: 5px !default;
$mat-progress-bar-full-animation-duration: 2000ms !default;
$mat-progress-bar-piece-animation-duration: 250ms !default;


.mat-progress-bar {
@include _noop-animation();
display: block;
// Height is provided for mat-progress-bar to act as a default.
height: $mat-progress-bar-height;
Expand All @@ -33,6 +35,7 @@ $mat-progress-bar-piece-animation-duration: 250ms !default;
// The progress bar buffer is the bar indicator showing the buffer value and is only visible
// beyond the current value of the primary progress bar.
.mat-progress-bar-buffer {
@include _noop-animation();
transform-origin: top left;
transition: transform $mat-progress-bar-piece-animation-duration ease;
}
Expand All @@ -45,13 +48,15 @@ $mat-progress-bar-piece-animation-duration: 250ms !default;

// The progress bar fill fills the progress bar with the indicator color.
.mat-progress-bar-fill {
@include _noop-animation();
animation: none;
transform-origin: top left;
transition: transform $mat-progress-bar-piece-animation-duration ease;
}

// A pseudo element is created for each progress bar bar that fills with the indicator color.
.mat-progress-bar-fill::after {
@include _noop-animation();
animation: none;
content: '';
display: inline-block;
Expand All @@ -76,23 +81,27 @@ $mat-progress-bar-piece-animation-duration: 250ms !default;
&[mode='indeterminate'],
&[mode='query'] {
.mat-progress-bar-fill {
@include _noop-animation();
transition: none;
}
.mat-progress-bar-primary {
// Avoids stacked animation tearing in Firefox >= 57.
@include _noop-animation();
@include backface-visibility(hidden);
animation: mat-progress-bar-primary-indeterminate-translate
$mat-progress-bar-full-animation-duration infinite linear;
left: -145.166611%;
}
.mat-progress-bar-primary.mat-progress-bar-fill::after {
// Avoids stacked animation tearing in Firefox >= 57.
@include _noop-animation();
@include backface-visibility(hidden);
animation: mat-progress-bar-primary-indeterminate-scale
$mat-progress-bar-full-animation-duration infinite linear;
}
.mat-progress-bar-secondary {
// Avoids stacked animation tearing in Firefox >= 57.
@include _noop-animation();
@include backface-visibility(hidden);
animation: mat-progress-bar-secondary-indeterminate-translate
$mat-progress-bar-full-animation-duration infinite linear;
Expand All @@ -101,6 +110,7 @@ $mat-progress-bar-piece-animation-duration: 250ms !default;
}
.mat-progress-bar-secondary.mat-progress-bar-fill::after {
// Avoids stacked animation tearing in Firefox >= 57.
@include _noop-animation();
@include backface-visibility(hidden);
animation: mat-progress-bar-secondary-indeterminate-scale
$mat-progress-bar-full-animation-duration infinite linear;
Expand All @@ -110,6 +120,7 @@ $mat-progress-bar-piece-animation-duration: 250ms !default;
&[mode='buffer'] {
.mat-progress-bar-background {
// Avoids stacked animation tearing in Firefox >= 57.
@include _noop-animation();
@include backface-visibility(hidden);
animation: mat-progress-bar-background-scroll
$mat-progress-bar-piece-animation-duration infinite linear;
Expand Down
8 changes: 7 additions & 1 deletion src/lib/progress-bar/progress-bar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ import {
Component,
ChangeDetectionStrategy,
ElementRef,
Inject,
Input,
Optional,
ViewEncapsulation
} from '@angular/core';
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't adding this import be considered a breaking change, in the case where somebody might not have the animations package? I think we had a discussion along the same lines with @mmalerba where we considered adding animations to a component, that didn't have any, a breaking change. That being said, most people should have platform-browser already.

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 this is different because it doesn't require a module to be loaded; for animations, the breaking change is that you need to include BrowserAnimationsModule at the root for it to work. For this, I don't think it actually breaks anything.

import {CanColor, mixinColor} from '@angular/material/core';

// TODO(josephperrott): Benchpress tests.
Expand Down Expand Up @@ -42,6 +45,7 @@ let progressbarId = 0;
'[attr.aria-valuenow]': 'value',
'[attr.mode]': 'mode',
'class': 'mat-progress-bar',
'[class._mat-animation-noopable]': `_animationMode === 'NoopAnimations'`,
},
inputs: ['color'],
templateUrl: 'progress-bar.html',
Expand All @@ -51,7 +55,9 @@ let progressbarId = 0;
})
export class MatProgressBar extends _MatProgressBarMixinBase implements CanColor {

constructor(public _elementRef: ElementRef) {

constructor(public _elementRef: ElementRef,
@Optional() @Inject(ANIMATION_MODULE_TYPE) public _animationMode?: string) {
super(_elementRef);
}

Expand Down
6 changes: 6 additions & 0 deletions src/lib/progress-spinner/progress-spinner.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
@import '../core/style/variables';
@import '../core/style/noop-animation';


// Animation config
Expand All @@ -22,16 +23,19 @@ $_mat-progress-spinner-default-circumference: $pi * $_mat-progress-spinner-defau
}

circle {
@include _noop-animation();
fill: transparent;
transform-origin: center;
transition: stroke-dashoffset 225ms linear;
}

&.mat-progress-spinner-indeterminate-animation[mode='indeterminate'] {
@include _noop-animation();
animation: mat-progress-spinner-linear-rotate $swift-ease-in-out-duration * 4
linear infinite;

circle {
@include _noop-animation();
transition-property: stroke;
// Note: we multiply the duration by 8, because the animation is spread out in 8 stages.
animation-duration: $swift-ease-in-out-duration * 8;
Expand All @@ -41,12 +45,14 @@ $_mat-progress-spinner-default-circumference: $pi * $_mat-progress-spinner-defau
}

&.mat-progress-spinner-indeterminate-fallback-animation[mode='indeterminate'] {
@include _noop-animation();
animation: mat-progress-spinner-stroke-rotate-fallback
$mat-progress-spinner-stroke-rotate-fallback-duration
$mat-progress-spinner-stroke-rotate-fallback-ease
infinite;

circle {
@include _noop-animation();
transition-property: stroke;
}
}
Expand Down
13 changes: 9 additions & 4 deletions src/lib/progress-spinner/progress-spinner.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,13 @@
import {
Component,
ChangeDetectionStrategy,
Inject,
Input,
ElementRef,
ViewEncapsulation,
Optional,
Inject,
} from '@angular/core';
import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations';
import {CanColor, mixinColor} from '@angular/material/core';
import {Platform} from '@angular/cdk/platform';
import {DOCUMENT} from '@angular/common';
Expand Down Expand Up @@ -80,6 +81,7 @@ const INDETERMINATE_ANIMATION_TEMPLATE = `
host: {
'role': 'progressbar',
'class': 'mat-progress-spinner',
'[class._mat-animation-noopable]': `_animationMode === 'NoopAnimations'`,
'[style.width.px]': 'diameter',
'[style.height.px]': 'diameter',
'[attr.aria-valuemin]': 'mode === "determinate" ? 0 : null',
Expand Down Expand Up @@ -144,7 +146,8 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements

constructor(public _elementRef: ElementRef,
platform: Platform,
@Optional() @Inject(DOCUMENT) private _document: any) {
@Optional() @Inject(DOCUMENT) private _document: any,
@Optional() @Inject(ANIMATION_MODULE_TYPE) public _animationMode?: string) {

super(_elementRef);
this._fallbackAnimation = platform.EDGE || platform.TRIDENT;
Expand Down Expand Up @@ -233,6 +236,7 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
'role': 'progressbar',
'mode': 'indeterminate',
'class': 'mat-spinner mat-progress-spinner',
'[class._mat-animation-noopable]': `_animationMode === 'NoopAnimations'`,
'[style.width.px]': 'diameter',
'[style.height.px]': 'diameter',
},
Expand All @@ -244,8 +248,9 @@ export class MatProgressSpinner extends _MatProgressSpinnerMixinBase implements
})
export class MatSpinner extends MatProgressSpinner {
constructor(elementRef: ElementRef, platform: Platform,
@Optional() @Inject(DOCUMENT) document: any) {
super(elementRef, platform, document);
@Optional() @Inject(DOCUMENT) document: any,
@Optional() @Inject(ANIMATION_MODULE_TYPE) _animationMode?: string) {
super(elementRef, platform, document, _animationMode);
this.mode = 'indeterminate';
}
}