Build image / build-and-push (push) Successful in 7s
Fixes from the PR #27 adversarial review (considered; not graded). Correctness / error-handling - Modal: focus once on mount and attach the keydown listener once (latest onClose/busy via refs), so a parent re-render (e.g. a background refetch) no longer re-fires focus() and steals it from the input being typed in. - Modal takes a `busy` prop; while a save/delete is in flight, Escape and backdrop clicks don't dismiss it (form/delete modals pass busy=pending), so an action can't be interrupted mid-flight and lose its error. - Form validates the *converted* centimeter values against the server's [1cm, 100m] bounds, so sub-cm / over-100m sizes fail client-side with a clear message instead of a generic server error. - displayFromCm rounds imperial to 0.1 ft so an 8 ft garden (244 cm) prefills as "8", not "8.01". Maintainability - Extracted ui/field.ts (useFieldId + fieldControlClass) shared by TextField/TextArea/Select. Added a `danger` Button variant, used by the delete modal; card edit/delete buttons gained focus-visible rings. - useUpdateGarden takes the id in its mutation variables (no dummy 0 during create). GardensPage shares a close handler; dropped a redundant zod annotation; 409 rebase reuses dimString; renamed `del` -> `deletion`. Verified in a browser: 8 ft round-trips to a prefilled "8"; a sub-cm value is rejected client-side with the modal staying open. tsc clean. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi
59 lines
1.7 KiB
TypeScript
59 lines
1.7 KiB
TypeScript
import { useEffect, useRef, type ReactNode } from 'react'
|
|
|
|
/**
|
|
* A centered modal dialog over a dimmed backdrop. Closes on Escape or a backdrop
|
|
* click, unless `busy` (a mutation is in flight) — then it stays put so the
|
|
* action can finish and report. The caller owns open/closed state (render only
|
|
* when open).
|
|
*/
|
|
export function Modal({
|
|
title,
|
|
onClose,
|
|
busy = false,
|
|
children,
|
|
}: {
|
|
title: string
|
|
onClose: () => void
|
|
busy?: boolean
|
|
children: ReactNode
|
|
}) {
|
|
const cardRef = useRef<HTMLDivElement>(null)
|
|
// Keep the latest onClose/busy in refs so the mount-only effect below never
|
|
// re-runs (which would re-attach the listener and steal focus on every parent
|
|
// re-render, e.g. during a background refetch).
|
|
const onCloseRef = useRef(onClose)
|
|
onCloseRef.current = onClose
|
|
const busyRef = useRef(busy)
|
|
busyRef.current = busy
|
|
|
|
useEffect(() => {
|
|
cardRef.current?.focus()
|
|
function onKey(e: KeyboardEvent) {
|
|
if (e.key === 'Escape' && !busyRef.current) onCloseRef.current()
|
|
}
|
|
document.addEventListener('keydown', onKey)
|
|
return () => document.removeEventListener('keydown', onKey)
|
|
}, [])
|
|
|
|
return (
|
|
<div
|
|
className="fixed inset-0 z-50 flex items-end justify-center bg-black/40 p-4 sm:items-center"
|
|
onMouseDown={(e) => {
|
|
if (e.target === e.currentTarget && !busy) onClose()
|
|
}}
|
|
>
|
|
<div
|
|
ref={cardRef}
|
|
role="dialog"
|
|
aria-modal="true"
|
|
aria-label={title}
|
|
tabIndex={-1}
|
|
className="w-full max-w-md rounded-xl border border-border bg-surface p-6 shadow-lg outline-none"
|
|
>
|
|
<h2 className="text-lg font-semibold tracking-tight text-fg">{title}</h2>
|
|
<div className="mt-4">{children}</div>
|
|
</div>
|
|
</div>
|
|
)
|
|
}
|