Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(OverlayToaster): Fix toasts being cut off if show() called too quickly #7049

Merged
merged 4 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/core/changelog/@unreleased/pr-7049.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type: fix
fix:
description: 'fix(OverlayToaster): Fix toasts being cut off if show() called too
quickly'
links:
- https://github.com/palantir/blueprint/pull/7049
75 changes: 64 additions & 11 deletions packages/core/src/components/toast/overlayToaster.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ export interface OverlayToasterState {

export type OverlayToasterCreateOptions = DOMMountOptions<OverlayToasterProps>;

interface OverlayToasterQueueState {
cancel: (() => void) | undefined;
isRunning: boolean;
toasts: ToastOptions[];
}

export const OVERLAY_TOASTER_DELAY_MS = 50;

/**
* OverlayToaster component.
*
Expand Down Expand Up @@ -132,6 +140,14 @@ export class OverlayToaster extends AbstractPureComponent<OverlayToasterProps, O
toasts: [],
};

// Queue of toasts to be displayed. If toasts are shown too quickly back to back, it can result in cut off toasts.
// The queue ensures that toasts are only displayed in QUEUE_TIMEOUT_MS increments.
private queue: OverlayToasterQueueState = {
cancel: undefined,
isRunning: false,
toasts: [],
};

// auto-incrementing identifier for un-keyed toasts
private toastId = 0;

Expand All @@ -152,24 +168,63 @@ export class OverlayToaster extends AbstractPureComponent<OverlayToasterProps, O
return options.key;
}

if (this.props.maxToasts) {
// check if active number of toasts are at the maxToasts limit
this.dismissIfAtLimit();
if (this.queue.isRunning) {
// If a toast has been shown recently, push to the queued toasts to prevent toasts from being shown too
// quickly for the animations to keep up
this.queue.toasts.push(options);
} else {
// If we have not recently shown a toast, we can immediately show the given toast
this.immediatelyShowToast(options);
this.startQueueTimeout();
}

this.updateToastsInState(toasts => [options, ...toasts]);
return options.key;
}

private maybeUpdateExistingToast(options: ToastOptions, key: string | undefined) {
if (key == null || this.isNewToastKey(key)) {
if (key == null) {
return false;
}

this.updateToastsInState(toasts => toasts.map(t => (t.key === key ? options : t)));
return true;
const isExistingQueuedToast = this.queue.toasts.some(toast => toast.key === key);
if (isExistingQueuedToast) {
this.queue.toasts = this.queue.toasts.map(t => (t.key === key ? options : t));
return true;
}

const isExistingShownToast = this.state.toasts.some(toast => toast.key === key);
if (isExistingShownToast) {
this.updateToastsInState(toasts => toasts.map(t => (t.key === key ? options : t)));
return true;
}

return false;
}

private immediatelyShowToast(options: ToastOptions) {
if (this.props.maxToasts) {
// check if active number of toasts are at the maxToasts limit
this.dismissIfAtLimit();
}

this.updateToastsInState(toasts => [options, ...toasts]);
}

private startQueueTimeout() {
this.queue.isRunning = true;
this.queue.cancel = this.setTimeout(this.handleQueueTimeout, OVERLAY_TOASTER_DELAY_MS);
}

private handleQueueTimeout = () => {
const nextToast = this.queue.toasts.shift();
if (nextToast != null) {
this.immediatelyShowToast(nextToast);
this.startQueueTimeout();
} else {
this.queue.isRunning = false;
}
};

private updateToastsInState(getNewToasts: (toasts: ToastOptions[]) => ToastOptions[]) {
this.setState(prevState => {
const toasts = getNewToasts(prevState.toasts);
Expand All @@ -191,6 +246,8 @@ export class OverlayToaster extends AbstractPureComponent<OverlayToasterProps, O
}

public clear() {
this.queue.cancel?.();
this.queue = { cancel: undefined, isRunning: false, toasts: [] };
this.state.toasts.forEach(t => t.onDismiss?.(false));
this.setState({ toasts: [], toastRefs: {} });
}
Expand Down Expand Up @@ -252,10 +309,6 @@ export class OverlayToaster extends AbstractPureComponent<OverlayToasterProps, O
});
}

private isNewToastKey(key: string) {
return this.state.toasts.every(toast => toast.key !== key);
}

private dismissIfAtLimit() {
if (this.state.toasts.length === this.props.maxToasts) {
// dismiss the oldest toast to stay within the maxToasts limit
Expand Down
54 changes: 51 additions & 3 deletions packages/core/test/toast/overlayToasterTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { expectPropValidationError } from "@blueprintjs/test-commons";

import { Classes, OverlayToaster, type OverlayToasterProps, type Toaster } from "../../src";
import { TOASTER_CREATE_NULL, TOASTER_MAX_TOASTS_INVALID } from "../../src/common/errors";
import { OVERLAY_TOASTER_DELAY_MS } from "../../src/components/toast/overlayToaster";

const SPECS = [
{
Expand Down Expand Up @@ -64,6 +65,7 @@ function unmountReact16Toaster(containerElement: HTMLElement) {
}

describe("OverlayToaster", () => {
let clock: sinon.SinonFakeTimers;
let testsContainerElement: HTMLElement;
let toaster: Toaster;

Expand All @@ -75,7 +77,12 @@ describe("OverlayToaster", () => {
toaster = await spec.create({}, testsContainerElement);
});

beforeEach(() => {
clock = sinon.useFakeTimers();
});

afterEach(() => {
clock.restore();
toaster.clear();
});

Expand All @@ -93,11 +100,11 @@ describe("OverlayToaster", () => {
});

it("show() renders toast on next tick", done => {
clock.restore();
toaster.show({
message: "Hello world",
});
assert.lengthOf(toaster.getToasts(), 1, "expected 1 toast");

// setState needs a tick to flush DOM updates
setTimeout(() => {
assert.isNotNull(
Expand All @@ -112,9 +119,27 @@ describe("OverlayToaster", () => {
toaster.show({ message: "one" });
toaster.show({ message: "two" });
toaster.show({ message: "six" });
clock.tick(3 * OVERLAY_TOASTER_DELAY_MS);
assert.lengthOf(toaster.getToasts(), 3, "expected 3 toasts");
});

it("multiple shows() get queued if provided too quickly", () => {
toaster.show({ message: "one" });
toaster.show({ message: "two" });
toaster.show({ message: "three" });
assert.lengthOf(toaster.getToasts(), 1, "expected 1 toast");
clock.tick(3 * OVERLAY_TOASTER_DELAY_MS);
assert.lengthOf(toaster.getToasts(), 3, "expected 3 toasts after delay");
});

it("show() immediately displays a toast when waiting after the previous show()", () => {
toaster.show({ message: "one" });
assert.lengthOf(toaster.getToasts(), 1, "expected 1 toast");
clock.tick(2 * OVERLAY_TOASTER_DELAY_MS);
toaster.show({ message: "two" });
assert.lengthOf(toaster.getToasts(), 2, "expected 2 toasts");
});

it("show() updates existing toast", () => {
const key = toaster.show({ message: "one" });
assert.deepEqual(toaster.getToasts()[0].message, "one");
Expand All @@ -123,10 +148,20 @@ describe("OverlayToaster", () => {
assert.deepEqual(toaster.getToasts()[0].message, "two");
});

it("show() updates existing toast in queue", () => {
toaster.show({ message: "one" });
const key = toaster.show({ message: "two" });
toaster.show({ message: "two updated" }, key);
clock.tick(2 * OVERLAY_TOASTER_DELAY_MS);
assert.lengthOf(toaster.getToasts(), 2, "expected 2 toasts");
assert.deepEqual(toaster.getToasts()[0].message, "two updated");
});

it("dismiss() removes just the toast in question", () => {
toaster.show({ message: "one" });
const key = toaster.show({ message: "two" });
toaster.show({ message: "six" });
clock.tick(3 * OVERLAY_TOASTER_DELAY_MS);
toaster.dismiss(key);
assert.deepEqual(
toaster.getToasts().map(t => t.message),
Expand All @@ -138,9 +173,13 @@ describe("OverlayToaster", () => {
toaster.show({ message: "one" });
toaster.show({ message: "two" });
toaster.show({ message: "six" });
assert.lengthOf(toaster.getToasts(), 3, "expected 3 toasts");
clock.tick(OVERLAY_TOASTER_DELAY_MS);
assert.lengthOf(toaster.getToasts(), 2, "expected 2 toasts");
toaster.clear();
assert.lengthOf(toaster.getToasts(), 0, "expected 0 toasts");
// Ensure the queue is cleared
clock.tick(2 * OVERLAY_TOASTER_DELAY_MS);
assert.lengthOf(toaster.getToasts(), 0, "expected 0 toasts");
});

it("action onClick callback invoked when action clicked", () => {
Expand Down Expand Up @@ -209,24 +248,33 @@ describe("OverlayToaster", () => {
toaster = await spec.create({ maxToasts: 3 }, testsContainerElement);
});

beforeEach(() => {
clock = sinon.useFakeTimers();
});

after(() => {
unmountReact16Toaster(testsContainerElement);
document.documentElement.removeChild(testsContainerElement);
});
afterEach(() => {
clock.restore();
});

it("does not exceed the maximum toast limit set", () => {
toaster.show({ message: "one" });
toaster.show({ message: "two" });
toaster.show({ message: "three" });
toaster.show({ message: "oh no" });
clock.tick(4 * OVERLAY_TOASTER_DELAY_MS);
assert.lengthOf(toaster.getToasts(), 3, "expected 3 toasts");
});

it("does not dismiss toasts when updating an existing toast at the limit", () => {
it("does not dismiss toasts when updating an existing toast at the limit", async () => {
toaster.show({ message: "one" });
toaster.show({ message: "two" });
toaster.show({ message: "three" }, "3");
toaster.show({ message: "three updated" }, "3");
clock.tick(4 * OVERLAY_TOASTER_DELAY_MS);
assert.lengthOf(toaster.getToasts(), 3, "expected 3 toasts");
});
});
Expand Down