fix(ripple): fade-out-all should hide all ripples (#3400)

src/lib/core/ripple/index.ts

@@ -5,7 +5,7 @@ import {VIEWPORT_RULER_PROVIDER} from '../overlay/position/viewport-ruler';
 import {SCROLL_DISPATCHER_PROVIDER} from '../overlay/scroll/scroll-dispatcher';
 
 export {MdRipple} from './ripple';
-export {RippleRef} from './ripple-ref';
+export {RippleRef, RippleState} from './ripple-ref';
 export {RippleConfig} from './ripple-renderer';
 
 @NgModule({

src/lib/core/ripple/ripple-ref.ts

@@ -1,10 +1,18 @@
 import {RippleConfig, RippleRenderer} from './ripple-renderer';
 
+/** Possible states for a ripple element. */
+export enum RippleState {
+  FADING_IN, VISIBLE, FADING_OUT, HIDDEN
+}
+
 /**
  * Reference to a previously launched ripple element.
  */
 export class RippleRef {
 
+  /** Current state of the ripple reference. */
+  state: RippleState = RippleState.HIDDEN;
+
   constructor(
     private _renderer: RippleRenderer,
     public element: HTMLElement,

src/lib/core/ripple/ripple-renderer.ts

@@ -1,6 +1,6 @@
 import {ElementRef, NgZone} from '@angular/core';
 import {ViewportRuler} from '../overlay/position/viewport-ruler';
-import {RippleRef} from './ripple-ref';
+import {RippleRef, RippleState} from './ripple-ref';
 
 /** Fade-in duration for the ripples. Can be modified with the speedFactor option. */
 export const RIPPLE_FADE_IN_DURATION = 450;
@@ -101,12 +101,17 @@ export class RippleRenderer {
     // Exposed reference to the ripple that will be returned.
     let rippleRef = new RippleRef(this, ripple, config);
 
+    rippleRef.state = RippleState.FADING_IN;
+
+    // Add the ripple reference to the list of all active ripples.
+    this._activeRipples.add(rippleRef);
+
     // Wait for the ripple element to be completely faded in.
     // Once it's faded in, the ripple can be hidden immediately if the mouse is released.
     this.runTimeoutOutsideZone(() => {
-      if (config.persistent || this._isMousedown) {
-        this._activeRipples.add(rippleRef);
-      } else {
+      rippleRef.state = RippleState.VISIBLE;
+
+      if (!config.persistent && !this._isMousedown) {
         rippleRef.fadeOut();
       }
     }, duration);
@@ -115,16 +120,22 @@ export class RippleRenderer {
   }
 
   /** Fades out a ripple reference. */
-  fadeOutRipple(ripple: RippleRef) {
-    let rippleEl = ripple.element;
+  fadeOutRipple(rippleRef: RippleRef) {
+    // For ripples that are not active anymore, don't re-un the fade-out animation.
+    if (!this._activeRipples.delete(rippleRef)) {
+      return;
+    }
 
-    this._activeRipples.delete(ripple);
+    let rippleEl = rippleRef.element;
 
     rippleEl.style.transitionDuration = `${RIPPLE_FADE_OUT_DURATION}ms`;
     rippleEl.style.opacity = '0';
 
+    rippleRef.state = RippleState.FADING_OUT;
+
     // Once the ripple faded out, the ripple can be safely removed from the DOM.
     this.runTimeoutOutsideZone(() => {
+      rippleRef.state = RippleState.HIDDEN;
       rippleEl.parentNode.removeChild(rippleEl);
     }, RIPPLE_FADE_OUT_DURATION);
   }
@@ -163,9 +174,9 @@ export class RippleRenderer {
   private onMouseup() {
     this._isMousedown = false;
 
-    // On mouseup, fade-out all ripples that are active and not persistent.
+    // Fade-out all ripples that are completely visible and not persistent.
     this._activeRipples.forEach(ripple => {
-      if (!ripple.config.persistent) {
+      if (!ripple.config.persistent && ripple.state === RippleState.VISIBLE) {
         ripple.fadeOut();
       }
     });

src/lib/core/ripple/ripple.spec.ts

@@ -1,6 +1,6 @@
 import {TestBed, ComponentFixture, fakeAsync, tick, inject} from '@angular/core/testing';
 import {Component, ViewChild} from '@angular/core';
-import {MdRipple, MdRippleModule} from './index';
+import {MdRipple, MdRippleModule, RippleState} from './index';
 import {ViewportRuler} from '../overlay/position/viewport-ruler';
 import {RIPPLE_FADE_OUT_DURATION, RIPPLE_FADE_IN_DURATION} from './ripple-renderer';
 import {dispatchMouseEvent} from '../testing/dispatch-events';
@@ -77,6 +77,39 @@ describe('MdRipple', () => {
       expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0);
     }));
 
+    it('should remove ripples after mouseup', fakeAsync(() => {
+      dispatchMouseEvent(rippleTarget, 'mousedown');
+
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1);
+
+      // Fakes the duration of fading-in and fading-out normal ripples.
+      // The fade-out duration has been added to ensure that didn't start fading out.
+      tick(RIPPLE_FADE_IN_DURATION + RIPPLE_FADE_OUT_DURATION);
+
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1);
+
+      dispatchMouseEvent(rippleTarget, 'mouseup');
+      tick(RIPPLE_FADE_OUT_DURATION);
+
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0);
+    }));
+
+    it('should not hide ripples while animating.', fakeAsync(() => {
+      // Calculates the duration for fading-in and fading-out the ripple.
+      let hideDuration = RIPPLE_FADE_IN_DURATION + RIPPLE_FADE_OUT_DURATION;
+
+      dispatchMouseEvent(rippleTarget, 'mousedown');
+      dispatchMouseEvent(rippleTarget, 'mouseup');
+
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1);
+
+      tick(hideDuration - 10);
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1);
+
+      tick(10);
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0);
+    }));
+
     it('creates ripples when manually triggered', () => {
       expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0);
 
@@ -270,6 +303,47 @@ describe('MdRipple', () => {
       expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0);
     }));
 
+   it('should remove ripples that are not done fading-in', fakeAsync(() => {
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0);
+
+      rippleDirective.launch(0, 0);
+
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1);
+
+      tick(RIPPLE_FADE_IN_DURATION / 2);
+
+      rippleDirective.fadeOutAll();
+
+      tick(RIPPLE_FADE_OUT_DURATION);
+
+      expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
+        .toBe(0, 'Expected no ripples to be active after calling fadeOutAll.');
+    }));
+
+   it('should properly set ripple states', fakeAsync(() => {
+     expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0);
+
+     let rippleRef = rippleDirective.launch(0, 0, { persistent: true });
+
+     expect(rippleRef.state).toBe(RippleState.FADING_IN);
+     expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1);
+
+     tick(RIPPLE_FADE_IN_DURATION);
+
+     expect(rippleRef.state).toBe(RippleState.VISIBLE);
+     expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1);
+
+     rippleRef.fadeOut();
+
+     expect(rippleRef.state).toBe(RippleState.FADING_OUT);
+     expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1);
+
+     tick(RIPPLE_FADE_OUT_DURATION);
+
+     expect(rippleRef.state).toBe(RippleState.HIDDEN);
+     expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0);
+   }));
+
   });
 
   describe('configuring behavior', () => {