gsadmin/sencho
1
0
Fork 0
mirror of https://github.com/Studio-Saelix/sencho.git synced 2026-07-27 20:29:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

chore(frontend): polish error boundaries and dismissal sync (#877)

Browse Source 
Bundles three small follow-ups deferred from the recent lazy-loading
and gate-refactor PRs:

ErrorBoundary visual harmonization. The top-level boundary used a red
banner with custom button styling that no longer matched the glass-
card aesthetic the lock cards and LazyBoundary settled on. Replace
with the same glass-card + AlertTriangle layout. The user already
knows something broke when this fires; a calm card with a clear Try
again CTA is more actionable than the louder treatment, and a
consistent recovery surface across both boundaries means a user never
sees two different "something went wrong" treatments depending on
which boundary catches the error.

Keyboard focus on boundary trip. When either boundary trips, focus
typically falls back to <body> because the throwing subtree
unmounted. Keyboard users would have to tab from the top to reach the
recovery action. Add a ref on the CTA button and a componentDidUpdate
gate that focuses it on the false-to-true hasError transition. The
gate fires once per trip, not on every error-state re-render, so a
user who tabbed elsewhere within the card does not get focus stolen
back. Verified the gate also fires on a re-error after Try again
(setState({hasError:false}) re-renders with prevState.hasError=false,
the next throw flips to true and the transition condition triggers).

Cross-tab dismissal sync in useDismissalState. The hook previously
read localStorage only in the lazy initializer, so dismissing in tab
A did not propagate to tab B until tab B re-mounted. Add a useEffect
that listens for storage events on the configured key. The browser
fires storage events only in OTHER tabs than the one that wrote the
change, so this handles the tab B receives tab A's dismiss case;
same-tab updates flow through setDismissed directly, unchanged.
Malformed event.newValue (NaN, empty string) defaults to dismissed=
false, the conservative outcome.

Adds 4 new vitest cases for the storage-event paths: recent
timestamp, null newValue (restore), unrelated key, and stale
timestamp.
This commit is contained in:
Anso
2026-05-02 04:15:49 -04:00
committed by GitHub
parent 677f0778e7
commit 72919ccd1b
4 changed files with 195 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/components/ErrorBoundary.tsx
+71 -31
View File
@@ -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 `<Suspense>` 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<Props, State> {
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 <body>; keyboard users would
* otherwise have to tab from the top to reach the recovery action.
*/
private ctaRef = createRef<HTMLButtonElement>();
public componentDidCatch(error: Error, errorInfo: ErrorInfo) {
console.error('ErrorBoundary caught an error:', error, errorInfo);
}
public render() {
if (this.state.hasError) {
return (
<div className="p-6 bg-red-900/20 border border-red-500 rounded-xl m-4">
<h2 className="text-lg font-bold text-red-500 mb-2">Something went wrong</h2>
<p className="text-red-300 text-sm mb-4">{this.state.error?.message || 'Unknown error'}</p>
<button
className="px-4 py-2 bg-red-500 text-white rounded-lg hover:bg-red-600"
onClick={() => this.setState({ hasError: false, error: null })}
>
Try again
</button>
</div>
);
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 (
<div className="flex flex-1 items-center justify-center min-h-[280px] p-8" role="alert">
<div className="flex flex-col items-center gap-4 rounded-xl border border-glass-border bg-glass px-10 py-8 text-center max-w-md">
<div className="flex items-center justify-center w-12 h-12 rounded-full border border-glass-border bg-glass">
<AlertTriangle className="w-5 h-5 text-stat-subtitle" strokeWidth={1.5} />
</div>
<div className="flex flex-col gap-1">
<p className="text-sm font-semibold text-stat-value">Something went wrong</p>
<p className="text-sm text-stat-subtitle">{this.state.error?.message || 'Unknown error'}</p>
</div>
<Button
ref={this.ctaRef}
variant="outline"
size="sm"
onClick={() => this.setState({ hasError: false, error: null })}
>
Try again
</Button>
</div>
</div>
);
}
}
export default ErrorBoundary;
frontend/src/components/LazyBoundary.tsx
+19 -2
View File
@@ -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<Props, State> {
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 <body>; keyboard users would
* otherwise have to tab from the top to reach the recovery action.
*/
private ctaRef = createRef<HTMLButtonElement>();
public static getDerivedStateFromError(error: Error): State {
return { hasError: true, error };
}
@@ -45,6 +53,15 @@ class LazyBoundary extends Component<Props, State> {
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<Props, State> {
<p className="text-sm font-semibold text-stat-value">{title}</p>
<p className="text-sm text-stat-subtitle">{body}</p>
</div>
<Button variant="outline" size="sm" onClick={onCta}>
<Button ref={this.ctaRef} variant="outline" size="sm" onClick={onCta}>
{ctaLabel}
</Button>
</div>
frontend/src/hooks/__tests__/useDismissalState.test.ts
+68
View File
@@ -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);
});
});
frontend/src/hooks/useDismissalState.ts
+37 -13
View File
@@ -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 `<PaidGate>`
* 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;
}