Skip to content

Commit

Permalink
fix(ripple): Always remove ripple after a certain period (#1915)
Browse files Browse the repository at this point in the history
* Always remove ripple after a certain period

* Also remove the ripple when transition is ended

* remove ripple after timeout outside the angular zone

* fix lint

* Add comment
  • Loading branch information
tinayuangao authored and mmalerba committed Dec 7, 2016
1 parent 7b90d62 commit 62cc830
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 8 deletions.
19 changes: 17 additions & 2 deletions src/lib/core/ripple/ripple-renderer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
ElementRef,
NgZone,
} from '@angular/core';

/** TODO: internal */
Expand Down Expand Up @@ -44,7 +45,9 @@ export class RippleRenderer {
private _triggerElement: HTMLElement;
_opacity: string;

constructor(_elementRef: ElementRef, private _eventHandlers: Map<string, (e: Event) => void>) {
constructor(_elementRef: ElementRef,
private _eventHandlers: Map<string, (e: Event) => void>,
private _ngZone: NgZone) {
this._rippleElement = _elementRef.nativeElement;
// The background div is created in createBackgroundIfNeeded when the ripple becomes enabled.
// This avoids creating unneeded divs when the ripple is always disabled.
Expand Down Expand Up @@ -143,6 +146,16 @@ export class RippleRenderer {

rippleDiv.addEventListener('transitionend',
(event: TransitionEvent) => transitionEndCallback(ripple, event));
// Ensure that ripples are always removed, even when transitionend doesn't fire.
// Run this outside the Angular zone because there's nothing that Angular cares about.
// If it were to run inside the Angular zone, every test that used ripples would have to be
// either async or fakeAsync.
this._ngZone.runOutsideAngular(() => {
// The ripple lasts a time equal to the sum of fade-in, transform,
// and fade-out (3 * fade-in time).
let rippleDuration = fadeInSeconds * 3 * 1000;
setTimeout(() => this.removeRippleFromDom(ripple.rippleElement), rippleDuration);
});
}

/** Fades out a foreground ripple after it has fully expanded and faded in. */
Expand All @@ -153,7 +166,9 @@ export class RippleRenderer {

/** Removes a foreground ripple from the DOM after it has faded out. */
removeRippleFromDom(ripple: Element) {
ripple.parentElement.removeChild(ripple);
if (ripple && ripple.parentElement) {
ripple.parentElement.removeChild(ripple);
}
}

/** Fades in the ripple background. */
Expand Down
12 changes: 11 additions & 1 deletion src/lib/core/ripple/ripple.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {TestBed, ComponentFixture} from '@angular/core/testing';
import {TestBed, ComponentFixture, async} from '@angular/core/testing';
import {Component, ViewChild} from '@angular/core';
import {MdRipple, MdRippleModule} from './ripple';

Expand Down Expand Up @@ -132,6 +132,16 @@ describe('MdRipple', () => {
expect(rippleElement.querySelectorAll('.md-ripple-foreground').length).toBe(0);
});

it('removes foreground ripples after timeout', async(() => {
rippleElement.click();
expect(rippleElement.querySelectorAll('.md-ripple-foreground').length).toBe(1);

// Use a real timeout because the ripple's timeout runs outside of the angular zone.
setTimeout(() => {
expect(rippleElement.querySelectorAll('.md-ripple-foreground').length).toBe(0);
}, 1600);
}));

it('creates ripples when manually triggered', () => {
const rippleComponent = fixture.debugElement.componentInstance.ripple;
// start() should show the background, but no foreground ripple yet.
Expand Down
5 changes: 3 additions & 2 deletions src/lib/core/ripple/ripple.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
ElementRef,
HostBinding,
Input,
NgZone,
OnChanges,
OnDestroy,
OnInit,
Expand Down Expand Up @@ -62,13 +63,13 @@ export class MdRipple implements OnInit, OnDestroy, OnChanges {

private _rippleRenderer: RippleRenderer;

constructor(_elementRef: ElementRef) {
constructor(_elementRef: ElementRef, _ngZone: NgZone) {
// These event handlers are attached to the element that triggers the ripple animations.
const eventHandlers = new Map<string, (e: Event) => void>();
eventHandlers.set('mousedown', (event: MouseEvent) => this._mouseDown(event));
eventHandlers.set('click', (event: MouseEvent) => this._click(event));
eventHandlers.set('mouseleave', (event: MouseEvent) => this._mouseLeave(event));
this._rippleRenderer = new RippleRenderer(_elementRef, eventHandlers);
this._rippleRenderer = new RippleRenderer(_elementRef, eventHandlers, _ngZone);
}

/** TODO: internal */
Expand Down
6 changes: 3 additions & 3 deletions src/lib/tabs/tab-nav-bar/tab-nav-bar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import {
ElementRef,
ViewEncapsulation,
Directive,
NgZone,
OnDestroy,
} from '@angular/core';

import {MdInkBar} from '../ink-bar';
import {MdRipple} from '../../core/ripple/ripple';

Expand Down Expand Up @@ -60,8 +60,8 @@ export class MdTabLink {
selector: '[md-tab-link], [mat-tab-link]',
})
export class MdTabLinkRipple extends MdRipple implements OnDestroy {
constructor(private _element: ElementRef) {
super(_element);
constructor(private _element: ElementRef, private _ngZone: NgZone) {
super(_element, _ngZone);
}

// In certain cases the parent destroy handler
Expand Down

0 comments on commit 62cc830

Please sign in to comment.