From ac1280cae42a320aa69508ba1fe915d50bee4398 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Fri, 11 Sep 2020 12:22:17 +0200 Subject: [PATCH 1/3] 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 --- .../mdc-list/list.spec.ts | 24 +- .../mdc-list/selection-list.spec.ts | 12 +- .../mdc-slider/slider.spec.ts | 20 +- .../mdc-tabs/tab-group.spec.ts | 1 - src/material/core/ripple/ripple-renderer.ts | 79 ++++-- src/material/core/ripple/ripple.spec.ts | 252 +++++++++--------- src/material/list/list.spec.ts | 24 +- src/material/list/selection-list.spec.ts | 27 +- src/material/tabs/tab-group.spec.ts | 2 - 9 files changed, 247 insertions(+), 194 deletions(-) diff --git a/src/material-experimental/mdc-list/list.spec.ts b/src/material-experimental/mdc-list/list.spec.ts index 6fe496a663ec..c9382089eea4 100644 --- a/src/material-experimental/mdc-list/list.spec.ts +++ b/src/material-experimental/mdc-list/list.spec.ts @@ -1,14 +1,10 @@ -import {waitForAsync, TestBed, fakeAsync, tick} from '@angular/core/testing'; +import {fakeAsync, TestBed, waitForAsync} from '@angular/core/testing'; +import {dispatchFakeEvent, dispatchMouseEvent} from '@angular/cdk/testing/private'; import {Component, QueryList, ViewChildren} from '@angular/core'; -import {defaultRippleAnimationConfig} from '@angular/material-experimental/mdc-core'; -import {dispatchMouseEvent} from '../../cdk/testing/private'; import {By} from '@angular/platform-browser'; import {MatListItem, MatListModule} from './index'; describe('MDC-based MatList', () => { - // Default ripple durations used for testing. - const {enterDuration, exitDuration} = defaultRippleAnimationConfig; - beforeEach( waitForAsync(() => { TestBed.configureTestingModule({ @@ -243,12 +239,16 @@ describe('MDC-based MatList', () => { dispatchMouseEvent(rippleTarget, 'mousedown'); dispatchMouseEvent(rippleTarget, 'mouseup'); + // Flush the ripple enter animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to be enabled by default.') .toBe(1); - // Wait for the ripples to go away. - tick(enterDuration + exitDuration); + // Flush the ripple exit animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to go away.') .toBe(0); @@ -273,12 +273,16 @@ describe('MDC-based MatList', () => { dispatchMouseEvent(rippleTarget, 'mousedown'); dispatchMouseEvent(rippleTarget, 'mouseup'); + // Flush the ripple enter animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to be enabled by default.') .toBe(1); - // Wait for the ripples to go away. - tick(enterDuration + exitDuration); + // Flush the ripple exit animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to go away.') .toBe(0); diff --git a/src/material-experimental/mdc-list/selection-list.spec.ts b/src/material-experimental/mdc-list/selection-list.spec.ts index 66c82416d2c6..f66811c1c816 100644 --- a/src/material-experimental/mdc-list/selection-list.spec.ts +++ b/src/material-experimental/mdc-list/selection-list.spec.ts @@ -22,7 +22,7 @@ import { waitForAsync, } from '@angular/core/testing'; import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms'; -import {defaultRippleAnimationConfig, ThemePalette} from '@angular/material-experimental/mdc-core'; +import {ThemePalette} from '@angular/material-experimental/mdc-core'; import {By} from '@angular/platform-browser'; import {numbers} from '@material/list'; import { @@ -612,17 +612,19 @@ describe('MDC-based MatSelectionList without forms', () => { const rippleTarget = fixture.nativeElement.querySelector( '.mat-mdc-list-option:not(.mdc-list-item--disabled)', ); - const {enterDuration, exitDuration} = defaultRippleAnimationConfig; - dispatchMouseEvent(rippleTarget, 'mousedown'); dispatchMouseEvent(rippleTarget, 'mouseup'); + // Flush the ripple enter animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to be enabled by default.') .toBe(1); - // Wait for the ripples to go away. - tick(enterDuration + exitDuration); + // Flush the ripple exit animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to go away.') .toBe(0); diff --git a/src/material-experimental/mdc-slider/slider.spec.ts b/src/material-experimental/mdc-slider/slider.spec.ts index e0fe487d2352..2a372dcca054 100644 --- a/src/material-experimental/mdc-slider/slider.spec.ts +++ b/src/material-experimental/mdc-slider/slider.spec.ts @@ -9,19 +9,13 @@ import {BidiModule, Directionality} from '@angular/cdk/bidi'; import {Platform} from '@angular/cdk/platform'; import { + dispatchFakeEvent, dispatchMouseEvent, dispatchPointerEvent, dispatchTouchEvent, } from '../../cdk/testing/private'; import {Component, Provider, QueryList, Type, ViewChild, ViewChildren} from '@angular/core'; -import { - ComponentFixture, - fakeAsync, - flush, - TestBed, - tick, - waitForAsync, -} from '@angular/core/testing'; +import {ComponentFixture, fakeAsync, flush, TestBed, waitForAsync} from '@angular/core/testing'; import {FormControl, FormsModule, ReactiveFormsModule} from '@angular/forms'; import {By} from '@angular/platform-browser'; import {Thumb} from '@material/slider'; @@ -297,8 +291,14 @@ describe('MDC-based MatSlider', () => { ); function isRippleVisible(selector: string) { - tick(500); - return !!document.querySelector(`.mat-mdc-slider-${selector}-ripple`); + flushRippleTransitions(); + return thumbElement.querySelector(`.mat-mdc-slider-${selector}-ripple`) !== null; + } + + function flushRippleTransitions() { + thumbElement.querySelectorAll('.mat-ripple-element').forEach(el => { + dispatchFakeEvent(el, 'transitionend'); + }); } function blur() { diff --git a/src/material-experimental/mdc-tabs/tab-group.spec.ts b/src/material-experimental/mdc-tabs/tab-group.spec.ts index dacd95956c01..55ae2938fd6f 100644 --- a/src/material-experimental/mdc-tabs/tab-group.spec.ts +++ b/src/material-experimental/mdc-tabs/tab-group.spec.ts @@ -199,7 +199,6 @@ describe('MDC-based MatTabGroup', () => { .toBe(0); dispatchFakeEvent(tabLabel.nativeElement, 'mousedown'); - dispatchFakeEvent(tabLabel.nativeElement, 'mouseup'); expect(testElement.querySelectorAll('.mat-ripple-element').length) .withContext('Expected one ripple to show up on label mousedown.') diff --git a/src/material/core/ripple/ripple-renderer.ts b/src/material/core/ripple/ripple-renderer.ts index 8887d83b4d96..9106f4d09c12 100644 --- a/src/material/core/ripple/ripple-renderer.ts +++ b/src/material/core/ripple/ripple-renderer.ts @@ -154,21 +154,19 @@ export class RippleRenderer implements EventListenerObject { this._mostRecentTransientRipple = 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(() => { - const isMostRecentTransientRipple = rippleRef === this._mostRecentTransientRipple; - - rippleRef.state = RippleState.VISIBLE; - - // When the timer runs out while the user has kept their pointer down, we want to - // keep only the persistent ripples and the latest transient ripple. We do this, - // because we don't want stacked transient ripples to appear after their enter - // animation has finished. - if (!config.persistent && (!isMostRecentTransientRipple || !this._isPointerDown)) { - rippleRef.fadeOut(); - } - }, duration); + // Do not register the `transition` event listener if fade-in and fade-out duration + // are set to zero. The events won't fire anyway and we can save resources here. + if (duration || animationConfig.exitDuration) { + this._ngZone.runOutsideAngular(() => { + ripple.addEventListener('transitionend', () => this._finishRippleTransition(rippleRef)); + }); + } + + // In case there is no fade-in transition duration, we need to manually call the transition + // end listener because `transitionend` doesn't fire if there is no transition. + if (!duration) { + this._finishRippleTransition(rippleRef); + } return rippleRef; } @@ -194,15 +192,17 @@ export class RippleRenderer implements EventListenerObject { const rippleEl = rippleRef.element; const animationConfig = {...defaultRippleAnimationConfig, ...rippleRef.config.animation}; + // This starts the fade-out transition and will fire the transition end listener that + // removes the ripple element from the DOM. rippleEl.style.transitionDuration = `${animationConfig.exitDuration}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.remove(); - }, animationConfig.exitDuration); + // In case there is no fade-out transition duration, we need to manually call the + // transition end listener because `transitionend` doesn't fire if there is no transition. + if (!animationConfig.exitDuration) { + this._finishRippleTransition(rippleRef); + } } /** Fades out all currently active ripples. */ @@ -256,6 +256,40 @@ export class RippleRenderer implements EventListenerObject { } } + /** Method that will be called if the fade-in or fade-in transition completed. */ + private _finishRippleTransition(rippleRef: RippleRef) { + if (rippleRef.state === RippleState.FADING_IN) { + this._startFadeOutTransition(rippleRef); + } else if (rippleRef.state === RippleState.FADING_OUT) { + this._destroyRipple(rippleRef); + } + } + + /** + * Starts the fade-out transition of the given ripple if it's not persistent and the pointer + * is not held down anymore. + */ + private _startFadeOutTransition(rippleRef: RippleRef) { + const isMostRecentTransientRipple = rippleRef === this._mostRecentTransientRipple; + const {persistent} = rippleRef.config; + + rippleRef.state = RippleState.VISIBLE; + + // When the timer runs out while the user has kept their pointer down, we want to + // keep only the persistent ripples and the latest transient ripple. We do this, + // because we don't want stacked transient ripples to appear after their enter + // animation has finished. + if (!persistent && (!isMostRecentTransientRipple || !this._isPointerDown)) { + rippleRef.fadeOut(); + } + } + + /** Destroys the given ripple by removing it from the DOM and updating its state. */ + private _destroyRipple(rippleRef: RippleRef) { + rippleRef.state = RippleState.HIDDEN; + rippleRef.element.remove(); + } + /** Function being called whenever the trigger is being pressed using mouse. */ private _onMousedown(event: MouseEvent) { // Screen readers will fire fake mouse events for space/enter. Skip launching a @@ -312,11 +346,6 @@ export class RippleRenderer implements EventListenerObject { }); } - /** Runs a timeout outside of the Angular zone to avoid triggering the change detection. */ - private _runTimeoutOutsideZone(fn: Function, delay = 0) { - this._ngZone.runOutsideAngular(() => setTimeout(fn, delay)); - } - /** Registers event listeners for a given list of events. */ private _registerEvents(eventTypes: string[]) { this._ngZone.runOutsideAngular(() => { diff --git a/src/material/core/ripple/ripple.spec.ts b/src/material/core/ripple/ripple.spec.ts index 0ea9e63a64e9..62720eb2874f 100644 --- a/src/material/core/ripple/ripple.spec.ts +++ b/src/material/core/ripple/ripple.spec.ts @@ -1,26 +1,23 @@ -import {TestBed, ComponentFixture, fakeAsync, tick, inject} from '@angular/core/testing'; -import {Component, ViewChild} from '@angular/core'; import {Platform} from '@angular/cdk/platform'; import { - dispatchEvent, + createMouseEvent, createTouchEvent, + dispatchEvent, + dispatchFakeEvent, dispatchMouseEvent, dispatchTouchEvent, - createMouseEvent, -} from '../../../cdk/testing/private'; -import {defaultRippleAnimationConfig} from './ripple-renderer'; +} from '@angular/cdk/testing/private'; +import {Component, ViewChild} from '@angular/core'; +import {ComponentFixture, inject, TestBed} from '@angular/core/testing'; +import {NoopAnimationsModule} from '@angular/platform-browser/animations'; import { + MAT_RIPPLE_GLOBAL_OPTIONS, MatRipple, MatRippleModule, - MAT_RIPPLE_GLOBAL_OPTIONS, - RippleState, - RippleGlobalOptions, RippleAnimationConfig, + RippleGlobalOptions, + RippleState, } from './index'; -import {NoopAnimationsModule} from '@angular/platform-browser/animations'; - -/** Shorthands for the enter and exit duration of ripples. */ -const {enterDuration, exitDuration} = defaultRippleAnimationConfig; describe('MatRipple', () => { let fixture: ComponentFixture; @@ -33,6 +30,11 @@ describe('MatRipple', () => { const startingWindowWidth = window.innerWidth; const startingWindowHeight = window.innerHeight; + /** Flushes the transition of the ripple element inside of the ripple target. */ + function flushTransition() { + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + } + beforeEach(() => { TestBed.configureTestingModule({ imports: [MatRippleModule], @@ -118,32 +120,33 @@ describe('MatRipple', () => { expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(2); }); - it('should launch ripples on touchstart', fakeAsync(() => { + it('should launch ripples on touchstart', () => { dispatchTouchEvent(rippleTarget, 'touchstart'); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - tick(enterDuration); + flushTransition(); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); dispatchTouchEvent(rippleTarget, 'touchend'); - - tick(exitDuration); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - })); + }); - it('should clear ripples if the touch sequence is cancelled', fakeAsync(() => { + it('should clear ripples if the touch sequence is cancelled', () => { dispatchTouchEvent(rippleTarget, 'touchstart'); - tick(enterDuration); + flushTransition(); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); dispatchTouchEvent(rippleTarget, 'touchcancel'); - tick(exitDuration); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - })); + }); - it('should launch multiple ripples for multi-touch', fakeAsync(() => { + it('should launch multiple ripples for multi-touch', () => { const touchEvent = createTouchEvent('touchstart'); Object.defineProperties(touchEvent, { @@ -157,86 +160,81 @@ describe('MatRipple', () => { }); dispatchEvent(rippleTarget, touchEvent); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(3); - tick(enterDuration); + const rippleElements = rippleTarget.querySelectorAll('.mat-ripple-element'); + + // Flush the fade-in transition of all three ripples. + dispatchFakeEvent(rippleElements[0], 'transitionend'); + dispatchFakeEvent(rippleElements[1], 'transitionend'); + dispatchFakeEvent(rippleElements[2], 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(3); dispatchTouchEvent(rippleTarget, 'touchend'); - tick(exitDuration); + // Flush the fade-out transition of all three ripples. + dispatchFakeEvent(rippleElements[0], 'transitionend'); + dispatchFakeEvent(rippleElements[1], 'transitionend'); + dispatchFakeEvent(rippleElements[2], 'transitionend'); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - })); + }); - it('should ignore synthetic mouse events after touchstart', () => - fakeAsync(() => { - dispatchTouchEvent(rippleTarget, 'touchstart'); - dispatchTouchEvent(rippleTarget, 'mousedown'); + it('should ignore synthetic mouse events after touchstart', () => { + dispatchTouchEvent(rippleTarget, 'touchstart'); + dispatchTouchEvent(rippleTarget, 'mousedown'); - tick(enterDuration); - expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); + flushTransition(); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - dispatchTouchEvent(rippleTarget, 'touchend'); + dispatchTouchEvent(rippleTarget, 'touchend'); - tick(exitDuration); + flushTransition(); - expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - })); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); + }); - it('should ignore fake mouse events from screen readers', fakeAsync(() => { + it('should ignore fake mouse events from screen readers', () => { const event = createMouseEvent('mousedown'); Object.defineProperties(event, {offsetX: {get: () => 0}, offsetY: {get: () => 0}}); dispatchEvent(rippleTarget, event); - tick(enterDuration); + expect(rippleTarget.querySelector('.mat-ripple-element')).toBeFalsy(); - })); + }); - it('removes ripple after timeout', fakeAsync(() => { + it('removes ripple after timeout', () => { dispatchMouseEvent(rippleTarget, 'mousedown'); dispatchMouseEvent(rippleTarget, 'mouseup'); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - // Calculates the duration for fading-in and fading-out the ripple. - tick(enterDuration + exitDuration); + // Flush fade-in and fade-out transition. + flushTransition(); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - })); + }); - it('should remove ripples after mouseup', fakeAsync(() => { + it('should remove ripples after mouseup', () => { 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(enterDuration + exitDuration); + // Flush the transition of fading in. Also flush the potential fading-out transition in + // order to make sure that the ripples didn't fade-out before mouseup. + flushTransition(); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); dispatchMouseEvent(rippleTarget, 'mouseup'); - tick(exitDuration); - - 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 = enterDuration + exitDuration; - - 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); + flushTransition(); - 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); @@ -294,17 +292,20 @@ describe('MatRipple', () => { subscription.unsubscribe(); }); - it('should only persist the latest ripple on pointer down', fakeAsync(() => { + it('should only persist the latest ripple on pointer down', () => { dispatchMouseEvent(rippleTarget, 'mousedown'); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); dispatchMouseEvent(rippleTarget, 'mousedown'); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(2); - tick(enterDuration + exitDuration); + // Flush the fade-in transition. + flushTransition(); + // Flush the fade-out transition. + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - })); + }); describe('when page is scrolled', () => { let veryLargeElement: HTMLDivElement = document.createElement('div'); @@ -392,45 +393,45 @@ describe('MatRipple', () => { rippleDirective = fixture.componentInstance.ripple; }); - it('should allow persistent ripple elements', fakeAsync(() => { + it('should allow persistent ripple elements', () => { expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); let rippleRef = rippleDirective.launch(0, 0, {persistent: true}); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - // Calculates the duration for fading-in and fading-out the ripple. Also adds some - // extra time to demonstrate that the ripples are persistent. - tick(enterDuration + exitDuration + 5000); + // Flush the fade-in transition. Additionally flush the potential fade-out transition + // in order to make sure that the ripple is persistent and won't fade-out. + flushTransition(); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); rippleRef.fadeOut(); - - tick(exitDuration); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - })); + }); - it('should remove ripples that are not done fading-in', fakeAsync(() => { + it('should remove ripples that are not done fading in', () => { expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); rippleDirective.launch(0, 0); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - tick(enterDuration / 2); - + // The ripple should still fade in right now. Now by calling `fadeOutAll` the ripple should + // immediately start fading out. We can verify this by just flushing the current transition + // and verifying if the ripple has been removed from the DOM. rippleDirective.fadeOutAll(); - - tick(exitDuration); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected no ripples to be active after calling fadeOutAll.') .toBe(0); - })); + }); - it('should properly set ripple states', fakeAsync(() => { + it('should properly set ripple states', () => { expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); let rippleRef = rippleDirective.launch(0, 0, {persistent: true}); @@ -438,7 +439,7 @@ describe('MatRipple', () => { expect(rippleRef.state).toBe(RippleState.FADING_IN); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - tick(enterDuration); + flushTransition(); expect(rippleRef.state).toBe(RippleState.VISIBLE); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); @@ -448,43 +449,51 @@ describe('MatRipple', () => { expect(rippleRef.state).toBe(RippleState.FADING_OUT); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - tick(exitDuration); + flushTransition(); expect(rippleRef.state).toBe(RippleState.HIDDEN); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - })); + }); - it('should allow setting a specific animation config for a ripple', fakeAsync(() => { + it('should allow setting a specific animation config for a ripple', () => { expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - rippleDirective.launch(0, 0, { + const rippleRef = rippleDirective.launch(0, 0, { animation: {enterDuration: 120, exitDuration: 0}, }); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - tick(120); + // Since we cannot use `fakeAsync`, we manually verify that the element has + // the specified transition duration. + expect(rippleRef.element.style.transitionDuration).toBe('120ms'); + + // We still flush the 120ms transition and should check if the 0ms exit transition happened + // properly. + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - })); + }); - it('should allow passing only a configuration', fakeAsync(() => { + it('should allow passing only a configuration', () => { expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); const rippleRef = rippleDirective.launch({persistent: true}); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - tick(enterDuration + exitDuration); + // Flush the fade-in transition. Additionally flush the potential fade-out transition + // in order to make sure that the ripple is persistent and won't fade-out. + flushTransition(); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); rippleRef.fadeOut(); - - tick(exitDuration); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - })); + }); }); describe('global ripple options', () => { @@ -549,22 +558,25 @@ describe('MatRipple', () => { expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); }); - it('should support changing the animation duration', fakeAsync(() => { + it('should support changing the animation duration', () => { createTestComponent({ - animation: {enterDuration: 100, exitDuration: 100}, + animation: {enterDuration: 100, exitDuration: 150}, }); - rippleDirective.launch(0, 0); + const rippleRef = rippleDirective.launch(0, 0); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - // Wait the 200ms of the enter duration and exit duration. - tick(100 + 100); + expect(rippleRef.element.style.transitionDuration).toBe('100ms'); + flushTransition(); + + expect(rippleRef.element.style.transitionDuration).toBe('150ms'); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - })); + }); - it('should allow ripples to fade out immediately on pointer up', fakeAsync(() => { + it('should allow ripples to fade out immediately on pointer up', () => { createTestComponent({ terminateOnPointerUp: true, }); @@ -574,16 +586,13 @@ describe('MatRipple', () => { expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - // Ignore the enter duration, because we immediately fired the mouseup after the mousedown. - // This means that the ripple should just fade out, and there shouldn't be an enter animation. - tick(exitDuration); + // Just flush the fade-out duration because we immediately fired the mouseup after the + // mousedown. This means that the ripple should just fade out, and there shouldn't be an + // enter animation. + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - - // Since the enter duration is bigger than the exit duration, the enter duration timer - // will still exist. To properly finish all timers, we just wait the remaining time. - tick(enterDuration - exitDuration); - })); + }); it('should not mutate the global options when NoopAnimationsModule is present', () => { const options: RippleGlobalOptions = {}; @@ -659,11 +668,11 @@ describe('MatRipple', () => { expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); }); - it('fades out non-persistent ripples when disabled input is set', fakeAsync(() => { + it('fades out non-persistent ripples when disabled input is set', () => { dispatchMouseEvent(rippleTarget, 'mousedown'); controller.ripple.launch(0, 0, {persistent: true}); - tick(enterDuration); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(2); spyOn(controller.ripple, 'fadeOutAllNonPersistent').and.callThrough(); @@ -672,9 +681,9 @@ describe('MatRipple', () => { expect(controller.ripple.fadeOutAllNonPersistent).toHaveBeenCalled(); - tick(exitDuration); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - })); + }); it('allows specifying custom trigger element', () => { let alternateTrigger = fixture.debugElement.nativeElement.querySelector( @@ -744,8 +753,8 @@ describe('MatRipple', () => { expect(pxStringToFloat(ripple.style.height)).toBeCloseTo(2 * customRadius, 1); }); - it('should be able to specify animation config through binding', fakeAsync(() => { - controller.animationConfig = {enterDuration: 150, exitDuration: 150}; + it('should be able to specify animation config through binding', () => { + controller.animationConfig = {enterDuration: 120, exitDuration: 150}; fixture.detectChanges(); dispatchMouseEvent(rippleTarget, 'mousedown'); @@ -753,10 +762,16 @@ describe('MatRipple', () => { expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); - tick(150 + 150); + const rippleElement = rippleTarget.querySelector('.mat-ripple-element')! as HTMLElement; + + expect(rippleElement.style.transitionDuration).toBe('120ms'); + flushTransition(); + + expect(rippleElement.style.transitionDuration).toBe('150ms'); + flushTransition(); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); - })); + }); }); }); @@ -801,8 +816,7 @@ class RippleContainerWithInputBindings { class RippleContainerWithoutBindings {} @Component({ - template: `
`, + template: `
`, }) class RippleContainerWithNgIf { @ViewChild(MatRipple) ripple: MatRipple; diff --git a/src/material/list/list.spec.ts b/src/material/list/list.spec.ts index 0d85de214d98..97e673617d57 100644 --- a/src/material/list/list.spec.ts +++ b/src/material/list/list.spec.ts @@ -1,14 +1,10 @@ -import {waitForAsync, TestBed, fakeAsync, tick} from '@angular/core/testing'; +import {fakeAsync, TestBed, waitForAsync} from '@angular/core/testing'; +import {dispatchFakeEvent, dispatchMouseEvent} from '@angular/cdk/testing/private'; import {Component, QueryList, ViewChildren} from '@angular/core'; -import {defaultRippleAnimationConfig} from '@angular/material/core'; -import {dispatchMouseEvent} from '../../cdk/testing/private'; import {By} from '@angular/platform-browser'; import {MatListItem, MatListModule} from './index'; describe('MatList', () => { - // Default ripple durations used for testing. - const {enterDuration, exitDuration} = defaultRippleAnimationConfig; - beforeEach( waitForAsync(() => { TestBed.configureTestingModule({ @@ -239,12 +235,16 @@ describe('MatList', () => { dispatchMouseEvent(rippleTarget, 'mousedown'); dispatchMouseEvent(rippleTarget, 'mouseup'); + // Flush the ripple enter animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to be enabled by default.') .toBe(1); - // Wait for the ripples to go away. - tick(enterDuration + exitDuration); + // Flush the ripple exit animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to go away.') .toBe(0); @@ -269,12 +269,16 @@ describe('MatList', () => { dispatchMouseEvent(rippleTarget, 'mousedown'); dispatchMouseEvent(rippleTarget, 'mouseup'); + // Flush the ripple enter animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to be enabled by default.') .toBe(1); - // Wait for the ripples to go away. - tick(enterDuration + exitDuration); + // Flush the ripple exit animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to go away.') .toBe(0); diff --git a/src/material/list/selection-list.spec.ts b/src/material/list/selection-list.spec.ts index 99b05a308cbf..683868a142f4 100644 --- a/src/material/list/selection-list.spec.ts +++ b/src/material/list/selection-list.spec.ts @@ -1,32 +1,32 @@ -import {DOWN_ARROW, SPACE, ENTER, UP_ARROW, HOME, END, A, D, TAB} from '@angular/cdk/keycodes'; +import {FocusMonitor} from '@angular/cdk/a11y'; +import {A, D, DOWN_ARROW, END, ENTER, HOME, SPACE, TAB, UP_ARROW} from '@angular/cdk/keycodes'; import { createKeyboardEvent, - dispatchFakeEvent, dispatchEvent, + dispatchFakeEvent, dispatchKeyboardEvent, dispatchMouseEvent, } from '../../cdk/testing/private'; import { + ChangeDetectionStrategy, Component, DebugElement, - ChangeDetectionStrategy, QueryList, ViewChildren, } from '@angular/core'; import { - waitForAsync, ComponentFixture, fakeAsync, - TestBed, - tick, flush, inject, + TestBed, + tick, + waitForAsync, } from '@angular/core/testing'; -import {MatRipple, defaultRippleAnimationConfig, ThemePalette} from '@angular/material/core'; +import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms'; +import {MatRipple, ThemePalette} from '@angular/material/core'; import {By} from '@angular/platform-browser'; import {MatListModule, MatListOption, MatSelectionList, MatSelectionListChange} from './index'; -import {FormControl, FormsModule, NgModel, ReactiveFormsModule} from '@angular/forms'; -import {FocusMonitor} from '@angular/cdk/a11y'; describe('MatSelectionList without forms', () => { describe('with list option', () => { @@ -770,17 +770,20 @@ describe('MatSelectionList without forms', () => { const rippleTarget = fixture.nativeElement.querySelector( '.mat-list-option:not(.mat-list-item-disabled) .mat-list-item-content', ); - const {enterDuration, exitDuration} = defaultRippleAnimationConfig; dispatchMouseEvent(rippleTarget, 'mousedown'); dispatchMouseEvent(rippleTarget, 'mouseup'); + // Flush the ripple enter animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to be enabled by default.') .toBe(1); - // Wait for the ripples to go away. - tick(enterDuration + exitDuration); + // Flush the ripple exit animation. + dispatchFakeEvent(rippleTarget.querySelector('.mat-ripple-element')!, 'transitionend'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length) .withContext('Expected ripples to go away.') .toBe(0); diff --git a/src/material/tabs/tab-group.spec.ts b/src/material/tabs/tab-group.spec.ts index 1be121f13985..911a2e4dd24c 100644 --- a/src/material/tabs/tab-group.spec.ts +++ b/src/material/tabs/tab-group.spec.ts @@ -198,7 +198,6 @@ describe('MatTabGroup', () => { .toBe(0); dispatchFakeEvent(tabLabel.nativeElement, 'mousedown'); - dispatchFakeEvent(tabLabel.nativeElement, 'mouseup'); expect(testElement.querySelectorAll('.mat-ripple-element').length) .withContext('Expected one ripple to show up on label mousedown.') @@ -217,7 +216,6 @@ describe('MatTabGroup', () => { .toBe(0); dispatchFakeEvent(tabLabel.nativeElement, 'mousedown'); - dispatchFakeEvent(tabLabel.nativeElement, 'mouseup'); expect(testElement.querySelectorAll('.mat-ripple-element').length) .withContext('Expected no ripple to show up on label mousedown.') From 0b3cf774a1efedb7b3a5402ed0b6cb5794fd5e2d Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 22 Feb 2022 21:53:41 +0100 Subject: [PATCH 2/3] fixup! fix(material/core): ripples not fading out on touch devices when scrolling Backwards compatibility change for g3 tests using just transition: none without the noopanimations module --- src/material/core/ripple/ripple-ref.ts | 2 ++ src/material/core/ripple/ripple-renderer.ts | 40 +++++++++++---------- src/material/core/ripple/ripple.spec.ts | 25 ++++++++++++- 3 files changed, 47 insertions(+), 20 deletions(-) diff --git a/src/material/core/ripple/ripple-ref.ts b/src/material/core/ripple/ripple-ref.ts index 562eb15c000d..38887a71a4aa 100644 --- a/src/material/core/ripple/ripple-ref.ts +++ b/src/material/core/ripple/ripple-ref.ts @@ -47,6 +47,8 @@ export class RippleRef { public element: HTMLElement, /** Ripple configuration used for the ripple. */ public config: RippleConfig, + /* Whether animations are forcibly disabled for ripples through CSS. */ + public _animationForciblyDisabledThroughCss = false, ) {} /** Fades out the ripple element. */ diff --git a/src/material/core/ripple/ripple-renderer.ts b/src/material/core/ripple/ripple-renderer.ts index 9106f4d09c12..0f0fe4a559bb 100644 --- a/src/material/core/ripple/ripple-renderer.ts +++ b/src/material/core/ripple/ripple-renderer.ts @@ -114,7 +114,7 @@ export class RippleRenderer implements EventListenerObject { const radius = config.radius || distanceToFurthestCorner(x, y, containerRect); const offsetX = x - containerRect.left; const offsetY = y - containerRect.top; - const duration = animationConfig.enterDuration; + const enterDuration = animationConfig.enterDuration; const ripple = document.createElement('div'); ripple.classList.add('mat-ripple-element'); @@ -130,20 +130,30 @@ export class RippleRenderer implements EventListenerObject { ripple.style.backgroundColor = config.color; } - ripple.style.transitionDuration = `${duration}ms`; + ripple.style.transitionDuration = `${enterDuration}ms`; this._containerElement.appendChild(ripple); // By default the browser does not recalculate the styles of dynamically created - // ripple elements. This is critical because then the `scale` would not animate properly. - enforceStyleRecalculation(ripple); + // ripple elements. This is critical to ensure that the `scale` animates properly. + // We enforce a style recalculation by calling `getComputedStyle` and *accessing* a property. + // See: https://gist.github.com/paulirish/5d52fb081b3570c81e3a + const computedStyles = window.getComputedStyle(ripple); + const transitionProperty = computedStyles.transitionProperty; - // We use a 3d transform here in order to avoid an issue in Safari where - // the ripples aren't clipped when inside the shadow DOM (see #24028). - ripple.style.transform = 'scale3d(1, 1, 1)'; + // Note: We detect whether animation is forcibly disabled through CSS by the use + // of `transition: none`. This is technically unexpected since animations are + // controlled through the animation config, but this exists for backwards compatibility + const animationForciblyDisabledThroughCss = transitionProperty === 'none'; // Exposed reference to the ripple that will be returned. - const rippleRef = new RippleRef(this, ripple, config); + const rippleRef = new RippleRef(this, ripple, config, animationForciblyDisabledThroughCss); + + // Start the enter animation by setting the transform/scale to 100%. The animation will + // execute as part of this statement because we forced a style recalculation before. + // Note: We use a 3d transform here in order to avoid an issue in Safari where + // the ripples aren't clipped when inside the shadow DOM (see #24028). + ripple.style.transform = 'scale3d(1, 1, 1)'; rippleRef.state = RippleState.FADING_IN; @@ -156,7 +166,7 @@ export class RippleRenderer implements EventListenerObject { // Do not register the `transition` event listener if fade-in and fade-out duration // are set to zero. The events won't fire anyway and we can save resources here. - if (duration || animationConfig.exitDuration) { + if (!animationForciblyDisabledThroughCss && (enterDuration || animationConfig.exitDuration)) { this._ngZone.runOutsideAngular(() => { ripple.addEventListener('transitionend', () => this._finishRippleTransition(rippleRef)); }); @@ -164,7 +174,7 @@ export class RippleRenderer implements EventListenerObject { // In case there is no fade-in transition duration, we need to manually call the transition // end listener because `transitionend` doesn't fire if there is no transition. - if (!duration) { + if (animationForciblyDisabledThroughCss || !enterDuration) { this._finishRippleTransition(rippleRef); } @@ -200,7 +210,7 @@ export class RippleRenderer implements EventListenerObject { // In case there is no fade-out transition duration, we need to manually call the // transition end listener because `transitionend` doesn't fire if there is no transition. - if (!animationConfig.exitDuration) { + if (rippleRef._animationForciblyDisabledThroughCss || !animationConfig.exitDuration) { this._finishRippleTransition(rippleRef); } } @@ -371,14 +381,6 @@ export class RippleRenderer implements EventListenerObject { } } -/** Enforces a style recalculation of a DOM element by computing its styles. */ -function enforceStyleRecalculation(element: HTMLElement) { - // Enforce a style recalculation by calling `getComputedStyle` and accessing any property. - // Calling `getPropertyValue` is important to let optimizers know that this is not a noop. - // See: https://gist.github.com/paulirish/5d52fb081b3570c81e3a - window.getComputedStyle(element).getPropertyValue('opacity'); -} - /** * Returns the distance from the point (x, y) to the furthest corner of a rectangle. */ diff --git a/src/material/core/ripple/ripple.spec.ts b/src/material/core/ripple/ripple.spec.ts index 62720eb2874f..033f5a48de6b 100644 --- a/src/material/core/ripple/ripple.spec.ts +++ b/src/material/core/ripple/ripple.spec.ts @@ -7,7 +7,7 @@ import { dispatchMouseEvent, dispatchTouchEvent, } from '@angular/cdk/testing/private'; -import {Component, ViewChild} from '@angular/core'; +import {Component, ViewChild, ViewEncapsulation} from '@angular/core'; import {ComponentFixture, inject, TestBed} from '@angular/core/testing'; import {NoopAnimationsModule} from '@angular/platform-browser/animations'; import { @@ -43,6 +43,7 @@ describe('MatRipple', () => { RippleContainerWithInputBindings, RippleContainerWithoutBindings, RippleContainerWithNgIf, + RippleCssTransitionNone, ], }); }); @@ -773,6 +774,21 @@ describe('MatRipple', () => { expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); }); }); + + describe('edge cases', () => { + it('should handle forcibly disabled animations through CSS `transition: none`', async () => { + fixture = TestBed.createComponent(RippleCssTransitionNone); + fixture.detectChanges(); + + rippleTarget = fixture.nativeElement.querySelector('.mat-ripple'); + + dispatchMouseEvent(rippleTarget, 'mousedown'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(1); + + dispatchMouseEvent(rippleTarget, 'mouseup'); + expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); + }); + }); }); @Component({ @@ -822,3 +838,10 @@ class RippleContainerWithNgIf { @ViewChild(MatRipple) ripple: MatRipple; isDestroyed = false; } + +@Component({ + styles: [`* { transition: none !important; }`], + template: `
`, + encapsulation: ViewEncapsulation.None, +}) +class RippleCssTransitionNone {} From cbc5f7c46800842ffcc8c3bad0ff59398ee4e008 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 22 Feb 2022 22:09:00 +0100 Subject: [PATCH 3/3] fixup! fix(material/core): ripples not fading out on touch devices when scrolling Support transition-duration --- src/material/core/ripple/ripple-renderer.ts | 19 +++++++++++++------ src/material/core/ripple/ripple.spec.ts | 21 +++++++++++++++++++++ tools/public_api_guard/material/core.md | 4 +++- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/material/core/ripple/ripple-renderer.ts b/src/material/core/ripple/ripple-renderer.ts index 0f0fe4a559bb..18b21d64c5ff 100644 --- a/src/material/core/ripple/ripple-renderer.ts +++ b/src/material/core/ripple/ripple-renderer.ts @@ -139,12 +139,19 @@ export class RippleRenderer implements EventListenerObject { // We enforce a style recalculation by calling `getComputedStyle` and *accessing* a property. // See: https://gist.github.com/paulirish/5d52fb081b3570c81e3a const computedStyles = window.getComputedStyle(ripple); - const transitionProperty = computedStyles.transitionProperty; - - // Note: We detect whether animation is forcibly disabled through CSS by the use - // of `transition: none`. This is technically unexpected since animations are - // controlled through the animation config, but this exists for backwards compatibility - const animationForciblyDisabledThroughCss = transitionProperty === 'none'; + const userTransitionProperty = computedStyles.transitionProperty; + const userTransitionDuration = computedStyles.transitionDuration; + + // Note: We detect whether animation is forcibly disabled through CSS by the use of + // `transition: none`. This is technically unexpected since animations are controlled + // through the animation config, but this exists for backwards compatibility. This logic does + // not need to be super accurate since it covers some edge cases which can be easily avoided by users. + const animationForciblyDisabledThroughCss = + userTransitionProperty === 'none' || + // Note: The canonical unit for serialized CSS `