diff --git a/frontend/package-lock.json b/frontend/package-lock.json index ad53dd26..6bbc19f5 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -14,7 +14,6 @@ "@radix-ui/react-dropdown-menu": "^2.1.19", "@radix-ui/react-popover": "^1.1.18", "@radix-ui/react-tabs": "^1.1.16", - "@radix-ui/react-toast": "^1.2.18", "@radix-ui/react-tooltip": "^1.2.8", "@tamagui/config": "2.6.0", "@tamagui/core": "2.6.0", @@ -4224,40 +4223,6 @@ } } }, - "node_modules/@radix-ui/react-toast": { - "version": "1.2.23", - "resolved": "https://registry.npmjs.org/@radix-ui/react-toast/-/react-toast-1.2.23.tgz", - "integrity": "sha512-ofhyAsYaocRGOs/n0XWdUOSVzEAG6BfrMVM8z0c0kLEWY38w/0WuMFPTJP/HVaZPYkMvHZoKIIhNcjbTCBILPg==", - "license": "MIT", - "dependencies": { - "@radix-ui/primitive": "1.1.7", - "@radix-ui/react-collection": "1.1.15", - "@radix-ui/react-compose-refs": "1.1.5", - "@radix-ui/react-context": "1.2.2", - "@radix-ui/react-dismissable-layer": "1.1.19", - "@radix-ui/react-portal": "1.1.17", - "@radix-ui/react-presence": "1.1.10", - "@radix-ui/react-primitive": "2.1.10", - "@radix-ui/react-use-callback-ref": "1.1.4", - "@radix-ui/react-use-controllable-state": "1.2.6", - "@radix-ui/react-use-layout-effect": "1.1.4", - "@radix-ui/react-visually-hidden": "1.2.11" - }, - "peerDependencies": { - "@types/react": "*", - "@types/react-dom": "*", - "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", - "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" - }, - "peerDependenciesMeta": { - "@types/react": { - "optional": true - }, - "@types/react-dom": { - "optional": true - } - } - }, "node_modules/@radix-ui/react-tooltip": { "version": "1.2.16", "resolved": "https://registry.npmjs.org/@radix-ui/react-tooltip/-/react-tooltip-1.2.16.tgz", diff --git a/frontend/package.json b/frontend/package.json index 1c8a07f2..b2f71b15 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -28,7 +28,6 @@ "@radix-ui/react-dropdown-menu": "^2.1.19", "@radix-ui/react-popover": "^1.1.18", "@radix-ui/react-tabs": "^1.1.16", - "@radix-ui/react-toast": "^1.2.18", "@radix-ui/react-tooltip": "^1.2.8", "@tamagui/config": "2.6.0", "@tamagui/core": "2.6.0", diff --git a/frontend/src/components/Toast/Toast.module.scss b/frontend/src/components/Toast/Toast.module.scss index 43e2fae3..bc9c6246 100644 --- a/frontend/src/components/Toast/Toast.module.scss +++ b/frontend/src/components/Toast/Toast.module.scss @@ -17,7 +17,8 @@ align-items: center; gap: sp.$spacing-gap; width: min(96vw, 720px); - /* Required by Radix to be a list */ + /* Tamagui's ToastViewport (#581) renders a View, not Radix's
    - these + resets are vestigial now but harmless to keep. */ list-style: none; padding: 0; margin: 0; diff --git a/frontend/src/components/Toast/ToastManager.test.tsx b/frontend/src/components/Toast/ToastManager.test.tsx index 1bd83586..c5ae6f28 100644 --- a/frontend/src/components/Toast/ToastManager.test.tsx +++ b/frontend/src/components/Toast/ToastManager.test.tsx @@ -1,17 +1,42 @@ import { describe, it, expect, vi } from 'vitest'; -import { render, screen } from '@testing-library/react'; +import { act, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import * as RadixToast from '@radix-ui/react-toast'; +import { TamaguiProvider, ToastProvider } from 'tamagui'; import ToastManager from './ToastManager'; +import tamaguiConfig from '../../../tamagui.config'; +// ToastManager's Toast/ToastViewport (#581) need both a TamaguiProvider +// ancestor (for tokens/theme) and a ToastProvider (duration/swipe context) - +// ToastContext.tsx supplies both in the app; tests need their own. function Wrapper({ children }: { children: React.ReactNode }) { - return {children}; + return ( + + {children} + + ); } function renderManager(props: Parameters[0]) { return render(, { wrapper: Wrapper }); } +// Auto-dismiss (duration -> onOpenChange(false) -> onDismiss) needs its own +// duration, distinct from the shared Wrapper's default - a per-test provider +// lets the fake-timer test below use a short duration without affecting the +// other tests' default 5000ms Tamagui provider timing. +function renderWithDuration( + props: Parameters[0], + duration: number +) { + return render( + + + + + + ); +} + describe('ToastManager', () => { it('renders empty when no messages', () => { const { container } = renderManager({ messages: [], onDismiss: vi.fn() }); @@ -35,26 +60,88 @@ describe('ToastManager', () => { expect(screen.getByText('Second message')).toBeInTheDocument(); }); - it('each toast has role="status" (Radix default for type=background)', () => { + // Unlike Radix (where only a transient hidden announce copy carries + // role="status", and the visible
  1. carries no role at all), Tamagui's + // fork puts role="status" on *both* the persistent visible toast + // (aria-live="off", so it doesn't double-announce) and the transient + // announce copy (aria-live="polite" for type="background", matching + // Radix's politeness exactly) - a different DOM shape, same announcement + // behavior. getAllByRole (not getByRole) because both are legitimately + // present at once here. + it('exposes role="status" with background-type ("polite") live-region politeness', () => { renderManager({ messages: [{ id: '1', message: 'Hello' }], onDismiss: vi.fn() }); - expect(screen.getByRole('status')).toBeInTheDocument(); + const statusElements = screen.getAllByRole('status'); + expect(statusElements.length).toBeGreaterThanOrEqual(1); + expect(statusElements.some((el) => el.getAttribute('aria-live') === 'polite')).toBe(true); + // None should be "assertive" - that's reserved for type="foreground", which this never uses. + expect(statusElements.some((el) => el.getAttribute('aria-live') === 'assertive')).toBe(false); }); it('calls onDismiss when toast closes', async () => { const onDismiss = vi.fn(); renderManager({ messages: [{ id: 'abc', message: 'Bye' }], onDismiss }); - // Radix renders a close button; simulate closing + // Neither Radix nor this Tamagui swap render a close button by default + // (closeButton defaults to false) - dismissal here is via the provider's + // duration timeout, covered by the fake-timer test below rather than a + // click in this test. const closeButton = screen.queryByRole('button'); if (closeButton) { await userEvent.click(closeButton); expect(onDismiss).toHaveBeenCalledWith('abc'); } - // If no close button rendered, onDismiss fires via duration timeout — covered by integration }); - it('renders viewport as an ordered list', () => { - const { container } = renderManager({ messages: [], onDismiss: vi.fn() }); - expect(container.querySelector('ol')).toBeInTheDocument(); + // #581 acceptance criterion: "Toasts auto-dismiss after the configured + // duration ... and clean up their timers on unmount" - untested even under + // the original Radix implementation (no prior test exercised this at all). + it('auto-dismisses after the configured duration', () => { + vi.useFakeTimers(); + try { + const onDismiss = vi.fn(); + renderWithDuration({ messages: [{ id: 'abc', message: 'Bye' }], onDismiss }, 100); + + expect(onDismiss).not.toHaveBeenCalled(); + + act(() => { + vi.advanceTimersByTime(150); + }); + + expect(onDismiss).toHaveBeenCalledWith('abc'); + } finally { + vi.useRealTimers(); + } }); + + // Unmounting before the timer fires must not call onDismiss (or throw) - + // the timer needs to be cleared, not just left to fire into a gone + // component. This is the "clean up their timers on unmount" half of the + // same acceptance criterion. + it('does not call onDismiss after unmounting before the duration elapses', () => { + vi.useFakeTimers(); + try { + const onDismiss = vi.fn(); + const { unmount } = renderWithDuration( + { messages: [{ id: 'abc', message: 'Bye' }], onDismiss }, + 100 + ); + + unmount(); + + act(() => { + vi.advanceTimersByTime(150); + }); + + expect(onDismiss).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + // Accepted a11y gap (#581): Radix's Toast.Root/Viewport render as a real + //
  2. /
      (native list navigation for screen readers across stacked + // toasts); Tamagui's are View-based with no way to override the rendered + // tag from this call site. See the comment in ToastManager.tsx - the + // "renders viewport as an ordered list" / "renders each toast as a list + // item" assertions this file used to have no longer hold, deliberately. }); diff --git a/frontend/src/components/Toast/ToastManager.tsx b/frontend/src/components/Toast/ToastManager.tsx index 21e1bbde..eed08216 100644 --- a/frontend/src/components/Toast/ToastManager.tsx +++ b/frontend/src/components/Toast/ToastManager.tsx @@ -1,4 +1,4 @@ -import * as Toast from '@radix-ui/react-toast'; +import { Toast, ToastViewport } from 'tamagui'; import styles from './Toast.module.scss'; import type { Toast as ToastMessage } from '../../context/toastContextDef'; @@ -11,8 +11,23 @@ export default function ToastManager({ messages, onDismiss }: ToastManagerProps) return ( <> {messages.map(({ id, message }) => ( - /
        (Primitive.li / Primitive.ol in its source), giving + // screen readers native list navigation ("item 2 of 3") across + // stacked toasts. Tamagui's Toast/ToastViewport are View-based + // (styled(YStack, ...)) with no tag-override escape hatch - there is + // no way to make them render as li/ol from this call site. The + // announcement itself (role="status", aria-live politeness below) + // is preserved; only the native list *structure* is lost. Recorded + // here rather than silently dropped, per #587's baseline. + { if (!open) onDismiss(id); }} className={styles.toast} @@ -24,9 +39,9 @@ export default function ToastManager({ messages, onDismiss }: ToastManagerProps) {message} - + ))} - + ); } diff --git a/frontend/src/context/ToastContext.tsx b/frontend/src/context/ToastContext.tsx index 3ee8c981..d0d27375 100644 --- a/frontend/src/context/ToastContext.tsx +++ b/frontend/src/context/ToastContext.tsx @@ -1,6 +1,6 @@ import { useState, useCallback } from 'react'; import type { ReactElement, ReactNode } from 'react'; -import * as RadixToast from '@radix-ui/react-toast'; +import { ToastProvider as TamaguiToastProvider } from 'tamagui'; import { v4 as uuidv4 } from 'uuid'; import { ToastContext } from './toastContextDef'; import type { Toast } from './toastContextDef'; @@ -26,11 +26,11 @@ export function ToastProvider({ children, duration = 3300 }: ProviderProps): Rea const toastsEnabled = useFeatureFlag('toastsFeature'); return ( - + {children} {toastsEnabled && } - + ); }