Address code review feedback on error pages
- Move statusIcon out of ErrorBoundary render as a plain module-level
function to avoid React re-creating a component type on every render
- Collapse 401 action into a single descriptive button label
("Go to Home Page to Sign In") instead of a fragmented button + hint
- Fix details/summary border radius collision by using overflow-hidden
on the container and removing rounded-md from the summary element;
separate the pre block with a border-t instead
https://claude.ai/code/session_011jv8desa5vhSkjZHSjWZiV
This commit is contained in:
parent
c8c1238963
commit
773ea7f932
1 changed files with 15 additions and 21 deletions
36
app/root.tsx
36
app/root.tsx
|
|
@ -89,7 +89,6 @@ export function ErrorBoundary({ error }: Route.ErrorBoundaryProps) {
|
||||||
let message = "An unexpected error occurred. Please try again later.";
|
let message = "An unexpected error occurred. Please try again later.";
|
||||||
let stack: string | undefined;
|
let stack: string | undefined;
|
||||||
let showSignInHint = false;
|
let showSignInHint = false;
|
||||||
|
|
||||||
if (isRouteErrorResponse(error)) {
|
if (isRouteErrorResponse(error)) {
|
||||||
status = error.status;
|
status = error.status;
|
||||||
switch (error.status) {
|
switch (error.status) {
|
||||||
|
|
@ -119,15 +118,6 @@ export function ErrorBoundary({ error }: Route.ErrorBoundaryProps) {
|
||||||
stack = import.meta.env.DEV ? error.stack : undefined;
|
stack = import.meta.env.DEV ? error.stack : undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
const StatusIcon = () => {
|
|
||||||
const cls = "h-14 w-14 text-muted-foreground";
|
|
||||||
if (status === 401) return <Lock className={cls} />;
|
|
||||||
if (status === 403) return <ShieldOff className={cls} />;
|
|
||||||
if (status === 404) return <FileQuestion className={cls} />;
|
|
||||||
if (status >= 500) return <ServerCrash className={cls} />;
|
|
||||||
return <AlertCircle className={cls} />;
|
|
||||||
};
|
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="min-h-screen bg-background text-foreground flex flex-col">
|
<div className="min-h-screen bg-background text-foreground flex flex-col">
|
||||||
<header className="border-b border-border/40 bg-background/95 backdrop-blur">
|
<header className="border-b border-border/40 bg-background/95 backdrop-blur">
|
||||||
|
|
@ -145,7 +135,7 @@ export function ErrorBoundary({ error }: Route.ErrorBoundaryProps) {
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<div className="flex justify-center">
|
<div className="flex justify-center">
|
||||||
<StatusIcon />
|
{statusIcon(status)}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<div className="space-y-2">
|
<div className="space-y-2">
|
||||||
|
|
@ -153,26 +143,21 @@ export function ErrorBoundary({ error }: Route.ErrorBoundaryProps) {
|
||||||
<p className="text-muted-foreground">{message}</p>
|
<p className="text-muted-foreground">{message}</p>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<div className="flex flex-col sm:flex-row gap-3 justify-center pt-2">
|
<div className="flex justify-center pt-2">
|
||||||
<a
|
<a
|
||||||
href="/"
|
href="/"
|
||||||
className="inline-flex items-center justify-center rounded-md text-sm font-medium bg-primary text-primary-foreground hover:bg-primary/90 h-9 px-4 py-2 transition-colors"
|
className="inline-flex items-center justify-center rounded-md text-sm font-medium bg-primary text-primary-foreground hover:bg-primary/90 h-9 px-4 py-2 transition-colors"
|
||||||
>
|
>
|
||||||
Go Home
|
{showSignInHint ? "Go to Home Page to Sign In" : "Go Home"}
|
||||||
</a>
|
</a>
|
||||||
{showSignInHint && (
|
|
||||||
<p className="text-sm text-muted-foreground self-center">
|
|
||||||
You can sign in from the home page.
|
|
||||||
</p>
|
|
||||||
)}
|
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{stack && (
|
{stack && (
|
||||||
<details className="text-left mt-8 border border-border rounded-md">
|
<details className="text-left mt-8 border border-border rounded-md overflow-hidden">
|
||||||
<summary className="cursor-pointer text-sm text-muted-foreground px-4 py-3 hover:bg-muted/50 rounded-md">
|
<summary className="cursor-pointer text-sm text-muted-foreground px-4 py-3 hover:bg-muted/50">
|
||||||
Stack trace (development only)
|
Stack trace (development only)
|
||||||
</summary>
|
</summary>
|
||||||
<pre className="p-4 overflow-x-auto text-xs text-muted-foreground">
|
<pre className="p-4 overflow-x-auto text-xs text-muted-foreground border-t border-border">
|
||||||
<code>{stack}</code>
|
<code>{stack}</code>
|
||||||
</pre>
|
</pre>
|
||||||
</details>
|
</details>
|
||||||
|
|
@ -182,3 +167,12 @@ export function ErrorBoundary({ error }: Route.ErrorBoundaryProps) {
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function statusIcon(status: number) {
|
||||||
|
const cls = "h-14 w-14 text-muted-foreground";
|
||||||
|
if (status === 401) return <Lock className={cls} />;
|
||||||
|
if (status === 403) return <ShieldOff className={cls} />;
|
||||||
|
if (status === 404) return <FileQuestion className={cls} />;
|
||||||
|
if (status >= 500) return <ServerCrash className={cls} />;
|
||||||
|
return <AlertCircle className={cls} />;
|
||||||
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue