diff --git a/frontend/src/components/ErrorBoundary.tsx b/frontend/src/components/ErrorBoundary.tsx index 6b4211ee..2a672be8 100644 --- a/frontend/src/components/ErrorBoundary.tsx +++ b/frontend/src/components/ErrorBoundary.tsx @@ -1,47 +1,87 @@ -import { Component } from 'react'; +import { Component, createRef } from 'react'; import type { ErrorInfo, ReactNode } from 'react'; +import { AlertTriangle } from 'lucide-react'; +import { Button } from '@/components/ui/button'; interface Props { - children: ReactNode; + children: ReactNode; } interface State { - hasError: boolean; - error: Error | null; + hasError: boolean; + error: Error | null; } +/** + * App-level catch-all error boundary. Sits above feature-specific + * boundaries (e.g. `LazyBoundary` around `` blocks) so any + * uncaught render error in a non-lazy subtree still produces a + * consistent recovery card instead of a blank page. + * + * Visually matches `LazyBoundary`: glass card, AlertTriangle icon, + * single Try-again CTA. The shared aesthetic is intentional so a user + * never sees two different "something went wrong" treatments depending + * on which boundary catches the error. + * + * "Try again" resets the boundary's state, which causes React to + * re-render the children. For deterministic errors this just shows the + * card again, which is the expected behavior of any error boundary; + * for transient errors (stale API response, race) it can recover + * cleanly. + */ class ErrorBoundary extends Component { - public state: State = { - hasError: false, - error: null, - }; + public state: State = { + hasError: false, + error: null, + }; - public static getDerivedStateFromError(error: Error): State { - return { hasError: true, error }; - } + /** + * Ref on the CTA button so we can focus it when the boundary trips. + * When a render error fires, focus is typically inside the now- + * unmounted subtree and falls back to ; keyboard users would + * otherwise have to tab from the top to reach the recovery action. + */ + private ctaRef = createRef(); - public componentDidCatch(error: Error, errorInfo: ErrorInfo) { - console.error('ErrorBoundary caught an error:', error, errorInfo); - } - - public render() { - if (this.state.hasError) { - return ( -
-

Something went wrong

-

{this.state.error?.message || 'Unknown error'}

- -
- ); + public static getDerivedStateFromError(error: Error): State { + return { hasError: true, error }; } - return this.props.children; - } + public componentDidCatch(error: Error, errorInfo: ErrorInfo) { + console.error('ErrorBoundary caught an error:', error, errorInfo); + } + + public componentDidUpdate(_prevProps: Props, prevState: State) { + if (!prevState.hasError && this.state.hasError) { + this.ctaRef.current?.focus(); + } + } + + public render() { + if (!this.state.hasError) return this.props.children; + + return ( +
+
+
+ +
+
+

Something went wrong

+

{this.state.error?.message || 'Unknown error'}

+
+ +
+
+ ); + } } export default ErrorBoundary; diff --git a/frontend/src/components/LazyBoundary.tsx b/frontend/src/components/LazyBoundary.tsx index 93e150ba..0b328788 100644 --- a/frontend/src/components/LazyBoundary.tsx +++ b/frontend/src/components/LazyBoundary.tsx @@ -1,4 +1,4 @@ -import { Component } from 'react'; +import { Component, createRef } from 'react'; import type { ErrorInfo, ReactNode } from 'react'; import { AlertTriangle } from 'lucide-react'; import { Button } from '@/components/ui/button'; @@ -37,6 +37,14 @@ class LazyBoundary extends Component { error: null, }; + /** + * Ref on the CTA button so we can focus it when the boundary trips. + * When a render error fires, focus is typically inside the now- + * unmounted subtree and falls back to ; keyboard users would + * otherwise have to tab from the top to reach the recovery action. + */ + private ctaRef = createRef(); + public static getDerivedStateFromError(error: Error): State { return { hasError: true, error }; } @@ -45,6 +53,15 @@ class LazyBoundary extends Component { console.error('LazyBoundary caught an error:', error, errorInfo); } + public componentDidUpdate(_prevProps: Props, prevState: State) { + // Focus the CTA the moment the error UI mounts, not on every + // re-render of the error state, so we don't steal focus from a + // user who has already tabbed elsewhere within the card. + if (!prevState.hasError && this.state.hasError) { + this.ctaRef.current?.focus(); + } + } + public render() { if (!this.state.hasError) return this.props.children; @@ -68,7 +85,7 @@ class LazyBoundary extends Component {

{title}

{body}

- diff --git a/frontend/src/hooks/__tests__/useDismissalState.test.ts b/frontend/src/hooks/__tests__/useDismissalState.test.ts index 43a8f409..33dd93cd 100644 --- a/frontend/src/hooks/__tests__/useDismissalState.test.ts +++ b/frontend/src/hooks/__tests__/useDismissalState.test.ts @@ -89,4 +89,72 @@ describe('useDismissalState', () => { expect(a.current.dismissed).toBe(true); expect(b.current.dismissed).toBe(false); }); + + it('syncs to dismissed=true when another tab fires a storage event with a recent timestamp', () => { + // Browsers fire `storage` events only in OTHER tabs than the one + // that wrote the change, so this test simulates "tab B receives + // a dismiss from tab A" by dispatching a synthetic StorageEvent. + const { result } = renderHook(() => useDismissalState(KEY)); + expect(result.current.dismissed).toBe(false); + + act(() => { + window.dispatchEvent( + new StorageEvent('storage', { + key: KEY, + newValue: String(Date.now()), + }), + ); + }); + + expect(result.current.dismissed).toBe(true); + }); + + it('syncs to dismissed=false when another tab fires a storage event with a null newValue', () => { + // null newValue is what the spec emits when localStorage.removeItem + // is called in another tab. + localStorage.setItem(KEY, String(Date.now())); + const { result } = renderHook(() => useDismissalState(KEY)); + expect(result.current.dismissed).toBe(true); + + act(() => { + window.dispatchEvent( + new StorageEvent('storage', { + key: KEY, + newValue: null, + }), + ); + }); + + expect(result.current.dismissed).toBe(false); + }); + + it('ignores storage events for unrelated keys', () => { + const { result } = renderHook(() => useDismissalState(KEY)); + + act(() => { + window.dispatchEvent( + new StorageEvent('storage', { + key: 'unrelated-key', + newValue: String(Date.now()), + }), + ); + }); + + expect(result.current.dismissed).toBe(false); + }); + + it('treats a storage event carrying a stale timestamp as not-dismissed', () => { + const { result } = renderHook(() => useDismissalState(KEY)); + + act(() => { + window.dispatchEvent( + new StorageEvent('storage', { + key: KEY, + newValue: String(Date.now() - DISMISS_DURATION_MS - 1), + }), + ); + }); + + expect(result.current.dismissed).toBe(false); + }); }); diff --git a/frontend/src/hooks/useDismissalState.ts b/frontend/src/hooks/useDismissalState.ts index 5e36f28a..608efd71 100644 --- a/frontend/src/hooks/useDismissalState.ts +++ b/frontend/src/hooks/useDismissalState.ts @@ -1,4 +1,4 @@ -import { useState } from 'react'; +import { useEffect, useState } from 'react'; const DISMISS_DURATION_MS = 24 * 60 * 60 * 1000; @@ -11,20 +11,36 @@ const DISMISS_DURATION_MS = 24 * 60 * 60 * 1000; * lazy initializer treats it as expired and returns `dismissed: false`, * so the gate shows the full upsell again on next mount. * - * Each hook instance owns its own React state. If two `` - * mounts share the same key in the same tab, dismissing one does not - * sync to the other until that other re-mounts. Cross-instance pub-sub - * is not provided; the dismissal is a soft UX nicety, not a security - * boundary. + * Cross-tab sync: a `storage` event listener on `window` propagates + * dismiss / restore actions from other tabs. The browser fires + * `storage` events only on tabs OTHER than the one that wrote the + * change, so this listener handles tab B updates after tab A + * dismisses or restores; same-tab updates flow through `setDismissed` + * directly. Stale timestamps that arrive past the 24h window are + * treated as not-dismissed. */ export function useDismissalState(key: string) { - const [dismissed, setDismissed] = useState(() => { - const dismissedAt = localStorage.getItem(key); - if (!dismissedAt) return false; - const ts = Number.parseInt(dismissedAt, 10); - if (!Number.isFinite(ts)) return false; - return Date.now() - ts < DISMISS_DURATION_MS; - }); + const [dismissed, setDismissed] = useState(() => readDismissedFromStorage(key)); + + useEffect(() => { + const onStorage = (event: StorageEvent) => { + if (event.key !== key) return; + // event.newValue is null when the entry was removed (restore) + // and a string when it was set (dismiss). + if (event.newValue === null) { + setDismissed(false); + return; + } + const ts = Number.parseInt(event.newValue, 10); + setDismissed(Number.isFinite(ts) && Date.now() - ts < DISMISS_DURATION_MS); + }; + window.addEventListener('storage', onStorage); + return () => window.removeEventListener('storage', onStorage); + // setDismissed is stable per React's setter guarantee; the handler + // closes over `key` only. Do not add setDismissed (harmless) or + // dismissed (would re-attach the listener on every state change) + // to satisfy a future drive-by exhaustive-deps "fix." + }, [key]); const dismiss = () => { localStorage.setItem(key, Date.now().toString()); @@ -38,3 +54,11 @@ export function useDismissalState(key: string) { return { dismissed, dismiss, restore }; } + +function readDismissedFromStorage(key: string): boolean { + const dismissedAt = localStorage.getItem(key); + if (!dismissedAt) return false; + const ts = Number.parseInt(dismissedAt, 10); + if (!Number.isFinite(ts)) return false; + return Date.now() - ts < DISMISS_DURATION_MS; +}