diff --git a/src/components/AppHeader.tsx b/src/components/AppHeader.tsx index 0ec0976..c100d06 100644 --- a/src/components/AppHeader.tsx +++ b/src/components/AppHeader.tsx @@ -1,7 +1,8 @@ -import { useState, useEffect, useRef } from "react"; +import { useState, useEffect } from "react"; import { useShallow } from "zustand/react/shallow"; import { useMachineStore } from "../stores/useMachineStore"; import { useUIStore } from "../stores/useUIStore"; +import { usePrevious } from "../hooks/usePrevious"; import { WorkflowStepper } from "./WorkflowStepper"; import { ErrorPopoverContent } from "./ErrorPopover"; import { @@ -65,9 +66,12 @@ export function AppHeader() { const [dismissedErrorCode, setDismissedErrorCode] = useState( null, ); - const prevMachineErrorRef = useRef(undefined); - const prevErrorMessageRef = useRef(null); - const prevPyodideErrorRef = useRef(null); + const [wasManuallyDismissed, setWasManuallyDismissed] = useState(false); + + // Track previous values for comparison + const prevMachineError = usePrevious(machineError); + const prevErrorMessage = usePrevious(machineErrorMessage); + const prevPyodideError = usePrevious(pyodideError); // Get state visual info for header status badge const stateVisual = getStateVisualInfo(machineStatus); @@ -89,31 +93,26 @@ export function AppHeader() { // Auto-open/close error popover based on error state changes /* eslint-disable react-hooks/set-state-in-effect */ useEffect(() => { - const currentError = machineError; - const prevError = prevMachineErrorRef.current; - const currentErrorMessage = machineErrorMessage; - const prevErrorMessage = prevErrorMessageRef.current; - const currentPyodideError = pyodideError; - const prevPyodideError = prevPyodideErrorRef.current; - // Check if there's any error now const hasAnyError = - machineErrorMessage || pyodideError || hasError(currentError); + machineErrorMessage || pyodideError || hasError(machineError); // Check if there was any error before const hadAnyError = - prevErrorMessage || prevPyodideError || hasError(prevError); + prevErrorMessage || prevPyodideError || hasError(prevMachineError); - // Auto-open popover when new error appears + // Auto-open popover when new error appears (but not if user manually dismissed) const isNewMachineError = - hasError(currentError) && - currentError !== prevError && - currentError !== dismissedErrorCode; + hasError(machineError) && + machineError !== prevMachineError && + machineError !== dismissedErrorCode; const isNewErrorMessage = - currentErrorMessage && currentErrorMessage !== prevErrorMessage; - const isNewPyodideError = - currentPyodideError && currentPyodideError !== prevPyodideError; + machineErrorMessage && machineErrorMessage !== prevErrorMessage; + const isNewPyodideError = pyodideError && pyodideError !== prevPyodideError; - if (isNewMachineError || isNewErrorMessage || isNewPyodideError) { + if ( + !wasManuallyDismissed && + (isNewMachineError || isNewErrorMessage || isNewPyodideError) + ) { setErrorPopoverOpen(true); } @@ -121,28 +120,34 @@ export function AppHeader() { if (!hasAnyError && hadAnyError) { setErrorPopoverOpen(false); setDismissedErrorCode(null); // Reset dismissed tracking + setWasManuallyDismissed(false); // Reset manual dismissal flag } - - // Update refs for next comparison - prevMachineErrorRef.current = currentError; - prevErrorMessageRef.current = currentErrorMessage; - prevPyodideErrorRef.current = currentPyodideError; - }, [machineError, machineErrorMessage, pyodideError, dismissedErrorCode]); + }, [ + machineError, + machineErrorMessage, + pyodideError, + dismissedErrorCode, + wasManuallyDismissed, + prevMachineError, + prevErrorMessage, + prevPyodideError, + ]); /* eslint-enable react-hooks/set-state-in-effect */ // Handle manual popover dismiss const handlePopoverOpenChange = (open: boolean) => { setErrorPopoverOpen(open); - // If user manually closes it, remember the current error state to prevent reopening - if (!open) { - // For machine errors, track the error code + // If user manually closes it while any error is present, remember this to prevent reopening + if ( + !open && + (hasError(machineError) || machineErrorMessage || pyodideError) + ) { + setWasManuallyDismissed(true); + // Also track the specific machine error code if present if (hasError(machineError)) { setDismissedErrorCode(machineError); } - // Update refs so we don't reopen for the same error message/pyodide error - prevErrorMessageRef.current = machineErrorMessage; - prevPyodideErrorRef.current = pyodideError; } }; diff --git a/src/hooks/usePrevious.ts b/src/hooks/usePrevious.ts new file mode 100644 index 0000000..f081f95 --- /dev/null +++ b/src/hooks/usePrevious.ts @@ -0,0 +1,26 @@ +/** + * usePrevious Hook + * + * Returns the previous value of a state or prop + * Useful for comparing current vs previous values in effects + * + * This implementation updates the ref in an effect instead of during render, + * which is compatible with React Concurrent Mode and Strict Mode. + * + * Note: The ref read on return is safe because it's only returning the previous value + * (from the last render), not the current value being passed in. This is the standard + * pattern for usePrevious hooks. + */ + +import { useEffect, useRef } from "react"; + +export function usePrevious(value: T): T | undefined { + const ref = useRef(undefined); + + useEffect(() => { + ref.current = value; + }, [value]); + + // eslint-disable-next-line react-hooks/refs + return ref.current; +}