🔔 fix: Honor Toast Duration and Support Persistent Toasts (#15049)

* 🔔 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

@@ -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 (
     <RadixToast.Root
+      key={toast.id}
       open={toast.open}
-      onOpenChange={onOpenChange}
+      onOpenChange={(open) => 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 {
           <RadixToast.Description className="flex-1 justify-center gap-2">
             <div className="whitespace-pre-wrap text-left">{toast.message}</div>
           </RadixToast.Description>
+          {persistent && (
+            <RadixToast.Close
+              aria-label={localize('com_ui_close')}
+              className="ml-2 inline-flex flex-shrink-0 flex-grow-0 items-center justify-center self-center rounded-sm opacity-80 transition-opacity hover:opacity-100 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-current"
+            >
+              <X className="h-4 w-4" strokeWidth={3} />
+            </RadixToast.Close>
+          )}
         </div>
       </div>
     </RadixToast.Root>

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(
+    <Provider>
+      <RadixToast.Provider>
+        {trigger}
+        <Toast />
+        <RadixToast.Viewport />
+      </RadixToast.Provider>
+    </Provider>,
+  );
+}
+
+function setup(duration?: number): void {
+  renderToast(<ShowOnMount duration={duration} />);
+}
+
+/** 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(<ShowTwice duration={3000} gap={2000} />);
+    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');
+  });
+});

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<number | null>(null);
-  const hideTimerRef = useRef<number | null>(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,
   };
 }

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<ToastState> & {
@@ -22,4 +26,6 @@ export const toastState: PrimitiveAtom<ToastState> & {
   message: '',
   severity: NotificationSeverity.SUCCESS,
   showIcon: true,
+  duration: 3000,
+  id: 0,
 });