From 6ef4c9797b58727b02e1abe668ec987cfddd235d Mon Sep 17 00:00:00 2001 From: crisbeto Date: Sat, 4 Apr 2020 10:34:00 +0200 Subject: [PATCH] fix(core): ripple mutating global options when animations are disabled 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. --- src/material/core/ripple/ripple.spec.ts | 15 ++++++++++++--- src/material/core/ripple/ripple.ts | 12 ++++++------ tools/public_api_guard/material/core.d.ts | 2 +- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/material/core/ripple/ripple.spec.ts b/src/material/core/ripple/ripple.spec.ts index 307ef4c1a90b..f95c1d038065 100644 --- a/src/material/core/ripple/ripple.spec.ts +++ b/src/material/core/ripple/ripple.spec.ts @@ -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); @@ -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', () => { diff --git a/src/material/core/ripple/ripple.ts b/src/material/core/ripple/ripple.ts index ffa882831dd1..4f108ef5d840 100644 --- a/src/material/core/ripple/ripple.ts +++ b/src/material/core/ripple/ripple.ts @@ -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() { @@ -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, }; } diff --git a/tools/public_api_guard/material/core.d.ts b/tools/public_api_guard/material/core.d.ts index ccb80b11e08a..a100dfe103c9 100644 --- a/tools/public_api_guard/material/core.d.ts +++ b/tools/public_api_guard/material/core.d.ts @@ -327,7 +327,7 @@ export declare class MatRipple implements OnInit, OnDestroy, RippleTarget { get trigger(): HTMLElement; set trigger(trigger: HTMLElement); unbounded: boolean; - constructor(_elementRef: ElementRef, ngZone: NgZone, platform: Platform, globalOptions?: RippleGlobalOptions, animationMode?: string); + constructor(_elementRef: ElementRef, ngZone: NgZone, platform: Platform, globalOptions?: RippleGlobalOptions, _animationMode?: string | undefined); fadeOutAll(): void; launch(config: RippleConfig): RippleRef; launch(x: number, y: number, config?: RippleConfig): RippleRef;