Field editor: palette, select/move/resize/rotate, inspector, optimistic sync (#11) #30

Merged
steve merged 2 commits from phase-3-field-editor into main 2026-07-19 01:01:39 +00:00
13 changed files with 989 additions and 74 deletions
+3
View File
@@ -1,4 +1,5 @@
import { Link, Outlet, useNavigate } from '@tanstack/react-router' import { Link, Outlet, useNavigate } from '@tanstack/react-router'
import { Toaster } from '@/components/ui/toast'
import { useLogout, useMe } from '@/lib/auth' import { useLogout, useMe } from '@/lib/auth'
const navLinks = [ const navLinks = [
@@ -81,6 +82,8 @@ export function AppShell() {
<main className="mx-auto w-full max-w-5xl flex-1 px-4 py-6"> <main className="mx-auto w-full max-w-5xl flex-1 px-4 py-6">
<Outlet /> <Outlet />
</main> </main>
<Toaster />
</div> </div>
) )
} }
+65
View File
@@ -0,0 +1,65 @@
import { useEffect } from 'react'
import { create } from 'zustand'
import { cn } from '@/lib/cn'
type ToastTone = 'info' | 'error'
interface Toast {
id: number
message: string
tone: ToastTone
}
interface ToastState {
toasts: Toast[]
push: (message: string, tone?: ToastTone) => void
dismiss: (id: number) => void
}
let nextId = 1
export const useToastStore = create<ToastState>((set) => ({
toasts: [],
push: (message, tone = 'info') => set((s) => ({ toasts: [...s.toasts, { id: nextId++, message, tone }] })),
dismiss: (id) => set((s) => ({ toasts: s.toasts.filter((t) => t.id !== id) })),
}))
/** Fire a toast from anywhere (including non-component code). */
export const toast = {
info: (m: string) => useToastStore.getState().push(m, 'info'),
error: (m: string) => useToastStore.getState().push(m, 'error'),
}
// Param is `item`, not `toast`, so it doesn't shadow the module's `toast` export.
Outdated
Review

🟡 ToastItem prop toast shadows the same-file exported toast helper

maintainability · flagged by 2 models

  • web/src/components/ui/toast.tsx:32ToastItem prop toast shadows the imported/exported toast helper. The component parameter { toast } (line 32) shadows the module-level export const toast = {...} (lines 27-30) within the component body. It works (the component only reads toast.id/toast.tone/toast.message), but shadowing a same-file exported symbol is a readability trap — rename the prop to data or t.

🪰 Gadfly · advisory

