Skip to content

Commit 64ccbfd

Browse files
committed
fix(ripple): not fading out on touch devices
* Makes the ripple animations no longer dependent on `setTimeout` that does not always fire properly / or within the specified duration. (related chrome issue: https://bugs.chromium.org/p/chromium/issues/detail?id=567800) * Fix indentation of a few ripple tests * Fixes that the speed factor tests are basically not checking anything (even though they will be removed in the future; they need to pass right now) Fixes #12470
1 parent fbee9c7 commit 64ccbfd

File tree

4 files changed

+222
-172
lines changed

4 files changed

+222
-172
lines changed

src/lib/checkbox/checkbox.spec.ts

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import {
22
ComponentFixture,
33
fakeAsync,
44
TestBed,
5-
tick,
65
flush,
76
flushMicrotasks,
87
} from '@angular/core/testing';
@@ -11,7 +10,6 @@ import {Component, DebugElement, ViewChild, Type} from '@angular/core';
1110
import {By} from '@angular/platform-browser';
1211
import {dispatchFakeEvent} from '@angular/cdk/testing';
1312
import {MatCheckbox, MatCheckboxChange, MatCheckboxModule} from './index';
14-
import {defaultRippleAnimationConfig} from '@angular/material/core';
1513
import {MAT_CHECKBOX_CLICK_ACTION} from './checkbox-config';
1614
import {MutationObserverFactory} from '@angular/cdk/observers';
1715

@@ -379,24 +377,29 @@ describe('MatCheckbox', () => {
379377
expect(inputElement.value).toBe('basic_checkbox');
380378
});
381379

382-
it('should show a ripple when focused by a keyboard action', fakeAsync(() => {
380+
it('should show a ripple when focused by a keyboard action', () => {
383381
expect(fixture.nativeElement.querySelectorAll('.mat-ripple-element').length)
384382
.toBe(0, 'Expected no ripples on load.');
385383

386384
dispatchFakeEvent(inputElement, 'keydown');
387385
dispatchFakeEvent(inputElement, 'focus');
388386

389-
tick(defaultRippleAnimationConfig.enterDuration);
390-
391387
expect(fixture.nativeElement.querySelectorAll('.mat-ripple-element').length)
392388
.toBe(1, 'Expected ripple after element is focused.');
393389

394-
dispatchFakeEvent(checkboxInstance._inputElement.nativeElement, 'blur');
395-
tick(defaultRippleAnimationConfig.exitDuration);
390+
const rippleElement = fixture.nativeElement
391+
.querySelector('.mat-ripple-element')! as HTMLElement;
392+
393+
// Flush the fade-in transition of the ripple.
394+
dispatchFakeEvent(rippleElement, 'transitionend');
395+
396+
// Blur the input element and flush the fade-out transition of the ripple.
397+
dispatchFakeEvent(inputElement, 'blur');
398+
dispatchFakeEvent(rippleElement, 'transitionend');
396399

397400
expect(fixture.nativeElement.querySelectorAll('.mat-ripple-element').length)
398401
.toBe(0, 'Expected no ripple after element is blurred.');
399-
}));
402+
});
400403

401404
it('should remove the SVG checkmark from the tab order', () => {
402405
expect(checkboxNativeElement.querySelector('svg')!.getAttribute('focusable')).toBe('false');

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

Lines changed: 53 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -172,21 +172,19 @@ export class RippleRenderer {
172172
this._mostRecentTransientRipple = rippleRef;
173173
}
174174

175-
// Wait for the ripple element to be completely faded in.
176-
// Once it's faded in, the ripple can be hidden immediately if the mouse is released.
177-
this.runTimeoutOutsideZone(() => {
178-
const isMostRecentTransientRipple = rippleRef === this._mostRecentTransientRipple;
179-
180-
rippleRef.state = RippleState.VISIBLE;
181-
182-
// When the timer runs out while the user has kept their pointer down, we want to
183-
// keep only the persistent ripples and the latest transient ripple. We do this,
184-
// because we don't want stacked transient ripples to appear after their enter
185-
// animation has finished.
186-
if (!config.persistent && (!isMostRecentTransientRipple || !this._isPointerDown)) {
187-
rippleRef.fadeOut();
188-
}
189-
}, duration);
175+
// Do not register the transition event listener if the fade-in and fade-out duration
176+
// are set to zero because the event won't be fired at all.
177+
if (duration || animationConfig.exitDuration) {
178+
this._ngZone.runOutsideAngular(() => {
179+
ripple.addEventListener('transitionend', () => this._finishRippleTransition(rippleRef));
180+
});
181+
}
182+
183+
// In case there is no fade-in transition duration, we need to manually call the
184+
// transition end listener because `transitionend` doesn't fire if there is no transition.
185+
if (!duration) {
186+
this._finishRippleTransition(rippleRef);
187+
}
190188

191189
return rippleRef;
192190
}
@@ -212,15 +210,17 @@ export class RippleRenderer {
212210
const rippleEl = rippleRef.element;
213211
const animationConfig = {...defaultRippleAnimationConfig, ...rippleRef.config.animation};
214212

213+
// This starts the fade-out transition and will fire the transition end listener that
214+
// removes the ripple element from the DOM.
215215
rippleEl.style.transitionDuration = `${animationConfig.exitDuration}ms`;
216216
rippleEl.style.opacity = '0';
217217
rippleRef.state = RippleState.FADING_OUT;
218218

219-
// Once the ripple faded out, the ripple can be safely removed from the DOM.
220-
this.runTimeoutOutsideZone(() => {
221-
rippleRef.state = RippleState.HIDDEN;
222-
rippleEl.parentNode!.removeChild(rippleEl);
223-
}, animationConfig.exitDuration);
219+
// In case there is no fade-out transition duration, we need to manually call the
220+
// transition end listener because `transitionend` doesn't fire if there is no transition.
221+
if (!animationConfig.exitDuration) {
222+
this._finishRippleTransition(rippleRef);
223+
}
224224
}
225225

226226
/** Fades out all currently active ripples. */
@@ -245,6 +245,39 @@ export class RippleRenderer {
245245
this._triggerElement = element;
246246
}
247247

248+
/** Method that will be called if the fade-in or fade-in transition completed. */
249+
private _finishRippleTransition(rippleRef: RippleRef) {
250+
if (rippleRef.state === RippleState.FADING_IN) {
251+
this._startFadeOutTransition(rippleRef);
252+
} else if (rippleRef.state === RippleState.FADING_OUT) {
253+
this._destroyRipple(rippleRef);
254+
}
255+
}
256+
257+
/**
258+
* Starts the fade-out transition of the given ripple if it's not persistent and the pointer
259+
* is not held down anymore.
260+
*/
261+
private _startFadeOutTransition(rippleRef: RippleRef) {
262+
const isMostRecentTransientRipple = rippleRef === this._mostRecentTransientRipple;
263+
264+
rippleRef.state = RippleState.VISIBLE;
265+
266+
// When the timer runs out while the user has kept their pointer down, we want to
267+
// keep only the persistent ripples and the latest transient ripple. We do this,
268+
// because we don't want stacked transient ripples to appear after their enter
269+
// animation has finished.
270+
if (!rippleRef.config.persistent && (!isMostRecentTransientRipple || !this._isPointerDown)) {
271+
rippleRef.fadeOut();
272+
}
273+
}
274+
275+
/** Destroys the given ripple by removing it from the DOM and updating its state. */
276+
private _destroyRipple(rippleRef: RippleRef) {
277+
rippleRef.state = RippleState.HIDDEN;
278+
rippleRef.element.parentNode!.removeChild(rippleRef.element);
279+
}
280+
248281
/** Function being called whenever the trigger is being pressed using mouse. */
249282
private onMousedown = (event: MouseEvent) => {
250283
// Screen readers will fire fake mouse events for space/enter. Skip launching a
@@ -299,11 +332,6 @@ export class RippleRenderer {
299332
});
300333
}
301334

302-
/** Runs a timeout outside of the Angular zone to avoid triggering the change detection. */
303-
private runTimeoutOutsideZone(fn: Function, delay = 0) {
304-
this._ngZone.runOutsideAngular(() => setTimeout(fn, delay));
305-
}
306-
307335
/** Removes previously registered event listeners from the trigger element. */
308336
_removeTriggerEvents() {
309337
if (this._triggerElement) {

0 commit comments

Comments
 (0)