Gardens UI: list page + create/edit/delete (#8) #27
@@ -0,0 +1,42 @@
|
|||||||
|
import { useState } from 'react'
|
||||||
|
import { Modal } from '@/components/ui/Modal'
|
||||||
|
import { Alert } from '@/components/ui/Alert'
|
||||||
|
import { Button } from '@/components/ui/Button'
|
||||||
|
import { errorMessage } from '@/lib/api'
|
||||||
|
import { useDeleteGarden, type Garden } from '@/lib/gardens'
|
||||||
|
|
||||||
|
/** Confirmation dialog for deleting a garden (and everything in it). */
|
||||||
|
export function DeleteGardenModal({ garden, onClose }: { garden: Garden; onClose: () => void }) {
|
||||||
|
const deletion = useDeleteGarden()
|
||||||
|
const [error, setError] = useState<string | null>(null)
|
||||||
|
|
||||||
|
async function onConfirm() {
|
||||||
|
setError(null)
|
||||||
|
try {
|
||||||
|
await deletion.mutateAsync(garden.id)
|
||||||
|
onClose()
|
||||||
|
} catch (err) {
|
||||||
|
setError(errorMessage(err, 'Could not delete the garden.'))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return (
|
||||||
|
<Modal title="Delete garden" onClose={onClose} busy={deletion.isPending}>
|
||||||
|
<div className="flex flex-col gap-4">
|
||||||
|
<p className="text-sm text-muted">
|
||||||
|
Delete <span className="font-medium text-fg">{garden.name}</span> and everything planned in it?
|
||||||
|
This can't be undone.
|
||||||
|
</p>
|
||||||
|
{error && <Alert>{error}</Alert>}
|
||||||
|
<div className="flex justify-end gap-2">
|
||||||
|
<Button type="button" variant="ghost" onClick={onClose} disabled={deletion.isPending}>
|
||||||
|
Cancel
|
||||||
|
|
|||||||
|
</Button>
|
||||||
|
<Button type="button" variant="danger" onClick={onConfirm} disabled={deletion.isPending}>
|
||||||
|
{deletion.isPending ? 'Deleting…' : 'Delete'}
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</Modal>
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -0,0 +1,49 @@
|
|||||||
|
import { Link } from '@tanstack/react-router'
|
||||||
|
import type { Garden } from '@/lib/gardens'
|
||||||
|
import { formatDimensions } from '@/lib/units'
|
||||||
|
|
||||||
|
/**
|
||||||
|
* One garden as a card: the body links into the editor (/gardens/:id); the
|
||||||
|
* footer has edit/delete actions (kept out of the link so they don't navigate).
|
||||||
|
*/
|
||||||
|
export function GardenCard({
|
||||||
|
garden,
|
||||||
|
onEdit,
|
||||||
|
onDelete,
|
||||||
|
}: {
|
||||||
|
garden: Garden
|
||||||
|
onEdit: () => void
|
||||||
|
onDelete: () => void
|
||||||
|
}) {
|
||||||
|
return (
|
||||||
|
<div className="flex flex-col rounded-xl border border-border bg-surface transition-colors hover:border-accent/50">
|
||||||
|
<Link
|
||||||
|
to="/gardens/$gardenId"
|
||||||
|
params={{ gardenId: String(garden.id) }}
|
||||||
|
className="flex-1 rounded-t-xl p-4 outline-none focus-visible:ring-2 focus-visible:ring-accent/40"
|
||||||
|
>
|
||||||
|
<h3 className="truncate font-semibold text-fg">{garden.name}</h3>
|
||||||
|
<p className="mt-1 text-sm text-muted">
|
||||||
|
{formatDimensions(garden.widthCm, garden.heightCm, garden.unitPref)}
|
||||||
|
</p>
|
||||||
|
{garden.notes && <p className="mt-2 line-clamp-2 text-sm text-muted">{garden.notes}</p>}
|
||||||
|
</Link>
|
||||||
|
<div className="flex justify-end gap-1 border-t border-border px-2 py-1.5">
|
||||||
|
<button
|
||||||
|
gitea-actions
commented
🟠 Raw elements bypass shared Button component styles and accessibility maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟠 **Raw <button> elements bypass shared Button component styles and accessibility**
_maintainability · flagged by 2 models_
- **`web/src/components/gardens/GardenCard.tsx:32`** — The card footer uses raw `<button>` elements instead of the shared `<Button>` component introduced/used everywhere else in this PR. This duplicates hover/transition styles and, more importantly, omits the `focus-visible:ring-2 focus-visible:ring-accent/40` and `disabled:cursor-not-allowed disabled:opacity-60` behaviour that `Button` provides. If these actions ever need disabled states or if the design system changes, edits must be made in bo…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
type="button"
|
||||||
|
onClick={onEdit}
|
||||||
|
className="rounded-md px-2.5 py-1 text-sm font-medium text-muted outline-none transition-colors hover:bg-border/50 hover:text-fg focus-visible:ring-2 focus-visible:ring-accent/40"
|
||||||
|
>
|
||||||
|
Edit
|
||||||
|
</button>
|
||||||
|
<button
|
||||||
|
gitea-actions
commented
🟠 Raw elements bypass shared Button component styles and accessibility maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟠 **Raw <button> elements bypass shared Button component styles and accessibility**
_maintainability · flagged by 1 model_
- **`web/src/components/gardens/GardenCard.tsx:39`** — The card footer uses raw `<button>` elements instead of the shared `<Button>` component introduced/used everywhere else in this PR. This duplicates hover/transition styles and, more importantly, omits the `focus-visible:ring-2 focus-visible:ring-accent/40` and `disabled:cursor-not-allowed disabled:opacity-60` behaviour that `Button` provides. If these actions ever need disabled states or if the design system changes, edits must be made in bo…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
type="button"
|
||||||
|
onClick={onDelete}
|
||||||
|
className="rounded-md px-2.5 py-1 text-sm font-medium text-muted outline-none transition-colors hover:bg-red-500/10 hover:text-red-600 focus-visible:ring-2 focus-visible:ring-red-500/40 dark:hover:text-red-400"
|
||||||
|
>
|
||||||
|
Delete
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -0,0 +1,163 @@
|
|||||||
|
import { useState, type FormEvent } from 'react'
|
||||||
|
import { Modal } from '@/components/ui/Modal'
|
||||||
|
import { Alert } from '@/components/ui/Alert'
|
||||||
|
import { Button } from '@/components/ui/Button'
|
||||||
|
import { Select } from '@/components/ui/Select'
|
||||||
|
import { TextArea } from '@/components/ui/TextArea'
|
||||||
|
import { TextField } from '@/components/ui/TextField'
|
||||||
|
import { errorMessage } from '@/lib/api'
|
||||||
|
import { conflictGarden, useCreateGarden, useUpdateGarden, type Garden } from '@/lib/gardens'
|
||||||
|
import {
|
||||||
|
cmFromDisplay,
|
||||||
|
dimensionUnitLabel,
|
||||||
|
displayFromCm,
|
||||||
|
isValidDimensionCm,
|
||||||
|
type UnitPref,
|
||||||
|
} from '@/lib/units'
|
||||||
|
|
||||||
|
const DEFAULT_METERS = 10 // matches the server's 10 m default
|
||||||
|
|
||||||
|
const unitOptions = [
|
||||||
|
{ value: 'metric', label: 'Metric (m)' },
|
||||||
|
{ value: 'imperial', label: 'Imperial (ft)' },
|
||||||
|
]
|
||||||
|
|
||||||
|
function dimString(cm: number | undefined, unit: UnitPref): string {
|
||||||
|
return cm === undefined ? String(DEFAULT_METERS) : String(displayFromCm(cm, unit))
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Create (no garden) or edit (garden given) form. Dimensions are entered in the
|
||||||
|
* selected unit and converted to centimeters for the API; switching units
|
||||||
|
* converts the current values so the physical size is preserved. A 409 rebases
|
||||||
|
gitea-actions
commented
🟡 Dummy id 0 passed to useUpdateGarden during creation is a leaky abstraction maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟡 **Dummy id 0 passed to useUpdateGarden during creation is a leaky abstraction**
_maintainability · flagged by 2 models_
- `web/src/components/gardens/GardenFormModal.tsx:32` — `useUpdateGarden(garden?.id ?? 0)` passes a dummy `0` during creation. The hook is never called, but the `?? 0` is a leaky abstraction—future readers must infer it is safe. **Fix:** make `useUpdateGarden` accept `id?: number` and return a no-op mutation when absent, removing the magic placeholder.
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
* the form onto the server's fresh row.
|
||||||
|
*/
|
||||||
|
export function GardenFormModal({ garden, onClose }: { garden?: Garden; onClose: () => void }) {
|
||||||
|
const isEdit = !!garden
|
||||||
|
const create = useCreateGarden()
|
||||||
|
const update = useUpdateGarden()
|
||||||
|
const pending = create.isPending || update.isPending
|
||||||
|
|
||||||
|
const [name, setName] = useState(garden?.name ?? '')
|
||||||
|
const [unit, setUnit] = useState<UnitPref>(garden?.unitPref ?? 'metric')
|
||||||
|
const [width, setWidth] = useState(() => dimString(garden?.widthCm, garden?.unitPref ?? 'metric'))
|
||||||
|
const [height, setHeight] = useState(() => dimString(garden?.heightCm, garden?.unitPref ?? 'metric'))
|
||||||
|
gitea-actions
commented
🟡 Unit-switching conversion is lossy and contradicts preserve-physical-size claim error-handling, maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟡 **Unit-switching conversion is lossy and contradicts preserve-physical-size claim**
_error-handling, maintainability · flagged by 2 models_
- `web/src/components/gardens/GardenFormModal.tsx:44-52` — Unit switching goes through `cmFromDisplay` → `Math.round` → `displayFromCm`. This is lossy: repeated toggles can drift. The comment claims the physical size is preserved, which is only true to ±1 cm. **Fix:** keep a hidden `cm` state (or ref) for the true physical size and derive the display string from it, eliminating round-trip noise.
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
const [notes, setNotes] = useState(garden?.notes ?? '')
|
||||||
|
const [version, setVersion] = useState(garden?.version ?? 0)
|
||||||
|
const [conflict, setConflict] = useState<string | null>(null)
|
||||||
|
const [formError, setFormError] = useState<string | null>(null)
|
||||||
|
|
||||||
|
function changeUnit(next: UnitPref) {
|
||||||
|
const convert = (s: string) => {
|
||||||
|
const v = parseFloat(s)
|
||||||
|
return Number.isFinite(v) ? String(displayFromCm(cmFromDisplay(v, unit), next)) : s
|
||||||
|
}
|
||||||
|
setWidth(convert(width))
|
||||||
|
setHeight(convert(height))
|
||||||
|
setUnit(next)
|
||||||
|
}
|
||||||
|
|
||||||
|
gitea-actions
commented
🟡 Client validates dimension positivity before unit conversion, missing server's [1cm,100m] bounds after rounding correctness · flagged by 1 model
🪰 Gadfly · advisory 🟡 **Client validates dimension positivity before unit conversion, missing server's [1cm,100m] bounds after rounding**
_correctness · flagged by 1 model_
- `web/src/components/gardens/GardenFormModal.tsx:59-64` vs. `internal/service/gardens.go:16-18,146-148` — Client-side validation only checks `w <= 0` / `h <= 0` in the display unit before conversion, while the server enforces `[minGardenCM=1, maxGardenCM=10_000]` (1 cm–100 m) on the converted value via `validDimensionCM`. A small value (e.g. `0.001` m) passes the client check but rounds to `0` cm through `cmFromMeters`/`cmFromFtIn` (`units.ts:12-19`) and fails server validation; a value like `5…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
async function onSubmit(e: FormEvent) {
|
||||||
|
e.preventDefault()
|
||||||
|
setFormError(null)
|
||||||
|
setConflict(null)
|
||||||
|
|
||||||
|
if (!name.trim()) {
|
||||||
|
setFormError('Enter a name for the garden.')
|
||||||
|
return
|
||||||
|
}
|
||||||
|
// Validate the converted centimeter values against the same bounds the
|
||||||
|
// server enforces, so sub-cm or over-100m sizes fail here with a clear
|
||||||
|
// message instead of a generic server error.
|
||||||
|
const widthCm = cmFromDisplay(parseFloat(width), unit)
|
||||||
|
const heightCm = cmFromDisplay(parseFloat(height), unit)
|
||||||
|
if (!isValidDimensionCm(widthCm) || !isValidDimensionCm(heightCm)) {
|
||||||
|
setFormError('Width and height must be between 1 cm and 100 m.')
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
const input = { name: name.trim(), widthCm, heightCm, unitPref: unit, notes: notes.trim() }
|
||||||
|
|
||||||
|
try {
|
||||||
|
if (isEdit) {
|
||||||
|
await update.mutateAsync({ id: garden.id, ...input, version })
|
||||||
|
} else {
|
||||||
|
await create.mutateAsync(input)
|
||||||
|
}
|
||||||
|
onClose()
|
||||||
|
} catch (err) {
|
||||||
|
const current = conflictGarden(err)
|
||||||
|
gitea-actions
commented
🟡 409 rebase duplicates dimString helper instead of reusing it maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **409 rebase duplicates dimString helper instead of reusing it**
_maintainability · flagged by 1 model_
- **`web/src/components/gardens/GardenFormModal.tsx:89-90`** — The 409 rebase path calls `String(displayFromCm(current.widthCm, current.unitPref))` directly, duplicating what the `dimString` helper (lines 19-21) already does. `dimString(current.widthCm, current.unitPref)` would read better and keep the "cm → input string" rule in one place (`dimString` accepts `number | undefined`, so a `number` arg is fine). Minor.
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
if (current) {
|
||||||
|
// Someone else changed this garden: rebase the form onto the fresh row so
|
||||||
|
// a re-save applies against the current version.
|
||||||
|
setVersion(current.version)
|
||||||
|
setName(current.name)
|
||||||
|
setUnit(current.unitPref)
|
||||||
|
setWidth(dimString(current.widthCm, current.unitPref))
|
||||||
|
setHeight(dimString(current.heightCm, current.unitPref))
|
||||||
|
setNotes(current.notes)
|
||||||
|
setConflict('This garden changed elsewhere. The latest values are shown — review and save again.')
|
||||||
|
return
|
||||||
|
}
|
||||||
|
setFormError(errorMessage(err, isEdit ? 'Could not save changes.' : 'Could not create the garden.'))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const unitLabel = dimensionUnitLabel(unit)
|
||||||
|
|
||||||
|
return (
|
||||||
|
<Modal title={isEdit ? 'Edit garden' : 'New garden'} onClose={onClose} busy={pending}>
|
||||||
|
<form onSubmit={onSubmit} className="flex flex-col gap-3">
|
||||||
|
{conflict && <Alert tone="info">{conflict}</Alert>}
|
||||||
|
|
||||||
|
<TextField label="Name" name="name" required value={name} onChange={(e) => setName(e.target.value)} />
|
||||||
|
|
||||||
|
<Select
|
||||||
|
label="Units"
|
||||||
|
name="unitPref"
|
||||||
|
value={unit}
|
||||||
|
onChange={(e) => changeUnit(e.target.value as UnitPref)}
|
||||||
|
options={unitOptions}
|
||||||
|
/>
|
||||||
|
|
||||||
|
<div className="grid grid-cols-2 gap-3">
|
||||||
|
<TextField
|
||||||
|
label={`Width (${unitLabel})`}
|
||||||
|
name="width"
|
||||||
|
type="number"
|
||||||
|
inputMode="decimal"
|
||||||
|
step="any"
|
||||||
|
min="0"
|
||||||
|
required
|
||||||
|
value={width}
|
||||||
|
onChange={(e) => setWidth(e.target.value)}
|
||||||
|
/>
|
||||||
|
<TextField
|
||||||
|
label={`Height (${unitLabel})`}
|
||||||
|
name="height"
|
||||||
|
type="number"
|
||||||
|
inputMode="decimal"
|
||||||
|
step="any"
|
||||||
|
min="0"
|
||||||
|
required
|
||||||
|
value={height}
|
||||||
|
onChange={(e) => setHeight(e.target.value)}
|
||||||
|
/>
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<TextArea label="Notes" name="notes" rows={2} value={notes} onChange={(e) => setNotes(e.target.value)} />
|
||||||
|
|
||||||
|
{formError && <Alert>{formError}</Alert>}
|
||||||
|
|
||||||
|
<div className="mt-1 flex justify-end gap-2">
|
||||||
|
<Button type="button" variant="ghost" onClick={onClose} disabled={pending}>
|
||||||
|
Cancel
|
||||||
|
</Button>
|
||||||
|
<Button type="submit" disabled={pending}>
|
||||||
|
{pending ? 'Saving…' : isEdit ? 'Save' : 'Create garden'}
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
</form>
|
||||||
|
</Modal>
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -1,7 +1,7 @@
|
|||||||
import type { ButtonHTMLAttributes } from 'react'
|
import type { ButtonHTMLAttributes } from 'react'
|
||||||
import { cn } from '@/lib/cn'
|
import { cn } from '@/lib/cn'
|
||||||
|
|
||||||
export type ButtonVariant = 'primary' | 'ghost'
|
export type ButtonVariant = 'primary' | 'ghost' | 'danger'
|
||||||
|
|
||||||
/** Shared button styling, exported so anchor "buttons" (e.g. the OIDC link) match. */
|
/** Shared button styling, exported so anchor "buttons" (e.g. the OIDC link) match. */
|
||||||
export function buttonClasses(variant: ButtonVariant = 'primary', className?: string) {
|
export function buttonClasses(variant: ButtonVariant = 'primary', className?: string) {
|
||||||
@@ -11,6 +11,7 @@ export function buttonClasses(variant: ButtonVariant = 'primary', className?: st
|
|||||||
'disabled:cursor-not-allowed disabled:opacity-60',
|
'disabled:cursor-not-allowed disabled:opacity-60',
|
||||||
variant === 'primary' && 'bg-accent text-accent-contrast hover:bg-accent-strong',
|
variant === 'primary' && 'bg-accent text-accent-contrast hover:bg-accent-strong',
|
||||||
variant === 'ghost' && 'border border-border text-fg hover:bg-border/50',
|
variant === 'ghost' && 'border border-border text-fg hover:bg-border/50',
|
||||||
|
variant === 'danger' && 'bg-red-600 text-white hover:bg-red-700 focus-visible:ring-red-500/40',
|
||||||
className,
|
className,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,58 @@
|
|||||||
|
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) {
|
||||||
|
gitea-actions
commented
🔴 Modal dismissible during pending mutation causing lost error feedback correctness · flagged by 1 model
🪰 Gadfly · advisory 🔴 **Modal dismissible during pending mutation causing lost error feedback**
_correctness · flagged by 1 model_
- **`web/src/components/ui/Modal.tsx:31` and `web/src/components/gardens/GardenFormModal.tsx:102`** — The modal can be dismissed while a mutation is in flight. Escape and backdrop click unconditionally invoke `onClose`. If the user hits Escape (or clicks the backdrop) after pressing Save but before the network call finishes, the modal unmounts. Should the mutation then fail, the catch block in `GardenFormModal` sets error state on an unmounted component and the user never sees the failure. **Fix…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
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>
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -0,0 +1,29 @@
|
|||||||
|
import { forwardRef, type SelectHTMLAttributes } from 'react'
|
||||||
|
import { cn } from '@/lib/cn'
|
||||||
|
import { fieldControlClass, useFieldId } from './field'
|
||||||
|
|
||||||
|
interface SelectProps extends SelectHTMLAttributes<HTMLSelectElement> {
|
||||||
|
label: string
|
||||||
|
options: { value: string; label: string }[]
|
||||||
|
}
|
||||||
|
|
||||||
|
export const Select = forwardRef<HTMLSelectElement, SelectProps>(function Select(
|
||||||
|
{ label, options, id, name, className, ...props },
|
||||||
|
ref,
|
||||||
|
) {
|
||||||
|
const fieldId = useFieldId(id, name)
|
||||||
|
return (
|
||||||
|
<div className="flex flex-col gap-1.5">
|
||||||
|
<label htmlFor={fieldId} className="text-sm font-medium text-fg">
|
||||||
|
{label}
|
||||||
|
</label>
|
||||||
|
<select ref={ref} id={fieldId} name={name} className={cn(fieldControlClass, className)} {...props}>
|
||||||
|
{options.map((o) => (
|
||||||
|
<option key={o.value} value={o.value}>
|
||||||
|
{o.label}
|
||||||
|
</option>
|
||||||
|
))}
|
||||||
|
gitea-actions
commented
🟡 Field chrome (styling + label/id-fallback pattern) duplicated across Select, TextArea, and TextField — extract a shared FieldShell/fieldClasses maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **Field chrome (styling + label/id-fallback pattern) duplicated across Select, TextArea, and TextField — extract a shared FieldShell/fieldClasses**
_maintainability · flagged by 1 model_
- **`web/src/components/ui/Select.tsx:25`** (with `TextArea.tsx:25` and `TextField.tsx:29`) — The field chrome styling string (`'w-full rounded-md border border-border bg-surface px-3 py-2 text-base text-fg', '...focus:border-accent focus:ring-2 focus:ring-accent/30 ... disabled:opacity-60'`) is copy-pasted across all three labelled-field components. The surrounding label+id-fallback pattern (`generatedId = useId(); fieldId = id ?? name ?? generatedId`, the wrapping `<div className="flex flex-co…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
</select>
|
||||||
|
</div>
|
||||||
|
)
|
||||||
|
})
|
||||||
@@ -0,0 +1,22 @@
|
|||||||
|
import { forwardRef, type TextareaHTMLAttributes } from 'react'
|
||||||
|
import { cn } from '@/lib/cn'
|
||||||
|
import { fieldControlClass, useFieldId } from './field'
|
||||||
|
|
||||||
|
interface TextAreaProps extends TextareaHTMLAttributes<HTMLTextAreaElement> {
|
||||||
|
label: string
|
||||||
|
}
|
||||||
|
|
||||||
|
export const TextArea = forwardRef<HTMLTextAreaElement, TextAreaProps>(function TextArea(
|
||||||
|
{ label, id, name, className, ...props },
|
||||||
|
ref,
|
||||||
|
) {
|
||||||
|
const fieldId = useFieldId(id, name)
|
||||||
|
return (
|
||||||
|
gitea-actions
commented
🟡 Copied useId fallback logic from TextField instead of extracting shared hook maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟡 **Copied useId fallback logic from TextField instead of extracting shared hook**
_maintainability · flagged by 2 models_
- **`web/src/components/ui/TextArea.tsx:14`** — The `useId()` + `id ?? name ?? generatedId` fallback chain is copied verbatim from the pre-existing `TextField` (the `TextArea` comment is even a near-duplicate of `TextField`’s). Adding two more copies of this three-line pattern makes future changes—e.g. adding `aria-describedby` support or changing fallback priority—require touching three components. Fix: extract a tiny `useFieldId(id?: string, name?: string)` hook (or a shared `FormControl` wrap…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
<div className="flex flex-col gap-1.5">
|
||||||
|
<label htmlFor={fieldId} className="text-sm font-medium text-fg">
|
||||||
|
{label}
|
||||||
|
</label>
|
||||||
|
<textarea ref={ref} id={fieldId} name={name} className={cn(fieldControlClass, className)} {...props} />
|
||||||
|
</div>
|
||||||
|
)
|
||||||
|
})
|
||||||
@@ -1,20 +1,17 @@
|
|||||||
import { forwardRef, useId, type InputHTMLAttributes } from 'react'
|
import { forwardRef, type InputHTMLAttributes } from 'react'
|
||||||
import { cn } from '@/lib/cn'
|
import { cn } from '@/lib/cn'
|
||||||
|
import { fieldControlClass, useFieldId } from './field'
|
||||||
|
|
||||||
interface TextFieldProps extends InputHTMLAttributes<HTMLInputElement> {
|
interface TextFieldProps extends InputHTMLAttributes<HTMLInputElement> {
|
||||||
label: string
|
label: string
|
||||||
hint?: string
|
hint?: string
|
||||||
}
|
}
|
||||||
|
|
||||||
// A labelled text input. text-base (16px) is deliberate: smaller fonts make iOS
|
|
||||||
// Safari zoom on focus. The field id falls back to the name, then to a generated
|
|
||||||
// id, so the label/hint associations always hold.
|
|
||||||
export const TextField = forwardRef<HTMLInputElement, TextFieldProps>(function TextField(
|
export const TextField = forwardRef<HTMLInputElement, TextFieldProps>(function TextField(
|
||||||
{ label, hint, id, name, className, ...props },
|
{ label, hint, id, name, className, ...props },
|
||||||
ref,
|
ref,
|
||||||
) {
|
) {
|
||||||
const generatedId = useId()
|
const inputId = useFieldId(id, name)
|
||||||
const inputId = id ?? name ?? generatedId
|
|
||||||
const hintId = hint ? `${inputId}-hint` : undefined
|
const hintId = hint ? `${inputId}-hint` : undefined
|
||||||
return (
|
return (
|
||||||
<div className="flex flex-col gap-1.5">
|
<div className="flex flex-col gap-1.5">
|
||||||
@@ -26,13 +23,7 @@ export const TextField = forwardRef<HTMLInputElement, TextFieldProps>(function T
|
|||||||
id={inputId}
|
id={inputId}
|
||||||
name={name}
|
name={name}
|
||||||
aria-describedby={hintId}
|
aria-describedby={hintId}
|
||||||
className={cn(
|
className={cn(fieldControlClass, className)}
|
||||||
'w-full rounded-md border border-border bg-surface px-3 py-2 text-base text-fg',
|
|
||||||
'placeholder:text-muted/70 outline-none transition-colors',
|
|
||||||
'focus:border-accent focus:ring-2 focus:ring-accent/30',
|
|
||||||
'disabled:opacity-60',
|
|
||||||
className,
|
|
||||||
)}
|
|
||||||
{...props}
|
{...props}
|
||||||
/>
|
/>
|
||||||
{hint && (
|
{hint && (
|
||||||
|
|||||||
@@ -0,0 +1,18 @@
|
|||||||
|
import { useId } from 'react'
|
||||||
|
|
||||||
|
// Shared field plumbing for the labelled form controls (TextField, TextArea,
|
||||||
|
// Select) so their id-fallback and base styling stay in one place.
|
||||||
|
|
||||||
|
/** The field id: an explicit id, else the name, else a generated stable id, so
|
||||||
|
* the label's htmlFor always binds to something. */
|
||||||
|
export function useFieldId(id?: string, name?: string): string {
|
||||||
|
const generated = useId()
|
||||||
|
return id ?? name ?? generated
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Base classes shared by the text/select/textarea controls. text-base (16px)
|
||||||
|
* keeps iOS Safari from zooming on focus. */
|
||||||
|
export const fieldControlClass =
|
||||||
|
'w-full rounded-md border border-border bg-surface px-3 py-2 text-base text-fg ' +
|
||||||
|
'outline-none transition-colors placeholder:text-muted/70 ' +
|
||||||
|
'focus:border-accent focus:ring-2 focus:ring-accent/30 disabled:opacity-60'
|
||||||
@@ -0,0 +1,88 @@
|
|||||||
|
// Gardens data layer: zod-validated shapes for /api/v1/gardens plus the
|
||||||
|
// react-query hooks the /gardens page uses.
|
||||||
|
|
||||||
|
import { queryOptions, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
|
||||||
|
import { z } from 'zod'
|
||||||
|
import { ApiError, api } from './api'
|
||||||
|
import type { UnitPref } from './units'
|
||||||
|
|
||||||
|
const unitPrefSchema = z.enum(['metric', 'imperial'])
|
||||||
|
|
||||||
|
export const gardenSchema = z.object({
|
||||||
|
id: z.number(),
|
||||||
|
ownerId: z.number(),
|
||||||
|
name: z.string(),
|
||||||
|
widthCm: z.number(),
|
||||||
|
gitea-actions
commented
🟠 Garden schema does not enforce positive dimensions, allowing zero/negative widthCm and heightCm from API error-handling · flagged by 1 model
🪰 Gadfly · advisory 🟠 **Garden schema does not enforce positive dimensions, allowing zero/negative widthCm and heightCm from API**
_error-handling · flagged by 1 model_
* **web/src/lib/gardens.ts:15-16** — `widthCm: z.number()` and `heightCm: z.number()` accept zero and negative values. The form guards against this on write, but the schema allows invalid server data to propagate to the UI and into `formatDimensions`. Defensive fix: use `z.number().positive()` for both.
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
heightCm: z.number(),
|
||||||
|
unitPref: unitPrefSchema,
|
||||||
|
notes: z.string(),
|
||||||
|
version: z.number(),
|
||||||
|
createdAt: z.string(),
|
||||||
|
updatedAt: z.string(),
|
||||||
|
})
|
||||||
|
export type Garden = z.infer<typeof gardenSchema>
|
||||||
|
|
||||||
|
const gardensKey = ['gardens'] as const
|
||||||
|
|
||||||
|
export const gardensQueryOptions = queryOptions({
|
||||||
|
queryKey: gardensKey,
|
||||||
|
queryFn: async (): Promise<Garden[]> => z.array(gardenSchema).parse(await api.get('/gardens')),
|
||||||
|
})
|
||||||
|
|
||||||
|
export function useGardens() {
|
||||||
|
return useQuery(gardensQueryOptions)
|
||||||
|
}
|
||||||
|
|
||||||
|
export interface GardenInput {
|
||||||
|
name: string
|
||||||
|
widthCm: number
|
||||||
|
heightCm: number
|
||||||
|
unitPref: UnitPref
|
||||||
|
notes: string
|
||||||
|
}
|
||||||
|
|
||||||
|
export function useCreateGarden() {
|
||||||
|
const qc = useQueryClient()
|
||||||
|
return useMutation({
|
||||||
|
gitea-actions
commented
🟠 Redundant full-list refetch after create mutation performance · flagged by 2 models
🪰 Gadfly · advisory 🟠 **Redundant full-list refetch after create mutation**
_performance · flagged by 2 models_
- `web/src/lib/gardens.ts:49` / `web/src/lib/gardens.ts:62` — **Redundant full-list refetch after every create/update.** `useCreateGarden` and `useUpdateGarden` both receive the fresh `Garden` row from the server (`api.post` / `api.patch` return the parsed object), but their `onSuccess` handlers ignore it and call `qc.invalidateQueries({ queryKey: gardensKey })`. That forces an extra `GET /gardens` round-trip even though the response already contains everything needed to update the cache. On slo…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
mutationFn: async (input: GardenInput): Promise<Garden> =>
|
||||||
|
gardenSchema.parse(await api.post('/gardens', input)),
|
||||||
|
onSuccess: () => qc.invalidateQueries({ queryKey: gardensKey }),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
export interface GardenUpdate extends GardenInput {
|
||||||
|
id: number
|
||||||
|
version: number
|
||||||
|
}
|
||||||
|
|
||||||
|
export function useUpdateGarden() {
|
||||||
|
const qc = useQueryClient()
|
||||||
|
return useMutation({
|
||||||
|
// id travels with the mutation variables (not the hook) so callers don't need
|
||||||
|
// a placeholder id before a garden is chosen.
|
||||||
|
mutationFn: async ({ id, ...input }: GardenUpdate): Promise<Garden> =>
|
||||||
|
gardenSchema.parse(await api.patch(`/gardens/${id}`, input)),
|
||||||
|
onSuccess: () => qc.invalidateQueries({ queryKey: gardensKey }),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
export function useDeleteGarden() {
|
||||||
|
const qc = useQueryClient()
|
||||||
|
return useMutation({
|
||||||
|
mutationFn: (id: number) => api.delete(`/gardens/${id}`),
|
||||||
|
onSuccess: () => qc.invalidateQueries({ queryKey: gardensKey }),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* If err is a 409 version conflict, return the fresh server row it carries
|
||||||
|
* (under `current`), so a form can rebase onto it; otherwise null.
|
||||||
|
*/
|
||||||
|
export function conflictGarden(err: unknown): Garden | null {
|
||||||
|
if (err instanceof ApiError && err.isConflict && err.body && typeof err.body === 'object') {
|
||||||
|
const current = (err.body as { current?: unknown }).current
|
||||||
|
const parsed = gardenSchema.safeParse(current)
|
||||||
|
if (parsed.success) return parsed.data
|
||||||
|
}
|
||||||
|
return null
|
||||||
|
}
|
||||||
@@ -0,0 +1,70 @@
|
|||||||
|
// Unit conversion + display for garden dimensions. The API and DB are metric
|
||||||
|
// only (centimeters); imperial is purely a display/entry concern here. A minimal
|
||||||
|
// helper set for #8 — #9's geometry lib consolidates/extends it.
|
||||||
|
|
||||||
|
export type UnitPref = 'metric' | 'imperial'
|
||||||
|
|
||||||
|
const CM_PER_INCH = 2.54
|
||||||
|
const CM_PER_METER = 100
|
||||||
|
const INCHES_PER_FOOT = 12
|
||||||
|
|
||||||
|
// Mirrors the server's garden dimension bounds (service/gardens.go): [1cm, 100m].
|
||||||
|
export const MIN_DIMENSION_CM = 1
|
||||||
|
export const MAX_DIMENSION_CM = 10_000
|
||||||
|
|
||||||
|
/** Whether a centimeter dimension is within the server's accepted range. */
|
||||||
|
export function isValidDimensionCm(cm: number): boolean {
|
||||||
|
return Number.isFinite(cm) && cm >= MIN_DIMENSION_CM && cm <= MAX_DIMENSION_CM
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Feet (+ optional inches) → whole centimeters. */
|
||||||
|
export function cmFromFtIn(feet: number, inches = 0): number {
|
||||||
|
return Math.round((feet * INCHES_PER_FOOT + inches) * CM_PER_INCH)
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Meters → whole centimeters. */
|
||||||
|
export function cmFromMeters(meters: number): number {
|
||||||
|
return Math.round(meters * CM_PER_METER)
|
||||||
|
gitea-actions
commented
🔴 displayFromCm 2-decimal rounding causes systematic imperial round-trip errors (8 ft becomes 8.01 ft) correctness · flagged by 1 model
🪰 Gadfly · advisory 🔴 **displayFromCm 2-decimal rounding causes systematic imperial round-trip errors (8 ft becomes 8.01 ft)**
_correctness · flagged by 1 model_
- **`web/src/lib/units.ts:27-31`** / **`web/src/components/gardens/GardenFormModal.tsx:71-72`** — `displayFromCm` rounds to 2 decimal places, causing systematic round-trip errors for common imperial dimensions. For example, 8 ft → `cmFromFtIn(8)` = 244 cm → `displayFromCm(244, 'imperial')` = 8.01 ft. A user who creates a garden as exactly 8 ft will later see 8.01 ft in the edit form. The same happens for 10 ft → 10.01 ft, 12 ft → 12.01 ft, etc. The PR notes that 4 ft happens to round-trip cleanl…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
}
|
||||||
|
|
||||||
|
/** A value typed in the given unit (meters, or feet) → centimeters. */
|
||||||
|
export function cmFromDisplay(value: number, unit: UnitPref): number {
|
||||||
|
return unit === 'imperial' ? cmFromFtIn(value) : cmFromMeters(value)
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Centimeters → a number in the given unit, for prefilling an input field.
|
||||||
|
* Rounded so cm-quantization doesn't show through (e.g. 244cm shows as 8 ft,
|
||||||
|
* not 8.01): meters to the cm (2 dp), feet to 0.1 ft. */
|
||||||
|
export function displayFromCm(cm: number, unit: UnitPref): number {
|
||||||
|
if (unit === 'imperial') {
|
||||||
|
const feet = cm / CM_PER_INCH / INCHES_PER_FOOT
|
||||||
|
return Math.round(feet * 10) / 10
|
||||||
|
}
|
||||||
|
return Math.round((cm / CM_PER_METER) * 100) / 100
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Centimeters → a human string in the given unit (e.g. "1.22 m" or "4′ 0″"). */
|
||||||
|
export function formatCm(cm: number, unit: UnitPref): string {
|
||||||
|
if (unit === 'imperial') {
|
||||||
|
const totalInches = cm / CM_PER_INCH
|
||||||
|
let feet = Math.floor(totalInches / INCHES_PER_FOOT)
|
||||||
|
let inches = Math.round(totalInches - feet * INCHES_PER_FOOT)
|
||||||
|
if (inches === INCHES_PER_FOOT) {
|
||||||
|
feet += 1
|
||||||
|
inches = 0
|
||||||
|
}
|
||||||
|
return `${feet}′ ${inches}″`
|
||||||
|
}
|
||||||
|
const meters = Math.round((cm / CM_PER_METER) * 100) / 100
|
||||||
|
return `${meters} m`
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Centimeters width × height → a human string in the given unit. */
|
||||||
|
export function formatDimensions(widthCm: number, heightCm: number, unit: UnitPref): string {
|
||||||
|
return `${formatCm(widthCm, unit)} × ${formatCm(heightCm, unit)}`
|
||||||
|
}
|
||||||
|
|
||||||
|
/** The input-field label for a single dimension in the given unit. */
|
||||||
|
export function dimensionUnitLabel(unit: UnitPref): string {
|
||||||
|
return unit === 'imperial' ? 'ft' : 'm'
|
||||||
|
}
|
||||||
@@ -1,5 +1,58 @@
|
|||||||
import { PageStub } from '@/components/PageStub'
|
import { useState } from 'react'
|
||||||
|
import { Alert } from '@/components/ui/Alert'
|
||||||
|
import { Button } from '@/components/ui/Button'
|
||||||
|
import { DeleteGardenModal } from '@/components/gardens/DeleteGardenModal'
|
||||||
|
import { GardenCard } from '@/components/gardens/GardenCard'
|
||||||
|
import { GardenFormModal } from '@/components/gardens/GardenFormModal'
|
||||||
|
import { useGardens, type Garden } from '@/lib/gardens'
|
||||||
|
|
||||||
|
// Which modal is open, if any. `edit`/`delete` carry the target garden.
|
||||||
|
type Dialog = { kind: 'create' } | { kind: 'edit'; garden: Garden } | { kind: 'delete'; garden: Garden } | null
|
||||||
|
|
||||||
export function GardensPage() {
|
export function GardensPage() {
|
||||||
return <PageStub title="Gardens">The garden list and create flow arrive in issue #8.</PageStub>
|
const gardens = useGardens()
|
||||||
|
const [dialog, setDialog] = useState<Dialog>(null)
|
||||||
|
const close = () => setDialog(null)
|
||||||
|
const hasGardens = !!gardens.data && gardens.data.length > 0
|
||||||
|
|
||||||
|
return (
|
||||||
|
<section>
|
||||||
|
<div className="flex items-center justify-between gap-4">
|
||||||
|
gitea-actions
commented
⚪ 'New garden' visibility check duplicates grid condition with inconsistent isSuccess/data usage maintainability · flagged by 1 model
🪰 Gadfly · advisory ⚪ **'New garden' visibility check duplicates grid condition with inconsistent isSuccess/data usage**
_maintainability · flagged by 1 model_
- **`web/src/pages/GardensPage.tsx:20` vs `:39`** — The "New garden" button visibility (`gardens.data && gardens.data.length > 0`) and the grid visibility (`gardens.isSuccess && gardens.data.length > 0`) check the same condition two different ways. Using `gardens.isSuccess &&` in both keeps them aligned and avoids relying on `data` truthiness while pending. Trivial.
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
<h1 className="text-2xl font-semibold tracking-tight">Gardens</h1>
|
||||||
|
{hasGardens && <Button onClick={() => setDialog({ kind: 'create' })}>New garden</Button>}
|
||||||
|
</div>
|
||||||
|
|
||||||
|
<div className="mt-6">
|
||||||
|
{gardens.isPending && <p className="text-sm text-muted">Loading your gardens…</p>}
|
||||||
|
{gardens.isError && <Alert>Could not load your gardens. Please refresh.</Alert>}
|
||||||
|
|
||||||
|
{gardens.isSuccess && gardens.data.length === 0 && (
|
||||||
|
<div className="rounded-xl border border-dashed border-border p-10 text-center">
|
||||||
|
<p className="text-fg">No gardens yet.</p>
|
||||||
|
<p className="mt-1 text-sm text-muted">Create your first garden to start planning.</p>
|
||||||
|
<Button className="mt-4" onClick={() => setDialog({ kind: 'create' })}>
|
||||||
|
Create your first garden
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
|
||||||
|
{gardens.isSuccess && gardens.data.length > 0 && (
|
||||||
|
<div className="grid grid-cols-1 gap-4 sm:grid-cols-2 lg:grid-cols-3">
|
||||||
|
{gardens.data.map((g) => (
|
||||||
|
<GardenCard
|
||||||
|
key={g.id}
|
||||||
|
garden={g}
|
||||||
|
onEdit={() => setDialog({ kind: 'edit', garden: g })}
|
||||||
|
onDelete={() => setDialog({ kind: 'delete', garden: g })}
|
||||||
|
/>
|
||||||
|
))}
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
</div>
|
||||||
|
|
||||||
|
{dialog?.kind === 'create' && <GardenFormModal onClose={close} />}
|
||||||
|
{dialog?.kind === 'edit' && <GardenFormModal garden={dialog.garden} onClose={close} />}
|
||||||
|
{dialog?.kind === 'delete' && <DeleteGardenModal garden={dialog.garden} onClose={close} />}
|
||||||
|
</section>
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
🟡 Destructive-action button hardcodes red classes instead of adding a danger variant to buttonClasses
maintainability · flagged by 1 model
web/src/components/gardens/DeleteGardenModal.tsx:33-42— the destructive Delete button uses<Button>(confirmed) but overrides its background via a hardcodedclassName="bg-red-600 text-white hover:bg-red-700"rather than the component exposing adangervariant. SinceButtonVariantinButton.tsx:4is only'primary' | 'ghost', this ad hoc override is the only way to get a red button today, and it's the kind of styling that's likely to get copy-pasted for the next destructive action…🪰 Gadfly · advisory