Skip to content

Commit ac1280c

Browse files
committed
fix(material/core): ripples not fading out on touch devices when scrolling
* 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 feaf50b commit ac1280c

File tree

9 files changed

+247
-194
lines changed

9 files changed

+247
-194
lines changed

src/material-experimental/mdc-list/list.spec.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
1-
import {waitForAsync, TestBed, fakeAsync, tick} from '@angular/core/testing';
1+
import {fakeAsync, TestBed, waitForAsync} from '@angular/core/testing';
2+
import {dispatchFakeEvent, dispatchMouseEvent} from '@angular/cdk/testing/private';
23
import {Component, QueryList, ViewChildren} from '@angular/core';
3-
import {defaultRippleAnimationConfig} from '@angular/material-experimental/mdc-core';
4-
import {dispatchMouseEvent} from '../../cdk/testing/private';
54
import {By} from '@angular/platform-browser';
65
import {MatListItem, MatListModule} from './index';
76

87
describe('MDC-based MatList', () => {
9-
// Default ripple durations used for testing.
10-
const {enterDuration, exitDuration} = defaultRippleAnimationConfig;
11-
128
beforeEach(
139
waitForAsync(() => {
1410
TestBed.configureTestingModule({
@@ -243,12 +239,16 @@ describe('MDC-based MatList', () => {
243239
dispatchMouseEvent(rippleTarget, 'mousedown');
244240
dispatchMouseEvent(rippleTarget, 'mouseup');
245241

242+
// Flush the ripple enter animation.
243+
dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend');
244+
246245
expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
247246
.withContext('Expected ripples to be enabled by default.')
248247
.toBe(1);
249248

250-
// Wait for the ripples to go away.
251-
tick(enterDuration + exitDuration);
249+
// Flush the ripple exit animation.
250+
dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend');
251+
252252
expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
253253
.withContext('Expected ripples to go away.')
254254
.toBe(0);
@@ -273,12 +273,16 @@ describe('MDC-based MatList', () => {
273273
dispatchMouseEvent(rippleTarget, 'mousedown');
274274
dispatchMouseEvent(rippleTarget, 'mouseup');
275275

276+
// Flush the ripple enter animation.
277+
dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend');
278+
276279
expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
277280
.withContext('Expected ripples to be enabled by default.')
278281
.toBe(1);
279282

280-
// Wait for the ripples to go away.
281-
tick(enterDuration + exitDuration);
283+
// Flush the ripple exit animation.
284+
dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend');
285+
282286
expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
283287
.withContext('Expected ripples to go away.')
284288
.toBe(0);

src/material-experimental/mdc-list/selection-list.spec.ts

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
waitForAsync,
2323
} from '@angular/core/testing';
2424
import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms';
25-
import {defaultRippleAnimationConfig, ThemePalette} from '@angular/material-experimental/mdc-core';
25+
import {ThemePalette} from '@angular/material-experimental/mdc-core';
2626
import {By} from '@angular/platform-browser';
2727
import {numbers} from '@material/list';
2828
import {
@@ -612,17 +612,19 @@ describe('MDC-based MatSelectionList without forms', () => {
612612
const rippleTarget = fixture.nativeElement.querySelector(
613613
'.mat-mdc-list-option:not(.mdc-list-item--disabled)',
614614
);
615-
const {enterDuration, exitDuration} = defaultRippleAnimationConfig;
616-
617615
dispatchMouseEvent(rippleTarget, 'mousedown');
618616
dispatchMouseEvent(rippleTarget, 'mouseup');
619617

618+
// Flush the ripple enter animation.
619+
dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend');
620+
620621
expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
621622
.withContext('Expected ripples to be enabled by default.')
622623
.toBe(1);
623624

624-
// Wait for the ripples to go away.
625-
tick(enterDuration + exitDuration);
625+
// Flush the ripple exit animation.
626+
dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend');
627+
626628
expect(rippleTarget.querySelectorAll('.mat-ripple-element').length)
627629
.withContext('Expected ripples to go away.')
628630
.toBe(0);

src/material-experimental/mdc-slider/slider.spec.ts

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,13 @@
99
import {BidiModule, Directionality} from '@angular/cdk/bidi';
1010
import {Platform} from '@angular/cdk/platform';
1111
import {
12+
dispatchFakeEvent,
1213
dispatchMouseEvent,
1314
dispatchPointerEvent,
1415
dispatchTouchEvent,
1516
} from '../../cdk/testing/private';
1617
import {Component, Provider, QueryList, Type, ViewChild, ViewChildren} from '@angular/core';
17-
import {
18-
ComponentFixture,
19-
fakeAsync,
20-
flush,
21-
TestBed,
22-
tick,
23-
waitForAsync,
24-
} from '@angular/core/testing';
18+
import {ComponentFixture, fakeAsync, flush, TestBed, waitForAsync} from '@angular/core/testing';
2519
import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms';
2620
import {By} from '@angular/platform-browser';
2721
import {Thumb} from '@material/slider';
@@ -297,8 +291,14 @@ describe('MDC-based MatSlider', () => {
297291
);
298292

299293
function isRippleVisible(selector: string) {
300-
tick(500);
301-
return !!document.querySelector(`.mat-mdc-slider-${selector}-ripple`);
294+
flushRippleTransitions();
295+
return thumbElement.querySelector(`.mat-mdc-slider-${selector}-ripple`) !== null;
296+
}
297+
298+
function flushRippleTransitions() {
299+
thumbElement.querySelectorAll('.mat-ripple-element').forEach(el => {
300+
dispatchFakeEvent(el, 'transitionend');
301+
});
302302
}
303303

304304
function blur() {

src/material-experimental/mdc-tabs/tab-group.spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,6 @@ describe('MDC-based MatTabGroup', () => {
199199
.toBe(0);
200200

201201
dispatchFakeEvent(tabLabel.nativeElement, 'mousedown');
202-
dispatchFakeEvent(tabLabel.nativeElement, 'mouseup');
203202

204203
expect(testElement.querySelectorAll('.mat-ripple-element').length)
205204
.withContext('Expected one ripple to show up on label mousedown.')

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

Lines changed: 54 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -154,21 +154,19 @@ export class RippleRenderer implements EventListenerObject {
154154
this._mostRecentTransientRipple = rippleRef;
155155
}
156156

157-
// Wait for the ripple element to be completely faded in.
158-
// Once it's faded in, the ripple can be hidden immediately if the mouse is released.
159-
this._runTimeoutOutsideZone(() => {
160-
const isMostRecentTransientRipple = rippleRef === this._mostRecentTransientRipple;
161-
162-
rippleRef.state = RippleState.VISIBLE;
163-
164-
// When the timer runs out while the user has kept their pointer down, we want to
165-
// keep only the persistent ripples and the latest transient ripple. We do this,
166-
// because we don't want stacked transient ripples to appear after their enter
167-
// animation has finished.
168-
if (!config.persistent && (!isMostRecentTransientRipple || !this._isPointerDown)) {
169-
rippleRef.fadeOut();
170-
}
171-
}, duration);
157+
// Do not register the `transition` event listener if fade-in and fade-out duration
158+
// are set to zero. The events won't fire anyway and we can save resources here.
159+
if (duration || animationConfig.exitDuration) {
160+
this._ngZone.runOutsideAngular(() => {
161+
ripple.addEventListener('transitionend', () => this._finishRippleTransition(rippleRef));
162+
});
163+
}
164+
165+
// In case there is no fade-in transition duration, we need to manually call the transition
166+
// end listener because `transitionend` doesn't fire if there is no transition.
167+
if (!duration) {
168+
this._finishRippleTransition(rippleRef);
169+
}
172170

173171
return rippleRef;
174172
}
@@ -194,15 +192,17 @@ export class RippleRenderer implements EventListenerObject {
194192
const rippleEl = rippleRef.element;
195193
const animationConfig = {...defaultRippleAnimationConfig, ...rippleRef.config.animation};
196194

195+
// This starts the fade-out transition and will fire the transition end listener that
196+
// removes the ripple element from the DOM.
197197
rippleEl.style.transitionDuration = `${animationConfig.exitDuration}ms`;
198198
rippleEl.style.opacity = '0';
199199
rippleRef.state = RippleState.FADING_OUT;
200200

201-
// Once the ripple faded out, the ripple can be safely removed from the DOM.
202-
this._runTimeoutOutsideZone(() => {
203-
rippleRef.state = RippleState.HIDDEN;
204-
rippleEl.remove();
205-
}, animationConfig.exitDuration);
201+
// In case there is no fade-out transition duration, we need to manually call the
202+
// transition end listener because `transitionend` doesn't fire if there is no transition.
203+
if (!animationConfig.exitDuration) {
204+
this._finishRippleTransition(rippleRef);
205+
}
206206
}
207207

208208
/** Fades out all currently active ripples. */
@@ -256,6 +256,40 @@ export class RippleRenderer implements EventListenerObject {
256256
}
257257
}
258258

259+
/** Method that will be called if the fade-in or fade-in transition completed. */
260+
private _finishRippleTransition(rippleRef: RippleRef) {
261+
if (rippleRef.state === RippleState.FADING_IN) {
262+
this._startFadeOutTransition(rippleRef);
263+
} else if (rippleRef.state === RippleState.FADING_OUT) {
264+
this._destroyRipple(rippleRef);
265+
}
266+
}
267+
268+
/**
269+
* Starts the fade-out transition of the given ripple if it's not persistent and the pointer
270+
* is not held down anymore.
271+
*/
272+
private _startFadeOutTransition(rippleRef: RippleRef) {
273+
const isMostRecentTransientRipple = rippleRef === this._mostRecentTransientRipple;
274+
const {persistent} = rippleRef.config;
275+
276+
rippleRef.state = RippleState.VISIBLE;
277+
278+
// When the timer runs out while the user has kept their pointer down, we want to
279+
// keep only the persistent ripples and the latest transient ripple. We do this,
280+
// because we don't want stacked transient ripples to appear after their enter
281+
// animation has finished.
282+
if (!persistent && (!isMostRecentTransientRipple || !this._isPointerDown)) {
283+
rippleRef.fadeOut();
284+
}
285+
}
286+
287+
/** Destroys the given ripple by removing it from the DOM and updating its state. */
288+
private _destroyRipple(rippleRef: RippleRef) {
289+
rippleRef.state = RippleState.HIDDEN;
290+
rippleRef.element.remove();
291+
}
292+
259293
/** Function being called whenever the trigger is being pressed using mouse. */
260294
private _onMousedown(event: MouseEvent) {
261295
// Screen readers will fire fake mouse events for space/enter. Skip launching a
@@ -312,11 +346,6 @@ export class RippleRenderer implements EventListenerObject {
312346
});
313347
}
314348

315-
/** Runs a timeout outside of the Angular zone to avoid triggering the change detection. */
316-
private _runTimeoutOutsideZone(fn: Function, delay = 0) {
317-
this._ngZone.runOutsideAngular(() => setTimeout(fn, delay));
318-
}
319-
320349
/** Registers event listeners for a given list of events. */
321350
private _registerEvents(eventTypes: string[]) {
322351
this._ngZone.runOutsideAngular(() => {

0 commit comments

Comments
 (0)