From 65fb5f44911b3839c1f40ab87cf380381e030434 Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Wed, 23 Feb 2022 18:44:30 +0100 Subject: [PATCH] fix(ripple): not fading out on touch devices (#12488) * 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 * 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 * fixup! fix(material/core): ripples not fading out on touch devices when scrolling Support transition-duration --- .../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-ref.ts | 2 + src/material/core/ripple/ripple-renderer.ts | 118 ++++--- src/material/core/ripple/ripple.spec.ts | 296 +++++++++++------- src/material/list/list.spec.ts | 24 +- src/material/list/selection-list.spec.ts | 27 +- src/material/tabs/tab-group.spec.ts | 2 - tools/public_api_guard/material/core.md | 4 +- 11 files changed, 320 insertions(+), 210 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-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 8887d83b4d96..18b21d64c5ff 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,21 +130,38 @@ 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 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 `