feature: Migrate ErrorPopover to shadcn Popover component

- Migrated ErrorPopover to use shadcn PopoverContent
  - Removed forwardRef wrapper (handled by shadcn internally)
  - Replaced custom absolute positioning with PopoverContent component
  - Used cn() utility for cleaner className management
  - Maintained all styling with info/danger color variants

- Updated AppHeader to use Popover pattern
  - Replaced manual state management (showErrorPopover/setErrorPopover)
  - Removed refs and click-outside detection useEffect
  - Wrapped error button in Popover component with PopoverTrigger
  - Simplified code by removing 30+ lines of manual popover handling

Benefits:
- Better keyboard navigation and accessibility (built into Radix UI)
- Automatic focus management and ARIA attributes
- Cleaner, more maintainable code
- Consistent with other shadcn components in the app

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Jan-Henrik Bruhn 2025-12-21 00:06:38 +01:00
parent 621de91bc8
commit 5849a1e854
2 changed files with 123 additions and 150 deletions

View file

@ -1,9 +1,8 @@
import { useRef, useEffect } from "react";
import { useShallow } from "zustand/react/shallow"; import { useShallow } from "zustand/react/shallow";
import { useMachineStore } from "../stores/useMachineStore"; import { useMachineStore } from "../stores/useMachineStore";
import { useUIStore } from "../stores/useUIStore"; import { useUIStore } from "../stores/useUIStore";
import { WorkflowStepper } from "./WorkflowStepper"; import { WorkflowStepper } from "./WorkflowStepper";
import { ErrorPopover } from "./ErrorPopover"; import { ErrorPopoverContent } from "./ErrorPopover";
import { getStateVisualInfo } from "../utils/machineStateHelpers"; import { getStateVisualInfo } from "../utils/machineStateHelpers";
import { import {
CheckCircleIcon, CheckCircleIcon,
@ -15,6 +14,7 @@ import {
} from "@heroicons/react/24/solid"; } from "@heroicons/react/24/solid";
import { Button } from "@/components/ui/button"; import { Button } from "@/components/ui/button";
import { Badge } from "@/components/ui/badge"; import { Badge } from "@/components/ui/badge";
import { Popover, PopoverTrigger } from "@/components/ui/popover";
import { cn } from "@/lib/utils"; import { cn } from "@/lib/utils";
export function AppHeader() { export function AppHeader() {
@ -42,17 +42,12 @@ export function AppHeader() {
})), })),
); );
const { pyodideError, showErrorPopover, setErrorPopover } = useUIStore( const { pyodideError } = useUIStore(
useShallow((state) => ({ useShallow((state) => ({
pyodideError: state.pyodideError, pyodideError: state.pyodideError,
showErrorPopover: state.showErrorPopover,
setErrorPopover: state.setErrorPopover,
})), })),
); );
const errorPopoverRef = useRef<HTMLDivElement>(null);
const errorButtonRef = useRef<HTMLButtonElement>(null);
// Get state visual info for header status badge // Get state visual info for header status badge
const stateVisual = getStateVisualInfo(machineStatus); const stateVisual = getStateVisualInfo(machineStatus);
const stateIcons = { const stateIcons = {
@ -65,26 +60,6 @@ export function AppHeader() {
}; };
const StatusIcon = stateIcons[stateVisual.iconName]; const StatusIcon = stateIcons[stateVisual.iconName];
// Close error popover when clicking outside
useEffect(() => {
const handleClickOutside = (event: MouseEvent) => {
if (
errorPopoverRef.current &&
!errorPopoverRef.current.contains(event.target as Node) &&
errorButtonRef.current &&
!errorButtonRef.current.contains(event.target as Node)
) {
setErrorPopover(false);
}
};
if (showErrorPopover) {
document.addEventListener("mousedown", handleClickOutside);
return () =>
document.removeEventListener("mousedown", handleClickOutside);
}
}, [showErrorPopover, setErrorPopover]);
return ( return (
<header className="bg-gradient-to-r from-primary-600 via-primary-700 to-primary-800 dark:from-primary-700 dark:via-primary-800 dark:to-primary-900 px-4 sm:px-6 lg:px-8 py-3 shadow-lg border-b-2 border-primary-900/20 dark:border-primary-800/30 flex-shrink-0"> <header className="bg-gradient-to-r from-primary-600 via-primary-700 to-primary-800 dark:from-primary-700 dark:via-primary-800 dark:to-primary-900 px-4 sm:px-6 lg:px-8 py-3 shadow-lg border-b-2 border-primary-900/20 dark:border-primary-800/30 flex-shrink-0">
<div className="grid grid-cols-1 lg:grid-cols-[280px_1fr] gap-4 lg:gap-8 items-center"> <div className="grid grid-cols-1 lg:grid-cols-[280px_1fr] gap-4 lg:gap-8 items-center">
@ -157,10 +132,9 @@ export function AppHeader() {
)} )}
{/* Error indicator - always render to prevent layout shift */} {/* Error indicator - always render to prevent layout shift */}
<div className="relative"> <Popover>
<PopoverTrigger asChild>
<Button <Button
ref={errorButtonRef}
onClick={() => setErrorPopover(!showErrorPopover)}
size="sm" size="sm"
variant="destructive" variant="destructive"
className={cn( className={cn(
@ -203,18 +177,18 @@ export function AppHeader() {
})()} })()}
</span> </span>
</Button> </Button>
</PopoverTrigger>
{/* Error popover */} {/* Error popover content */}
{showErrorPopover && (machineErrorMessage || pyodideError) && ( {(machineErrorMessage || pyodideError) && (
<ErrorPopover <ErrorPopoverContent
ref={errorPopoverRef}
machineError={machineError} machineError={machineError}
isPairingError={isPairingError} isPairingError={isPairingError}
errorMessage={machineErrorMessage} errorMessage={machineErrorMessage}
pyodideError={pyodideError} pyodideError={pyodideError}
/> />
)} )}
</div> </Popover>
</div> </div>
</div> </div>
</div> </div>