🟡 **ToastItem prop `toast` shadows the same-file exported `toast` helper** _maintainability · flagged by 2 models_ - **`web/src/components/ui/toast.tsx:32` — `ToastItem` prop `toast` shadows the imported/exported `toast` helper.** The component parameter `{ toast }` (line 32) shadows the module-level `export const toast = {...}` (lines 27-30) within the component body. It works (the component only reads `toast.id`/`toast.tone`/`toast.message`), but shadowing a same-file exported symbol is a readability trap — rename the prop to `data` or `t`. <sub>🪰 Gadfly · advisory</sub>
function ToastItem({ item }: { item: Toast }) {
const dismiss = useToastStore((s) => s.dismiss)
useEffect(() => {
const t = setTimeout(() => dismiss(item.id), 4000)
return () => clearTimeout(t)
}, [item.id, dismiss])
return (
<div
role={item.tone === 'error' ? 'alert' : 'status'}
className={cn(
'pointer-events-auto rounded-md border px-3 py-2 text-sm shadow-md',
item.tone === 'error'
? 'border-red-500/40 bg-red-500/10 text-red-700 dark:text-red-300'
: 'border-border bg-surface text-fg',
)}
>
{item.message}
</div>
)
}
/** Fixed stack of active toasts. Mount once near the app root. */
export function Toaster() {
const toasts = useToastStore((s) => s.toasts)
if (toasts.length === 0) return null
return (
<div className="pointer-events-none fixed bottom-4 left-1/2 z-[60] flex -translate-x-1/2 flex-col items-center gap-2">
{toasts.map((t) => (
<ToastItem key={t.id} item={t} />
))}
</div>
)
}
+78 -49
View File
@@ -1,22 +1,32 @@
import { useEffect, useId, useMemo, useRef, useState } from 'react' import { useEffect, useId, useMemo, useRef, useState, type PointerEvent as ReactPointerEvent } from 'react'
import { Button } from '@/components/ui/Button' import { screenToWorld, type Rect, type Size } from '@/lib/geometry'
import type { Size } from '@/lib/geometry' import { useCreateObject } from '@/lib/objects'
import { ObjectShape } from './ObjectShape' import { ObjectShape } from './ObjectShape'
import { SelectionOverlay } from './SelectionOverlay'
import { kindDef } from './kinds'
import { useEditorStore } from './store' import { useEditorStore } from './store'
import { useViewport } from './useViewport' import { useViewport } from './useViewport'
import type { EditorGarden, EditorObject } from './types' import type { EditorGarden, EditorObject } from './types'
const GRID_CM = 100 // 1 m grid const GRID_CM = 100 // 1 m grid
const GRID_MIN_CELL_PX = 6 // grid is invisible below this cell size (zoomed out) const GRID_MIN_CELL_PX = 6
const GRID_MAX_OPACITY = 0.18 const GRID_MAX_OPACITY = 0.18
/** /**
Outdated
Review

🟠 defaultZForKind hard-codes kind strings that must stay in sync with kinds.ts

maintainability · flagged by 3 models

  • web/src/editor/SelectionOverlay.tsx:138-150 — The circle-vs-rect conditional is repeated twice (transparent drag body + selection outline) with only a few props differing. Extract a small helper (<ShapeBody shape={…} halfW={…} halfH={…} {...props} />) so the shape fork lives in one place. - web/src/lib/objects.ts:188-202applyServerPatch manually spreads each of the 10 patchable fields with !== undefined guards. Adding a new patchable field requires touching this function,…

🪰 Gadfly · advisory

🟠 **defaultZForKind hard-codes kind strings that must stay in sync with kinds.ts** _maintainability · flagged by 3 models_ - **`web/src/editor/SelectionOverlay.tsx:138-150`** — The circle-vs-rect conditional is repeated twice (transparent drag body + selection outline) with only a few props differing. Extract a small helper (`<ShapeBody shape={…} halfW={…} halfH={…} {...props} />`) so the shape fork lives in one place. - **`web/src/lib/objects.ts:188-202`** — `applyServerPatch` manually spreads each of the 10 patchable fields with `!== undefined` guards. Adding a new patchable field requires touching this function,… <sub>🪰 Gadfly · advisory</sub>
* The editor canvas: a full-size SVG with a single world <g> that pan/zoom/pinch * The editor canvas. Pan/zoom/pinch (from useViewport), tap-to-place from the
* transforms. Renders a 1 m grid that fades in as you zoom, the garden boundary, * armed palette kind, click to select, and — for the selected object —
* and its objects. Interactions beyond viewport + basic select (move/resize/ * move/resize/rotate via SelectionOverlay. The object being dragged is rendered
* rotate, palette) land in #11; this is the foundation, driven by mock data. * from liveObject for instant feedback; the PATCH fires on release.
*/ */
export function GardenCanvas({ garden, objects }: { garden: EditorGarden; objects: EditorObject[] }) { export function GardenCanvas({
garden,
objects,
focusId,
}: {
garden: EditorGarden
objects: EditorObject[]
focusId?: number
}) {
const svgRef = useRef<SVGSVGElement>(null) const svgRef = useRef<SVGSVGElement>(null)
const containerRef = useRef<HTMLDivElement>(null) const containerRef = useRef<HTMLDivElement>(null)
const [size, setSize] = useState<Size>({ w: 0, h: 0 }) const [size, setSize] = useState<Size>({ w: 0, h: 0 })
@@ -26,39 +36,77 @@ export function GardenCanvas({ garden, objects }: { garden: EditorGarden; object
const viewport = useEditorStore((s) => s.viewport) const viewport = useEditorStore((s) => s.viewport)
const selectedId = useEditorStore((s) => s.selectedId) const selectedId = useEditorStore((s) => s.selectedId)
const select = useEditorStore((s) => s.select) const select = useEditorStore((s) => s.select)
const liveObject = useEditorStore((s) => s.liveObject)
const { fitToRect } = useViewport(svgRef) const { fitToRect } = useViewport(svgRef)
const create = useCreateObject(garden.id)
const gardenRect = useMemo( const gardenRect: Rect = useMemo(
() => ({ x: 0, y: 0, w: garden.widthCm, h: garden.heightCm }), () => ({ x: 0, y: 0, w: garden.widthCm, h: garden.heightCm }),
[garden.widthCm, garden.heightCm], [garden.widthCm, garden.heightCm],
) )
// Track the container's pixel size for fit math.
useEffect(() => { useEffect(() => {
const el = containerRef.current const el = containerRef.current
if (!el) return if (!el) return
const ro = new ResizeObserver(([entry]) => { const ro = new ResizeObserver(([entry]) => setSize({ w: entry.contentRect.width, h: entry.contentRect.height }))
setSize({ w: entry.contentRect.width, h: entry.contentRect.height })
})
ro.observe(el) ro.observe(el)
return () => ro.disconnect() return () => ro.disconnect()
}, []) }, [])
// Frame the garden once the canvas size is known, and again if we switch to a
// different garden.
useEffect(() => { useEffect(() => {
const usable = size.w > 0 && size.h > 0 && garden.widthCm > 0 && garden.heightCm > 0 const usable = size.w > 0 && size.h > 0 && garden.widthCm > 0 && garden.heightCm > 0
if (usable && fittedForRef.current !== garden.id) { if (usable && fittedForRef.current !== garden.id) {
fittedForRef.current = garden.id fittedForRef.current = garden.id
fitToRect(gardenRect, size) // ?focus=<id> frames that object (groundwork for #15's focus mode);
// otherwise frame the whole garden.
const focus = focusId != null ? objects.find((o) => o.id === focusId) : undefined
const target: Rect = focus
? { x: focus.xCm - focus.widthCm / 2, y: focus.yCm - focus.heightCm / 2, w: focus.widthCm, h: focus.heightCm }
: gardenRect
fitToRect(target, size)
} }
}, [size, garden.id, garden.widthCm, garden.heightCm, gardenRect, fitToRect]) }, [size, garden.id, garden.widthCm, garden.heightCm, gardenRect, fitToRect, focusId, objects])
// Grid fades in as cells grow from GRID_MIN_CELL_PX to 2× that.
const cellPx = GRID_CM * viewport.scale const cellPx = GRID_CM * viewport.scale
const gridOpacity = Math.max(0, Math.min(GRID_MAX_OPACITY, ((cellPx - GRID_MIN_CELL_PX) / GRID_MIN_CELL_PX) * GRID_MAX_OPACITY)) const gridOpacity = Math.max(0, Math.min(GRID_MAX_OPACITY, ((cellPx - GRID_MIN_CELL_PX) / GRID_MIN_CELL_PX) * GRID_MAX_OPACITY))
Outdated
Review

🔴 fittedForRef guard prevents re-framing when focusId changes for same garden

correctness · flagged by 2 models

  • web/src/editor/GardenCanvas.tsx:74-82 — The fittedForRef.current !== garden.id guard means the canvas frames the garden (or a ?focus= object) exactly once per garden mount. If the focusId search param changes client-side without remounting the page, the viewport does not re-fit to the new object because the guard only tracks garden.id, not the focus target. Track the last focused id and re-frame when it changes.

🪰 Gadfly · advisory

🔴 **fittedForRef guard prevents re-framing when focusId changes for same garden** _correctness · flagged by 2 models_ - `web/src/editor/GardenCanvas.tsx:74-82` — The `fittedForRef.current !== garden.id` guard means the canvas frames the garden (or a `?focus=` object) exactly once per garden mount. If the `focusId` search param changes client-side without remounting the page, the viewport does **not** re-fit to the new object because the guard only tracks `garden.id`, not the focus target. Track the last focused id and re-frame when it changes. <sub>🪰 Gadfly · advisory</sub>
const ordered = useMemo(() => [...objects].sort((a, b) => a.zIndex - b.zIndex), [objects]) // Sort once by z-order; only re-sorts when the server objects change.
const sorted = useMemo(() => [...objects].sort((a, b) => a.zIndex - b.zIndex), [objects])
// Merge the in-flight live geometry over the sorted list. A gesture never
// changes z, so this stays a cheap O(n) map on every pointermove — no re-sort.
const rendered = useMemo(
() => (liveObject ? sorted.map((o) => (o.id === liveObject.id ? liveObject : o)) : sorted),
[sorted, liveObject],
)
const selected = rendered.find((o) => o.id === selectedId) ?? null
Review

🟠 Full object array re-mapped and re-sorted on every drag pointermove instead of only repositioning the dragged object

maintainability, performance · flagged by 5 models

  • web/src/editor/GardenCanvas.tsx:82-85rendered re-maps and re-sorts the full objects array on every liveObject change (every pointermove during a drag, per SelectionOverlay's setLiveObject calls). Confirmed the useMemo dependency array is [objects, liveObject] (line 85), so it recomputes each frame even though only one object's position/size changes and z-order among the rest is unaffected. Additionally confirmed GardenEditorPage.tsx:51 (`full.data.objects.map(toEditorO…

🪰 Gadfly · advisory

🟠 **Full object array re-mapped and re-sorted on every drag pointermove instead of only repositioning the dragged object** _maintainability, performance · flagged by 5 models_ - **`web/src/editor/GardenCanvas.tsx:82-85`** — `rendered` re-maps and re-sorts the full `objects` array on every `liveObject` change (every pointermove during a drag, per `SelectionOverlay`'s `setLiveObject` calls). Confirmed the `useMemo` dependency array is `[objects, liveObject]` (line 85), so it recomputes each frame even though only one object's position/size changes and z-order among the rest is unaffected. Additionally confirmed `GardenEditorPage.tsx:51` (`full.data.objects.map(toEditorO… <sub>🪰 Gadfly · advisory</sub>
// A pointerdown reaching the svg is empty space: place the armed kind there, or
// deselect. (Objects/handles stopPropagation.)
function onCanvasPointerDown(e: ReactPointerEvent) {
const armed = useEditorStore.getState().armedKind
if (!armed) {
select(null)
return
}
const def = kindDef(armed)
useEditorStore.getState().setArmedKind(null)
if (!def) return
const rect = svgRef.current?.getBoundingClientRect()
if (!rect) return
const world = screenToWorld({ x: e.clientX - rect.left, y: e.clientY - rect.top }, viewport)
create.mutate(
{
kind: def.kind,
shape: def.shape,
xCm: world.x,
yCm: world.y,
widthCm: def.widthCm,
heightCm: def.heightCm,
zIndex: def.defaultZ,
},
{ onSuccess: (o) => select(o.id) },
)
}
return ( return (
<div ref={containerRef} className="relative h-full w-full overflow-hidden rounded-xl border border-border bg-bg"> <div ref={containerRef} className="relative h-full w-full overflow-hidden rounded-xl border border-border bg-bg">
@@ -66,59 +114,40 @@ export function GardenCanvas({ garden, objects }: { garden: EditorGarden; object
ref={svgRef} ref={svgRef}
className="h-full w-full select-none" className="h-full w-full select-none"
style={{ touchAction: 'none' }} style={{ touchAction: 'none' }}
// Any pointerdown reaching the svg is empty space (objects stopPropagation), onPointerDown={onCanvasPointerDown}
// so it deselects.
onPointerDown={() => select(null)}
> >
<g transform={`translate(${viewport.tx} ${viewport.ty}) scale(${viewport.scale})`}> <g transform={`translate(${viewport.tx} ${viewport.ty}) scale(${viewport.scale})`}>
{gridOpacity > 0.005 && ( {gridOpacity > 0.005 && (
<> <>
<defs> <defs>
<pattern id={gridId} width={GRID_CM} height={GRID_CM} patternUnits="userSpaceOnUse"> <pattern id={gridId} width={GRID_CM} height={GRID_CM} patternUnits="userSpaceOnUse">
<path <path d={`M ${GRID_CM} 0 L 0 0 0 ${GRID_CM}`} fill="none" stroke="#808080" strokeOpacity={gridOpacity} strokeWidth={1} vectorEffect="non-scaling-stroke" />
d={`M ${GRID_CM} 0 L 0 0 0 ${GRID_CM}`}
fill="none"
stroke="#808080"
strokeOpacity={gridOpacity}
strokeWidth={1}
vectorEffect="non-scaling-stroke"
/>
</pattern> </pattern>
</defs> </defs>
<rect x={0} y={0} width={garden.widthCm} height={garden.heightCm} fill={`url(#${gridId})`} /> <rect x={0} y={0} width={garden.widthCm} height={garden.heightCm} fill={`url(#${gridId})`} />
</> </>
)} )}
Outdated
Review

🟡 Grid path collapsed from multi-line to single long line, hurting readability

maintainability · flagged by 1 model

  • web/src/editor/SelectionOverlay.tsx:138-150 — The circle-vs-rect conditional is repeated twice (transparent drag body + selection outline) with only a few props differing. Extract a small helper (<ShapeBody shape={…} halfW={…} halfH={…} {...props} />) so the shape fork lives in one place. - web/src/lib/objects.ts:188-202applyServerPatch manually spreads each of the 10 patchable fields with !== undefined guards. Adding a new patchable field requires touching this function,…

🪰 Gadfly · advisory

🟡 **Grid path collapsed from multi-line to single long line, hurting readability** _maintainability · flagged by 1 model_ - **`web/src/editor/SelectionOverlay.tsx:138-150`** — The circle-vs-rect conditional is repeated twice (transparent drag body + selection outline) with only a few props differing. Extract a small helper (`<ShapeBody shape={…} halfW={…} halfH={…} {...props} />`) so the shape fork lives in one place. - **`web/src/lib/objects.ts:188-202`** — `applyServerPatch` manually spreads each of the 10 patchable fields with `!== undefined` guards. Adding a new patchable field requires touching this function,… <sub>🪰 Gadfly · advisory</sub>
{/* Garden boundary. */} <rect x={0} y={0} width={garden.widthCm} height={garden.heightCm} fill="none" stroke="#3f8f4f" strokeOpacity={0.7} strokeWidth={1.5} vectorEffect="non-scaling-stroke" />
<rect
x={0}
y={0}
width={garden.widthCm}
height={garden.heightCm}
fill="none"
stroke="#3f8f4f"
strokeOpacity={0.7}
strokeWidth={1.5}
vectorEffect="non-scaling-stroke"
/>
{ordered.map((o) => ( {rendered.map((o) => (
<ObjectShape key={o.id} object={o} selected={o.id === selectedId} onSelect={select} /> <ObjectShape key={o.id} object={o} selected={o.id === selectedId} onSelect={select} />
))} ))}
{selected && <SelectionOverlay object={selected} gardenId={garden.id} svgRef={svgRef} />}
</g> </g>
</svg> </svg>
{/* Overlay: zoom readout + fit control. */}
<div className="pointer-events-none absolute left-3 top-3 rounded-md bg-surface/80 px-2 py-1 text-xs text-muted backdrop-blur"> <div className="pointer-events-none absolute left-3 top-3 rounded-md bg-surface/80 px-2 py-1 text-xs text-muted backdrop-blur">
{viewport.scale.toFixed(2)} px/cm {viewport.scale.toFixed(2)} px/cm
</div> </div>
<Button <button
variant="ghost" type="button"
onClick={() => size.w > 0 && fitToRect(gardenRect, size)} onClick={() => size.w > 0 && fitToRect(gardenRect, size)}
className="absolute bottom-3 right-3 bg-surface/90 py-1.5 shadow-sm backdrop-blur" className="absolute bottom-3 right-3 rounded-md border border-border bg-surface/90 px-3 py-1.5 text-sm font-medium text-fg shadow-sm outline-none backdrop-blur transition-colors hover:bg-border/50 focus-visible:ring-2 focus-visible:ring-accent/40"
> >
Fit Fit
Outdated
Review

🟡 Hand-rolled 'Fit' button duplicates the shared Button/buttonClasses ghost styling instead of reusing it

maintainability · flagged by 1 model

  • web/src/editor/GardenCanvas.tsx:149-155 — The "Fit" button was changed from <Button variant="ghost"> to a raw <button> that hand-duplicates the Button/buttonClasses ghost styling (border border-border, text-fg, hover:bg-border/50, outline-none focus-visible:ring-2 focus-visible:ring-accent/40, transition-colors — confirmed identical to web/src/components/ui/Button.tsx:9-16). The Button import was dropped from this file entirely. The prior code already used className t…

🪰 Gadfly · advisory

🟡 **Hand-rolled 'Fit' button duplicates the shared Button/buttonClasses ghost styling instead of reusing it** _maintainability · flagged by 1 model_ - `web/src/editor/GardenCanvas.tsx:149-155` — The "Fit" button was changed from `<Button variant="ghost">` to a raw `<button>` that hand-duplicates the `Button`/`buttonClasses` ghost styling (`border border-border`, `text-fg`, `hover:bg-border/50`, `outline-none focus-visible:ring-2 focus-visible:ring-accent/40`, `transition-colors` — confirmed identical to `web/src/components/ui/Button.tsx:9-16`). The `Button` import was dropped from this file entirely. The prior code already used `className` t… <sub>🪰 Gadfly · advisory</sub>
</Button> </button>
</div> </div>
) )
} }
+235
View File
@@ -0,0 +1,235 @@
import { useEffect, useRef, useState, type ChangeEvent } from 'react'
import { Button } from '@/components/ui/Button'
import { TextField } from '@/components/ui/TextField'
import { TextArea } from '@/components/ui/TextArea'
import { useDeleteObject, useUpdateObject } from '@/lib/objects'
import {
cmFromDisplay,
dimensionUnitLabel,
displayFromCm,
MIN_DIMENSION_CM,
type UnitPref,
} from '@/lib/units'
import { kindDef } from './kinds'
import { useEditorStore } from './store'
import type { EditorObject } from './types'
const DEFAULT_COLOR = '#8a8a8a'
/**
* Property panel for the selected object. Each field commits a PATCH on
* blur/change (carrying the object's version); dimensions and position are shown
* in the garden's unit. Keyed by object id in the parent so it re-inits cleanly
* on selection change.
*/
export function Inspector({
object,
gardenId,
unit,
}: {
object: EditorObject
gardenId: number
Review

🟡 web/src/editor/Inspector.tsx:31-37 and :44-53 — the field-init and re-sync logic duplicate the same String(displayFromCm(...)) / String(Math.round(...)) conversions.

maintainability · flagged by 1 model

  • web/src/editor/Inspector.tsx:31-37 and :44-53 — the field-init and re-sync logic duplicate the same String(displayFromCm(...)) / String(Math.round(...)) conversions. Seven useState initializers and the useEffect both spell out the same per-field formatting. A small fieldsFromObject(object, unit) helper returning { name, notes, width, height, x, y, rotation } would remove the duplication and make a future field a one-line add in both places. Trivial.

🪰 Gadfly · advisory

🟡 **`web/src/editor/Inspector.tsx:31-37` and `:44-53` — the field-init and re-sync logic duplicate the same `String(displayFromCm(...))` / `String(Math.round(...))` conversions.** _maintainability · flagged by 1 model_ - **`web/src/editor/Inspector.tsx:31-37` and `:44-53` — the field-init and re-sync logic duplicate the same `String(displayFromCm(...))` / `String(Math.round(...))` conversions.** Seven `useState` initializers and the `useEffect` both spell out the same per-field formatting. A small `fieldsFromObject(object, unit)` helper returning `{ name, notes, width, height, x, y, rotation }` would remove the duplication and make a future field a one-line add in both places. Trivial. <sub>🪰 Gadfly · advisory</sub>
unit: UnitPref
}) {
const update = useUpdateObject(gardenId)
const del = useDeleteObject(gardenId)
const select = useEditorStore((s) => s.select)
// Local field state (initialized once; committed on blur/change).
const [name, setName] = useState(object.name)
const [notes, setNotes] = useState(object.notes)
const [width, setWidth] = useState(String(displayFromCm(object.widthCm, unit)))
const [height, setHeight] = useState(String(displayFromCm(object.heightCm, unit)))
const [x, setX] = useState(String(displayFromCm(object.xCm, unit)))
const [y, setY] = useState(String(displayFromCm(object.yCm, unit)))
Review

🟠 Re-sync skips all inspector fields when any field has focus

error-handling · flagged by 1 model

  • web/src/editor/Inspector.tsx:44-53 — The re-sync useEffect aborts entirely when any field inside the inspector has focus (rootRef.current?.contains(document.activeElement)). If another client updates the object while the user is typing in the Notes field, the Width/Height/X/Y/Rotation fields do not refresh and remain stale even though the user is not actively editing them. Fix: track focus per-field (e.g., by field name) and only block re-sync for the focused field.

🪰 Gadfly · advisory

🟠 **Re-sync skips all inspector fields when any field has focus** _error-handling · flagged by 1 model_ - **`web/src/editor/Inspector.tsx:44-53`** — The re-sync `useEffect` aborts entirely when *any* field inside the inspector has focus (`rootRef.current?.contains(document.activeElement)`). If another client updates the object while the user is typing in the Notes field, the Width/Height/X/Y/Rotation fields do not refresh and remain stale even though the user is not actively editing them. Fix: track focus per-field (e.g., by field name) and only block re-sync for the focused field. <sub>🪰 Gadfly · advisory</sub>
const [rotation, setRotation] = useState(String(Math.round(object.rotationDeg)))
const [color, setColor] = useState(object.color ?? DEFAULT_COLOR)
const [confirmingDelete, setConfirmingDelete] = useState(false)
const rootRef = useRef<HTMLDivElement>(null)
// When the object changes underneath us (e.g. a canvas move/resize/rotate, or
// an optimistic PATCH result), re-sync the fields — unless the user is
// actively editing one here, so we don't clobber their typing.
useEffect(() => {
if (rootRef.current?.contains(document.activeElement)) return
setName(object.name)
Review

🔴 Inspector commitDim accepts zero and negative dimensions

error-handling, maintainability · flagged by 4 models

  • web/src/editor/Inspector.tsx:58-63commitDim parses the raw input with parseFloat and sends the result without validating that it is positive or at least MIN_OBJ_CM. This allows zero and negative dimensions to be PATCHed to the server, which the canvas resize logic explicitly forbids (MIN_OBJ_CM = 1). The inspector and canvas are inconsistent, and negative sizes can corrupt the SVG bounding boxes. Fix: reject v <= 0 (or cm <= 0) in commitDim and surface a brief inline war…

🪰 Gadfly · advisory

🔴 **Inspector commitDim accepts zero and negative dimensions** _error-handling, maintainability · flagged by 4 models_ - **`web/src/editor/Inspector.tsx:58-63`** — `commitDim` parses the raw input with `parseFloat` and sends the result without validating that it is positive or at least `MIN_OBJ_CM`. This allows zero and negative dimensions to be PATCHed to the server, which the canvas resize logic explicitly forbids (`MIN_OBJ_CM = 1`). The inspector and canvas are inconsistent, and negative sizes can corrupt the SVG bounding boxes. Fix: reject `v <= 0` (or `cm <= 0`) in `commitDim` and surface a brief inline war… <sub>🪰 Gadfly · advisory</sub>
setNotes(object.notes)
setWidth(String(displayFromCm(object.widthCm, unit)))
setHeight(String(displayFromCm(object.heightCm, unit)))
setX(String(displayFromCm(object.xCm, unit)))
setY(String(displayFromCm(object.yCm, unit)))
setRotation(String(Math.round(object.rotationDeg)))
setColor(object.color ?? DEFAULT_COLOR)
Outdated
Review

🟠 Blur of an unchanged dimension/position/rotation field fires a spurious PATCH due to lossy unit round-trip (e.g. 100cm -> 3.3ft -> 101cm), mutating the object and bumping version on a no-op focus/blur

correctness · flagged by 2 models

  • web/src/editor/Inspector.tsx:62 — spurious PATCH on blur of an unchanged dimension/position field, with unit-dependent drift. commitDim runs on every blur and compares cmFromDisplay(v, unit) to current with no dirty guard. Because displayFromCm rounds (imperial → 0.1 ft, metric → 2 dp cm) and cmFromDisplay re-quantizes to whole cm, the round trip is lossy. Verified: 100 cm → displayFromCm gives 3.3 ft → cmFromDisplay(3.3,'imperial') = round(3.3*12*2.54) = 101 cm, so…

🪰 Gadfly · advisory

🟠 **Blur of an unchanged dimension/position/rotation field fires a spurious PATCH due to lossy unit round-trip (e.g. 100cm -> 3.3ft -> 101cm), mutating the object and bumping version on a no-op focus/blur** _correctness · flagged by 2 models_ - **`web/src/editor/Inspector.tsx:62` — spurious PATCH on blur of an unchanged dimension/position field, with unit-dependent drift.** `commitDim` runs on every blur and compares `cmFromDisplay(v, unit)` to `current` with no dirty guard. Because `displayFromCm` rounds (imperial → 0.1 ft, metric → 2 dp cm) and `cmFromDisplay` re-quantizes to whole cm, the round trip is lossy. Verified: 100 cm → `displayFromCm` gives `3.3` ft → `cmFromDisplay(3.3,'imperial')` = `round(3.3*12*2.54)` = **101 cm**, so… <sub>🪰 Gadfly · advisory</sub>
}, [object.version, object.widthCm, object.heightCm, object.xCm, object.yCm, object.rotationDeg, object.name, object.notes, object.color, unit])
const patch = (fields: Partial<Omit<EditorObject, 'id' | 'version'>>) =>
update.mutate({ id: object.id, version: object.version, ...fields })
// Commit a dimension/position field. `positive` gates width/height (must be
// ≥ the server minimum) but not x/y, which may be zero or negative. Compare at
// display precision first so a blur without an edit doesn't fire a spurious
// PATCH from cm↔ft rounding drift (e.g. 240cm shows 7.9ft → 241cm).
const commitDim = (raw: string, current: number, apply: (cm: number) => void, positive = false) => {
const v = parseFloat(raw)
if (!Number.isFinite(v)) return
if (v === displayFromCm(current, unit)) return
const cm = cmFromDisplay(v, unit)
if (positive && cm < MIN_DIMENSION_CM) return
if (cm !== current) apply(cm)
}
const u = dimensionUnitLabel(unit)
return (
<div ref={rootRef} className="flex flex-col gap-3">
<div className="flex items-center justify-between">
<h2 className="text-sm font-semibold text-fg">{kindDef(object.kind)?.label ?? object.kind}</h2>
<button
type="button"
onClick={() => select(null)}
Review

🟠 Four dimension TextFields are copy-pasted with only label/value differing

maintainability · flagged by 2 models

  • web/src/editor/SelectionOverlay.tsx:138-150 — The circle-vs-rect conditional is repeated twice (transparent drag body + selection outline) with only a few props differing. Extract a small helper (<ShapeBody shape={…} halfW={…} halfH={…} {...props} />) so the shape fork lives in one place. - web/src/lib/objects.ts:188-202applyServerPatch manually spreads each of the 10 patchable fields with !== undefined guards. Adding a new patchable field requires touching this function,…

🪰 Gadfly · advisory

🟠 **Four dimension TextFields are copy-pasted with only label/value differing** _maintainability · flagged by 2 models_ - **`web/src/editor/SelectionOverlay.tsx:138-150`** — The circle-vs-rect conditional is repeated twice (transparent drag body + selection outline) with only a few props differing. Extract a small helper (`<ShapeBody shape={…} halfW={…} halfH={…} {...props} />`) so the shape fork lives in one place. - **`web/src/lib/objects.ts:188-202`** — `applyServerPatch` manually spreads each of the 10 patchable fields with `!== undefined` guards. Adding a new patchable field requires touching this function,… <sub>🪰 Gadfly · advisory</sub>
className="rounded px-1.5 text-sm text-muted hover:text-fg"
aria-label="Close inspector"
>
</button>
</div>
<TextField
label="Name"
name="name"
value={name}
onChange={(e) => setName(e.target.value)}
onBlur={() => name !== object.name && patch({ name })}
/>
<div className="grid grid-cols-2 gap-2">
<TextField
label={`Width (${u})`}
name="width"
type="number"
inputMode="decimal"
step="any"
value={width}
onChange={(e) => setWidth(e.target.value)}
onBlur={() => commitDim(width, object.widthCm, (cm) => patch({ widthCm: cm }), true)}
/>
<TextField
label={`Height (${u})`}
name="height"
type="number"
inputMode="decimal"
step="any"
value={height}
onChange={(e) => setHeight(e.target.value)}
onBlur={() => commitDim(height, object.heightCm, (cm) => patch({ heightCm: cm }), true)}
/>
<TextField
label={`X (${u})`}
name="x"
type="number"
inputMode="decimal"
step="any"
value={x}
onChange={(e) => setX(e.target.value)}
onBlur={() => commitDim(x, object.xCm, (cm) => patch({ xCm: cm }))}
/>
<TextField
label={`Y (${u})`}
name="y"
type="number"
inputMode="decimal"
step="any"
value={y}
onChange={(e) => setY(e.target.value)}
onBlur={() => commitDim(y, object.yCm, (cm) => patch({ yCm: cm }))}
/>
</div>
<TextField
label="Rotation (°)"
name="rotation"
type="number"
Review

🔴 Color onChange fires a PATCH per picker drag step → version storms and spurious 409 rollbacks with a misleading 'updated elsewhere' toast

correctness, error-handling, performance · flagged by 3 models

  • web/src/editor/Inspector.tsx:155 — color onChange fires a PATCH on every picker drag step, causing version storms and spurious 409 rollbacks. The native <input type="color"> emits input events continuously while the user drags through the OS color picker; React's onChange maps to that input event. Each tick calls patch({ color }), sending the current object.version. The first PATCH succeeds (N→N+1), but a second PATCH emitted before the cache re-renders with the new versi…

🪰 Gadfly · advisory

🔴 **Color onChange fires a PATCH per picker drag step → version storms and spurious 409 rollbacks with a misleading 'updated elsewhere' toast** _correctness, error-handling, performance · flagged by 3 models_ - **`web/src/editor/Inspector.tsx:155` — color `onChange` fires a PATCH on every picker drag step, causing version storms and spurious 409 rollbacks.** The native `<input type="color">` emits `input` events continuously while the user drags through the OS color picker; React's `onChange` maps to that `input` event. Each tick calls `patch({ color })`, sending the *current* `object.version`. The first PATCH succeeds (N→N+1), but a second PATCH emitted before the cache re-renders with the new versi… <sub>🪰 Gadfly · advisory</sub>
inputMode="numeric"
step="1"
value={rotation}
onChange={(e) => setRotation(e.target.value)}
onBlur={() => {
const v = parseFloat(rotation)
if (Number.isFinite(v) && v !== object.rotationDeg) patch({ rotationDeg: v })
}}
/>
<div className="flex items-end gap-2">
<div className="flex flex-col gap-1.5">
<label htmlFor="obj-color" className="text-sm font-medium text-fg">
Color
</label>
<input
id="obj-color"
type="color"
value={color}
Outdated
Review

🟠 Checkbox commits PATCH immediately onChange, rapid toggles self-conflict with version

error-handling · flagged by 1 model

🪰 Gadfly · advisory

🟠 **Checkbox commits PATCH immediately onChange, rapid toggles self-conflict with version** _error-handling · flagged by 1 model_ <sub>🪰 Gadfly · advisory</sub>
// onChange fires continuously while dragging in the native picker;
// track it locally for the live swatch and commit one PATCH on blur.
onChange={(e: ChangeEvent<HTMLInputElement>) => setColor(e.target.value)}
onBlur={() => color !== (object.color ?? DEFAULT_COLOR) && patch({ color })}
className="h-9 w-14 cursor-pointer rounded-md border border-border bg-surface"
/>
</div>
{object.color && (
<Button
variant="ghost"
className="px-2 py-1.5 text-xs"
onClick={() => {
setColor(DEFAULT_COLOR)
patch({ color: null })
}}
>
Clear
</Button>
)}
</div>
Review

🟡 On delete failure the object is restored to the cache but selection was already cleared, leaving it unselected on retry

error-handling · flagged by 1 model

  • **web/src/editor/Inspector.tsx:191-194 — on delete error the cache rolls back but selection stays cleared.** select(null)runs beforedel.mutate(object.id). If the DELETE fails (network/403/500), onError` (objects.ts:173-176) restores the object to the cache, but the inspector is already gone (selection cleared at Inspector.tsx:192). The object reappears on the canvas unselected, and the user has to re-select to retry. Minor, but an unhappy-path inconsistency. Suggested fix: clear the se…

🪰 Gadfly · advisory

🟡 **On delete failure the object is restored to the cache but selection was already cleared, leaving it unselected on retry** _error-handling · flagged by 1 model_ - **`web/src/editor/Inspector.tsx:191-194 — on delete error the cache rolls back but selection stays cleared.** `select(null)` runs before `del.mutate(object.id)`. If the DELETE fails (network/403/500), `onError` (objects.ts:173-176) restores the object to the cache, but the inspector is already gone (selection cleared at Inspector.tsx:192). The object reappears on the canvas unselected, and the user has to re-select to retry. Minor, but an unhappy-path inconsistency. Suggested fix: clear the se… <sub>🪰 Gadfly · advisory</sub>
<label className="flex items-center gap-2 text-sm text-fg">
<input
type="checkbox"
checked={object.plantable}
onChange={(e) => patch({ plantable: e.target.checked })}
className="h-4 w-4 rounded border-border"
/>
Plantable
</label>
<TextArea
label="Notes"
name="notes"
rows={2}
value={notes}
onChange={(e) => setNotes(e.target.value)}
onBlur={() => notes !== object.notes && patch({ notes })}
/>
{confirmingDelete ? (
<div className="flex items-center gap-2">
<Button
variant="danger"
className="flex-1"
disabled={del.isPending}
onClick={() => {
select(null)
del.mutate(object.id)
}}
>
Confirm delete
</Button>
<Button variant="ghost" onClick={() => setConfirmingDelete(false)}>
Cancel
</Button>
</div>
) : (
<Button variant="ghost" className="text-red-600 dark:text-red-400" onClick={() => setConfirmingDelete(true)}>
Delete object
</Button>
)}
</div>
)
}
+49
View File
@@ -0,0 +1,49 @@
import { cn } from '@/lib/cn'
import { OBJECT_KINDS, kindDef } from './kinds'
import { useEditorStore } from './store'
/**
* The object-kind palette. Tap a kind to arm it, then tap the canvas to place
* (works on desktop and touch). Tapping the armed kind again disarms it.
*/
export function Palette() {
const armedKind = useEditorStore((s) => s.armedKind)
const setArmedKind = useEditorStore((s) => s.setArmedKind)
const select = useEditorStore((s) => s.select)
return (
<div className="flex flex-col gap-2">
<div className="flex flex-wrap gap-1.5 md:flex-col">
{OBJECT_KINDS.map((k) => {
const active = armedKind === k.kind
return (
<button
key={k.kind}
type="button"
onClick={() => {
select(null)
setArmedKind(active ? null : k.kind)
}}
className={cn(
'flex items-center gap-2 rounded-md border px-2.5 py-1.5 text-sm font-medium outline-none transition-colors',
'focus-visible:ring-2 focus-visible:ring-accent/40',
active
? 'border-accent bg-accent/10 text-accent-strong'
: 'border-border bg-surface text-fg hover:bg-border/50',
)}
aria-pressed={active}
>
<span aria-hidden className="text-base leading-none">
{k.icon}
</span>
<span>{k.label}</span>
</button>
)
})}
</div>
{armedKind && (
<p className="text-xs text-muted">Tap the canvas to place a {kindDef(armedKind)?.label ?? 'object'}.</p>
)}
</div>
)
}
+202
View File
@@ -0,0 +1,202 @@
import { useEffect, useRef, type PointerEvent as ReactPointerEvent, type RefObject } from 'react'
import { localToWorld, screenToWorld, worldToLocal, type Point } from '@/lib/geometry'
import { useUpdateObject } from '@/lib/objects'
import { useEditorStore } from './store'
import type { EditorObject } from './types'
const HANDLE_PX = 12 // on-screen size of the resize handles
const ROTATE_OFFSET_PX = 28 // distance of the rotate knob above the object
const MIN_OBJ_CM = 1 // smallest allowed dimension
const ROTATE_SNAP_DEG = 15
const SELECT_COLOR = '#2f7a3e'
const corners: [number, number][] = [
[-1, -1],
[1, -1],
[1, 1],
[-1, 1],
]
Review

🔴 Window pointer listeners leak when SelectionOverlay unmounts mid-drag

error-handling · flagged by 4 models

  • web/src/editor/SelectionOverlay.tsx:21-31trackPointer attaches window listeners for pointermove/up/cancel but never cleans them up if the component unmounts mid-gesture. If the user navigates to a different garden, the object is deleted by another client, or a background update removes the selection, the overlay unmounts while the drag listeners are still on window. On pointerup the onEnd closure fires, calling setObjectDragging(false) and update.mutate(fields(final)) ag…

🪰 Gadfly · advisory

🔴 **Window pointer listeners leak when SelectionOverlay unmounts mid-drag** _error-handling · flagged by 4 models_ - **`web/src/editor/SelectionOverlay.tsx:21-31`** — `trackPointer` attaches `window` listeners for pointermove/up/cancel but never cleans them up if the component unmounts mid-gesture. If the user navigates to a different garden, the object is deleted by another client, or a background update removes the selection, the overlay unmounts while the drag listeners are still on `window`. On `pointerup` the `onEnd` closure fires, calling `setObjectDragging(false)` and `update.mutate(fields(final))` ag… <sub>🪰 Gadfly · advisory</sub>
/**
* Handles for the selected object: a transparent body to move, four corner
* handles to resize (opposite corner stays put, honoring rotation via the
* object-local frame), and a knob to rotate (snaps to 15°, free with Shift).
* Each gesture updates liveObject for instant feedback and fires exactly one
* PATCH on release.
*/
export function SelectionOverlay({
object,
gardenId,
svgRef,
}: {
object: EditorObject
gardenId: number
svgRef: RefObject<SVGSVGElement | null>
}) {
const setLiveObject = useEditorStore((s) => s.setLiveObject)
const setObjectDragging = useEditorStore((s) => s.setObjectDragging)
const scale = useEditorStore((s) => s.viewport.scale)
const update = useUpdateObject(gardenId)
// Detach for an in-flight gesture. If the overlay unmounts mid-drag (the
// object is deleted or deselected), this runs on unmount so window listeners
// don't leak and objectDragging can't stay stuck true (which would freeze pan).
const cleanupRef = useRef<(() => void) | null>(null)
useEffect(
() => () => {
cleanupRef.current?.()
cleanupRef.current = null
},
[],
)
const halfW = object.widthCm / 2
const halfH = object.heightCm / 2
const handleCm = HANDLE_PX / scale
const rotateOffsetCm = ROTATE_OFFSET_PX / scale
Review

🟠 getBoundingClientRect() called on every pointermove during drags — forces layout reflow each move event (layout thrash)

maintainability, performance · flagged by 4 models

  • web/src/editor/SelectionOverlay.tsx:60-65getBoundingClientRect() called on every pointermove during a drag. pointerWorld reads svgRef.current?.getBoundingClientRect() and is invoked from the trackPointer move callback (line 78) on every pointer event. getBoundingClientRect forces a synchronous layout/reflow, and during a move/resize/rotate this fires dozens of times per second — exactly when the live setLiveObject re-render is also mutating the SVG, so the layout is dirt…

🪰 Gadfly · advisory

🟠 **getBoundingClientRect() called on every pointermove during drags — forces layout reflow each move event (layout thrash)** _maintainability, performance · flagged by 4 models_ - **`web/src/editor/SelectionOverlay.tsx:60-65` — `getBoundingClientRect()` called on every `pointermove` during a drag.** `pointerWorld` reads `svgRef.current?.getBoundingClientRect()` and is invoked from the `trackPointer` move callback (line 78) on every pointer event. `getBoundingClientRect` forces a synchronous layout/reflow, and during a move/resize/rotate this fires dozens of times per second — exactly when the live `setLiveObject` re-render is also mutating the SVG, so the layout is dirt… <sub>🪰 Gadfly · advisory</sub>
// The svg's screen rect doesn't move during a drag, so snapshot it once at
// gesture start rather than calling getBoundingClientRect (a layout reflow) on
// every pointermove.
const makePointerWorld = () => {
const rect = svgRef.current?.getBoundingClientRect()
return (e: { clientX: number; clientY: number }): Point => {
const vp = useEditorStore.getState().viewport
const local = rect ? { x: e.clientX - rect.left, y: e.clientY - rect.top } : { x: e.clientX, y: e.clientY }
return screenToWorld(local, vp)
}
}
// Common gesture scaffolding: mark dragging, track the pointer on window, and
// on release fire one PATCH built from the final liveObject. onMove receives
// the live pointer event so modifier keys (Shift) reflect their current state.
const begin = (
e: ReactPointerEvent,
onMove: (e: PointerEvent) => EditorObject,
fields: (final: EditorObject) => Parameters<typeof update.mutate>[0],
) => {
e.stopPropagation()
e.preventDefault()
setObjectDragging(true)
const move = (ev: PointerEvent) => setLiveObject(onMove(ev))
const detach = () => {
window.removeEventListener('pointermove', move)
window.removeEventListener('pointerup', finish)
window.removeEventListener('pointercancel', finish)
}
const finish = () => {
detach()
Outdated
Review

🔴 Stale version captured in gesture closure causes spurious 409s

correctness · flagged by 1 model

  • web/src/editor/SelectionOverlay.tsx:88base = { ...object } snapshots the object's version at render time. The gesture-end PATCH sends base.version, so if the server's version increments during the drag (another field edit, another client, or a background refetch), the mutation 409s and rolls back the user's gesture even though there was no edit conflict during the drag. The fix is to read the current cached row's version when building the patch on release, or to derive it from t…

🪰 Gadfly · advisory

🔴 **Stale version captured in gesture closure causes spurious 409s** _correctness · flagged by 1 model_ - `web/src/editor/SelectionOverlay.tsx:88` — `base = { ...object }` snapshots the object's `version` at render time. The gesture-end PATCH sends `base.version`, so if the server's version increments during the drag (another field edit, another client, or a background refetch), the mutation 409s and rolls back the user's gesture even though there was no edit conflict *during* the drag. The fix is to read the current cached row's `version` when building the patch on release, or to derive it from t… <sub>🪰 Gadfly · advisory</sub>
cleanupRef.current = null
const final = useEditorStore.getState().liveObject
setObjectDragging(false)
setLiveObject(null)
if (final) update.mutate(fields(final))
}
// On unmount mid-gesture: detach and reset, but don't fire a PATCH.
cleanupRef.current = () => {
detach()
setObjectDragging(false)
setLiveObject(null)
}
window.addEventListener('pointermove', move)
window.addEventListener('pointerup', finish)
window.addEventListener('pointercancel', finish)
}
const base = { ...object }
const center0: Point = { x: base.xCm, y: base.yCm }
Review

🟡 Resize clamps size to MIN_OBJ_CM but keeps recomputing/moving the center when the dragged corner crosses the fixed opposite corner.

error-handling · flagged by 1 model

  • web/src/editor/SelectionOverlay.tsx:108 — resize recomputes/moves the center even when the dragged corner crosses the fixed opposite corner. newW/newH are clamped to MIN_OBJ_CM, but newCenterLocal is still derived from the live pointer position p, so once the pointer passes the opposite corner the object stays at minimum size while its center keeps sliding with the pointer. Visible UX edge, non-crashing, minor.

🪰 Gadfly · advisory

🟡 **Resize clamps size to MIN_OBJ_CM but keeps recomputing/moving the center when the dragged corner crosses the fixed opposite corner.** _error-handling · flagged by 1 model_ - **`web/src/editor/SelectionOverlay.tsx:108` — resize recomputes/moves the center even when the dragged corner crosses the fixed opposite corner.** `newW`/`newH` are clamped to `MIN_OBJ_CM`, but `newCenterLocal` is still derived from the live pointer position `p`, so once the pointer passes the opposite corner the object stays at minimum size while its center keeps sliding with the pointer. Visible UX edge, non-crashing, minor. <sub>🪰 Gadfly · advisory</sub>
const startMove = (e: ReactPointerEvent) => {
const pointerWorld = makePointerWorld()
const start = pointerWorld(e.nativeEvent)
begin(
e,
(ev) => {
const world = pointerWorld(ev)
return { ...base, xCm: base.xCm + (world.x - start.x), yCm: base.yCm + (world.y - start.y) }
},
(f) => ({ id: base.id, version: base.version, xCm: f.xCm, yCm: f.yCm }),
)
}
const startResize = (e: ReactPointerEvent, sx: number, sy: number) => {
Review

🟡 Rotation shiftKey state locked at gesture start

error-handling · flagged by 1 model

  • web/src/editor/SelectionOverlay.tsx:122-134startRotate captures e.shiftKey from the initial ReactPointerEvent closure. The snap/free behavior is therefore locked to the Shift state at pointer-down; the user cannot press or release Shift mid-gesture to toggle snapping, which contradicts typical drawing-tool behavior. Fix: read shiftKey from the live PointerEvent passed to the move handler instead of the stale closed-over React event.

🪰 Gadfly · advisory

🟡 **Rotation shiftKey state locked at gesture start** _error-handling · flagged by 1 model_ - **`web/src/editor/SelectionOverlay.tsx:122-134`** — `startRotate` captures `e.shiftKey` from the initial `ReactPointerEvent` closure. The snap/free behavior is therefore locked to the Shift state at pointer-down; the user cannot press or release Shift mid-gesture to toggle snapping, which contradicts typical drawing-tool behavior. Fix: read `shiftKey` from the live `PointerEvent` passed to the move handler instead of the stale closed-over React event. <sub>🪰 Gadfly · advisory</sub>
// The corner opposite the dragged one stays fixed (in the object's local
// frame, which is anchored at the original center).
const oppositeLocal: Point = { x: -sx * halfW, y: -sy * halfH }
const pointerWorld = makePointerWorld()
begin(
e,
Review

🟠 Shift key state locked at gesture start instead of evaluated live during rotate

correctness · flagged by 3 models

  • web/src/editor/SelectionOverlay.tsx:128 — Shift state captured at gesture start, not tracked during the drag. The onMove closure in startRotate references e.shiftKey from the original pointerdown event (if (!e.shiftKey) deg = Math.round(...)). Pressing or releasing Shift mid-rotate has no effect; "free with Shift" only works if Shift was held at pointerdown. Fix: read Shift state from the live pointer event (ev.shiftKey) inside onMove instead of from e.

🪰 Gadfly · advisory

🟠 **Shift key state locked at gesture start instead of evaluated live during rotate** _correctness · flagged by 3 models_ - **`web/src/editor/SelectionOverlay.tsx:128` — Shift state captured at gesture start, not tracked during the drag.** The `onMove` closure in `startRotate` references `e.shiftKey` from the original `pointerdown` event (`if (!e.shiftKey) deg = Math.round(...)`). Pressing or releasing Shift mid-rotate has no effect; "free with Shift" only works if Shift was held at pointerdown. Fix: read Shift state from the live pointer event (`ev.shiftKey`) inside `onMove` instead of from `e`. <sub>🪰 Gadfly · advisory</sub>
(ev) => {
const world = pointerWorld(ev)
const p = worldToLocal(world, center0, base.rotationDeg)
const newW = Math.max(MIN_OBJ_CM, Math.abs(p.x - oppositeLocal.x))
const newH = Math.max(MIN_OBJ_CM, Math.abs(p.y - oppositeLocal.y))
const draggedLocal: Point = { x: oppositeLocal.x + sx * newW, y: oppositeLocal.y + sy * newH }
const newCenterLocal: Point = {
x: (oppositeLocal.x + draggedLocal.x) / 2,
y: (oppositeLocal.y + draggedLocal.y) / 2,
}
Review

🟠 Duplicate circle-vs-rect shape rendering inside SelectionOverlay

maintainability · flagged by 1 model

  • web/src/editor/SelectionOverlay.tsx:138-150 — The circle-vs-rect conditional is repeated twice (transparent drag body + selection outline) with only a few props differing. Extract a small helper (<ShapeBody shape={…} halfW={…} halfH={…} {...props} />) so the shape fork lives in one place. - web/src/lib/objects.ts:188-202applyServerPatch manually spreads each of the 10 patchable fields with !== undefined guards. Adding a new patchable field requires touching this function,…

🪰 Gadfly · advisory

🟠 **Duplicate circle-vs-rect shape rendering inside SelectionOverlay** _maintainability · flagged by 1 model_ - **`web/src/editor/SelectionOverlay.tsx:138-150`** — The circle-vs-rect conditional is repeated twice (transparent drag body + selection outline) with only a few props differing. Extract a small helper (`<ShapeBody shape={…} halfW={…} halfH={…} {...props} />`) so the shape fork lives in one place. - **`web/src/lib/objects.ts:188-202`** — `applyServerPatch` manually spreads each of the 10 patchable fields with `!== undefined` guards. Adding a new patchable field requires touching this function,… <sub>🪰 Gadfly · advisory</sub>
const c = localToWorld(newCenterLocal, center0, base.rotationDeg)
return { ...base, xCm: c.x, yCm: c.y, widthCm: newW, heightCm: newH }
},
(f) => ({ id: base.id, version: base.version, xCm: f.xCm, yCm: f.yCm, widthCm: f.widthCm, heightCm: f.heightCm }),
)
}
const startRotate = (e: ReactPointerEvent) => {
const pointerWorld = makePointerWorld()
begin(
e,
(ev) => {
const world = pointerWorld(ev)
// +90° because the knob points up (local -y) at rotation 0.
let deg = (Math.atan2(world.y - center0.y, world.x - center0.x) * 180) / Math.PI + 90
// Read Shift live off the move event so toggling mid-drag switches
// between snapped and free rotation.
if (!ev.shiftKey) deg = Math.round(deg / ROTATE_SNAP_DEG) * ROTATE_SNAP_DEG
deg = ((deg % 360) + 360) % 360
return { ...base, rotationDeg: deg }
},
(f) => ({ id: base.id, version: base.version, rotationDeg: f.rotationDeg }),
)
}
return (
<g transform={`translate(${object.xCm} ${object.yCm}) rotate(${object.rotationDeg})`}>
{/* Transparent body: drag to move. */}
{object.shape === 'circle' ? (
<ellipse cx={0} cy={0} rx={halfW} ry={halfH} fill="transparent" pointerEvents="all" style={{ cursor: 'move' }} onPointerDown={startMove} />
) : (
<rect x={-halfW} y={-halfH} width={object.widthCm} height={object.heightCm} fill="transparent" pointerEvents="all" style={{ cursor: 'move' }} onPointerDown={startMove} />
)}
{/* Selection outline. */}
{object.shape === 'circle' ? (
<ellipse cx={0} cy={0} rx={halfW} ry={halfH} fill="none" stroke={SELECT_COLOR} strokeWidth={1.5} vectorEffect="non-scaling-stroke" pointerEvents="none" />
) : (
<rect x={-halfW} y={-halfH} width={object.widthCm} height={object.heightCm} fill="none" stroke={SELECT_COLOR} strokeWidth={1.5} vectorEffect="non-scaling-stroke" pointerEvents="none" />
)}
{/* Rotate knob. */}
<line x1={0} y1={-halfH} x2={0} y2={-halfH - rotateOffsetCm} stroke={SELECT_COLOR} strokeWidth={1} vectorEffect="non-scaling-stroke" pointerEvents="none" />
<circle cx={0} cy={-halfH - rotateOffsetCm} r={handleCm * 0.7} fill={SELECT_COLOR} style={{ cursor: 'grab' }} onPointerDown={startRotate} />
{/* Resize corners. */}
{corners.map(([sx, sy]) => (
<rect
key={`${sx},${sy}`}
x={sx * halfW - handleCm / 2}
y={sy * halfH - handleCm / 2}
width={handleCm}
height={handleCm}
fill="#ffffff"
stroke={SELECT_COLOR}
strokeWidth={1}
vectorEffect="non-scaling-stroke"
style={{ cursor: 'nwse-resize' }}
onPointerDown={(e) => startResize(e, sx, sy)}
/>
))}
</g>
)
}
+29
View File
@@ -0,0 +1,29 @@
import type { ObjectShapeKind } from './types'
// The seven placeable object kinds with their palette presentation and sensible
// default sizes (cm), per the #11 brief.
export interface KindDef {
kind: string
label: string
icon: string
shape: ObjectShapeKind
widthCm: number
heightCm: number
// Default z-index at placement. Paths/structures/in-ground sit under beds (0);
// beds and containers sit at 1; trees float above (2). Explicit z still wins.
defaultZ: number
}
export const OBJECT_KINDS: KindDef[] = [
{ kind: 'bed', label: 'Bed', icon: '▭', shape: 'rect', widthCm: 120, heightCm: 240, defaultZ: 1 },
{ kind: 'grow_bag', label: 'Grow bag', icon: '🛍️', shape: 'circle', widthCm: 40, heightCm: 40, defaultZ: 1 },
{ kind: 'container', label: 'Container', icon: '🪴', shape: 'circle', widthCm: 60, heightCm: 60, defaultZ: 1 },
{ kind: 'in_ground', label: 'In-ground', icon: '🟫', shape: 'rect', widthCm: 200, heightCm: 200, defaultZ: 0 },
{ kind: 'tree', label: 'Tree', icon: '🌳', shape: 'circle', widthCm: 300, heightCm: 300, defaultZ: 2 },
{ kind: 'path', label: 'Path', icon: '🧱', shape: 'rect', widthCm: 100, heightCm: 300, defaultZ: 0 },
{ kind: 'structure', label: 'Structure', icon: '🏠', shape: 'rect', widthCm: 200, heightCm: 200, defaultZ: 0 },
]
export function kindDef(kind: string): KindDef | undefined {
return OBJECT_KINDS.find((k) => k.kind === kind)
}
+28 -4
View File
@@ -1,9 +1,10 @@
import { create } from 'zustand' import { create } from 'zustand'
import type { Viewport } from '@/lib/geometry' import type { Viewport } from '@/lib/geometry'
import type { EditorObject } from './types'
// Ephemeral editor state only (per DESIGN § State): the viewport, the current // Ephemeral editor state only (per DESIGN § State): the viewport, the current
// selection, and which object is "focused" (zoomed into for plops, #15). All // selection, the object being live-edited mid-gesture, and the palette kind
// server state stays in react-query. Selection interactions grow in #11. // armed for tap-to-place. All server state stays in react-query.
export const MIN_SCALE = 0.05 // px per cm — fully zoomed out export const MIN_SCALE = 0.05 // px per cm — fully zoomed out
export const MAX_SCALE = 20 // px per cm — fully zoomed in export const MAX_SCALE = 20 // px per cm — fully zoomed in
@@ -17,16 +18,39 @@ interface EditorState {
focusedObjectId: number | null focusedObjectId: number | null
setFocusedObject: (id: number | null) => void setFocusedObject: (id: number | null) => void
// During a move/resize/rotate, the object's live geometry is held here so the
// canvas renders it instantly; the PATCH fires only on gesture end.
liveObject: EditorObject | null
setLiveObject: (o: EditorObject | null) => void
// The palette kind armed for tap-to-place (mobile-friendly; also set while
// dragging a kind from the palette). null when nothing is armed.
armedKind: string | null
setArmedKind: (kind: string | null) => void
// True while an object move/resize/rotate is in progress, so the viewport's
// pan gesture stands down (both listen on the same svg).
objectDragging: boolean
setObjectDragging: (v: boolean) => void
} }
export const useEditorStore = create<EditorState>((set) => ({ export const useEditorStore = create<EditorState>((set) => ({
viewport: { tx: 0, ty: 0, scale: 1 }, viewport: { tx: 0, ty: 0, scale: 1 },
setViewport: (next) => setViewport: (next) => set((s) => ({ viewport: typeof next === 'function' ? next(s.viewport) : next })),
set((s) => ({ viewport: typeof next === 'function' ? next(s.viewport) : next })),
selectedId: null, selectedId: null,
select: (id) => set({ selectedId: id }), select: (id) => set({ selectedId: id }),
focusedObjectId: null, focusedObjectId: null,
setFocusedObject: (id) => set({ focusedObjectId: id }), setFocusedObject: (id) => set({ focusedObjectId: id }),
liveObject: null,
setLiveObject: (o) => set({ liveObject: o }),
armedKind: null,
setArmedKind: (kind) => set({ armedKind: kind }),
objectDragging: false,
setObjectDragging: (v) => set({ objectDragging: v }),
})) }))
+2
View File
@@ -17,7 +17,9 @@ export interface EditorObject {
rotationDeg: number rotationDeg: number
zIndex: number zIndex: number
color?: string | null color?: string | null
notes: string
plantable: boolean plantable: boolean
version: number
} }
/** The garden being edited: its bounds define the canvas extent. */ /** The garden being edited: its bounds define the canvas extent. */
+2
View File
@@ -46,6 +46,8 @@ export function useViewport(svgRef: RefObject<SVGSVGElement | null>) {
{ {
onDragStart: cancelAnim, onDragStart: cancelAnim,
onDrag: ({ delta: [dx, dy], pinching, cancel }) => { onDrag: ({ delta: [dx, dy], pinching, cancel }) => {
// An object move/resize/rotate owns this drag; don't also pan.
if (useEditorStore.getState().objectDragging) return
if (pinching) { if (pinching) {
cancel() cancel()
return return
+222
View File
@@ -0,0 +1,222 @@
// Garden objects data layer: the /full editor payload plus optimistic
// create/update/delete mutations. Updates apply to the cache immediately and
// PATCH with the row version; a 409 rolls back to the server's current row and
// toasts (see DESIGN § Sync).
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 { toast } from '@/components/ui/toast'
import type { EditorObject, ObjectShapeKind } from '@/editor/types'
const shapeSchema = z.enum(['rect', 'circle', 'polygon'])
export const serverObjectSchema = z.object({
id: z.number(),
gardenId: z.number(),
kind: z.string(),
name: z.string(),
shape: shapeSchema,
xCm: z.number(),
yCm: z.number(),
widthCm: z.number(),
heightCm: z.number(),
rotationDeg: z.number(),
zIndex: z.number(),
plantable: z.boolean(),
color: z.string().nullable().optional(),
props: z.string().nullable().optional(),
notes: z.string(),
version: z.number(),
})
export type ServerObject = z.infer<typeof serverObjectSchema>
export const fullGardenSchema = z.object({
garden: gardenSchema,
objects: z.array(serverObjectSchema),
plantings: z.array(z.unknown()), // typed in #14
plants: z.array(z.unknown()), // typed in #12
})
export type FullGarden = z.infer<typeof fullGardenSchema>
/** Map a server object to the canvas's EditorObject (polygon never reaches the
* v1 editor; coerce it to rect defensively). */
export function toEditorObject(o: ServerObject): EditorObject {
const shape: ObjectShapeKind = o.shape === 'circle' ? 'circle' : 'rect'
return {
id: o.id,
kind: o.kind,
name: o.name,
shape,
xCm: o.xCm,
yCm: o.yCm,
widthCm: o.widthCm,
heightCm: o.heightCm,
rotationDeg: o.rotationDeg,
zIndex: o.zIndex,
color: o.color ?? null,
notes: o.notes,
plantable: o.plantable,
version: o.version,
}
}
const fullKey = (gardenId: number) => ['garden-full', gardenId] as const
export function gardenFullQueryOptions(gardenId: number) {
return queryOptions({
queryKey: fullKey(gardenId),
queryFn: async (): Promise<FullGarden> => fullGardenSchema.parse(await api.get(`/gardens/${gardenId}/full`)),
})
}
export function useGardenFull(gardenId: number) {
return useQuery(gardenFullQueryOptions(gardenId))
}
// --- mutations -------------------------------------------------------------
export interface ObjectCreate {
kind: string
name?: string
shape?: ObjectShapeKind
xCm: number
yCm: number
widthCm: number
heightCm: number
rotationDeg?: number
zIndex?: number
plantable?: boolean
color?: string | null
}
export function useCreateObject(gardenId: number) {
const qc = useQueryClient()
return useMutation({
mutationFn: async (input: ObjectCreate): Promise<ServerObject> =>
serverObjectSchema.parse(await api.post(`/gardens/${gardenId}/objects`, input)),
onSuccess: (created) => {
// Splice the created object into the cache (avoids a full refetch flash).
patchFullCache(qc, gardenId, (full) => ({ ...full, objects: [...full.objects, created] }))
},
onError: (err) => toast.error(objectErrorMessage(err, 'Could not add that object.')),
})
}
/** Fields the editor patches. version is required for the optimistic guard. */
export interface ObjectPatch {
id: number
version: number
name?: string
xCm?: number
yCm?: number
widthCm?: number
heightCm?: number
rotationDeg?: number
zIndex?: number
plantable?: boolean
color?: string | null
notes?: string
}
export function useUpdateObject(gardenId: number) {
const qc = useQueryClient()
return useMutation({
mutationFn: async ({ id, ...body }: ObjectPatch): Promise<ServerObject> =>
serverObjectSchema.parse(await api.patch(`/objects/${id}`, body)),
// Apply the change to the cache up front so the object doesn't snap back
// while the request is in flight. Snapshot for rollback.
onMutate: async (patch) => {
Review

🔴 Optimistic PATCH never bumps cached version, causing false 409 conflicts and silent loss of the user's own concurrent edits

correctness · flagged by 2 models

  • web/src/lib/objects.ts:130-137 + :188-201 (onMutate / applyServerPatch) — optimistic update never advances the cached version, so any second PATCH fired before the first resolves gets a false 409 "conflict" and silently loses the user's own edit. useUpdateObject's onMutate applies the patched fields to the cache via applyServerPatch, which copies over every field except version (objects.ts:188-201). The cached object therefore still carries the pre-mutation version afte…

🪰 Gadfly · advisory

🔴 **Optimistic PATCH never bumps cached version, causing false 409 conflicts and silent loss of the user's own concurrent edits** _correctness · flagged by 2 models_ - **`web/src/lib/objects.ts:130-137` + `:188-201` (`onMutate` / `applyServerPatch`) — optimistic update never advances the cached `version`, so any second PATCH fired before the first resolves gets a false 409 "conflict" and silently loses the user's own edit.** `useUpdateObject`'s `onMutate` applies the patched fields to the cache via `applyServerPatch`, which copies over every field except `version` (objects.ts:188-201). The cached object therefore still carries the pre-mutation `version` afte… <sub>🪰 Gadfly · advisory</sub>
await qc.cancelQueries({ queryKey: fullKey(gardenId) })
const prev = qc.getQueryData<FullGarden>(fullKey(gardenId))
patchFullCache(qc, gardenId, (full) => ({
...full,
objects: full.objects.map((o) => (o.id === patch.id ? applyServerPatch(o, patch) : o)),
}))
return { prev }
},
onSuccess: (updated) => {
patchFullCache(qc, gardenId, (full) => ({
...full,
objects: full.objects.map((o) => (o.id === updated.id ? updated : o)),
}))
},
onError: (err, _patch, ctx) => {
const current = conflictObject(err)
if (current) {
// Someone else changed it: adopt the server's row and tell the user.
patchFullCache(qc, gardenId, (full) => ({
...full,
objects: full.objects.map((o) => (o.id === current.id ? current : o)),
}))
toast.error('That object was updated elsewhere — your change was rolled back.')
} else if (ctx?.prev) {
qc.setQueryData(fullKey(gardenId), ctx.prev)
toast.error(objectErrorMessage(err, 'Could not save that change.'))
}
},
// No onSettled refetch: onSuccess already writes the authoritative server
Outdated
Review

🟠 Redundant full-garden refetch (invalidateQueries) after every object PATCH already synced via onMutate/onSuccess

performance · flagged by 1 model

  • web/src/lib/objects.ts:159 (and 177)useUpdateObject and useDeleteObject both add onSettled: () => qc.invalidateQueries({ queryKey: fullKey(gardenId) }) even though onSuccess/onMutate already write the authoritative result into the cache (patchFullCache at lines 139-144 for update, 170 for delete's optimistic removal). This triggers a full refetch of GET /gardens/:id/full after every successful PATCH/DELETE, in addition to the request itself. Since the Inspector fires o…

🪰 Gadfly · advisory

🟠 **Redundant full-garden refetch (invalidateQueries) after every object PATCH already synced via onMutate/onSuccess** _performance · flagged by 1 model_ - **`web/src/lib/objects.ts:159` (and `177`)** — `useUpdateObject` and `useDeleteObject` both add `onSettled: () => qc.invalidateQueries({ queryKey: fullKey(gardenId) })` even though `onSuccess`/`onMutate` already write the authoritative result into the cache (`patchFullCache` at lines 139-144 for update, 170 for delete's optimistic removal). This triggers a full refetch of `GET /gardens/:id/full` after every successful PATCH/DELETE, in addition to the request itself. Since the Inspector fires o… <sub>🪰 Gadfly · advisory</sub>
// row and onError reconciles. Invalidating here would fire a full-garden GET
// after every PATCH — a request storm during drags, and a refetch race that
// can clobber a fast follow-up edit.
})
}
export function useDeleteObject(gardenId: number) {
const qc = useQueryClient()
return useMutation({
mutationFn: (id: number) => api.delete(`/objects/${id}`),
onMutate: async (id) => {
await qc.cancelQueries({ queryKey: fullKey(gardenId) })
const prev = qc.getQueryData<FullGarden>(fullKey(gardenId))
patchFullCache(qc, gardenId, (full) => ({ ...full, objects: full.objects.filter((o) => o.id !== id) }))
return { prev }
},
onError: (err, _id, ctx) => {
if (ctx?.prev) qc.setQueryData(fullKey(gardenId), ctx.prev)
toast.error(objectErrorMessage(err, 'Could not delete that object.'))
},
onSettled: () => qc.invalidateQueries({ queryKey: fullKey(gardenId) }),
})
}
// --- helpers ---------------------------------------------------------------
function patchFullCache(qc: QueryClient, gardenId: number, fn: (full: FullGarden) => FullGarden) {
qc.setQueryData<FullGarden>(fullKey(gardenId), (full) => (full ? fn(full) : full))
}
Review

🔴 Optimistic PATCH doesn't bump cached version, causing false 409s that silently discard the user's own rapid sequential edits

error-handling, maintainability · flagged by 4 models

  • web/src/lib/objects.ts:188-201 (applyServerPatch) — Ten near-identical ...(patch.X !== undefined ? { X: patch.X } : {}) lines, one per ObjectPatch field. Every time a field is added to ObjectPatch (objects.ts:108-121), it must be independently added here too, and nothing enforces that — a forgotten field silently no-ops instead of failing to compile. Could be replaced with a small "merge only defined keys" helper.

🪰 Gadfly · advisory

🔴 **Optimistic PATCH doesn't bump cached version, causing false 409s that silently discard the user's own rapid sequential edits** _error-handling, maintainability · flagged by 4 models_ - `web/src/lib/objects.ts:188-201` (`applyServerPatch`) — Ten near-identical `...(patch.X !== undefined ? { X: patch.X } : {})` lines, one per `ObjectPatch` field. Every time a field is added to `ObjectPatch` (`objects.ts:108-121`), it must be independently added here too, and nothing enforces that — a forgotten field silently no-ops instead of failing to compile. Could be replaced with a small "merge only defined keys" helper. <sub>🪰 Gadfly · advisory</sub>
/** Apply a patch onto a cached server object for the optimistic pass. The
* version is bumped to mirror the server's post-commit increment, so a second
* edit started before the first PATCH resolves reads the next version instead
* of re-sending the stale one and self-conflicting (a spurious 409). */
function applyServerPatch(o: ServerObject, patch: ObjectPatch): ServerObject {
return {
...o,
version: o.version + 1,
...(patch.name !== undefined ? { name: patch.name } : {}),
...(patch.xCm !== undefined ? { xCm: patch.xCm } : {}),
...(patch.yCm !== undefined ? { yCm: patch.yCm } : {}),
...(patch.widthCm !== undefined ? { widthCm: patch.widthCm } : {}),
...(patch.heightCm !== undefined ? { heightCm: patch.heightCm } : {}),
...(patch.rotationDeg !== undefined ? { rotationDeg: patch.rotationDeg } : {}),
...(patch.zIndex !== undefined ? { zIndex: patch.zIndex } : {}),
...(patch.plantable !== undefined ? { plantable: patch.plantable } : {}),
...(patch.color !== undefined ? { color: patch.color } : {}),
...(patch.notes !== undefined ? { notes: patch.notes } : {}),
}
}
/** The current server row from a 409 conflict body, or null. */
function conflictObject(err: unknown): ServerObject | null {
if (err instanceof ApiError && err.isConflict && err.body && typeof err.body === 'object') {
const parsed = serverObjectSchema.safeParse((err.body as { current?: unknown }).current)
if (parsed.success) return parsed.data
}
return null
}
function objectErrorMessage(err: unknown, fallback: string): string {
return err instanceof ApiError ? err.message : fallback
}
+69 -21
View File
@@ -1,31 +1,79 @@
import { useEffect, useMemo } from 'react'
import { getRouteApi } from '@tanstack/react-router'
import { Alert } from '@/components/ui/Alert'
import { GardenCanvas } from '@/editor/GardenCanvas' import { GardenCanvas } from '@/editor/GardenCanvas'
import type { EditorGarden, EditorObject } from '@/editor/types' import { Inspector } from '@/editor/Inspector'
import { Palette } from '@/editor/Palette'
import { useEditorStore } from '@/editor/store'
import type { EditorGarden } from '@/editor/types'
import { toEditorObject, useGardenFull } from '@/lib/objects'
// Mock scene for the canvas foundation (#9). #11 replaces this with the garden's const routeApi = getRouteApi('/gardens/$gardenId')
// real /full payload keyed on the route's gardenId.
const mockGarden: EditorGarden = { id: 0, name: 'Demo garden', widthCm: 1200, heightCm: 800, unitPref: 'metric' }
const mockObjects: EditorObject[] = [
{ id: 1, kind: 'bed', name: 'Bed A', shape: 'rect', xCm: 300, yCm: 250, widthCm: 400, heightCm: 120, rotationDeg: 0, zIndex: 1, plantable: true },
{ id: 2, kind: 'bed', name: 'Bed B', shape: 'rect', xCm: 300, yCm: 450, widthCm: 400, heightCm: 120, rotationDeg: 0, zIndex: 1, plantable: true },
{ id: 3, kind: 'container', name: 'Pot', shape: 'circle', xCm: 800, yCm: 250, widthCm: 120, heightCm: 120, rotationDeg: 0, zIndex: 2, plantable: true },
{ id: 4, kind: 'path', name: '', shape: 'rect', xCm: 600, yCm: 400, widthCm: 60, heightCm: 720, rotationDeg: 0, zIndex: 0, plantable: false },
{ id: 5, kind: 'tree', name: 'Apple', shape: 'circle', xCm: 980, yCm: 620, widthCm: 200, heightCm: 200, rotationDeg: 0, zIndex: 2, plantable: false },
{ id: 6, kind: 'bed', name: 'Angled', shape: 'rect', xCm: 920, yCm: 480, widthCm: 260, heightCm: 100, rotationDeg: 30, zIndex: 1, plantable: true },
]
export function GardenEditorPage() { export function GardenEditorPage() {
const { gardenId } = routeApi.useParams()
const gid = Number(gardenId)
const { focus } = routeApi.useSearch()
const full = useGardenFull(gid)
const selectedId = useEditorStore((s) => s.selectedId)
const select = useEditorStore((s) => s.select)
const setArmedKind = useEditorStore((s) => s.setArmedKind)
const setLiveObject = useEditorStore((s) => s.setLiveObject)
// Map the server rows to EditorObjects once per cache change. Rebuilding this
// every render (e.g. on each viewport/selection update) would hand ObjectShape
// fresh identities and defeat its memo, re-rendering every object.
const serverObjects = full.data?.objects
const objects = useMemo(() => serverObjects?.map(toEditorObject) ?? [], [serverObjects])
// Clear transient editor state on entering/switching gardens and on leaving.
useEffect(() => {
const reset = () => {
select(null)
setArmedKind(null)
setLiveObject(null)
}
reset()
return reset
}, [gid, select, setArmedKind, setLiveObject])
if (full.isPending) return <p className="p-6 text-sm text-muted">Loading garden</p>
if (full.isError)
return ( return (
<div className="flex h-[calc(100vh-10rem)] flex-col gap-3"> <div className="p-6">
<div className="flex flex-wrap items-baseline gap-x-2 gap-y-1"> <Alert>Could not load this garden.</Alert>
<h1 className="text-lg font-semibold tracking-tight">Editor preview</h1>
<span className="text-sm text-muted">
Mock garden drag to pan, wheel/pinch to zoom, tap an object to select. Real data + editing land in #11.
</span>
</div> </div>
<div className="min-h-0 flex-1"> )
<GardenCanvas garden={mockGarden} objects={mockObjects} />
const g = full.data.garden
const garden: EditorGarden = {
id: g.id,
Review

🟠 Unmemoized toEditorObject mapping breaks downstream memoization and causes unnecessary allocations

performance · flagged by 3 models

  • web/src/pages/GardenEditorPage.tsx:51const objects = full.data.objects.map(toEditorObject) is unmemoized. toEditorObject creates a new EditorObject instance for every server object on every parent render. This defeats the React.memo on ObjectShape, forcing all objects to re-render whenever GardenEditorPage re-renders (e.g., on selectedId change or background refetch). Fix: wrap the mapping in useMemo(() => full.data.objects.map(toEditorObject), [full.data.objects])

🪰 Gadfly · advisory

🟠 **Unmemoized toEditorObject mapping breaks downstream memoization and causes unnecessary allocations** _performance · flagged by 3 models_ - **`web/src/pages/GardenEditorPage.tsx:51`** — `const objects = full.data.objects.map(toEditorObject)` is unmemoized. `toEditorObject` creates a new `EditorObject` instance for every server object on every parent render. This defeats the `React.memo` on `ObjectShape`, forcing all objects to re-render whenever `GardenEditorPage` re-renders (e.g., on `selectedId` change or background refetch). **Fix:** wrap the mapping in `useMemo(() => full.data.objects.map(toEditorObject), [full.data.objects])`… <sub>🪰 Gadfly · advisory</sub>
name: g.name,
widthCm: g.widthCm,
heightCm: g.heightCm,
unitPref: g.unitPref,
}
const selected = objects.find((o) => o.id === selectedId) ?? null
return (
<div className="flex h-[calc(100vh-8rem)] flex-col gap-3 md:flex-row">
<div className="shrink-0 md:w-40">
<h1 className="mb-2 truncate text-lg font-semibold tracking-tight" title={garden.name}>
{garden.name}
</h1>
<Palette />
</div> </div>
<div className="relative min-h-0 flex-1">
<GardenCanvas garden={garden} objects={objects} focusId={focus} />
</div>
{selected && (
<div className="fixed inset-x-0 bottom-0 z-30 max-h-[65vh] overflow-y-auto rounded-t-xl border-t border-border bg-surface p-4 shadow-lg md:static md:max-h-none md:w-72 md:shrink-0 md:rounded-xl md:border md:p-4 md:shadow-sm">
<Inspector key={selected.id} object={selected} gardenId={gid} unit={garden.unitPref} />
</div>
)}
</div> </div>
) )
} }
+5
View File
@@ -89,6 +89,11 @@ const gardensRoute = createRoute({
const gardenEditorRoute = createRoute({ const gardenEditorRoute = createRoute({
getParentRoute: () => rootRoute, getParentRoute: () => rootRoute,
path: 'gardens/$gardenId', path: 'gardens/$gardenId',
// ?focus=<objectId> frames that object on load.
validateSearch: (search: Record<string, unknown>): { focus?: number } => {
const f = Number(search.focus)
return Number.isInteger(f) && f > 0 ? { focus: f } : {}
},
beforeLoad: ({ context, location }) => requireAuth(context, location.href), beforeLoad: ({ context, location }) => requireAuth(context, location.href),
component: GardenEditorPage, component: GardenEditorPage,
}) })