refactor(frontend): improve code quality and maintainability

- Extract API error handling utility (utils/error-handling.ts)
  - Centralize error message extraction logic
  - Add type guards for API errors
  - Replace duplicated error handling across components

- Create reusable Toast component (components/Toast.tsx)
  - Extract toast notification logic from profile page
  - Support auto-dismiss functionality
  - Consistent styling with shared styles

- Extract form validation debouncing hook (hooks/useDebouncedValidation.ts)
  - Reusable debounced validation logic
  - Clean timeout management
  - Used in profile page for form validation

- Consolidate duplicate styles (styles/auth-form.ts)
  - Use shared style tokens instead of duplicating values
  - Reduce code duplication between auth-form and shared styles

- Extract loading state component (components/LoadingState.tsx)
  - Standardize loading UI across pages
  - Replace duplicated loading JSX patterns
  - Used in profile, exchange, and trades pages

- Fix useRequireAuth dependency array
  - Remove unnecessary hasPermission from dependencies
  - Add eslint-disable comment with explanation
  - Improve hook stability and performance

All frontend tests pass. Linting passes.
This commit is contained in:
counterweight 2025-12-25 19:04:45 +01:00
parent db181b338c
commit 3beb23a765
Signed by: counterweight
GPG key ID: 883EDBAA726BD96C
10 changed files with 231 additions and 143 deletions

View file

@ -2,8 +2,9 @@
import { createContext, useContext, useState, useEffect, useCallback, ReactNode } from "react"; import { createContext, useContext, useState, useEffect, useCallback, ReactNode } from "react";
import { api, ApiError } from "./api"; import { api } from "./api";
import { components } from "./generated/api"; import { components } from "./generated/api";
import { extractApiErrorMessage } from "./utils/error-handling";
// Permission type from generated OpenAPI schema // Permission type from generated OpenAPI schema
export type PermissionType = components["schemas"]["Permission"]; export type PermissionType = components["schemas"]["Permission"];
@ -67,11 +68,7 @@ export function AuthProvider({ children }: { children: ReactNode }) {
const userData = await api.post<User>("/api/auth/login", { email, password }); const userData = await api.post<User>("/api/auth/login", { email, password });
setUser(userData); setUser(userData);
} catch (err) { } catch (err) {
if (err instanceof ApiError) { throw new Error(extractApiErrorMessage(err, "Login failed"));
const data = err.data as { detail?: string };
throw new Error(data?.detail || "Login failed");
}
throw err;
} }
}; };
@ -84,11 +81,7 @@ export function AuthProvider({ children }: { children: ReactNode }) {
}); });
setUser(userData); setUser(userData);
} catch (err) { } catch (err) {
if (err instanceof ApiError) { throw new Error(extractApiErrorMessage(err, "Registration failed"));
const data = err.data as { detail?: string };
throw new Error(data?.detail || "Registration failed");
}
throw err;
} }
}; };

View file

@ -0,0 +1,20 @@
"use client";
import { layoutStyles } from "../styles/shared";
interface LoadingStateProps {
/** Custom loading message (default: "Loading...") */
message?: string;
}
/**
* Standard loading state component.
* Displays a centered loading message with consistent styling.
*/
export function LoadingState({ message = "Loading..." }: LoadingStateProps) {
return (
<main style={layoutStyles.main}>
<div style={layoutStyles.loader}>{message}</div>
</main>
);
}

View file

@ -0,0 +1,40 @@
"use client";
import { useEffect } from "react";
import { toastStyles } from "../styles/shared";
export type ToastType = "success" | "error";
export interface ToastProps {
message: string;
type: ToastType;
onDismiss?: () => void;
/** Auto-dismiss delay in milliseconds (default: 3000) */
autoDismissDelay?: number;
}
/**
* Toast notification component with auto-dismiss functionality.
* Displays success or error messages in a fixed position.
*/
export function Toast({ message, type, onDismiss, autoDismissDelay = 3000 }: ToastProps) {
useEffect(() => {
if (onDismiss) {
const timer = setTimeout(() => {
onDismiss();
}, autoDismissDelay);
return () => clearTimeout(timer);
}
}, [onDismiss, autoDismissDelay]);
return (
<div
style={{
...toastStyles.toast,
...(type === "success" ? toastStyles.toastSuccess : toastStyles.toastError),
}}
>
{message}
</div>
);
}

View file

