From b11ab8682297e6226e1e6a7768181d4a0f084d1b Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Tue, 26 Apr 2022 18:33:54 +0200 Subject: [PATCH] fix(material-experimental/mdc-snack-bar): avoid multiple snack bars on the page if opened in quick succession Fixes an issue where opening a snack bar while the previous one was being animated caused the former to remain in the DOM. The problem was that MDC always waits for an animation, even if it is interrupted. --- .../mdc-snack-bar/snack-bar-container.ts | 43 +++++++++++++------ .../mdc-snack-bar/snack-bar.spec.ts | 27 +++++++++--- src/material/snack-bar/snack-bar-ref.ts | 3 +- src/material/snack-bar/snack-bar.spec.ts | 18 +++++++- 4 files changed, 68 insertions(+), 23 deletions(-) diff --git a/src/material-experimental/mdc-snack-bar/snack-bar-container.ts b/src/material-experimental/mdc-snack-bar/snack-bar-container.ts index 11fd467869e6..5a9ff3629565 100644 --- a/src/material-experimental/mdc-snack-bar/snack-bar-container.ts +++ b/src/material-experimental/mdc-snack-bar/snack-bar-container.ts @@ -28,7 +28,7 @@ import { } from '@angular/core'; import {MatSnackBarConfig, _SnackBarContainer} from '@angular/material/snack-bar'; import {ANIMATION_MODULE_TYPE} from '@angular/platform-browser/animations'; -import {MDCSnackbarAdapter, MDCSnackbarFoundation} from '@material/snackbar'; +import {MDCSnackbarAdapter, MDCSnackbarFoundation, cssClasses} from '@material/snackbar'; import {Platform} from '@angular/cdk/platform'; import {Observable, Subject} from 'rxjs'; @@ -97,10 +97,7 @@ export class MatSnackBarContainer addClass: (className: string) => this._setClass(className, true), removeClass: (className: string) => this._setClass(className, false), announce: () => {}, - notifyClosed: () => { - this._onExit.next(); - this._mdcFoundation.destroy(); - }, + notifyClosed: () => this._finishExit(), notifyClosing: () => {}, notifyOpened: () => this._onEnter.next(), notifyOpening: () => {}, @@ -172,16 +169,24 @@ export class MatSnackBarContainer } exit(): Observable { - // It's common for snack bars to be opened by random outside calls like HTTP requests or - // errors. Run inside the NgZone to ensure that it functions correctly. - this._ngZone.run(() => { - this._exiting = true; - this._mdcFoundation.close(); + const classList = this._elementRef.nativeElement.classList; + + // MDC won't complete the closing sequence if it starts while opening hasn't finished. + // If that's the case, destroy immediately to ensure that our stream emits as expected. + if (classList.contains(cssClasses.OPENING) || !classList.contains(cssClasses.OPEN)) { + this._finishExit(); + } else { + // It's common for snack bars to be opened by random outside calls like HTTP requests or + // errors. Run inside the NgZone to ensure that it functions correctly. + this._ngZone.run(() => { + this._exiting = true; + this._mdcFoundation.close(); + }); + } - // If the snack bar hasn't been announced by the time it exits it wouldn't have been open - // long enough to visually read it either, so clear the timeout for announcing. - clearTimeout(this._announceTimeoutId); - }); + // If the snack bar hasn't been announced by the time it exits it wouldn't have been open + // long enough to visually read it either, so clear the timeout for announcing. + clearTimeout(this._announceTimeoutId); return this._onExit; } @@ -228,6 +233,16 @@ export class MatSnackBarContainer } } + /** Finishes the exit sequence of the container. */ + private _finishExit() { + this._onExit.next(); + this._onExit.complete(); + + if (this._platform.isBrowser) { + this._mdcFoundation.destroy(); + } + } + /** * Starts a timeout to move the snack bar content to the live region so screen readers will * announce it. diff --git a/src/material-experimental/mdc-snack-bar/snack-bar.spec.ts b/src/material-experimental/mdc-snack-bar/snack-bar.spec.ts index d682b03804a8..7c555a5e8db5 100644 --- a/src/material-experimental/mdc-snack-bar/snack-bar.spec.ts +++ b/src/material-experimental/mdc-snack-bar/snack-bar.spec.ts @@ -288,15 +288,16 @@ describe('MatSnackBar', () => { let snackBarRef = snackBar.open(simpleMessage, undefined, config); viewContainerFixture.detectChanges(); + flush(); expect(overlayContainerElement.childElementCount) .withContext('Expected overlay container element to have at least one child') .toBeGreaterThan(0); snackBarRef.afterDismissed().subscribe({complete: dismissCompleteSpy}); + const messageElement = overlayContainerElement.querySelector('mat-snack-bar-container')!; snackBarRef.dismiss(); viewContainerFixture.detectChanges(); - const messageElement = overlayContainerElement.querySelector('mat-snack-bar-container')!; expect(messageElement.hasAttribute('mat-exit')) .withContext('Expected the snackbar container to have the "exit" attribute upon dismiss') .toBe(true); @@ -412,23 +413,29 @@ describe('MatSnackBar', () => { })); it('should dismiss the snackbar when the action is called, notifying of both action and dismiss', fakeAsync(() => { + const dismissNextSpy = jasmine.createSpy('dismiss next spy'); const dismissCompleteSpy = jasmine.createSpy('dismiss complete spy'); + const actionNextSpy = jasmine.createSpy('action next spy'); const actionCompleteSpy = jasmine.createSpy('action complete spy'); const snackBarRef = snackBar.open('Some content', 'Dismiss'); viewContainerFixture.detectChanges(); - snackBarRef.afterDismissed().subscribe({complete: dismissCompleteSpy}); - snackBarRef.onAction().subscribe({complete: actionCompleteSpy}); + snackBarRef.afterDismissed().subscribe({next: dismissNextSpy, complete: dismissCompleteSpy}); + snackBarRef.onAction().subscribe({next: actionNextSpy, complete: actionCompleteSpy}); - let actionButton = overlayContainerElement.querySelector( + const actionButton = overlayContainerElement.querySelector( 'button.mat-mdc-button', ) as HTMLButtonElement; actionButton.click(); viewContainerFixture.detectChanges(); - flush(); + tick(); + expect(dismissNextSpy).toHaveBeenCalled(); expect(dismissCompleteSpy).toHaveBeenCalled(); + expect(actionNextSpy).toHaveBeenCalled(); expect(actionCompleteSpy).toHaveBeenCalled(); + + tick(500); })); it('should allow manually dismissing with an action', fakeAsync(() => { @@ -587,6 +594,16 @@ describe('MatSnackBar', () => { flush(); })); + it('should only keep one snack bar in the DOM if multiple are opened at the same time', fakeAsync(() => { + for (let i = 0; i < 10; i++) { + snackBar.open('Snack time!', 'Chew'); + viewContainerFixture.detectChanges(); + } + + flush(); + expect(overlayContainerElement.querySelectorAll('mat-snack-bar-container').length).toBe(1); + })); + describe('with custom component', () => { it('should open a custom component', () => { const snackBarRef = snackBar.openFromComponent(BurritosNotification); diff --git a/src/material/snack-bar/snack-bar-ref.ts b/src/material/snack-bar/snack-bar-ref.ts index 0d7de41d9706..40810536a2c4 100644 --- a/src/material/snack-bar/snack-bar-ref.ts +++ b/src/material/snack-bar/snack-bar-ref.ts @@ -52,8 +52,6 @@ export class MatSnackBarRef { constructor(containerInstance: _SnackBarContainer, private _overlayRef: OverlayRef) { this.containerInstance = containerInstance; - // Dismiss snackbar on action. - this.onAction().subscribe(() => this.dismiss()); containerInstance._onExit.subscribe(() => this._finishDismiss()); } @@ -71,6 +69,7 @@ export class MatSnackBarRef { this._dismissedByAction = true; this._onAction.next(); this._onAction.complete(); + this.dismiss(); } clearTimeout(this._durationTimeoutId); } diff --git a/src/material/snack-bar/snack-bar.spec.ts b/src/material/snack-bar/snack-bar.spec.ts index d1dc01f64606..0b30ab6b6993 100644 --- a/src/material/snack-bar/snack-bar.spec.ts +++ b/src/material/snack-bar/snack-bar.spec.ts @@ -465,13 +465,15 @@ describe('MatSnackBar', () => { })); it('should dismiss the snackbar when the action is called, notifying of both action and dismiss', fakeAsync(() => { + const dismissNextSpy = jasmine.createSpy('dismiss next spy'); const dismissCompleteSpy = jasmine.createSpy('dismiss complete spy'); + const actionNextSpy = jasmine.createSpy('action next spy'); const actionCompleteSpy = jasmine.createSpy('action complete spy'); const snackBarRef = snackBar.open('Some content', 'Dismiss'); viewContainerFixture.detectChanges(); - snackBarRef.afterDismissed().subscribe({complete: dismissCompleteSpy}); - snackBarRef.onAction().subscribe({complete: actionCompleteSpy}); + snackBarRef.afterDismissed().subscribe({next: dismissNextSpy, complete: dismissCompleteSpy}); + snackBarRef.onAction().subscribe({next: actionNextSpy, complete: actionCompleteSpy}); const actionButton = overlayContainerElement.querySelector( 'button.mat-button', @@ -480,7 +482,9 @@ describe('MatSnackBar', () => { viewContainerFixture.detectChanges(); tick(); + expect(dismissNextSpy).toHaveBeenCalled(); expect(dismissCompleteSpy).toHaveBeenCalled(); + expect(actionNextSpy).toHaveBeenCalled(); expect(actionCompleteSpy).toHaveBeenCalled(); tick(500); @@ -651,6 +655,16 @@ describe('MatSnackBar', () => { flush(); })); + it('should only keep one snack bar in the DOM if multiple are opened at the same time', fakeAsync(() => { + for (let i = 0; i < 10; i++) { + snackBar.open('Snack time!', 'Chew'); + viewContainerFixture.detectChanges(); + } + + flush(); + expect(overlayContainerElement.querySelectorAll('snack-bar-container').length).toBe(1); + })); + describe('with custom component', () => { it('should open a custom component', () => { const snackBarRef = snackBar.openFromComponent(BurritosNotification);