Plant catalog UI: /plants page + PlantPicker (#13) #32

Merged
steve merged 2 commits from phase-4-plants-ui into main 2026-07-19 02:18:12 +00:00
10 changed files with 848 additions and 3 deletions
@@ -0,0 +1,37 @@
import { cn } from '@/lib/cn'
import { CATEGORY_LABELS, PLANT_CATEGORIES, type CategoryFilter } from '@/lib/plants'
/**
* Horizontal, scrollable "All + each category" chip row. Shared by the /plants
* page and the PlantPicker so both filter the catalog identically.
*/
export function CategoryChips({
value,
onChange,
size = 'md',
}: {
value: CategoryFilter
onChange: (c: CategoryFilter) => void
size?: 'sm' | 'md'
}) {
const chip = (v: CategoryFilter, label: string) => (
<button
key={v}
type="button"
onClick={() => onChange(v)}
className={cn(
'shrink-0 rounded-full px-3 font-medium transition-colors',
size === 'sm' ? 'py-1 text-xs' : 'py-1 text-sm',
value === v ? 'bg-accent text-accent-contrast' : 'bg-border/50 text-muted hover:text-fg',
)}
>
{label}
</button>
)
return (
<div className="flex gap-1.5 overflow-x-auto">
{chip('all', 'All')}
{PLANT_CATEGORIES.map((c) => chip(c, CATEGORY_LABELS[c]))}
</div>
)
}
@@ -0,0 +1,46 @@
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 { useDeletePlant, type Plant } from '@/lib/plants'
/**
* Confirmation dialog for deleting a custom plant. A plant still used by
* plantings is refused by the server (409 PLANT_IN_USE); we surface that
* message inline rather than pretending it worked.
*/
export function DeletePlantModal({ plant, onClose }: { plant: Plant; onClose: () => void }) {
const deletion = useDeletePlant()
const [error, setError] = useState<string | null>(null)
async function onConfirm() {
setError(null)
try {
await deletion.mutateAsync(plant.id)
onClose()
} catch (err) {
setError(errorMessage(err, 'Could not delete the plant.'))
}
}
return (
<Modal title="Delete plant" 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">{plant.name}</span> from your catalog? 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>
)
}
+72
View File
@@ -0,0 +1,72 @@
import { PlantIcon } from './PlantIcon'
import { CATEGORY_LABELS, isBuiltin, type Plant } from '@/lib/plants'
import { formatSpacing, type UnitPref } from '@/lib/units'
Review

🟡 PlantCard's actionClass/dangerClass duplicate GardenCard's inline button classes verbatim

maintainability · flagged by 1 model

  • PlantCard's actionClass/dangerClass duplicate GardenCard's inline button classes verbatim (web/src/components/plants/PlantCard.tsx:4-9 vs web/src/components/gardens/GardenCard.tsx:35,42). The two class strings are character-identical to what GardenCard inlines per-button. Now that the pattern appears in three+ cards, hoist these to a shared ui/buttonStyles.ts (or a cardActionClass/cardDangerClass export) — otherwise the next card copy will diverge again, exactly as this…

🪰 Gadfly · advisory

🟡 **`PlantCard`'s `actionClass`/`dangerClass` duplicate `GardenCard`'s inline button classes verbatim** _maintainability · flagged by 1 model_ - **`PlantCard`'s `actionClass`/`dangerClass` duplicate `GardenCard`'s inline button classes verbatim** (`web/src/components/plants/PlantCard.tsx:4-9` vs `web/src/components/gardens/GardenCard.tsx:35,42`). The two class strings are character-identical to what `GardenCard` inlines per-button. Now that the pattern appears in three+ cards, hoist these to a shared `ui/buttonStyles.ts` (or a `cardActionClass`/`cardDangerClass` export) — otherwise the next card copy will diverge again, exactly as this… <sub>🪰 Gadfly · advisory</sub>
const actionClass =
'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'
const dangerClass =
'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'
/**
* One catalog plant as a card: icon tile tinted with the plant's color, name,
* category + mature spacing (unit-aware), and actions. Built-ins are badged and
* offer only "Duplicate" (they're read-only); own plants add Edit/Delete.
*/
export function PlantCard({
plant,
unit,
onEdit,
onDelete,
onDuplicate,
}: {
plant: Plant
unit: UnitPref
onEdit: () => void
onDelete: () => void
onDuplicate: () => void
}) {
const builtin = isBuiltin(plant)
return (
<div className="flex flex-col rounded-xl border border-border bg-surface">
Review

🟡 Icon-tile + color-mix styling duplicated

maintainability · flagged by 2 models

  • Icon-tile + color-mix styling duplicated between PlantCard (web/src/components/plants/PlantCard.tsx:32-39) and PlantPicker (web/src/editor/PlantPicker.tsx:92-99). Same grid h-N w-N place-items-center rounded-md text-xl/2xl tile with backgroundColor: color-mix(in srgb, ${color} 18%, transparent) and the emoji inside. Extract a <PlantIconTile plant size> component (or a tileStyle(plant) helper) so the tint formula lives in one place — important because the magic 18% const…

🪰 Gadfly · advisory

🟡 **Icon-tile + `color-mix` styling duplicated** _maintainability · flagged by 2 models_ - **Icon-tile + `color-mix` styling duplicated** between `PlantCard` (`web/src/components/plants/PlantCard.tsx:32-39`) and `PlantPicker` (`web/src/editor/PlantPicker.tsx:92-99`). Same `grid h-N w-N place-items-center rounded-md text-xl/2xl` tile with `backgroundColor: color-mix(in srgb, ${color} 18%, transparent)` and the emoji inside. Extract a `<PlantIconTile plant size>` component (or a `tileStyle(plant)` helper) so the tint formula lives in one place — important because the magic `18%` const… <sub>🪰 Gadfly · advisory</sub>
<div className="flex items-start gap-3 p-4">
<PlantIcon color={plant.color} icon={plant.icon} className="h-11 w-11 rounded-lg text-2xl" />
<div className="min-w-0 flex-1">
<div className="flex items-center gap-2">
<h3 className="truncate font-semibold text-fg">{plant.name}</h3>
{builtin && (
<span className="shrink-0 rounded bg-border/60 px-1.5 py-0.5 text-[10px] font-medium uppercase tracking-wide text-muted">
Built-in
</span>
)}
</div>
<p className="mt-0.5 text-sm text-muted">
{CATEGORY_LABELS[plant.category]} · {formatSpacing(plant.spacingCm, unit)} spacing
</p>
{plant.notes && <p className="mt-1 line-clamp-2 text-xs text-muted">{plant.notes}</p>}
</div>
<span
className="mt-1 h-4 w-4 shrink-0 rounded-full border border-black/10 dark:border-white/10"
style={{ backgroundColor: plant.color }}
title={plant.color}
/>
</div>
<div className="flex justify-end gap-1 border-t border-border px-2 py-1.5">
<button type="button" onClick={onDuplicate} className={actionClass}>
Duplicate
</button>
{!builtin && (
<button type="button" onClick={onEdit} className={actionClass}>
Edit
</button>
)}
{!builtin && (
<button type="button" onClick={onDelete} className={dangerClass}>
Delete
</button>
)}
</div>
</div>
)
}
@@ -0,0 +1,212 @@
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 {
CATEGORY_LABELS,
PLANT_CATEGORIES,
conflictPlant,
useCreatePlant,
useUpdatePlant,
type Plant,
type PlantCategory,
type PlantInput,
} from '@/lib/plants'
import { cmFromSpacing, spacingFromCm, spacingUnitLabel, type UnitPref } from '@/lib/units'
const DEFAULT_COLOR = '#4a7c3f'
const DEFAULT_ICON = '🌱'
const categoryOptions = PLANT_CATEGORIES.map((c) => ({ value: c, label: CATEGORY_LABELS[c] }))
/** Expand a #rgb shorthand to #rrggbb so the native color input renders it. */
function expandHex(color: string): string {
if (/^#[0-9a-fA-F]{3}$/.test(color)) {
const [, r, g, b] = color
return `#${r}${r}${g}${g}${b}${b}`
}
return /^#[0-9a-fA-F]{6}$/.test(color) ? color : DEFAULT_COLOR
}
/**
* Create or edit a custom plant. `plant` puts it in edit mode; `template` (a
* built-in or another plant, used by "Duplicate") pre-fills a fresh create. A
* 409 rebases the form onto the server's current row. Spacing is entered in the
* page's unit and converted to centimeters for the API.
*/
export function PlantFormModal({
plant,
template,
unit,
onClose,
}: {
plant?: Plant
template?: Plant
unit: UnitPref
onClose: () => void
}) {
const isEdit = !!plant
const source = plant ?? template
const create = useCreatePlant()
const update = useUpdatePlant()
const pending = create.isPending || update.isPending
const [name, setName] = useState(source ? (isEdit ? source.name : `${source.name} (copy)`) : '')
const [category, setCategory] = useState<PlantCategory>(source?.category ?? 'vegetable')
const [spacing, setSpacing] = useState(String(spacingFromCm(source?.spacingCm ?? 30, unit)))
const [color, setColor] = useState(expandHex(source?.color ?? DEFAULT_COLOR))
const [icon, setIcon] = useState(source?.icon ?? DEFAULT_ICON)
const [days, setDays] = useState(source?.daysToMaturity != null ? String(source.daysToMaturity) : '')
const [notes, setNotes] = useState(source?.notes ?? '')
Review

🟡 Client spacing threshold (< 1 cm) is stricter than server (>= 0.1 cm); misleading 'positive number' error

correctness · flagged by 1 model

  • web/src/components/plants/PlantFormModal.tsx:64 — Spacing validation rejects spacingCm < 1, but the server accepts >= 0.1 cm (minPlantSpacingCM = 0.1, internal/service/plants.go:18). In metric a user cannot enter a sub-centimeter spacing the backend would happily store, and the message "Spacing must be a positive number" is misleading since e.g. 0.5 is positive. The HTML min="0" on the field reinforces the inconsistency. Suggest aligning the client threshold to the server's `0.…

🪰 Gadfly · advisory

🟡 **Client spacing threshold (< 1 cm) is stricter than server (>= 0.1 cm); misleading 'positive number' error** _correctness · flagged by 1 model_ - `web/src/components/plants/PlantFormModal.tsx:64` — Spacing validation rejects `spacingCm < 1`, but the server accepts `>= 0.1` cm (`minPlantSpacingCM = 0.1`, `internal/service/plants.go:18`). In metric a user cannot enter a sub-centimeter spacing the backend would happily store, and the message "Spacing must be a positive number" is misleading since e.g. `0.5` *is* positive. The HTML `min="0"` on the field reinforces the inconsistency. Suggest aligning the client threshold to the server's `0.… <sub>🪰 Gadfly · advisory</sub>
const [version, setVersion] = useState(plant?.version ?? 0)
const [conflict, setConflict] = useState<string | null>(null)
const [formError, setFormError] = useState<string | null>(null)
const unitLabel = spacingUnitLabel(unit)
async function onSubmit(e: FormEvent) {
e.preventDefault()
setFormError(null)
setConflict(null)
Review

🟡 parseInt silently truncates non-integer days-to-maturity (e.g. '1.5' → 1) instead of rejecting it

correctness · flagged by 1 model

  • web/src/components/plants/PlantFormModal.tsx:73 — Days-to-maturity is parsed with parseInt(days, 10). Since the input is type="number" step="1", a pasted/entered value like "1.5" is silently truncated to 1, which then passes Number.isInteger(d) and is sent to the server as 1. Use Number(days) (or parseFloat + an explicit integer check) so a non-integer entry is rejected with the "whole number of days" message instead of being quietly accepted as a different value. Minor data-…

🪰 Gadfly · advisory

🟡 **parseInt silently truncates non-integer days-to-maturity (e.g. '1.5' → 1) instead of rejecting it** _correctness · flagged by 1 model_ - `web/src/components/plants/PlantFormModal.tsx:73` — Days-to-maturity is parsed with `parseInt(days, 10)`. Since the input is `type="number" step="1"`, a pasted/entered value like `"1.5"` is silently truncated to `1`, which then passes `Number.isInteger(d)` and is sent to the server as `1`. Use `Number(days)` (or `parseFloat` + an explicit integer check) so a non-integer entry is rejected with the "whole number of days" message instead of being quietly accepted as a different value. Minor data-… <sub>🪰 Gadfly · advisory</sub>
if (!name.trim()) {
setFormError('Enter a name for the plant.')
return
}
if (!icon.trim()) {
setFormError('Pick an emoji icon.')
return
}
const spacingCm = cmFromSpacing(parseFloat(spacing), unit)
if (!Number.isFinite(spacingCm) || spacingCm < 1) {
setFormError(`Spacing must be at least 1 ${unitLabel}.`)
return
}
let daysToMaturity: number | null = null
if (days.trim()) {
Review

🟠 parseInt truncates decimals in days-to-maturity validation

error-handling · flagged by 2 models

  • web/src/components/plants/PlantFormModal.tsx:89parseInt(days, 10) silently truncates fractional input instead of rejecting it. Impact: A user who types 1.5 into “Days to maturity” sees no validation error; the value is quietly rounded down to 1 and submitted, contradicting the error message “must be a whole number of days”. Fix: Use parseFloat(days) before the Number.isInteger check so decimals are caught and rejected: ```tsx const d = parseFloat(days) if (!Number.is…

🪰 Gadfly · advisory

🟠 **parseInt truncates decimals in days-to-maturity validation** _error-handling · flagged by 2 models_ - **`web/src/components/plants/PlantFormModal.tsx:89`** — `parseInt(days, 10)` silently truncates fractional input instead of rejecting it. **Impact:** A user who types `1.5` into “Days to maturity” sees no validation error; the value is quietly rounded down to `1` and submitted, contradicting the error message “must be a whole number of days”. **Fix:** Use `parseFloat(days)` before the `Number.isInteger` check so decimals are caught and rejected: ```tsx const d = parseFloat(days) if (!Number.is… <sub>🪰 Gadfly · advisory</sub>
// Number() (not parseInt) so "1.5" is rejected, not silently truncated to 1.
const d = Number(days)
if (!Number.isInteger(d) || d < 1) {
setFormError('Days to maturity must be a whole number of days, or left blank.')
return
}
daysToMaturity = d
}
const input: PlantInput = {
name: name.trim(),
category,
spacingCm,
color,
icon: icon.trim(),
daysToMaturity,
notes: notes.trim(),
}
try {
if (isEdit) {
await update.mutateAsync({ id: plant.id, ...input, version })
} else {
await create.mutateAsync(input)
}
onClose()
} catch (err) {
const current = conflictPlant(err)
if (current) {
// Someone else changed this plant: rebase onto the fresh row so a re-save
// applies against the current version.
setVersion(current.version)
setName(current.name)
setCategory(current.category)
setSpacing(String(spacingFromCm(current.spacingCm, unit)))
setColor(expandHex(current.color))
setIcon(current.icon)
setDays(current.daysToMaturity != null ? String(current.daysToMaturity) : '')
setNotes(current.notes)
setConflict('This plant changed elsewhere. The latest values are shown — review and save again.')
return
}
setFormError(errorMessage(err, isEdit ? 'Could not save changes.' : 'Could not create the plant.'))
}
}
return (
<Modal title={isEdit ? 'Edit plant' : 'New plant'} 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)} />
<div className="grid grid-cols-2 gap-3">
<Select
label="Category"
name="category"
value={category}
onChange={(e) => setCategory(e.target.value as PlantCategory)}
options={categoryOptions}
/>
<TextField
label={`Spacing (${unitLabel})`}
name="spacing"
type="number"
inputMode="decimal"
step="any"
min="1"
Review

Spacing input's min="0" doesn't match the actual minimum-1 validation, allowing a round-trip rejection instead of blocking at the field

error-handling · flagged by 1 model

  • web/src/components/plants/PlantFormModal.tsx:157 vs :83 — the spacing <input type="number" min="0"> allows the native stepper/typed input to land on 0, but the submit-time check requires spacingCm < 1 to fail with "Spacing must be a positive number." The min attribute doesn't reflect the real floor, so users hit an avoidable round-trip rejection instead of being blocked at the field. TextField (web/src/components/ui/TextField.tsx:21-28) forwards min directly to the native inp…

🪰 Gadfly · advisory

⚪ **Spacing input's min="0" doesn't match the actual minimum-1 validation, allowing a round-trip rejection instead of blocking at the field** _error-handling · flagged by 1 model_ - `web/src/components/plants/PlantFormModal.tsx:157` vs `:83` — the spacing `<input type="number" min="0">` allows the native stepper/typed input to land on `0`, but the submit-time check requires `spacingCm < 1` to fail with "Spacing must be a positive number." The `min` attribute doesn't reflect the real floor, so users hit an avoidable round-trip rejection instead of being blocked at the field. `TextField` (`web/src/components/ui/TextField.tsx:21-28`) forwards `min` directly to the native inp… <sub>🪰 Gadfly · advisory</sub>
required
value={spacing}
onChange={(e) => setSpacing(e.target.value)}
/>
</div>
<div className="grid grid-cols-2 gap-3">
<div className="flex flex-col gap-1.5">
<label htmlFor="plant-color" className="text-sm font-medium text-fg">
Color
</label>
<input
Review

🟡 Icon maxLength=8 truncates by UTF-16 code units, can split multi-codepoint emoji sequences

correctness · flagged by 1 model

  • web/src/components/plants/PlantFormModal.tsx:169 — the icon TextField renders a native <input maxLength={8} ...> (confirmed via TextField.tsx, which spreads ...props, including maxLength, straight onto a plain <input> with no app-level truncation logic). HTML's native maxlength is enforced in UTF-16 code units, not codepoints/graphemes. Hand-verified: 👨‍👩‍👧‍👦 (person+person+person+person joined by ZWJ) = 4×2 (surrogate pairs) + 3×1 (ZWJ) = 11 UTF-16 units, exceeding the 8-uni…

🪰 Gadfly · advisory

🟡 **Icon maxLength=8 truncates by UTF-16 code units, can split multi-codepoint emoji sequences** _correctness · flagged by 1 model_ - `web/src/components/plants/PlantFormModal.tsx:169` — the icon `TextField` renders a native `<input maxLength={8} ...>` (confirmed via `TextField.tsx`, which spreads `...props`, including `maxLength`, straight onto a plain `<input>` with no app-level truncation logic). HTML's native `maxlength` is enforced in UTF-16 code units, not codepoints/graphemes. Hand-verified: `👨‍👩‍👧‍👦` (person+person+person+person joined by ZWJ) = 4×2 (surrogate pairs) + 3×1 (ZWJ) = 11 UTF-16 units, exceeding the 8-uni… <sub>🪰 Gadfly · advisory</sub>
id="plant-color"
type="color"
value={color}
onChange={(e) => setColor(e.target.value)}
className="h-10 w-full cursor-pointer rounded-md border border-border bg-surface"
/>
</div>
<TextField
label="Icon (emoji)"
name="icon"
value={icon}
onChange={(e) => setIcon(e.target.value)}
hint="A single emoji, e.g. 🍅"
/>
</div>
<TextField
label="Days to maturity (optional)"
name="days"
type="number"
inputMode="numeric"
step="1"
min="1"
value={days}
onChange={(e) => setDays(e.target.value)}
/>
<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 plant'}
</Button>
</div>
</form>
</Modal>
)
}
+18
View File
@@ -0,0 +1,18 @@
import { cn } from '@/lib/cn'
/**
* A plant's emoji on a tile tinted with its own color (via color-mix, so any
* valid CSS color works). Shared by PlantCard and the PlantPicker rows. Size and
* shape come from `className`.
*/
export function PlantIcon({ color, icon, className }: { color: string; icon: string; className?: string }) {
return (
<span
className={cn('grid shrink-0 place-items-center', className)}
style={{ backgroundColor: `color-mix(in srgb, ${color} 18%, transparent)` }}
aria-hidden
>
{icon}
</span>
)
}
+156
View File
@@ -0,0 +1,156 @@
import { useEffect, useMemo, useRef, useState } from 'react'
import { cn } from '@/lib/cn'
import { fieldControlClass } from '@/components/ui/field'
import { CategoryChips } from '@/components/plants/CategoryChips'
import { PlantIcon } from '@/components/plants/PlantIcon'
import { CATEGORY_LABELS, filterPlants, usePlants, type CategoryFilter, type Plant } from '@/lib/plants'
import { formatSpacing, type UnitPref } from '@/lib/units'
const RECENT_KEY = 'pansy:recent-plants'
const RECENT_MAX = 8
/** Recently-picked plant ids, most-recent first, from localStorage. */
function loadRecent(): number[] {
try {
const v: unknown = JSON.parse(localStorage.getItem(RECENT_KEY) ?? '[]')
return Array.isArray(v) ? v.filter((n): n is number => typeof n === 'number') : []
} catch {
return []
}
}
/** Prepend id (dedup, capped) and persist; returns the new list. */
function recordRecent(id: number): number[] {
const next = [id, ...loadRecent().filter((n) => n !== id)].slice(0, RECENT_MAX)
try {
localStorage.setItem(RECENT_KEY, JSON.stringify(next))
} catch {
// Recents are a nicety; ignore quota/availability failures.
}
return next
}
/**
* Reusable plant chooser: a search-first list with a recently-used shortcut and
* category filter, rendered as a bottom sheet on mobile / centered panel on
* desktop. `onSelect` fires with the chosen plant (and records it as recent).
* The plops editor (#15) opens this when placing plants; /plants demos it.
*/
export function PlantPicker({
onSelect,
onClose,
unit = 'metric',
}: {
onSelect: (plant: Plant) => void
onClose: () => void
unit?: UnitPref
}) {
Review

🟠 Duplicated filter state and filtering logic with PlantsPage

maintainability · flagged by 2 models

  • web/src/pages/PlantsPage.tsx:58 / web/src/editor/PlantPicker.tsx:47Duplicated filter state shape and logic. Both files independently define type CategoryFilter = PlantCategory | 'all', query/category state, and an identical filtered useMemo. This means search behavior changes (e.g., fuzzy matching, searching notes) need to be made twice. Extract a shared usePlantFilter hook (or at least the CategoryFilter type and filter predicate).

🪰 Gadfly · advisory

🟠 **Duplicated filter state and filtering logic with PlantsPage** _maintainability · flagged by 2 models_ - `web/src/pages/PlantsPage.tsx:58` / `web/src/editor/PlantPicker.tsx:47` — **Duplicated filter state shape and logic.** Both files independently define `type CategoryFilter = PlantCategory | 'all'`, `query`/`category` state, and an identical `filtered` `useMemo`. This means search behavior changes (e.g., fuzzy matching, searching notes) need to be made twice. **Extract a shared `usePlantFilter` hook (or at least the `CategoryFilter` type and filter predicate).** <sub>🪰 Gadfly · advisory</sub>
const plants = usePlants()
const [query, setQuery] = useState('')
const [category, setCategory] = useState<CategoryFilter>('all')
const [recent, setRecent] = useState<number[]>(() => loadRecent())
const searchRef = useRef<HTMLInputElement>(null)
const onCloseRef = useRef(onClose)
Review

🟠 PlantPicker re-implements Modal close behavior instead of sharing it

maintainability · flagged by 1 model

  • web/src/editor/PlantPicker.tsx:53-60 / web/src/editor/PlantPicker.tsx:126PlantPicker re-implements Modal's close behavior. The file duplicates the onCloseRef + keydown Escape listener pattern and the onMouseDown backdrop-click-close logic that already lives in Modal.tsx. Because PlantPicker is a custom bottom sheet, it can't trivially use Modal, but the close-handling concern should still be shared (e.g., extract a useDialogClose(onClose, busy?) hook) so fixes like focus…

🪰 Gadfly · advisory

🟠 **PlantPicker re-implements Modal close behavior instead of sharing it** _maintainability · flagged by 1 model_ - `web/src/editor/PlantPicker.tsx:53-60` / `web/src/editor/PlantPicker.tsx:126` — **PlantPicker re-implements Modal's close behavior.** The file duplicates the `onCloseRef` + `keydown` Escape listener pattern and the `onMouseDown` backdrop-click-close logic that already lives in `Modal.tsx`. Because PlantPicker is a custom bottom sheet, it can't trivially use `Modal`, but the close-handling concern should still be shared (e.g., extract a `useDialogClose(onClose, busy?)` hook) so fixes like focus… <sub>🪰 Gadfly · advisory</sub>
onCloseRef.current = onClose
useEffect(() => {
searchRef.current?.focus()
function onKey(e: KeyboardEvent) {
if (e.key === 'Escape') onCloseRef.current()
}
document.addEventListener('keydown', onKey)
return () => document.removeEventListener('keydown', onKey)
}, [])
Review

🟡 Duplicated chip + filter logic between PlantsPage and PlantPicker

maintainability · flagged by 1 model

  • Duplicated chip + filter logic between PlantsPage and PlantPicker (web/src/pages/PlantsPage.tsx:60-72, web/src/editor/PlantPicker.tsx:63-72,109-121). Both files declare an identical CategoryFilter = PlantCategory | 'all' type, an identical filtered useMemo (category === 'all' || p.category === category) && (q === '' || p.name.toLowerCase().includes(q)), and a near-identical chip render helper (the only difference is text-sm vs text-xs/pb-1). This is exactly the kind…

🪰 Gadfly · advisory

🟡 **Duplicated `chip` + filter logic between `PlantsPage` and `PlantPicker`** _maintainability · flagged by 1 model_ - **Duplicated `chip` + filter logic between `PlantsPage` and `PlantPicker`** (`web/src/pages/PlantsPage.tsx:60-72`, `web/src/editor/PlantPicker.tsx:63-72,109-121`). Both files declare an identical `CategoryFilter = PlantCategory | 'all'` type, an identical `filtered` useMemo `(category === 'all' || p.category === category) && (q === '' || p.name.toLowerCase().includes(q))`, and a near-identical `chip` render helper (the only difference is `text-sm` vs `text-xs`/`pb-1`). This is exactly the kind… <sub>🪰 Gadfly · advisory</sub>
const all = useMemo(() => plants.data ?? [], [plants.data])
const filtered = useMemo(() => filterPlants(all, query, category), [all, query, category])
// Recents show only when not actively searching, and only rows still present.
const recentPlants = useMemo(() => {
if (query.trim() !== '') return []
const byId = new Map(all.map((p) => [p.id, p]))
return recent.map((id) => byId.get(id)).filter((p): p is Plant => !!p)
}, [all, recent, query])
function choose(p: Plant) {
setRecent(recordRecent(p.id))
onSelect(p)
}
// A plant option row. keyPrefix namespaces the key so a plant appearing in
// both the Recent and All sections doesn't collide on a duplicate React key.
const row = (p: Plant, keyPrefix: string) => (
<button
key={`${keyPrefix}-${p.id}`}
type="button"
Review

🟠 Duplicate React keys when recent plant also appears in filtered list

error-handling, maintainability · flagged by 2 models

  • web/src/editor/PlantPicker.tsx:86-171 — Duplicate React keys when a recent plant also appears in the filtered list Verified by reading the component source. When the search query is empty, recentPlants.map(row) renders <button key={p.id} …> for recently-used plants. Immediately afterward, filtered.map(row) renders the same <button key={p.id} …> for every plant in the catalog. Since filtered includes all plants when category === 'all' and query === '', any plant that is also…

🪰 Gadfly · advisory

🟠 **Duplicate React keys when recent plant also appears in filtered list** _error-handling, maintainability · flagged by 2 models_ - **`web/src/editor/PlantPicker.tsx:86-171` — Duplicate React keys when a recent plant also appears in the filtered list** Verified by reading the component source. When the search query is empty, `recentPlants.map(row)` renders `<button key={p.id} …>` for recently-used plants. Immediately afterward, `filtered.map(row)` renders the same `<button key={p.id} …>` for every plant in the catalog. Since `filtered` includes all plants when `category === 'all'` and `query === ''`, any plant that is also… <sub>🪰 Gadfly · advisory</sub>
onClick={() => choose(p)}
className="flex w-full items-center gap-3 rounded-lg px-3 py-2.5 text-left outline-none transition-colors hover:bg-border/50 focus-visible:bg-border/50"
>
<PlantIcon color={p.color} icon={p.icon} className="h-9 w-9 rounded-md text-xl" />
<span className="min-w-0 flex-1">
<span className="block truncate font-medium text-fg">{p.name}</span>
Review

🟡 Icon-tile + color-mix styling duplicated

maintainability · flagged by 1 model

  • Icon-tile + color-mix styling duplicated between PlantCard (web/src/components/plants/PlantCard.tsx:32-39) and PlantPicker (web/src/editor/PlantPicker.tsx:92-99). Same grid h-N w-N place-items-center rounded-md text-xl/2xl tile with backgroundColor: color-mix(in srgb, ${color} 18%, transparent) and the emoji inside. Extract a <PlantIconTile plant size> component (or a tileStyle(plant) helper) so the tint formula lives in one place — important because the magic 18% const…

🪰 Gadfly · advisory

🟡 **Icon-tile + `color-mix` styling duplicated** _maintainability · flagged by 1 model_ - **Icon-tile + `color-mix` styling duplicated** between `PlantCard` (`web/src/components/plants/PlantCard.tsx:32-39`) and `PlantPicker` (`web/src/editor/PlantPicker.tsx:92-99`). Same `grid h-N w-N place-items-center rounded-md text-xl/2xl` tile with `backgroundColor: color-mix(in srgb, ${color} 18%, transparent)` and the emoji inside. Extract a `<PlantIconTile plant size>` component (or a `tileStyle(plant)` helper) so the tint formula lives in one place — important because the magic `18%` const… <sub>🪰 Gadfly · advisory</sub>
<span className="block text-xs text-muted">
{CATEGORY_LABELS[p.category]} · {formatSpacing(p.spacingCm, unit)}
</span>
</span>
</button>
)
return (
<div
className="fixed inset-0 z-50 flex items-end justify-center bg-black/40 sm:items-center sm:p-4"
onMouseDown={(e) => e.target === e.currentTarget && onClose()}
>
<div
role="dialog"
aria-modal="true"
aria-label="Choose a plant"
className="flex max-h-[85vh] w-full flex-col rounded-t-2xl border border-border bg-surface shadow-xl sm:max-w-lg sm:rounded-2xl"
>
<div className="flex items-center gap-2 border-b border-border p-3">
<input
ref={searchRef}
type="search"
value={query}
Review

🟠 Duplicated inline chip renderer for category filters

maintainability · flagged by 1 model

  • web/src/pages/PlantsPage.tsx:70 / web/src/editor/PlantPicker.tsx:115Duplicated inline chip renderer for category filters. Both components define an almost identical inline chip helper that renders a pill-style category filter button. The only meaningful difference is text size (text-sm in the page vs text-xs in the picker), indicating copy-paste with a tweak. If chip styling needs to change (active ring, padding, dark mode), it must be updated in both places. **Extract a sha…

🪰 Gadfly · advisory

🟠 **Duplicated inline chip renderer for category filters** _maintainability · flagged by 1 model_ - `web/src/pages/PlantsPage.tsx:70` / `web/src/editor/PlantPicker.tsx:115` — **Duplicated inline `chip` renderer for category filters.** Both components define an almost identical inline `chip` helper that renders a pill-style category filter button. The only meaningful difference is text size (`text-sm` in the page vs `text-xs` in the picker), indicating copy-paste with a tweak. If chip styling needs to change (active ring, padding, dark mode), it must be updated in both places. **Extract a sha… <sub>🪰 Gadfly · advisory</sub>
onChange={(e) => setQuery(e.target.value)}
placeholder="Search plants…"
aria-label="Search plants"
className={cn(fieldControlClass, 'min-w-0 flex-1')}
/>
<button
type="button"
onClick={onClose}
aria-label="Close"
className="rounded-md px-2 py-2 text-muted outline-none transition-colors hover:bg-border/50 hover:text-fg focus-visible:ring-2 focus-visible:ring-accent/40"
>
</button>
</div>
<div className="border-b border-border px-3 py-2">
<CategoryChips value={category} onChange={setCategory} size="sm" />
</div>
<div className="min-h-0 flex-1 overflow-y-auto p-2">
{plants.isPending && <p className="p-4 text-sm text-muted">Loading plants</p>}
{plants.isError && <p className="p-4 text-sm text-red-600 dark:text-red-400">Couldn't load plants.</p>}
{recentPlants.length > 0 && (
<>
<p className="px-3 pb-1 pt-2 text-xs font-semibold uppercase tracking-wide text-muted">Recent</p>
{recentPlants.map((p) => row(p, 'recent'))}
<p className="px-3 pb-1 pt-3 text-xs font-semibold uppercase tracking-wide text-muted">All plants</p>
</>
)}
{filtered.map((p) => row(p, 'all'))}
{plants.isSuccess && filtered.length === 0 && (
<p className="p-4 text-sm text-muted">No plants match your search.</p>
)}
</div>
</div>
</div>
)
}
+122
View File
@@ -0,0 +1,122 @@
// Plant catalog data layer: zod-validated shapes for /api/v1/plants plus the
// react-query hooks the /plants page and PlantPicker use. Built-ins (ownerId
// null) are seeded and read-only; a user's own rows are editable. "Cloning" a
// built-in is just a create of a copy (no special endpoint).
import { queryOptions, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
import { z } from 'zod'
import { ApiError, api } from './api'
export const PLANT_CATEGORIES = ['vegetable', 'herb', 'flower', 'fruit', 'tree_shrub', 'cover'] as const
export type PlantCategory = (typeof PLANT_CATEGORIES)[number]
export const CATEGORY_LABELS: Record<PlantCategory, string> = {
vegetable: 'Vegetable',
herb: 'Herb',
flower: 'Flower',
fruit: 'Fruit',
tree_shrub: 'Tree / shrub',
cover: 'Cover crop',
}
const plantCategorySchema = z.enum(PLANT_CATEGORIES)
export const plantSchema = z.object({
id: z.number(),
// Absent (built-in) or a user id. The API omits ownerId for built-ins.
ownerId: z.number().nullable().optional(),
name: z.string(),
category: plantCategorySchema,
spacingCm: z.number(),
color: z.string(),
icon: z.string(),
daysToMaturity: z.number().nullable().optional(),
notes: z.string(),
version: z.number(),
createdAt: z.string(),
updatedAt: z.string(),
})
export type Plant = z.infer<typeof plantSchema>
/** A built-in (seeded, read-only) plant has no owner. */
export function isBuiltin(p: Plant): boolean {
return p.ownerId == null
}
/** Category filter value: a specific category, or 'all'. Shared by the catalog
* page and the PlantPicker so their chip rows and filtering stay in sync. */
export type CategoryFilter = PlantCategory | 'all'
/** Filter a plant list by a name query (case-insensitive substring) and
* category. The single source of truth for both the /plants grid and the
* PlantPicker list. */
export function filterPlants(plants: Plant[], query: string, category: CategoryFilter): Plant[] {
const q = query.trim().toLowerCase()
return plants.filter(
(p) => (category === 'all' || p.category === category) && (q === '' || p.name.toLowerCase().includes(q)),
)
}
const plantsKey = ['plants'] as const
export const plantsQueryOptions = queryOptions({
queryKey: plantsKey,
queryFn: async (): Promise<Plant[]> => z.array(plantSchema).parse(await api.get('/plants')),
})
export function usePlants() {
return useQuery(plantsQueryOptions)
}
export interface PlantInput {
name: string
category: PlantCategory
spacingCm: number
color: string
icon: string
daysToMaturity: number | null
notes: string
}
export function useCreatePlant() {
const qc = useQueryClient()
return useMutation({
mutationFn: async (input: PlantInput): Promise<Plant> => plantSchema.parse(await api.post('/plants', input)),
onSuccess: () => qc.invalidateQueries({ queryKey: plantsKey }),
})
}
export interface PlantUpdate extends PlantInput {
id: number
version: number
}
export function useUpdatePlant() {
const qc = useQueryClient()
return useMutation({
// id/version travel with the mutation variables so a single hook serves any row.
mutationFn: async ({ id, ...body }: PlantUpdate): Promise<Plant> =>
plantSchema.parse(await api.patch(`/plants/${id}`, body)),
onSuccess: () => qc.invalidateQueries({ queryKey: plantsKey }),
})
}
Review

conflictPlant duplicates conflictGarden; a shared generic conflictRow helper would prevent N copies

maintainability · flagged by 1 model

  • Duplicated category-chip + search/filter logic across PlantsPage.tsx and PlantPicker.tsx. Both files re-implement the same chip(value, label) component (near-identical JSX + cn(...) classes, differing only in text-sm vs text-xs) and the same filtered useMemo: q.trim().toLowerCase() over all.filter((p) => (category === 'all' || p.category === category) && (q === '' || p.name.toLowerCase().includes(q))) (web/src/pages/PlantsPage.tsx:64, `web/src/editor/PlantPicker.tsx:109…

🪰 Gadfly · advisory

⚪ **conflictPlant duplicates conflictGarden; a shared generic conflictRow helper would prevent N copies** _maintainability · flagged by 1 model_ - **Duplicated category-chip + search/filter logic across `PlantsPage.tsx` and `PlantPicker.tsx`.** Both files re-implement the same `chip(value, label)` component (near-identical JSX + `cn(...)` classes, differing only in `text-sm` vs `text-xs`) and the same `filtered` useMemo: `q.trim().toLowerCase()` over `all.filter((p) => (category === 'all' || p.category === category) && (q === '' || p.name.toLowerCase().includes(q)))` (`web/src/pages/PlantsPage.tsx:64`, `web/src/editor/PlantPicker.tsx:109… <sub>🪰 Gadfly · advisory</sub>
export function useDeletePlant() {
const qc = useQueryClient()
return useMutation({
mutationFn: (id: number) => api.delete(`/plants/${id}`),
onSuccess: () => qc.invalidateQueries({ queryKey: plantsKey }),
})
}
/**
* 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 conflictPlant(err: unknown): Plant | null {
if (err instanceof ApiError && err.isConflict && err.body && typeof err.body === 'object') {
const parsed = plantSchema.safeParse((err.body as { current?: unknown }).current)
if (parsed.success) return parsed.data
}
return null
}
+28 -1
View File
@@ -1,5 +1,14 @@
import { describe, expect, it } from 'vitest'
import { cmFromFtIn, cmFromMeters, displayFromCm, formatCm, formatDimensions } from './units'
import {
cmFromFtIn,
cmFromMeters,
cmFromSpacing,
displayFromCm,
formatCm,
formatDimensions,
formatSpacing,
spacingFromCm,
} from './units'
describe('cm conversions', () => {
it('feet+inches → whole cm', () => {
@@ -42,3 +51,21 @@ describe('formatCm', () => {
expect(formatDimensions(122, 244, 'imperial')).toBe('4 0″ × 8 0″')
})
})
describe('plant spacing (small scale)', () => {
it('formats cm in metric, whole inches in imperial', () => {
expect(formatSpacing(15, 'metric')).toBe('15 cm')
expect(formatSpacing(15, 'imperial')).toBe('6″') // 15/2.54 = 5.9 → 6
expect(formatSpacing(30, 'imperial')).toBe('12″')
})
it('parses an entered value back to whole cm', () => {
expect(cmFromSpacing(15, 'metric')).toBe(15)
expect(cmFromSpacing(6, 'imperial')).toBe(15) // 6in = 15.24 → 15
})
it('prefills an input from cm', () => {
expect(spacingFromCm(15, 'metric')).toBe(15)
expect(spacingFromCm(30, 'imperial')).toBe(11.8) // 30/2.54 = 11.81 → 11.8
})
})
+26
View File
@@ -68,3 +68,29 @@ export function formatDimensions(widthCm: number, heightCm: number, unit: UnitPr
export function dimensionUnitLabel(unit: UnitPref): string {
return unit === 'imperial' ? 'ft' : 'm'
}
// --- Plant spacing (small scale) -------------------------------------------
// Plant spacing is a handful of centimeters, so meters/feet read poorly here;
// metric shows cm and imperial shows whole inches.
/** Centimeters → a spacing string in the given unit (e.g. "15 cm" or "6″"). */
export function formatSpacing(cm: number, unit: UnitPref): string {
if (unit === 'imperial') return `${Math.round(cm / CM_PER_INCH)}`
return `${Math.round(cm)} cm`
}
/** A spacing value typed in the given unit (cm, or inches) → whole centimeters. */
export function cmFromSpacing(value: number, unit: UnitPref): number {
return unit === 'imperial' ? Math.round(value * CM_PER_INCH) : Math.round(value)
}
/** Centimeters → a spacing number in the given unit, for prefilling an input.
* Inches to 0.1 so cm-quantization doesn't show through. */
export function spacingFromCm(cm: number, unit: UnitPref): number {
return unit === 'imperial' ? Math.round((cm / CM_PER_INCH) * 10) / 10 : Math.round(cm)
}
/** The spacing input-field unit label. */
export function spacingUnitLabel(unit: UnitPref): string {
return unit === 'imperial' ? 'in' : 'cm'
}
+131 -2
View File
@@ -1,5 +1,134 @@
import { PageStub } from '@/components/PageStub'
import { useMemo, useState } from 'react'
import { Alert } from '@/components/ui/Alert'
import { Button } from '@/components/ui/Button'
import { fieldControlClass } from '@/components/ui/field'
import { toast } from '@/components/ui/toast'
import { CategoryChips } from '@/components/plants/CategoryChips'
import { PlantCard } from '@/components/plants/PlantCard'
import { PlantFormModal } from '@/components/plants/PlantFormModal'
import { DeletePlantModal } from '@/components/plants/DeletePlantModal'
import { PlantPicker } from '@/editor/PlantPicker'
import { filterPlants, usePlants, type CategoryFilter, type Plant } from '@/lib/plants'
import type { UnitPref } from '@/lib/units'
// Which modal is open, if any. edit/duplicate/delete carry the target plant.
Review

🟡 CategoryFilter type alias duplicated in PlantsPage and PlantPicker

maintainability · flagged by 1 model

  • web/src/pages/PlantsPage.tsx:14 / web/src/editor/PlantPicker.tsx:31type CategoryFilter = PlantCategory | 'all' is defined identically in both files. It belongs next to the category definitions in lib/plants.ts so the union is maintained in one place.

🪰 Gadfly · advisory

🟡 **CategoryFilter type alias duplicated in PlantsPage and PlantPicker** _maintainability · flagged by 1 model_ - **`web/src/pages/PlantsPage.tsx:14`** / **`web/src/editor/PlantPicker.tsx:31`** — `type CategoryFilter = PlantCategory | 'all'` is defined identically in both files. It belongs next to the category definitions in `lib/plants.ts` so the union is maintained in one place. <sub>🪰 Gadfly · advisory</sub>
type Dialog =
| { kind: 'create' }
| { kind: 'edit'; plant: Plant }
| { kind: 'duplicate'; plant: Plant }
| { kind: 'delete'; plant: Plant }
| { kind: 'picker' }
| null
// There's no per-user unit preference server-side (only per-garden), so the
// catalog page keeps its own, persisted locally.
const UNIT_KEY = 'pansy:unit-pref'
function loadUnit(): UnitPref {
try {
Review

🟡 Unit pref and recent-plants localStorage keys aren't user-scoped or cleared on logout, leaking prior user's data on shared devices

security · flagged by 1 model

  • web/src/pages/PlantsPage.tsx:27 (UNIT_KEY) and web/src/editor/PlantPicker.tsx:7-24 (RECENT_KEY) — persisted state is written to plain, unscoped localStorage and is never cleared on logout. useLogout in web/src/lib/auth.ts:86-97 explicitly clears the react-query cache "so the next user never sees the previous user's gardens/plants" (qc.removeQueries({ predicate: (q) => q.queryKey[0] !== 'auth' })), but this only touches react-query state, not localStorage. No `localStorage.rem…

🪰 Gadfly · advisory

🟡 **Unit pref and recent-plants localStorage keys aren't user-scoped or cleared on logout, leaking prior user's data on shared devices** _security · flagged by 1 model_ - `web/src/pages/PlantsPage.tsx:27` (`UNIT_KEY`) and `web/src/editor/PlantPicker.tsx:7-24` (`RECENT_KEY`) — persisted state is written to plain, unscoped `localStorage` and is never cleared on logout. `useLogout` in `web/src/lib/auth.ts:86-97` explicitly clears the react-query cache "so the next user never sees the previous user's gardens/plants" (`qc.removeQueries({ predicate: (q) => q.queryKey[0] !== 'auth' })`), but this only touches react-query state, not `localStorage`. No `localStorage.rem… <sub>🪰 Gadfly · advisory</sub>
return localStorage.getItem(UNIT_KEY) === 'imperial' ? 'imperial' : 'metric'
} catch {
return 'metric'
}
}
export function PlantsPage() {
return <PageStub title="Plants">The plant catalog arrives in issue #13.</PageStub>
const plants = usePlants()
const [unit, setUnit] = useState<UnitPref>(() => loadUnit())
const [query, setQuery] = useState('')
const [category, setCategory] = useState<CategoryFilter>('all')
const [dialog, setDialog] = useState<Dialog>(null)
const close = () => setDialog(null)
function toggleUnit() {
setUnit((u) => {
const next: UnitPref = u === 'metric' ? 'imperial' : 'metric'
try {
localStorage.setItem(UNIT_KEY, next)
} catch {
// Preference is a nicety; ignore storage failures.
}
return next
})
}
const all = useMemo(() => plants.data ?? [], [plants.data])
const filtered = useMemo(() => filterPlants(all, query, category), [all, query, category])
Review

🟠 Duplicated filter state and filtering logic with PlantPicker

maintainability · flagged by 3 models

  • web/src/pages/PlantsPage.tsx:58 / web/src/editor/PlantPicker.tsx:47Duplicated filter state shape and logic. Both files independently define type CategoryFilter = PlantCategory | 'all', query/category state, and an identical filtered useMemo. This means search behavior changes (e.g., fuzzy matching, searching notes) need to be made twice. Extract a shared usePlantFilter hook (or at least the CategoryFilter type and filter predicate).

🪰 Gadfly · advisory

🟠 **Duplicated filter state and filtering logic with PlantPicker** _maintainability · flagged by 3 models_ - `web/src/pages/PlantsPage.tsx:58` / `web/src/editor/PlantPicker.tsx:47` — **Duplicated filter state shape and logic.** Both files independently define `type CategoryFilter = PlantCategory | 'all'`, `query`/`category` state, and an identical `filtered` `useMemo`. This means search behavior changes (e.g., fuzzy matching, searching notes) need to be made twice. **Extract a shared `usePlantFilter` hook (or at least the `CategoryFilter` type and filter predicate).** <sub>🪰 Gadfly · advisory</sub>
return (
<section>
<div className="flex flex-wrap items-center justify-between gap-3">
<h1 className="text-2xl font-semibold tracking-tight">Plants</h1>
<div className="flex items-center gap-2">
<Button variant="ghost" onClick={() => setDialog({ kind: 'picker' })}>
Try the picker
</Button>
Review

🟡 CategoryChips + plant-filter logic duplicated between PlantsPage and PlantPicker

maintainability · flagged by 2 models

  • Duplicated category-chip + search/filter logic across PlantsPage.tsx and PlantPicker.tsx. Both files re-implement the same chip(value, label) component (near-identical JSX + cn(...) classes, differing only in text-sm vs text-xs) and the same filtered useMemo: q.trim().toLowerCase() over all.filter((p) => (category === 'all' || p.category === category) && (q === '' || p.name.toLowerCase().includes(q))) (web/src/pages/PlantsPage.tsx:64, `web/src/editor/PlantPicker.tsx:109…

🪰 Gadfly · advisory

🟡 **CategoryChips + plant-filter logic duplicated between PlantsPage and PlantPicker** _maintainability · flagged by 2 models_ - **Duplicated category-chip + search/filter logic across `PlantsPage.tsx` and `PlantPicker.tsx`.** Both files re-implement the same `chip(value, label)` component (near-identical JSX + `cn(...)` classes, differing only in `text-sm` vs `text-xs`) and the same `filtered` useMemo: `q.trim().toLowerCase()` over `all.filter((p) => (category === 'all' || p.category === category) && (q === '' || p.name.toLowerCase().includes(q)))` (`web/src/pages/PlantsPage.tsx:64`, `web/src/editor/PlantPicker.tsx:109… <sub>🪰 Gadfly · advisory</sub>
<Button
variant="ghost"
onClick={toggleUnit}
title="Toggle spacing units"
aria-label={`Spacing units: ${unit === 'metric' ? 'metric' : 'imperial'}`}
>
Review

🟠 Duplicated inline chip renderer for category filters

maintainability · flagged by 1 model

  • web/src/pages/PlantsPage.tsx:70 / web/src/editor/PlantPicker.tsx:115Duplicated inline chip renderer for category filters. Both components define an almost identical inline chip helper that renders a pill-style category filter button. The only meaningful difference is text size (text-sm in the page vs text-xs in the picker), indicating copy-paste with a tweak. If chip styling needs to change (active ring, padding, dark mode), it must be updated in both places. **Extract a sha…

🪰 Gadfly · advisory

🟠 **Duplicated inline chip renderer for category filters** _maintainability · flagged by 1 model_ - `web/src/pages/PlantsPage.tsx:70` / `web/src/editor/PlantPicker.tsx:115` — **Duplicated inline `chip` renderer for category filters.** Both components define an almost identical inline `chip` helper that renders a pill-style category filter button. The only meaningful difference is text size (`text-sm` in the page vs `text-xs` in the picker), indicating copy-paste with a tweak. If chip styling needs to change (active ring, padding, dark mode), it must be updated in both places. **Extract a sha… <sub>🪰 Gadfly · advisory</sub>
{unit === 'metric' ? 'cm' : 'in'}
</Button>
<Button onClick={() => setDialog({ kind: 'create' })}>New plant</Button>
</div>
</div>
<div className="mt-4 flex flex-col gap-3">
<input
type="search"
value={query}
onChange={(e) => setQuery(e.target.value)}
placeholder="Search plants by name…"
aria-label="Search plants"
className={fieldControlClass}
/>
<div className="pb-1">
<CategoryChips value={category} onChange={setCategory} />
</div>
</div>
<div className="mt-5">
{plants.isPending && <p className="text-sm text-muted">Loading the catalog</p>}
{plants.isError && <Alert>Could not load the plant catalog. Please refresh.</Alert>}
{plants.isSuccess && filtered.length === 0 && (
<div className="rounded-xl border border-dashed border-border p-10 text-center">
<p className="text-fg">No plants match.</p>
<p className="mt-1 text-sm text-muted">Try a different search or category, or add a custom plant.</p>
</div>
)}
{filtered.length > 0 && (
<div className="grid grid-cols-1 gap-4 sm:grid-cols-2 lg:grid-cols-3">
{filtered.map((p) => (
<PlantCard
key={p.id}
plant={p}
unit={unit}
onEdit={() => setDialog({ kind: 'edit', plant: p })}
onDelete={() => setDialog({ kind: 'delete', plant: p })}
onDuplicate={() => setDialog({ kind: 'duplicate', plant: p })}
/>
))}
</div>
)}
</div>
{dialog?.kind === 'create' && <PlantFormModal unit={unit} onClose={close} />}
{dialog?.kind === 'edit' && <PlantFormModal plant={dialog.plant} unit={unit} onClose={close} />}
{dialog?.kind === 'duplicate' && <PlantFormModal template={dialog.plant} unit={unit} onClose={close} />}
{dialog?.kind === 'delete' && <DeletePlantModal plant={dialog.plant} onClose={close} />}
{dialog?.kind === 'picker' && (
<PlantPicker
unit={unit}
onClose={close}
onSelect={(p) => {
toast.info(`Picked ${p.name}`)
close()
}}
/>
)}
</section>
)
}