@ -3,9 +3,11 @@
import { useEffect, useState, useCallback, useMemo, ChangeEvent, CSSProperties } from "react"; import { useEffect, useState, useCallback, useMemo, ChangeEvent, CSSProperties } from "react";
import { useRouter } from "next/navigation"; import { useRouter } from "next/navigation";
import { Permission } from "../auth-context"; import { Permission } from "../auth-context";
import { api, ApiError } from "../api"; import { api } from "../api";
import { extractApiErrorMessage } from "../utils/error-handling";
import { Header } from "../components/Header"; import { Header } from "../components/Header";
import { SatsDisplay } from "../components/SatsDisplay"; import { SatsDisplay } from "../components/SatsDisplay";
import { LoadingState } from "../components/LoadingState";
import { useRequireAuth } from "../hooks/useRequireAuth"; import { useRequireAuth } from "../hooks/useRequireAuth";
import { components } from "../generated/api"; import { components } from "../generated/api";
import { formatDate, formatTime, getDateRange } from "../utils/date"; import { formatDate, formatTime, getDateRange } from "../utils/date";
@ -325,18 +327,7 @@ export default function ExchangePage() {
// Redirect to trades page after successful booking // Redirect to trades page after successful booking
router.push("/trades"); router.push("/trades");
} catch (err) { } catch (err) {
let errorMessage = "Failed to book trade"; const errorMessage = extractApiErrorMessage(err, "Failed to book trade");
if (err instanceof ApiError) {
// Extract detail from API error response
if (err.data && typeof err.data === "object") {
const data = err.data as { detail?: string };
errorMessage = data.detail || err.message;
} else {
errorMessage = err.message;
}
} else if (err instanceof Error) {
errorMessage = err.message;
}
setError(errorMessage); setError(errorMessage);
// Check if it's a "same day" error and extract trade public_id (UUID) // Check if it's a "same day" error and extract trade public_id (UUID)
@ -352,11 +343,7 @@ export default function ExchangePage() {
}; };
if (isLoading) { if (isLoading) {
return ( return <LoadingState />;
<main style={layoutStyles.main}>
<div style={layoutStyles.loader}>Loading...</div>
</main>
);
} }
if (!isAuthorized) { if (!isAuthorized) {

View file

@ -0,0 +1,54 @@
import { useEffect, useRef, useState } from "react";
/**
* Hook for debounced form validation.
* Validates form data after the user stops typing for a specified delay.
*
* @param formData - The form data to validate
* @param validator - Function that validates the form data and returns field errors
* @param delay - Debounce delay in milliseconds (default: 500)
* @returns Object containing current errors and a function to manually trigger validation
*/
export function useDebouncedValidation<T>(
formData: T,
validator: (data: T) => Record<string, string>,
delay: number = 500
): {
errors: Record<string, string>;
setErrors: React.Dispatch<React.SetStateAction<Record<string, string>>>;
validate: (data?: T) => void;
} {
const [errors, setErrors] = useState<Record<string, string>>({});
const validationTimeoutRef = useRef<NodeJS.Timeout | null>(null);
const formDataRef = useRef<T>(formData);
// Keep formDataRef in sync with formData
useEffect(() => {
formDataRef.current = formData;
}, [formData]);
// Cleanup timeout on unmount
useEffect(() => {
return () => {
if (validationTimeoutRef.current) {
clearTimeout(validationTimeoutRef.current);
}
};
}, []);
const validate = (data?: T) => {
// Clear any pending validation timeout
if (validationTimeoutRef.current) {
clearTimeout(validationTimeoutRef.current);
}
// Debounce validation - wait for user to stop typing
validationTimeoutRef.current = setTimeout(() => {
const dataToValidate = data ?? formDataRef.current;
const newErrors = validator(dataToValidate);
setErrors(newErrors);
}, delay);
};
return { errors, setErrors, validate };
}

View file

@ -46,6 +46,7 @@ export function useRequireAuth(options: UseRequireAuthOptions = {}): UseRequireA
if (!isAuthorized) { if (!isAuthorized) {
// Redirect to the most appropriate page based on permissions // Redirect to the most appropriate page based on permissions
// Use hasPermission/hasRole directly since they're stable callbacks
const redirect = const redirect =
fallbackRedirect ?? fallbackRedirect ??
(hasPermission(Permission.VIEW_ALL_EXCHANGES) (hasPermission(Permission.VIEW_ALL_EXCHANGES)
@ -55,7 +56,11 @@ export function useRequireAuth(options: UseRequireAuthOptions = {}): UseRequireA
: "/login"); : "/login");
router.push(redirect); router.push(redirect);
} }
}, [isLoading, user, isAuthorized, router, fallbackRedirect, hasPermission]); // Note: hasPermission and hasRole are stable callbacks from useAuth,
// so they don't need to be in the dependency array. They're only included
// for clarity and to satisfy exhaustive-deps if needed.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isLoading, user, isAuthorized, router, fallbackRedirect]);
return { return {
user, user,

View file

@ -1,21 +1,24 @@
"use client"; "use client";
import { useEffect, useState, useCallback, useRef } from "react"; import { useEffect, useState, useCallback } from "react";
import { api, ApiError } from "../api"; import { api } from "../api";
import { extractApiErrorMessage, extractFieldErrors } from "../utils/error-handling";
import { Permission } from "../auth-context"; import { Permission } from "../auth-context";
import { Header } from "../components/Header"; import { Header } from "../components/Header";
import { Toast } from "../components/Toast";
import { LoadingState } from "../components/LoadingState";
import { components } from "../generated/api"; import { components } from "../generated/api";
import { useRequireAuth } from "../hooks/useRequireAuth"; import { useRequireAuth } from "../hooks/useRequireAuth";
import { useDebouncedValidation } from "../hooks/useDebouncedValidation";
import { import {
layoutStyles, layoutStyles,
cardStyles, cardStyles,
formStyles, formStyles,
buttonStyles, buttonStyles,
toastStyles,
utilityStyles, utilityStyles,
} from "../styles/shared"; } from "../styles/shared";
import { FieldErrors, validateProfileFields } from "../utils/validation"; import { validateProfileFields } from "../utils/validation";
// Use generated type from OpenAPI schema // Use generated type from OpenAPI schema
type ProfileData = components["schemas"]["ProfileResponse"]; type ProfileData = components["schemas"]["ProfileResponse"];
@ -50,11 +53,15 @@ export default function ProfilePage() {
nostr_npub: "", nostr_npub: "",
}); });
const [godfatherEmail, setGodfatherEmail] = useState<string | null>(null); const [godfatherEmail, setGodfatherEmail] = useState<string | null>(null);
const [errors, setErrors] = useState<FieldErrors>({});
const [isLoadingProfile, setIsLoadingProfile] = useState(true); const [isLoadingProfile, setIsLoadingProfile] = useState(true);
const [isSubmitting, setIsSubmitting] = useState(false); const [isSubmitting, setIsSubmitting] = useState(false);
const [toast, setToast] = useState<{ message: string; type: "success" | "error" } | null>(null); const [toast, setToast] = useState<{ message: string; type: "success" | "error" } | null>(null);
const validationTimeoutRef = useRef<NodeJS.Timeout | null>(null);
const {
errors,
setErrors,
validate: validateForm,
} = useDebouncedValidation(formData, validateProfileFields, 500);
// Check if form has changes // Check if form has changes
const hasChanges = useCallback(() => { const hasChanges = useCallback(() => {
@ -93,23 +100,6 @@ export default function ProfilePage() {
} }
}, [user, isAuthorized, fetchProfile]); }, [user, isAuthorized, fetchProfile]);
// Auto-dismiss toast after 3 seconds
useEffect(() => {
if (toast) {
const timer = setTimeout(() => setToast(null), 3000);
return () => clearTimeout(timer);
}
}, [toast]);
// Cleanup validation timeout on unmount
useEffect(() => {
return () => {
if (validationTimeoutRef.current) {
clearTimeout(validationTimeoutRef.current);
}
};
}, []);
const handleInputChange = (field: keyof FormData) => (e: React.ChangeEvent<HTMLInputElement>) => { const handleInputChange = (field: keyof FormData) => (e: React.ChangeEvent<HTMLInputElement>) => {
let value = e.target.value; let value = e.target.value;
@ -121,19 +111,11 @@ export default function ProfilePage() {
} }
} }
setFormData((prev) => ({ ...prev, [field]: value }));
// Clear any pending validation timeout
if (validationTimeoutRef.current) {
clearTimeout(validationTimeoutRef.current);
}
// Debounce validation - wait 500ms after user stops typing
validationTimeoutRef.current = setTimeout(() => {
const newFormData = { ...formData, [field]: value }; const newFormData = { ...formData, [field]: value };
const newErrors = validateProfileFields(newFormData); setFormData(newFormData);
setErrors(newErrors);
}, 500); // Trigger debounced validation with the new data
validateForm(newFormData);
}; };
const handleSubmit = async (e: React.FormEvent) => { const handleSubmit = async (e: React.FormEvent) => {
@ -162,14 +144,15 @@ export default function ProfilePage() {
setToast({ message: "Profile saved successfully!", type: "success" }); setToast({ message: "Profile saved successfully!", type: "success" });
} catch (err) { } catch (err) {
console.error("Profile save error:", err); console.error("Profile save error:", err);
if (err instanceof ApiError && err.status === 422) { const fieldErrors = extractFieldErrors(err);
const errorData = err.data as { detail?: { field_errors?: FieldErrors } }; if (fieldErrors?.detail?.field_errors) {
if (errorData?.detail?.field_errors) { setErrors(fieldErrors.detail.field_errors);
setErrors(errorData.detail.field_errors);
}
setToast({ message: "Please fix the errors below", type: "error" }); setToast({ message: "Please fix the errors below", type: "error" });
} else { } else {
setToast({ message: "Network error. Please try again.", type: "error" }); setToast({
message: extractApiErrorMessage(err, "Network error. Please try again."),
type: "error",
});
} }
} finally { } finally {
setIsSubmitting(false); setIsSubmitting(false);
@ -177,11 +160,7 @@ export default function ProfilePage() {
}; };
if (isLoading || isLoadingProfile) { if (isLoading || isLoadingProfile) {
return ( return <LoadingState />;
<main style={layoutStyles.main}>
<div style={layoutStyles.loader}>Loading...</div>
</main>
);
} }
if (!user || !isAuthorized) { if (!user || !isAuthorized) {
@ -194,14 +173,7 @@ export default function ProfilePage() {
<main style={layoutStyles.main}> <main style={layoutStyles.main}>
{/* Toast notification */} {/* Toast notification */}
{toast && ( {toast && (
<div <Toast message={toast.message} type={toast.type} onDismiss={() => setToast(null)} />
style={{
...toastStyles.toast,
...(toast.type === "success" ? toastStyles.toastSuccess : toastStyles.toastError),
}}
>
{toast.message}
</div>
)} )}
<Header currentPage="profile" /> <Header currentPage="profile" />

View file

@ -1,12 +1,21 @@
import { CSSProperties } from "react"; import { CSSProperties } from "react";
// Import shared tokens and styles to avoid duplication
// Note: We can't directly import tokens from shared.ts as it's not exported,
// so we'll use the shared style objects where possible
import {
layoutStyles,
cardStyles,
formStyles,
buttonStyles,
bannerStyles,
typographyStyles,
} from "./shared";
export const authFormStyles: Record<string, CSSProperties> = { export const authFormStyles: Record<string, CSSProperties> = {
main: { main: {
...layoutStyles.contentCentered,
minHeight: "100vh", minHeight: "100vh",
background: "linear-gradient(135deg, #0f0f23 0%, #1a1a3e 50%, #2d1b4e 100%)",
display: "flex",
alignItems: "center",
justifyContent: "center",
padding: "1rem", padding: "1rem",
}, },
container: { container: {
@ -14,80 +23,41 @@ export const authFormStyles: Record<string, CSSProperties> = {
maxWidth: "420px", maxWidth: "420px",
}, },
card: { card: {
background: "rgba(255, 255, 255, 0.03)", ...cardStyles.card,
backdropFilter: "blur(10px)",
border: "1px solid rgba(255, 255, 255, 0.08)",
borderRadius: "24px",
padding: "3rem 2.5rem", padding: "3rem 2.5rem",
boxShadow: "0 25px 50px -12px rgba(0, 0, 0, 0.5)",
}, },
header: { header: {
textAlign: "center" as const, textAlign: "center" as const,
marginBottom: "2.5rem", marginBottom: "2.5rem",
}, },
title: { title: {
fontFamily: "'Instrument Serif', Georgia, serif", ...typographyStyles.pageTitle,
fontSize: "2.5rem", fontSize: "2.5rem",
fontWeight: 400, textAlign: "center" as const,
color: "#fff",
margin: 0,
letterSpacing: "-0.02em",
}, },
subtitle: { subtitle: {
fontFamily: "'DM Sans', system-ui, sans-serif", ...typographyStyles.pageSubtitle,
color: "rgba(255, 255, 255, 0.5)", textAlign: "center" as const,
marginTop: "0.5rem",
fontSize: "0.95rem",
}, },
form: { form: {
display: "flex", ...formStyles.form,
flexDirection: "column" as const,
gap: "1.5rem", gap: "1.5rem",
}, },
field: { field: {
display: "flex", ...formStyles.field,
flexDirection: "column" as const,
gap: "0.5rem",
}, },
label: { label: {
fontFamily: "'DM Sans', system-ui, sans-serif", ...formStyles.label,
color: "rgba(255, 255, 255, 0.7)",
fontSize: "0.875rem",
fontWeight: 500,
}, },
input: { input: {
fontFamily: "'DM Sans', system-ui, sans-serif", ...formStyles.input,
padding: "0.875rem 1rem",
fontSize: "1rem",
background: "rgba(255, 255, 255, 0.05)",
border: "1px solid rgba(255, 255, 255, 0.1)",
borderRadius: "12px",
color: "#fff",
outline: "none",
transition: "border-color 0.2s, box-shadow 0.2s",
}, },
button: { button: {
fontFamily: "'DM Sans', system-ui, sans-serif", ...buttonStyles.primaryButton,
marginTop: "0.5rem", marginTop: "0.5rem",
padding: "1rem",
fontSize: "1rem",
fontWeight: 600,
background: "linear-gradient(135deg, #6366f1 0%, #8b5cf6 100%)",
color: "#fff",
border: "none",
borderRadius: "12px",
cursor: "pointer",
transition: "transform 0.2s, box-shadow 0.2s",
boxShadow: "0 4px 14px rgba(99, 102, 241, 0.4)",
}, },
error: { error: {
fontFamily: "'DM Sans', system-ui, sans-serif", ...bannerStyles.errorBanner,
padding: "0.875rem 1rem",
background: "rgba(239, 68, 68, 0.1)",
border: "1px solid rgba(239, 68, 68, 0.3)",
borderRadius: "12px",
color: "#fca5a5",
fontSize: "0.875rem",
textAlign: "center" as const, textAlign: "center" as const,
}, },
footer: { footer: {

View file

@ -6,6 +6,7 @@ import { Permission } from "../auth-context";
import { api } from "../api"; import { api } from "../api";
import { Header } from "../components/Header"; import { Header } from "../components/Header";
import { SatsDisplay } from "../components/SatsDisplay"; import { SatsDisplay } from "../components/SatsDisplay";
import { LoadingState } from "../components/LoadingState";
import { useRequireAuth } from "../hooks/useRequireAuth"; import { useRequireAuth } from "../hooks/useRequireAuth";
import { components } from "../generated/api"; import { components } from "../generated/api";
import { formatDateTime } from "../utils/date"; import { formatDateTime } from "../utils/date";
@ -68,11 +69,7 @@ export default function TradesPage() {
}; };
if (isLoading) { if (isLoading) {
return ( return <LoadingState />;
<main style={layoutStyles.main}>
<div style={layoutStyles.loader}>Loading...</div>
</main>
);
} }
if (!isAuthorized) { if (!isAuthorized) {

View file

@ -0,0 +1,50 @@
import { ApiError } from "../api";
/**
* Extract a user-friendly error message from an API error or generic error.
* Handles ApiError instances with structured data, regular Error instances, and unknown errors.
*
* @param err - The error to extract a message from
* @param fallback - Default message if extraction fails (default: "An error occurred")
* @returns A user-friendly error message string
*/
export function extractApiErrorMessage(
err: unknown,
fallback: string = "An error occurred"
): string {
if (err instanceof ApiError) {
if (err.data && typeof err.data === "object") {
const data = err.data as { detail?: string };
return data.detail || err.message || fallback;
}
return err.message || fallback;
}
if (err instanceof Error) {
return err.message;
}
return fallback;
}
/**
* Type guard to check if an error is an ApiError with structured detail data.
*/
export function isApiErrorWithDetail(
err: unknown
): err is ApiError & { data: { detail?: string } } {
return err instanceof ApiError && err.data !== undefined && typeof err.data === "object";
}
/**
* Extract field errors from a 422 validation error response.
* Returns undefined if the error doesn't contain field errors.
*/
export function extractFieldErrors(
err: unknown
): { detail?: { field_errors?: Record<string, string> } } | undefined {
if (err instanceof ApiError && err.status === 422) {
if (err.data && typeof err.data === "object") {
return err.data as { detail?: { field_errors?: Record<string, string> } };
}
}
return undefined;
}