From c9b9644163433f43244d7bb1dd97d325423ccec0 Mon Sep 17 00:00:00 2001 From: James Todaro <30529065+jtodaroii@users.noreply.github.com> Date: Sat, 5 Sep 2026 22:38:15 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=94=20fix:=20Honor=20Toast=20Duration?= =?UTF-8?q?=20and=20Support=20Persistent=20Toasts=20(#15049)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔔 fix: Honor Toast Duration and Support Persistent Toasts * 🔔 fix: Annotate the single-run effect in the toast spec * 🔔 fix: Give each toast its own close deadline - a toast replacing one still open inherited the superseded Radix deadline and could close early - the shown-toast id keys the Radix root and gates `onOpenChange`, so a stale timer is a no-op - drops the competing hide timer, leaving Radix as the single dismissal owner * fix: align and emphasize persistent toast close button --------- Co-authored-by: Marco Beretta <81851188+berry-13@users.noreply.github.com> --- packages/client/src/components/Toast.tsx | 19 ++- .../src/components/__tests__/Toast.spec.tsx | 145 ++++++++++++++++++ packages/client/src/hooks/useToast.ts | 32 ++-- packages/client/src/store.ts | 6 + 4 files changed, 183 insertions(+), 19 deletions(-) create mode 100644 packages/client/src/components/__tests__/Toast.spec.tsx diff --git a/packages/client/src/components/Toast.tsx b/packages/client/src/components/Toast.tsx index 8f160637b8..01d496e486 100644 --- a/packages/client/src/components/Toast.tsx +++ b/packages/client/src/components/Toast.tsx @@ -1,10 +1,13 @@ +import { X } from 'lucide-react'; import { JSX } from 'react/jsx-runtime'; import * as RadixToast from '@radix-ui/react-toast'; import { NotificationSeverity } from '~/common'; -import { useToast } from '~/hooks'; +import { useToast, useLocalize } from '~/hooks'; export function Toast(): JSX.Element { const { toast, onOpenChange } = useToast(); + const localize = useLocalize(); + const persistent = toast.duration === Infinity; const severityClassName = { [NotificationSeverity.INFO]: 'border-status-info-strong bg-status-info-strong', [NotificationSeverity.SUCCESS]: 'border-status-success-strong bg-status-success-strong', @@ -14,11 +17,13 @@ export function Toast(): JSX.Element { return ( onOpenChange(open, toast.id)} + duration={toast.duration} className="toast-root" style={{ - height: '74px', + minHeight: '74px', marginBottom: '0px', }} > @@ -51,6 +56,14 @@ export function Toast(): JSX.Element {
{toast.message}
 + {persistent && ( + + + + )} diff --git a/packages/client/src/components/__tests__/Toast.spec.tsx b/packages/client/src/components/__tests__/Toast.spec.tsx new file mode 100644 index 0000000000..25b04fc9a7 --- /dev/null +++ b/packages/client/src/components/__tests__/Toast.spec.tsx @@ -0,0 +1,145 @@ +import { useEffect } from 'react'; +import { Provider } from 'jotai'; +import * as RadixToast from '@radix-ui/react-toast'; +import { render, act, fireEvent } from '@testing-library/react'; +import type { ReactElement } from 'react'; +import { useToast } from '~/hooks'; +import { Toast } from '../Toast'; + +const MESSAGE = 'The file is too large.'; +const REPLACEMENT = 'The upload finished.'; + +function ShowOnMount({ duration }: { duration?: number }): null { + const { showToast } = useToast(0); + + /** One toast per mount — re-running on a changed identity would reset the timers under test. */ + useEffect(() => { + showToast({ message: MESSAGE, duration }); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + return null; +} + +function ShowTwice({ duration, gap }: { duration: number; gap: number }): null { + const { showToast } = useToast(0); + + /** One pair per mount, for the same reason ShowOnMount runs once. */ + useEffect(() => { + showToast({ message: MESSAGE, duration }); + const timer = window.setTimeout(() => showToast({ message: REPLACEMENT, duration }), gap); + return () => window.clearTimeout(timer); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); + + return null; +} + +function renderToast(trigger: ReactElement): void { + render( + + + {trigger} + + + + , + ); +} + +function setup(duration?: number): void { + renderToast(); +} + +/** Radix removes the root once it has closed, so an absent node and a node + * marked closed are the same observable outcome. */ +function state(): string { + return document.querySelector('.toast-root')?.getAttribute('data-state') ?? 'unmounted'; +} + +function message(): string { + return document.querySelector('.toast-root')?.textContent ?? ''; +} + +function advance(ms: number): void { + act(() => { + jest.advanceTimersByTime(ms); + }); +} + +describe('Toast duration', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + test('honors a duration shorter than the Radix default', () => { + setup(3000); + advance(0); + expect(state()).toBe('open'); + + advance(2900); + expect(state()).toBe('open'); + + advance(200); + expect(state()).not.toBe('open'); + }); + + test('honors a duration longer than the Radix default', () => { + setup(10000); + advance(0); + expect(state()).toBe('open'); + + advance(5500); + expect(state()).toBe('open'); + }); + + test('treats a duration of 0 as persistent', () => { + setup(0); + advance(0); + expect(state()).toBe('open'); + + advance(60000); + expect(state()).toBe('open'); + }); + + test('offers a dismissal only for a persistent toast', () => { + setup(0); + advance(0); + + const close = document.querySelector('[aria-label="com_ui_close"]'); + expect(close).not.toBeNull(); + + act(() => { + fireEvent.click(close as Element); + }); + + expect(state()).not.toBe('open'); + }); + + test('leaves a self-dismissing toast without a close control', () => { + setup(3000); + advance(0); + + expect(document.querySelector('[aria-label="com_ui_close"]')).toBeNull(); + }); + + test('restarts the deadline when a toast replaces one that is still open', () => { + renderToast(); + advance(0); + expect(state()).toBe('open'); + + advance(2001); + expect(message()).toContain(REPLACEMENT); + + /** Past the first toast's deadline: the replacement keeps its own full duration. */ + advance(1200); + expect(state()).toBe('open'); + + advance(2000); + expect(state()).not.toBe('open'); + }); +}); diff --git a/packages/client/src/hooks/useToast.ts b/packages/client/src/hooks/useToast.ts index fe9adf316e..5916c46d82 100644 --- a/packages/client/src/hooks/useToast.ts +++ b/packages/client/src/hooks/useToast.ts @@ -6,21 +6,17 @@ import { NotificationSeverity } from '~/common'; export default function useToast(showDelay = 100): { toast: ToastState; - onOpenChange: (open: boolean) => void; + onOpenChange: (open: boolean, id: number) => void; showToast: ({ message, severity, showIcon, duration, status }: TShowToast) => void; } { const [toast, setToast] = useAtom(toastState); const showTimerRef = useRef(null); - const hideTimerRef = useRef(null); useEffect(() => { return () => { if (showTimerRef.current !== null) { clearTimeout(showTimerRef.current); } - if (hideTimerRef.current !== null) { - clearTimeout(hideTimerRef.current); - } }; }, []); @@ -31,32 +27,36 @@ export default function useToast(showDelay = 100): { duration = 3000, // default duration for the toast to be visible status, }: TShowToast): void => { - // Clear existing timeouts + // Clear a pending show that has not fired yet if (showTimerRef.current !== null) { clearTimeout(showTimerRef.current); } - if (hideTimerRef.current !== null) { - clearTimeout(hideTimerRef.current); - } + + const closeAfter = Number.isFinite(duration) && duration > 0 ? duration : Infinity; // Timeout to show the toast showTimerRef.current = window.setTimeout(() => { - setToast({ + /** A new `id` gives the toast its own Radix lifecycle, so its close deadline + * starts now even when it replaces a toast that is still open. */ + setToast((prevToast: ToastState) => ({ open: true, message, severity: (status as NotificationSeverity) ?? severity, showIcon, - }); - // Hides the toast after the specified duration - hideTimerRef.current = window.setTimeout(() => { - setToast((prevToast: ToastState) => ({ ...prevToast, open: false })); - }, duration); + duration: closeAfter, + id: prevToast.id + 1, + })); }, showDelay); }; return { toast, - onOpenChange: (open: boolean): void => setToast({ ...toast, open }), + /** Radix keeps a superseded toast's close timer alive past unmount, so it can + * otherwise close the toast that replaced it; the id makes that a no-op. */ + onOpenChange: (open: boolean, id: number): void => + setToast((prevToast: ToastState) => + prevToast.id === id ? { ...prevToast, open } : prevToast, + ), showToast, }; } diff --git a/packages/client/src/store.ts b/packages/client/src/store.ts index 9b1d49e22a..660b825dca 100644 --- a/packages/client/src/store.ts +++ b/packages/client/src/store.ts @@ -13,6 +13,10 @@ export type ToastState = { message: string; severity: NotificationSeverity; showIcon: boolean; + /** Milliseconds until the toast closes itself, or `Infinity` to require a dismissal. */ + duration: number; + /** Increments per shown toast, so each one gets its own close deadline. */ + id: number; }; export const toastState: PrimitiveAtom & { @@ -22,4 +26,6 @@ export const toastState: PrimitiveAtom & { message: '', severity: NotificationSeverity.SUCCESS, showIcon: true, + duration: 3000, + id: 0, });