Fix plant placement + add a Seed Tray (#39) #42
@@ -0,0 +1,66 @@
|
||||
import { cn } from '@/lib/cn'
|
||||
import { PlantIcon } from '@/components/plants/PlantIcon'
|
||||
import type { Plant } from '@/lib/plants'
|
||||
|
||||
/**
|
||||
* The Seed Tray strip in the focus-mode toolbar: a row of the garden's
|
||||
* kept-at-hand plants. Tap a chip to arm that plant for tap-to-place (the armed
|
||||
* chip is highlighted); ✕ removes it from the tray; the trailing + chip opens
|
||||
* the full catalog picker. See useSeedTray for persistence.
|
||||
*/
|
||||
export function SeedTray({
|
||||
plants,
|
||||
armedPlantId,
|
||||
onArm,
|
||||
onRemove,
|
||||
onOpenPicker,
|
||||
}: {
|
||||
plants: Plant[]
|
||||
armedPlantId: number | null
|
||||
onArm: (plant: Plant) => void
|
||||
onRemove: (id: number) => void
|
||||
onOpenPicker: () => void
|
||||
}) {
|
||||
return (
|
||||
<div className="flex flex-wrap items-center gap-1.5">
|
||||
{plants.map((p) => {
|
||||
const active = p.id === armedPlantId
|
||||
return (
|
||||
<span
|
||||
|
|
||||
key={p.id}
|
||||
className={cn(
|
||||
'inline-flex items-center rounded-full border text-xs transition-colors',
|
||||
active ? 'border-accent bg-accent/10 text-accent-strong' : 'border-border bg-surface text-fg',
|
||||
)}
|
||||
>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => onArm(p)}
|
||||
aria-pressed={active}
|
||||
title={active ? `Placing ${p.name}` : `Place ${p.name}`}
|
||||
className="flex items-center gap-1.5 rounded-full py-1 pl-1.5 pr-1 outline-none focus-visible:ring-2 focus-visible:ring-accent/40"
|
||||
>
|
||||
<PlantIcon color={p.color} icon={p.icon} className="h-5 w-5 rounded-full text-[0.65rem]" />
|
||||
<span className="max-w-[6rem] truncate font-medium">{p.name}</span>
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => onRemove(p.id)}
|
||||
aria-label={`Remove ${p.name} from tray`}
|
||||
className="rounded-full px-1.5 py-1 text-muted outline-none hover:text-fg focus-visible:ring-2 focus-visible:ring-accent/40"
|
||||
>
|
||||
✕
|
||||
</button>
|
||||
</span>
|
||||
)
|
||||
})}
|
||||
<button
|
||||
type="button"
|
||||
onClick={onOpenPicker}
|
||||
className="inline-flex items-center gap-1 rounded-full border border-dashed border-border px-2.5 py-1 text-xs font-medium text-muted outline-none transition-colors hover:border-accent hover:text-accent-strong focus-visible:ring-2 focus-visible:ring-accent/40"
|
||||
>
|
||||
+ Plant
|
||||
</button>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
+20
-1
@@ -3,11 +3,12 @@
|
||||
// PATCH with the row version; a 409 rolls back to the server's current row and
|
||||
// toasts (see DESIGN § Sync).
|
||||
|
||||
import { useCallback } from 'react'
|
||||
import { queryOptions, useMutation, useQuery, useQueryClient, type QueryClient } from '@tanstack/react-query'
|
||||
import { z } from 'zod'
|
||||
import { ApiError, api } from './api'
|
||||
import { gardenSchema } from './gardens'
|
||||
import { plantSchema } from './plants'
|
||||
import { plantSchema, type Plant } from './plants'
|
||||
import { serverPlantingSchema, type ServerPlanting } from './plantings'
|
||||
import { toast } from '@/components/ui/toast'
|
||||
import type { EditorObject, ObjectShapeKind } from '@/editor/types'
|
||||
@@ -77,6 +78,24 @@ export function useGardenFull(gardenId: number) {
|
||||
return useQuery(gardenFullQueryOptions(gardenId))
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensure a plant is present in the /full cache's `plants` list. The full payload
|
||||
* carries only the garden's *referenced* plants (ListReferencedPlants), so a
|
||||
* plant chosen from the full catalog but not yet placed here is absent — its
|
||||
* plops would render without an icon/color until the next refetch. Call this
|
||||
* when arming/placing such a plant so it renders immediately. No-op if present.
|
||||
*/
|
||||
export function useEnsurePlantInFull(gardenId: number) {
|
||||
const qc = useQueryClient()
|
||||
return useCallback(
|
||||
(plant: Plant) =>
|
||||
patchFullCache(qc, gardenId, (full) =>
|
||||
|
gitea-actions
commented
🟡 useEnsurePlantInFull silently no-ops when cache is empty error-handling · flagged by 1 model
🪰 Gadfly · advisory 🟡 **useEnsurePlantInFull silently no-ops when cache is empty**
_error-handling · flagged by 1 model_
- **`web/src/lib/objects.ts:92`** — `useEnsurePlantInFull` silently no-ops when the `/full` cache has no data. `patchFullCache` returns `undefined` untouched if the cache entry is missing (e.g., during a race with cache eviction or invalidation), so the caller proceeds to arm the plant without its metadata ever being spliced in. The helper should return a success flag or warn so the caller can decide whether to proceed with arming.
<sub>🪰 Gadfly · advisory</sub>
|
||||
full.plants.some((p) => p.id === plant.id) ? full : { ...full, plants: [...full.plants, plant] },
|
||||
),
|
||||
[qc, gardenId],
|
||||
)
|
||||
}
|
||||
|
||||
// --- mutations -------------------------------------------------------------
|
||||
|
||||
export interface ObjectCreate {
|
||||
|
||||
@@ -0,0 +1,82 @@
|
||||
// The Seed Tray: a small, deliberately-curated set of plants a gardener keeps at
|
||||
// hand while planting a bed, so repeat placement is a single tap instead of a
|
||||
// trip through the full catalog. It's a per-garden convenience, persisted in
|
||||
// localStorage (same rationale as the PlantPicker's "recent plants"): a nicety,
|
||||
// not authoritative state, so a quota/availability failure is swallowed.
|
||||
//
|
||||
// Only plant ids are stored; they resolve against the live catalog on read, so a
|
||||
// renamed plant stays current and a deleted one silently drops out of the tray.
|
||||
|
||||
import { useCallback, useEffect, useMemo, useState } from 'react'
|
||||
import { usePlants, type Plant } from './plants'
|
||||
|
||||
const KEY = (gardenId: number) => `pansy:seed-tray:${gardenId}`
|
||||
const TRAY_MAX = 24 // a working set, not a catalog; caps pathological growth
|
||||
|
||||
function loadIds(gardenId: number): number[] {
|
||||
|
gitea-actions
commented
🟠 Unbounded localStorage read bypasses TRAY_MAX cap on load error-handling, maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟠 **Unbounded localStorage read bypasses TRAY_MAX cap on load**
_error-handling, maintainability · flagged by 2 models_
- **`web/src/lib/seedTray.ts:16` — Unbounded `localStorage` read, no `TRAY_MAX` cap on load** `loadIds` filters and returns every numeric ID stored in localStorage, but never applies `TRAY_MAX`. If the key is corrupted or manually edited to hold thousands of IDs, the component will attempt to render an unbounded number of chips on first mount, degrading or freezing the UI. The `TRAY_MAX` slice is only enforced inside `add`, so a malicious/corrupted payload bypasses the safeguard entirely. **Fix:…
<sub>🪰 Gadfly · advisory</sub>
|
||||
try {
|
||||
const v: unknown = JSON.parse(localStorage.getItem(KEY(gardenId)) ?? '[]')
|
||||
return Array.isArray(v) ? v.filter((n): n is number => typeof n === 'number') : []
|
||||
} catch {
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
function saveIds(gardenId: number, ids: number[]) {
|
||||
try {
|
||||
localStorage.setItem(KEY(gardenId), JSON.stringify(ids))
|
||||
} catch {
|
||||
// The tray is a convenience; ignore quota/availability failures.
|
||||
}
|
||||
}
|
||||
|
||||
export interface SeedTray {
|
||||
|
gitea-actions
commented
🟡 SeedTray interface name collides with the SeedTray component in editor/SeedTray.tsx maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟡 **SeedTray interface name collides with the SeedTray component in editor/SeedTray.tsx**
_maintainability · flagged by 2 models_
- `web/src/lib/seedTray.ts:33` vs `web/src/editor/SeedTray.tsx:11` — **Name collision**: `SeedTray` is both the presentational component (`export function SeedTray`) and the hook's return type (`export interface SeedTray`). They live in separate modules so it compiles, but `GardenEditorPage.tsx:10` imports the component while the hook's return shape flows in indirectly via `useSeedTray`, making "SeedTray" ambiguous across searches/stack traces. Renaming the interface (e.g. `SeedTrayApi`) would d…
<sub>🪰 Gadfly · advisory</sub>
|
||||
/** Tray plants in insertion order, resolved against the live catalog. */
|
||||
trayPlants: Plant[]
|
||||
/** Add a plant to the end of the tray (no-op if already present). */
|
||||
add: (plant: Plant) => void
|
||||
/** Remove a plant from the tray. */
|
||||
remove: (id: number) => void
|
||||
}
|
||||
|
||||
/** Per-garden Seed Tray backed by localStorage and the plant catalog. */
|
||||
export function useSeedTray(gardenId: number): SeedTray {
|
||||
const plants = usePlants()
|
||||
|
gitea-actions
commented
🟡 useSeedTray doesn't surface plants.isPending/isError, so a catalog fetch failure silently empties the tray with no indication to the user error-handling · flagged by 1 model
🪰 Gadfly · advisory 🟡 **useSeedTray doesn't surface plants.isPending/isError, so a catalog fetch failure silently empties the tray with no indication to the user**
_error-handling · flagged by 1 model_
- `web/src/lib/seedTray.ts:44` — `useSeedTray` never checks `plants.isPending`/`plants.isError` from `usePlants()` (`plants.ts:67-69`). If the catalog fetch is loading or fails, `byId` stays empty and `trayPlants` silently resolves to `[]`, leaving only the bare "+ Plant" chip with no indication anything went wrong — unlike `PlantPicker`, which explicitly renders "Loading plants…" and "Couldn't load plants." on `isPending`/`isError` (`PlantPicker.tsx:136-137`). Not data loss (ids stay in `localS…
<sub>🪰 Gadfly · advisory</sub>
|
||||
const [ids, setIds] = useState<number[]>(() => loadIds(gardenId))
|
||||
|
||||
// Re-read when switching gardens (the route param changes without remounting).
|
||||
useEffect(() => {
|
||||
setIds(loadIds(gardenId))
|
||||
}, [gardenId])
|
||||
|
||||
const byId = useMemo(() => new Map((plants.data ?? []).map((p) => [p.id, p])), [plants.data])
|
||||
|
||||
// Resolve ids → plants, dropping any no longer in the catalog; preserve order.
|
||||
const trayPlants = useMemo(
|
||||
() => ids.map((id) => byId.get(id)).filter((p): p is Plant => !!p),
|
||||
[ids, byId],
|
||||
)
|
||||
|
||||
const add = useCallback(
|
||||
(plant: Plant) =>
|
||||
setIds((prev) => {
|
||||
if (prev.includes(plant.id)) return prev
|
||||
const next = [...prev, plant.id].slice(-TRAY_MAX)
|
||||
saveIds(gardenId, next)
|
||||
return next
|
||||
}),
|
||||
[gardenId],
|
||||
)
|
||||
|
||||
const remove = useCallback(
|
||||
(id: number) =>
|
||||
setIds((prev) => {
|
||||
const next = prev.filter((x) => x !== id)
|
||||
saveIds(gardenId, next)
|
||||
return next
|
||||
}),
|
||||
[gardenId],
|
||||
)
|
||||
|
||||
return { trayPlants, add, remove }
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import { Inspector } from '@/editor/Inspector'
|
||||
import { PlopInspector } from '@/editor/PlopInspector'
|
||||
import { PlantPicker } from '@/editor/PlantPicker'
|
||||
import { Palette } from '@/editor/Palette'
|
||||
import { SeedTray } from '@/editor/SeedTray'
|
||||
import { ClearBedModal } from '@/editor/ClearBedModal'
|
||||
import { EditorHint } from '@/editor/EditorHint'
|
||||
import { objectDisplayName } from '@/editor/kinds'
|
||||
@@ -14,8 +15,10 @@ import { useEditorStore } from '@/editor/store'
|
||||
import type { EditorGarden } from '@/editor/types'
|
||||
import { ShareGardenModal } from '@/components/gardens/ShareGardenModal'
|
||||
import { useMe } from '@/lib/auth'
|
||||
import { toEditorObject, useGardenFull, useUpdateObject, useUpdatePlanting } from '@/lib/objects'
|
||||
import { toEditorObject, useEnsurePlantInFull, useGardenFull, useUpdateObject, useUpdatePlanting } from '@/lib/objects'
|
||||
import { toEditorPlanting } from '@/lib/plantings'
|
||||
import type { Plant } from '@/lib/plants'
|
||||
import { useSeedTray } from '@/lib/seedTray'
|
||||
import { usePageTitle } from '@/lib/usePageTitle'
|
||||
|
||||
const routeApi = getRouteApi('/gardens/$gardenId')
|
||||
@@ -43,6 +46,8 @@ export function GardenEditorPage() {
|
||||
|
||||
const updatePlanting = useUpdatePlanting(gid)
|
||||
const updateObject = useUpdateObject(gid)
|
||||
const ensurePlant = useEnsurePlantInFull(gid)
|
||||
const { trayPlants, add: addToTray, remove: removeFromTray } = useSeedTray(gid)
|
||||
|
||||
// Which plant-picker flow is open: 'place' arms a plant for repeat placement;
|
||||
// 'change' swaps the selected plop's plant.
|
||||
@@ -231,14 +236,26 @@ export function GardenEditorPage() {
|
||||
const selectedPlop = plantings.find((p) => p.id === selectedPlantingId) ?? null
|
||||
const focusedPlops = focusedObjectId != null ? plantings.filter((p) => p.objectId === focusedObjectId) : []
|
||||
|
||||
function onPickPlant(plantId: number) {
|
||||
// Arm a plant for tap-to-place. The full catalog carries plants not yet in this
|
||||
// garden's /full payload, so make sure the chosen one is in the cache first —
|
||||
// otherwise its placed plops render without an icon/color until a refetch.
|
||||
function armPlant(plant: Plant) {
|
||||
|
gitea-actions
commented
🟡 ensurePlant called in two different styles across armPlant and the 'change' branch; duplication worth hoisting maintainability · flagged by 1 model
🪰 Gadfly · advisory 🟡 **ensurePlant called in two different styles across armPlant and the 'change' branch; duplication worth hoisting**
_maintainability · flagged by 1 model_
- **`web/src/pages/GardenEditorPage.tsx:242-261`** — `ensurePlant(plant)` is invoked in `armPlant` (line 243) and *again* inline in the `'change'` branch of `onPickPlant` (line 255), while the `'place'` branch goes through `armPlant`. The branching mixes two call styles for the same pre-step. Hoisting a single `ensurePlant(plant)` to the top of `onPickPlant` (it's a no-op when already present) would remove the duplication and let `armPlant` focus on `setArmedPlant`. Verified `useEnsurePlantInFul…
<sub>🪰 Gadfly · advisory</sub>
|
||||
ensurePlant(plant)
|
||||
setArmedPlant(plant)
|
||||
}
|
||||
|
||||
// The picker hands back the full Plant (from the whole catalog), so use it
|
||||
// directly rather than re-resolving against the garden's referenced-plant map —
|
||||
// a not-yet-placed plant isn't in that map, which used to silently abort the
|
||||
// pick (nothing armed, picker left open).
|
||||
function onPickPlant(plant: Plant) {
|
||||
if (!canEdit) return // defense in depth: viewers can't reach the picker anyway
|
||||
const plant = plantsById.get(plantId)
|
||||
if (!plant) return
|
||||
addToTray(plant)
|
||||
if (picker === 'change' && selectedPlop) {
|
||||
ensurePlant(plant)
|
||||
updatePlanting.mutate({ id: selectedPlop.id, version: selectedPlop.version, plantId: plant.id })
|
||||
} else {
|
||||
setArmedPlant(plant) // 'place' mode: arm for repeat placement
|
||||
armPlant(plant) // 'place' mode: arm for repeat placement
|
||||
}
|
||||
setPicker(null)
|
||||
}
|
||||
@@ -267,18 +284,21 @@ export function GardenEditorPage() {
|
||||
← {garden.name}
|
||||
</button>
|
||||
<span className="max-w-[8rem] truncate text-muted">{objectDisplayName(focusedObject)}</span>
|
||||
{canEdit &&
|
||||
(!focusedObject.plantable ? (
|
||||
<span className="text-xs text-muted">Not plantable</span>
|
||||
) : armedPlant ? (
|
||||
{canEdit && !focusedObject.plantable && <span className="text-xs text-muted">Not plantable</span>}
|
||||
{canEdit && focusedObject.plantable && (
|
||||
<SeedTray
|
||||
plants={trayPlants}
|
||||
armedPlantId={armedPlant?.id ?? null}
|
||||
onArm={armPlant}
|
||||
onRemove={removeFromTray}
|
||||
onOpenPicker={() => setPicker('place')}
|
||||
/>
|
||||
)}
|
||||
{canEdit && focusedObject.plantable && armedPlant && (
|
||||
<Button variant="ghost" className="px-2 py-1 text-xs" onClick={() => setArmedPlant(null)}>
|
||||
Placing {armedPlant.icon} — Done
|
||||
Done
|
||||
</Button>
|
||||
) : (
|
||||
<Button className="px-2 py-1 text-xs" onClick={() => setPicker('place')}>
|
||||
+ Add plant
|
||||
</Button>
|
||||
))}
|
||||
)}
|
||||
{canEdit && focusedObject.plantable && focusedPlops.length > 0 && (
|
||||
<Button
|
||||
variant="ghost"
|
||||
@@ -297,7 +317,7 @@ export function GardenEditorPage() {
|
||||
<EditorHint>Pick a shape from the palette, then tap the field to place your first bed.</EditorHint>
|
||||
)}
|
||||
{canEdit && focusedObject?.plantable && focusedPlops.length === 0 && !armedPlant && (
|
||||
<EditorHint position="top">Tap “+ Add plant”, then tap inside the bed to place plops.</EditorHint>
|
||||
<EditorHint position="top">Pick a plant from the tray, then tap inside the bed to place it.</EditorHint>
|
||||
)}
|
||||
</div>
|
||||
|
||||
@@ -335,7 +355,7 @@ export function GardenEditorPage() {
|
||||
<PlantPicker
|
||||
unit={garden.unitPref}
|
||||
onClose={() => setPicker(null)}
|
||||
onSelect={(p) => onPickPlant(p.id)}
|
||||
onSelect={(p) => onPickPlant(p)}
|
||||
/>
|
||||
)}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user
⚪ Chip styled span wrapping two buttons splits interactive affordance awkwardly
maintainability · flagged by 1 model
web/src/editor/SeedTray.tsx:29-54— Each chip is a styled<span>wrapping two sibling<button>s, with the active-state border/bg on the outer span while each button carries its ownrounded-full/focus ring. The interactive affordance is split across a non-interactive wrapper and two buttons; a single button with an inner ✕ (orrole="group"on the span) would be cleaner. Not blocking — purely structural readability.🪰 Gadfly · advisory