Skip to content

Commit

Permalink
fix(core): ripple mutating global options when animations are… (#18983)
Browse files Browse the repository at this point in the history
When animations are disabled, the ripple elements sets the animation duration of its global options to zero. The problem is that it's mutating the object that's used by all other ripples. The issue can be observed by going to the MDC checkbox demo and then trying to interact with something that should have a ripple.
  • Loading branch information
crisbeto authored and mmalerba committed Apr 14, 2020
1 parent 7957423 commit 09f2872
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 10 deletions.
15 changes: 12 additions & 3 deletions src/material/core/ripple/ripple.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -479,14 +479,15 @@ describe('MatRipple', () => {
let rippleDirective: MatRipple;

function createTestComponent(rippleConfig: RippleGlobalOptions,
testComponent: any = BasicRippleContainer) {
testComponent: any = BasicRippleContainer,
extraImports: any[] = []) {
// Reset the previously configured testing module to be able set new providers.
// The testing module has been initialized in the root describe group for the ripples.
TestBed.resetTestingModule();
TestBed.configureTestingModule({
imports: [MatRippleModule],
imports: [MatRippleModule, ...extraImports],
declarations: [testComponent],
providers: [{ provide: MAT_RIPPLE_GLOBAL_OPTIONS, useValue: rippleConfig }]
providers: [{provide: MAT_RIPPLE_GLOBAL_OPTIONS, useValue: rippleConfig}]
});

fixture = TestBed.createComponent(testComponent);
Expand Down Expand Up @@ -569,6 +570,14 @@ describe('MatRipple', () => {
// will still exist. To properly finish all timers, we just wait the remaining time.
tick(enterDuration - exitDuration);
}));

it('should not mutate the global options when NoopAnimationsModule is present', () => {
const options: RippleGlobalOptions = {};

createTestComponent(options, RippleContainerWithoutBindings, [NoopAnimationsModule]);

expect(options.animation).toBeFalsy();
});
});

describe('with disabled animations', () => {
Expand Down
12 changes: 6 additions & 6 deletions src/material/core/ripple/ripple.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,10 @@ export class MatRipple implements OnInit, OnDestroy, RippleTarget {
ngZone: NgZone,
platform: Platform,
@Optional() @Inject(MAT_RIPPLE_GLOBAL_OPTIONS) globalOptions?: RippleGlobalOptions,
@Optional() @Inject(ANIMATION_MODULE_TYPE) animationMode?: string) {
@Optional() @Inject(ANIMATION_MODULE_TYPE) private _animationMode?: string) {

this._globalOptions = globalOptions || {};
this._rippleRenderer = new RippleRenderer(this, ngZone, _elementRef, platform);

if (animationMode === 'NoopAnimations') {
this._globalOptions.animation = {enterDuration: 0, exitDuration: 0};
}
}

ngOnInit() {
Expand All @@ -154,7 +150,11 @@ export class MatRipple implements OnInit, OnDestroy, RippleTarget {
centered: this.centered,
radius: this.radius,
color: this.color,
animation: {...this._globalOptions.animation, ...this.animation},
animation: {
...this._globalOptions.animation,
...(this._animationMode === 'NoopAnimations' ? {enterDuration: 0, exitDuration: 0} : {}),
...this.animation
},
terminateOnPointerUp: this._globalOptions.terminateOnPointerUp,
};
}
Expand Down
2 changes: 1 addition & 1 deletion tools/public_api_guard/material/core.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ export declare class MatRipple implements OnInit, OnDestroy, RippleTarget {
get trigger(): HTMLElement;
set trigger(trigger: HTMLElement);
unbounded: boolean;
constructor(_elementRef: ElementRef<HTMLElement>, ngZone: NgZone, platform: Platform, globalOptions?: RippleGlobalOptions, animationMode?: string);
constructor(_elementRef: ElementRef<HTMLElement>, ngZone: NgZone, platform: Platform, globalOptions?: RippleGlobalOptions, _animationMode?: string | undefined);
fadeOutAll(): void;
launch(config: RippleConfig): RippleRef;
launch(x: number, y: number, config?: RippleConfig): RippleRef;
Expand Down

0 comments on commit 09f2872

Please sign in to comment.