Skip to content

Commit e9c5d2b

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 dad0ed0 commit e9c5d2b

File tree

5 files changed

+232
-174
lines changed

5 files changed

+232
-174
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
@@ -171,21 +171,19 @@ export class RippleRenderer {
171171
this._mostRecentTransientRipple = rippleRef;
172172
}
173173

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

190188
return rippleRef;
191189
}
@@ -211,15 +209,17 @@ export class RippleRenderer {
211209
const rippleEl = rippleRef.element;
212210
const animationConfig = {...defaultRippleAnimationConfig, ...rippleRef.config.animation};
213211

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

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

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

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

298-
/** Runs a timeout outside of the Angular zone to avoid triggering the change detection. */
299-
private runTimeoutOutsideZone(fn: Function, delay = 0) {
300-
this._ngZone.runOutsideAngular(() => setTimeout(fn, delay));
301-
}
302-
303331
/** Removes previously registered event listeners from the trigger element. */
304332
_removeTriggerEvents() {
305333
if (this._triggerElement) {

0 commit comments

Comments
 (0)