From 0f67623da762549d073705bae32457fbb817863b Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Sat, 18 Jul 2026 23:21:17 -0400 Subject: [PATCH] Address Gadfly review on #15: real bugs + shared editor constants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Correctness / stuck-state: - PlopInspector: the planted date can now be cleared (blur commits null instead of being blocked); radius/count reset to the current value on invalid input. - applyPlantingPatch now applies removedAt too (completes the optimistic patch). - PlopMarker computes the count live from the current radius, so the on-canvas near-zoom label updates while resizing (not just the inspector). - GardenEditorPage clears focus when the focused object vanishes — deleted, or a stale ?focus id on load — so the canvas can't get stuck dimmed with no exit. - useRemovePlanting adopts the 409 current row, so a stale-version soft-remove can be retried instead of looping. - Placement is gated on plantable, so a deep-linked non-plantable focus can't try to POST a plop the server would 400. Dedup / perf (introduced by this PR): - editor/shared.ts: SELECT_COLOR, HANDLE_PX, MIN_RADIUS_CM, DIMMED_OPACITY, and an objectTransform() helper, now used by ObjectShape/SelectionOverlay/PlopLayer/ PlopOverlay/PlopMarker/GardenCanvas (unified the transform string + the two different dim opacities). - Dropped PlopMarker's always-false `dimmed` prop (dimming is at the group level). - Build plantsById once in the page and pass it to GardenCanvas. - Moved the livePlanting subscription into PlopInspector so a plop drag re-renders only the panel, not the whole page (matches the liveObject pattern). - Memoized PlopLayer's plop grouping so it doesn't rebuild every pan/zoom frame. Skipped: gesture-scaffolding hook extraction (risky churn in #11 code), elliptical clamp for circle objects (consistent with the server's rect bounds), optimistic create (matches useCreateObject). tsc --noEmit clean; 24/24 vitest; production build green. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi --- web/src/editor/GardenCanvas.tsx | 14 +++---- web/src/editor/ObjectShape.tsx | 7 +--- web/src/editor/PlopInspector.tsx | 64 ++++++++++++++++++----------- web/src/editor/PlopLayer.tsx | 26 ++++++------ web/src/editor/PlopMarker.tsx | 21 ++++------ web/src/editor/PlopOverlay.tsx | 6 +-- web/src/editor/SelectionOverlay.tsx | 5 +-- web/src/editor/shared.ts | 14 +++++++ web/src/lib/objects.ts | 14 ++++++- web/src/pages/GardenEditorPage.tsx | 20 ++++++--- 10 files changed, 115 insertions(+), 76 deletions(-) create mode 100644 web/src/editor/shared.ts diff --git a/web/src/editor/GardenCanvas.tsx b/web/src/editor/GardenCanvas.tsx index 7614fef..5b27758 100644 --- a/web/src/editor/GardenCanvas.tsx +++ b/web/src/editor/GardenCanvas.tsx @@ -8,6 +8,7 @@ import { PlopLayer } from './PlopLayer' import { PlopOverlay } from './PlopOverlay' import { SelectionOverlay } from './SelectionOverlay' import { kindDef } from './kinds' +import { DIMMED_OPACITY, objectTransform } from './shared' import { useEditorStore } from './store' import { useViewport } from './useViewport' import type { EditorGarden, EditorObject } from './types' @@ -15,7 +16,6 @@ import type { EditorGarden, EditorObject } from './types' const GRID_CM = 100 // 1 m grid const GRID_MIN_CELL_PX = 6 const GRID_MAX_OPACITY = 0.18 -const DIMMED_OPACITY = 0.35 /** The world-space bounds rect of an object (center + size → top-left rect). */ function objectRect(o: EditorObject): Rect { @@ -33,12 +33,12 @@ export function GardenCanvas({ garden, objects, plantings, - plants, + plantsById, }: { garden: EditorGarden objects: EditorObject[] plantings: EditorPlanting[] - plants: Plant[] + plantsById: Map }) { const svgRef = useRef(null) const containerRef = useRef(null) @@ -65,8 +65,6 @@ export function GardenCanvas({ [garden.widthCm, garden.heightCm], ) - const plantsById = useMemo(() => new Map(plants.map((p) => [p.id, p])), [plants]) - useEffect(() => { const el = containerRef.current if (!el) return @@ -136,7 +134,7 @@ export function GardenCanvas({ // armed for repeat-placement until Escape / Done). function onPlace(e: ReactPointerEvent) { e.stopPropagation() - if (!focusedObject || !armedPlant) return + if (!focusedObject || !armedPlant || !focusedObject.plantable) return const rect = svgRef.current?.getBoundingClientRect() if (!rect) return const world = screenToWorld({ x: e.clientX - rect.left, y: e.clientY - rect.top }, viewport) @@ -197,8 +195,8 @@ export function GardenCanvas({ {/* Placement capture: a transparent sheet over the focused object while a plant is armed, so taps drop plops instead of selecting the object. */} - {focusedObject && armedPlant && ( - + {focusedObject && armedPlant && focusedObject.plantable && ( + + {object.shape === 'circle' ? ( s.livePlanting) const rootRef = useRef(null) - const [radius, setRadius] = useState(String(spacingFromCm(plop.radiusCm, unit))) - const [count, setCount] = useState(plop.count != null ? String(plop.count) : '') - const [label, setLabel] = useState(plop.label ?? '') - const [planted, setPlanted] = useState(plop.plantedAt ?? '') + // The plop with any in-flight drag/resize geometry applied. + const p = livePlanting && livePlanting.id === plop.id ? livePlanting : plop + + const [radius, setRadius] = useState(String(spacingFromCm(p.radiusCm, unit))) + const [count, setCount] = useState(p.count != null ? String(p.count) : '') + const [label, setLabel] = useState(p.label ?? '') + const [planted, setPlanted] = useState(p.plantedAt ?? '') // Re-sync when the plop changes underneath us (a drag/resize or a server row), // unless the user is editing a field here. useEffect(() => { if (rootRef.current?.contains(document.activeElement)) return - setRadius(String(spacingFromCm(plop.radiusCm, unit))) - setCount(plop.count != null ? String(plop.count) : '') - setLabel(plop.label ?? '') - setPlanted(plop.plantedAt ?? '') - }, [plop.version, plop.radiusCm, plop.count, plop.label, plop.plantedAt, unit]) + setRadius(String(spacingFromCm(p.radiusCm, unit))) + setCount(p.count != null ? String(p.count) : '') + setLabel(p.label ?? '') + setPlanted(p.plantedAt ?? '') + }, [p.version, p.radiusCm, p.count, p.label, p.plantedAt, unit]) const patch = (fields: Omit[0], 'id' | 'version'>) => update.mutate({ id: plop.id, version: plop.version, ...fields }) const u = spacingUnitLabel(unit) - const derived = plant ? computeDerivedCount(plop.radiusCm, plant.spacingCm) : plop.derivedCount + const derived = plant ? computeDerivedCount(p.radiusCm, plant.spacingCm) : p.derivedCount function commitRadius() { - const v = parseFloat(radius) - if (!Number.isFinite(v)) return + const v = Number(radius) + if (radius.trim() === '' || !Number.isFinite(v)) { + setRadius(String(spacingFromCm(p.radiusCm, unit))) // reset stale/invalid text + return + } const cm = Math.max(MIN_RADIUS_CM, cmFromSpacing(v, unit)) - if (cm !== plop.radiusCm) patch({ radiusCm: cm }) + if (cm !== p.radiusCm) patch({ radiusCm: cm }) } function commitCount() { if (count.trim() === '') { - if (plop.count != null) patch({ count: null }) // restore derived + if (p.count != null) patch({ count: null }) // restore derived return } const n = Number(count) - if (!Number.isInteger(n) || n < 1) return - if (n !== plop.count) patch({ count: n }) + if (!Number.isInteger(n) || n < 1) { + setCount(p.count != null ? String(p.count) : '') // reject invalid, restore + return + } + if (n !== p.count) patch({ count: n }) + } + + function commitPlanted() { + const next = planted === '' ? null : planted + if (next !== (p.plantedAt ?? null)) patch({ plantedAt: next }) } return ( @@ -121,7 +139,7 @@ export function PlopInspector({ value={count} onChange={(e) => setCount(e.target.value)} onBlur={commitCount} - hint={plop.count != null ? 'Override — clear for auto' : 'Auto from area ÷ spacing²'} + hint={p.count != null ? 'Override — clear for auto' : 'Auto from area ÷ spacing²'} /> @@ -130,7 +148,7 @@ export function PlopInspector({ name="label" value={label} onChange={(e) => setLabel(e.target.value)} - onBlur={() => label !== (plop.label ?? '') && patch({ label: label.trim() || null })} + onBlur={() => label !== (p.label ?? '') && patch({ label: label.trim() || null })} /> setPlanted(e.target.value)} - onBlur={() => planted !== (plop.plantedAt ?? '') && planted !== '' && patch({ plantedAt: planted })} + onBlur={commitPlanted} /> @@ -160,7 +168,7 @@ export function GardenEditorPage() { )} )} - + {(selectedObject || selectedPlop) && (