diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index a24c2fd7ba72..61f90aa95b01 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -265,6 +265,7 @@ /src/e2e-app/progress-bar/** @andrewseguin @crisbeto /src/e2e-app/progress-spinner/** @andrewseguin @crisbeto /src/e2e-app/radio/** @andrewseguin @devversion +/src/e2e-app/select/** @crisbeto /src/e2e-app/sidenav/** @mmalerba /src/e2e-app/slide-toggle/** @devversion /src/e2e-app/stepper/** @mmalerba diff --git a/src/e2e-app/BUILD.bazel b/src/e2e-app/BUILD.bazel index 1e1e23876925..a83c64265888 100644 --- a/src/e2e-app/BUILD.bazel +++ b/src/e2e-app/BUILD.bazel @@ -70,6 +70,7 @@ ng_module( "//src/material/progress-bar", "//src/material/progress-spinner", "//src/material/radio", + "//src/material/select", "//src/material/sidenav", "//src/material/slide-toggle", "//src/material/tabs", diff --git a/src/e2e-app/e2e-app/e2e-app-layout.ts b/src/e2e-app/e2e-app/e2e-app-layout.ts index 8fc83ee7824f..37e072d32883 100644 --- a/src/e2e-app/e2e-app/e2e-app-layout.ts +++ b/src/e2e-app/e2e-app/e2e-app-layout.ts @@ -27,6 +27,7 @@ export class E2eAppLayout { {path: 'progress-bar', title: 'Progress bar'}, {path: 'progress-spinner', title: 'Progress Spinner'}, {path: 'radio', title: 'Radios'}, + {path: 'select', title: 'Select'}, {path: 'sidenav', title: 'Sidenav'}, {path: 'slide-toggle', title: 'Slide Toggle'}, {path: 'stepper', title: 'Stepper'}, diff --git a/src/e2e-app/main-module.ts b/src/e2e-app/main-module.ts index 3aca19b85dd5..c7d44b05b0e9 100644 --- a/src/e2e-app/main-module.ts +++ b/src/e2e-app/main-module.ts @@ -1,6 +1,6 @@ import {NgModule} from '@angular/core'; import {BrowserModule} from '@angular/platform-browser'; -import {NoopAnimationsModule} from '@angular/platform-browser/animations'; +import {BrowserAnimationsModule} from '@angular/platform-browser/animations'; import {RouterModule} from '@angular/router'; import {BlockScrollStrategyE2eModule} from './block-scroll-strategy/block-scroll-strategy-e2e-module'; import {ButtonToggleE2eModule} from './button-toggle/button-toggle-e2e-module'; @@ -41,11 +41,14 @@ import {VirtualScrollE2eModule} from './virtual-scroll/virtual-scroll-e2e-module import {MdcProgressBarE2eModule} from './mdc-progress-bar/mdc-progress-bar-e2e-module'; import {MdcProgressSpinnerE2eModule} from './mdc-progress-spinner/mdc-progress-spinner-module'; +/** We allow for animations to be explicitly enabled in certain e2e tests. */ +const enableAnimations = window.location.search.includes('animations=true'); + @NgModule({ imports: [ BrowserModule, E2eAppModule, - NoopAnimationsModule, + BrowserAnimationsModule.withConfig({disableAnimations: !enableAnimations}), RouterModule.forRoot(E2E_APP_ROUTES), // E2E demos diff --git a/src/e2e-app/routes.ts b/src/e2e-app/routes.ts index 17198e3ddd1d..6b4a0bbbc6f4 100644 --- a/src/e2e-app/routes.ts +++ b/src/e2e-app/routes.ts @@ -36,6 +36,7 @@ import {BasicTabs} from './tabs/tabs-e2e'; import {ToolbarE2e} from './toolbar/toolbar-e2e'; import {VirtualScrollE2E} from './virtual-scroll/virtual-scroll-e2e'; import {Home} from './e2e-app/e2e-app-layout'; +import {SelectE2e} from './select/select-e2e'; export const E2E_APP_ROUTES: Routes = [ {path: '', component: Home}, @@ -70,6 +71,7 @@ export const E2E_APP_ROUTES: Routes = [ {path: 'progress-spinner', component: ProgressSpinnerE2E}, {path: 'radio', component: SimpleRadioButtons}, {path: 'sidenav', component: SidenavE2E}, + {path: 'select', component: SelectE2e}, {path: 'slide-toggle', component: SlideToggleE2E}, {path: 'stepper', component: StepperE2e}, {path: 'tabs', component: BasicTabs}, diff --git a/src/e2e-app/select/select-e2e-module.ts b/src/e2e-app/select/select-e2e-module.ts new file mode 100644 index 000000000000..312de216486d --- /dev/null +++ b/src/e2e-app/select/select-e2e-module.ts @@ -0,0 +1,17 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {NgModule} from '@angular/core'; +import {ExampleViewerModule} from '../example-viewer/example-viewer-module'; +import {SelectE2e} from './select-e2e'; + +@NgModule({ + imports: [ExampleViewerModule], + declarations: [SelectE2e], +}) +export class SelectE2eModule {} diff --git a/src/e2e-app/select/select-e2e.ts b/src/e2e-app/select/select-e2e.ts new file mode 100644 index 000000000000..3ce153a17bc3 --- /dev/null +++ b/src/e2e-app/select/select-e2e.ts @@ -0,0 +1,17 @@ +/** + * @license + * Copyright Google LLC All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {Component} from '@angular/core'; + +@Component({ + selector: 'select-demo', + template: ``, +}) +export class SelectE2e { + examples = ['select-overview']; +} diff --git a/src/material/core/ripple/ripple-renderer.ts b/src/material/core/ripple/ripple-renderer.ts index 18b21d64c5ff..8babf9d671ae 100644 --- a/src/material/core/ripple/ripple-renderer.ts +++ b/src/material/core/ripple/ripple-renderer.ts @@ -176,6 +176,10 @@ export class RippleRenderer implements EventListenerObject { if (!animationForciblyDisabledThroughCss && (enterDuration || animationConfig.exitDuration)) { this._ngZone.runOutsideAngular(() => { ripple.addEventListener('transitionend', () => this._finishRippleTransition(rippleRef)); + // If the transition is cancelled (e.g. due to DOM removal), we destroy the ripple + // directly as otherwise we would keep it part of the ripple container forever. + // https://www.w3.org/TR/css-transitions-1/#:~:text=no%20longer%20in%20the%20document. + ripple.addEventListener('transitioncancel', () => this._destroyRipple(rippleRef)); }); } @@ -190,19 +194,8 @@ export class RippleRenderer implements EventListenerObject { /** Fades out a ripple reference. */ fadeOutRipple(rippleRef: RippleRef) { - const wasActive = this._activeRipples.delete(rippleRef); - - if (rippleRef === this._mostRecentTransientRipple) { - this._mostRecentTransientRipple = null; - } - - // Clear out the cached bounding rect if we have no more ripples. - if (!this._activeRipples.size) { - this._containerRect = null; - } - - // For ripples that are not active anymore, don't re-run the fade-out animation. - if (!wasActive) { + // For ripples already fading out or hidden, this should be a noop. + if (rippleRef.state === RippleState.FADING_OUT || rippleRef.state === RippleState.HIDDEN) { return; } @@ -303,6 +296,19 @@ export class RippleRenderer implements EventListenerObject { /** Destroys the given ripple by removing it from the DOM and updating its state. */ private _destroyRipple(rippleRef: RippleRef) { + this._activeRipples.delete(rippleRef); + + // Clear out the cached bounding rect if we have no more ripples. + if (!this._activeRipples.size) { + this._containerRect = null; + } + + // If the current ref is the most recent transient ripple, unset it + // avoid memory leaks. + if (rippleRef === this._mostRecentTransientRipple) { + this._mostRecentTransientRipple = null; + } + rippleRef.state = RippleState.HIDDEN; rippleRef.element.remove(); } diff --git a/src/material/core/ripple/ripple.spec.ts b/src/material/core/ripple/ripple.spec.ts index be191e69eb45..cc7a31120bcd 100644 --- a/src/material/core/ripple/ripple.spec.ts +++ b/src/material/core/ripple/ripple.spec.ts @@ -45,6 +45,7 @@ describe('MatRipple', () => { RippleContainerWithNgIf, RippleCssTransitionNone, RippleCssTransitionDurationZero, + RippleWithDomRemovalOnClick, ], }); }); @@ -802,6 +803,33 @@ describe('MatRipple', () => { dispatchMouseEvent(rippleTarget, 'mouseup'); expect(rippleTarget.querySelectorAll('.mat-ripple-element').length).toBe(0); }); + + it('should destroy the ripple if the transition is being canceled due to DOM removal', async () => { + fixture = TestBed.createComponent(RippleWithDomRemovalOnClick); + fixture.detectChanges(); + + rippleTarget = fixture.nativeElement.querySelector('.mat-ripple'); + + dispatchMouseEvent(rippleTarget, 'mousedown'); + dispatchMouseEvent(rippleTarget, 'mouseup'); + dispatchMouseEvent(rippleTarget, 'click'); + + const fadingRipple = rippleTarget.querySelector('.mat-ripple-element'); + expect(fadingRipple).not.toBe(null); + + // The ripple animation is still on-going but the element is now removed from DOM as + // part of the change detecton (given `show` being set to `false` on click) + fixture.detectChanges(); + + // The `transitioncancel` event is emitted when a CSS transition is canceled due + // to e.g. DOM removal. More details in the CSS transitions spec: + // https://www.w3.org/TR/css-transitions-1/#:~:text=no%20longer%20in%20the%20document. + dispatchFakeEvent(fadingRipple!, 'transitioncancel'); + + // There should be no ripple element anymore because the fading-in ripple from + // before had its animation canceled due the DOM removal. + expect(rippleTarget.querySelector('.mat-ripple-element')).toBeNull(); + }); }); }); @@ -866,3 +894,14 @@ class RippleCssTransitionNone {} encapsulation: ViewEncapsulation.None, }) class RippleCssTransitionDurationZero {} + +@Component({ + template: ` +
+ Click to remove this element. +
+ `, +}) +class RippleWithDomRemovalOnClick { + show = true; +} diff --git a/src/material/select/BUILD.bazel b/src/material/select/BUILD.bazel index 18f88d82d113..97b6d43f2141 100644 --- a/src/material/select/BUILD.bazel +++ b/src/material/select/BUILD.bazel @@ -1,6 +1,8 @@ +load("//src/e2e-app:test_suite.bzl", "e2e_test_suite") load( "//tools:defaults.bzl", "markdown_to_html", + "ng_e2e_test_library", "ng_module", "ng_test_library", "ng_web_test_suite", @@ -78,6 +80,19 @@ ng_web_test_suite( deps = [":unit_test_sources"], ) +ng_e2e_test_library( + name = "e2e_test_sources", + srcs = glob(["**/*.e2e.spec.ts"]), + deps = [ + "//src/cdk/testing/private/e2e", + ], +) + +e2e_test_suite( + name = "e2e_tests", + deps = [":e2e_test_sources"], +) + markdown_to_html( name = "overview", srcs = [":select.md"], diff --git a/src/material/select/select.e2e.spec.ts b/src/material/select/select.e2e.spec.ts new file mode 100644 index 000000000000..9e5ea06b3d56 --- /dev/null +++ b/src/material/select/select.e2e.spec.ts @@ -0,0 +1,36 @@ +import {browser, by, element, ExpectedConditions} from 'protractor'; +import {getElement} from '../../cdk/testing/private/e2e'; + +const presenceOf = ExpectedConditions.presenceOf; +const not = ExpectedConditions.not; + +describe('select', () => { + beforeEach(async () => await browser.get('/select?animations=true')); + + // Regression test which ensures that ripples within the select are not persisted + // accidentally. This could happen because the select panel is removed from DOM + // immediately when an option is clicked. Usually ripples still fade-in at that point. + it('should not accidentally persist ripples', async () => { + const select = getElement('.mat-select'); + const options = element.all(by.css('.mat-option')); + const ripples = element.all(by.css('.mat-ripple-element')); + + // Wait for select to be rendered. + await browser.wait(presenceOf(select)); + + // Opent the select and wait for options to be displayed. + await select.click(); + await browser.wait(presenceOf(options.get(0))); + + // Click the first option and wait for the select to be closed. + await options.get(0).click(); + await browser.wait(not(presenceOf(options.get(0)))); + + // Re-open the select and wait for it to be rendered. + await select.click(); + await browser.wait(presenceOf(options.get(0))); + + // Expect no ripples to be showing up without an option click. + expect(await ripples.isPresent()).toBe(false); + }); +});