Skip to content

Commit

Permalink
fix(material-experimental/mdc-snack-bar): avoid multiple snack bars o…
Browse files Browse the repository at this point in the history
…n the page if opened in quick succession (#24757)

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.

(cherry picked from commit 30f5181)
  • Loading branch information
crisbeto committed May 4, 2022
1 parent d6f9db3 commit 8ca629d
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 23 deletions.
43 changes: 29 additions & 14 deletions src/material-experimental/mdc-snack-bar/snack-bar-container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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: () => {},
Expand Down Expand Up @@ -172,16 +169,24 @@ export class MatSnackBarContainer
}

exit(): Observable<void> {
// 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;
}
Expand Down Expand Up @@ -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.
Expand Down
27 changes: 22 additions & 5 deletions src/material-experimental/mdc-snack-bar/snack-bar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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);
Expand Down
3 changes: 1 addition & 2 deletions src/material/snack-bar/snack-bar-ref.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,6 @@ export class MatSnackBarRef<T> {

constructor(containerInstance: _SnackBarContainer, private _overlayRef: OverlayRef) {
this.containerInstance = containerInstance;
// Dismiss snackbar on action.
this.onAction().subscribe(() => this.dismiss());
containerInstance._onExit.subscribe(() => this._finishDismiss());
}

Expand All @@ -71,6 +69,7 @@ export class MatSnackBarRef<T> {
this._dismissedByAction = true;
this._onAction.next();
this._onAction.complete();
this.dismiss();
}
clearTimeout(this._durationTimeoutId);
}
Expand Down
18 changes: 16 additions & 2 deletions src/material/snack-bar/snack-bar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -480,7 +482,9 @@ describe('MatSnackBar', () => {
viewContainerFixture.detectChanges();
tick();

expect(dismissNextSpy).toHaveBeenCalled();
expect(dismissCompleteSpy).toHaveBeenCalled();
expect(actionNextSpy).toHaveBeenCalled();
expect(actionCompleteSpy).toHaveBeenCalled();

tick(500);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 8ca629d

Please sign in to comment.