View file

@ -1,19 +1,24 @@
import { forwardRef } from "react";
import { import {
ExclamationTriangleIcon, ExclamationTriangleIcon,
InformationCircleIcon, InformationCircleIcon,
} from "@heroicons/react/24/solid"; } from "@heroicons/react/24/solid";
import { getErrorDetails } from "../utils/errorCodeHelpers"; import { getErrorDetails } from "../utils/errorCodeHelpers";
import { PopoverContent } from "@/components/ui/popover";
import { cn } from "@/lib/utils";
interface ErrorPopoverProps { interface ErrorPopoverContentProps {
machineError?: number; machineError?: number;
isPairingError: boolean; isPairingError: boolean;
errorMessage?: string | null; errorMessage?: string | null;
pyodideError?: string | null; pyodideError?: string | null;
} }
export const ErrorPopover = forwardRef<HTMLDivElement, ErrorPopoverProps>( export function ErrorPopoverContent({
({ machineError, isPairingError, errorMessage, pyodideError }, ref) => { machineError,
isPairingError,
errorMessage,
pyodideError,
}: ErrorPopoverContentProps) {
const errorDetails = getErrorDetails(machineError); const errorDetails = getErrorDetails(machineError);
const isPairingErr = isPairingError; const isPairingErr = isPairingError;
const errorMsg = pyodideError || errorMessage || ""; const errorMsg = pyodideError || errorMessage || "";
@ -44,31 +49,29 @@ export const ErrorPopover = forwardRef<HTMLDivElement, ErrorPopoverProps>(
errorDetails?.title || (isPairingErr ? "Pairing Required" : "Error"); errorDetails?.title || (isPairingErr ? "Pairing Required" : "Error");
return ( return (
<div <PopoverContent
ref={ref} className={cn("w-[600px] border-l-4 p-4 backdrop-blur-sm", bgColor)}
className="absolute top-full mt-2 left-0 w-[600px] z-50 animate-fadeIn" align="start"
role="dialog"
aria-label="Error details"
>
<div
className={`${bgColor} border-l-4 p-4 rounded-lg shadow-xl backdrop-blur-sm`}
> >
<div className="flex items-start gap-3"> <div className="flex items-start gap-3">
<Icon className={`w-6 h-6 ${iconColor} flex-shrink-0 mt-0.5`} /> <Icon className={cn("w-6 h-6 flex-shrink-0 mt-0.5", iconColor)} />
<div className="flex-1"> <div className="flex-1">
<h3 className={`text-base font-semibold ${textColor} mb-2`}> <h3 className={cn("text-base font-semibold mb-2", textColor)}>
{title} {title}
</h3> </h3>
<p className={`text-sm ${descColor} mb-3`}> <p className={cn("text-sm mb-3", descColor)}>
{errorDetails?.description || errorMsg} {errorDetails?.description || errorMsg}
</p> </p>
{errorDetails?.solutions && errorDetails.solutions.length > 0 && ( {errorDetails?.solutions && errorDetails.solutions.length > 0 && (
<> <>
<h4 className={`text-sm font-semibold ${textColor} mb-2`}> <h4 className={cn("text-sm font-semibold mb-2", textColor)}>
{isInfo ? "Steps:" : "How to Fix:"} {isInfo ? "Steps:" : "How to Fix:"}
</h4> </h4>
<ol <ol
className={`list-decimal list-inside text-sm ${listColor} space-y-1.5`} className={cn(
"list-decimal list-inside text-sm space-y-1.5",
listColor,
)}
> >
{errorDetails.solutions.map((solution, index) => ( {errorDetails.solutions.map((solution, index) => (
<li key={index} className="pl-2"> <li key={index} className="pl-2">
@ -79,17 +82,13 @@ export const ErrorPopover = forwardRef<HTMLDivElement, ErrorPopoverProps>(
</> </>
)} )}
{machineError !== undefined && !errorDetails?.isInformational && ( {machineError !== undefined && !errorDetails?.isInformational && (
<p className={`text-xs ${descColor} mt-3 font-mono`}> <p className={cn("text-xs mt-3 font-mono", descColor)}>
Error Code: 0x Error Code: 0x
{machineError.toString(16).toUpperCase().padStart(2, "0")} {machineError.toString(16).toUpperCase().padStart(2, "0")}
</p> </p>
)} )}
</div> </div>
</div> </div>
</div> </PopoverContent>
</div>
); );
}, }
);
ErrorPopover.displayName = "ErrorPopover";