From c4bc5e9f47f2716458c00ac588f8282dab370f3f Mon Sep 17 00:00:00 2001
From: Steve Dudenhoeffer <steve@stevedudenhoeffer.com>
Date: Sun, 19 Jul 2026 00:42:51 -0400
Subject: [PATCH] Address Gadfly review on #18: clear-bed + nudge robustness
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- useClearObject: Promise.allSettled + invalidate in onSettled (not onSuccess),
  so a partial failure still reconciles the cache with the server instead of
  losing the already-removed rows; reports how many failed.
- Keyboard nudge: the keydown effect is now mounted once and reads live values
  from a ref, so a data refetch can't re-subscribe and cancel a pending
  debounced commit. The pending move is flushed on unmount instead of dropped,
  a fire only commits if its live value is still present (a drag that cleared it
  already committed its own PATCH), and nudges are ignored while a pointer drag
  is active — closing the nudge/drag race.
- usePageTitle: dropped the restore-on-unmount, which raced and could clobber the
  incoming route's title on a fast navigation.
- ClearBedModal disables confirm on an empty plop list.
- Extracted EditorHint (empty-state overlays) and objectDisplayName (harmonized
  the name fallback across the toolbar and clear dialog).

Skipped (false positive): "nudge clamp ignores rotation" — the clamp is to the
object's LOCAL bounds (±w/2, ±h/2), exactly what the server enforces, so a plop
can't escape a rotated bed.

tsc --noEmit clean; 24/24 vitest; production build green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JdQpdYYsTgtkJBxbcpAszi
---
 web/src/editor/ClearBedModal.tsx   |  2 +-
 web/src/editor/EditorHint.tsx      | 18 +++++++
 web/src/editor/kinds.ts            |  5 ++
 web/src/lib/objects.ts             | 14 ++++--
 web/src/lib/usePageTitle.ts        |  9 ++--
 web/src/pages/GardenEditorPage.tsx | 75 ++++++++++++++++++------------
 6 files changed, 82 insertions(+), 41 deletions(-)
 create mode 100644 web/src/editor/EditorHint.tsx

diff --git a/web/src/editor/ClearBedModal.tsx b/web/src/editor/ClearBedModal.tsx
index 76fd2fc..ed5665e 100644
--- a/web/src/editor/ClearBedModal.tsx
+++ b/web/src/editor/ClearBedModal.tsx
@@ -31,7 +31,7 @@ export function ClearBedModal({
           <Button
             type="button"
             variant="danger"
-            disabled={clear.isPending}
+            disabled={clear.isPending || n === 0}
             onClick={() => clear.mutate(plops, { onSuccess: onClose })}
           >
             {clear.isPending ? 'Clearing…' : 'Clear bed'}
diff --git a/web/src/editor/EditorHint.tsx b/web/src/editor/EditorHint.tsx
new file mode 100644
index 0000000..9b4f9c9
--- /dev/null
+++ b/web/src/editor/EditorHint.tsx
@@ -0,0 +1,18 @@
+import type { ReactNode } from 'react'
+import { cn } from '@/lib/cn'
+
+/** A non-interactive hint overlaid on the canvas (empty-state guidance). */
+export function EditorHint({ position = 'center', children }: { position?: 'center' | 'top'; children: ReactNode }) {
+  return (
+    <div
+      className={cn(
+        'pointer-events-none absolute flex p-6 text-center',
+        position === 'top' ? 'inset-x-0 top-16 justify-center' : 'inset-0 items-center justify-center',
+      )}
+    >
+      <p className="max-w-xs rounded-lg bg-surface/85 px-4 py-3 text-sm text-muted shadow-sm backdrop-blur">
+        {children}
+      </p>
+    </div>
+  )
+}
diff --git a/web/src/editor/kinds.ts b/web/src/editor/kinds.ts
index a8567ed..cd2b1aa 100644
--- a/web/src/editor/kinds.ts
+++ b/web/src/editor/kinds.ts
@@ -27,3 +27,8 @@ export const OBJECT_KINDS: KindDef[] = [
 export function kindDef(kind: string): KindDef | undefined {
   return OBJECT_KINDS.find((k) => k.kind === kind)
 }
+
+/** A human label for an object: its name, else its kind's label, else "Object". */
+export function objectDisplayName(o: { name: string; kind: string }): string {
+  return o.name || kindDef(o.kind)?.label || 'Object'
+}
diff --git a/web/src/lib/objects.ts b/web/src/lib/objects.ts
index aa48de0..e97b0be 100644
--- a/web/src/lib/objects.ts
+++ b/web/src/lib/objects.ts
@@ -271,12 +271,20 @@ export function useClearObject(gardenId: number) {
   return useMutation({
     mutationFn: async (plops: { id: number; version: number }[]) => {
       const today = new Date().toISOString().slice(0, 10)
-      await Promise.all(
+      // allSettled, not all: a partial failure still soft-removed some rows
+      // server-side, so we must reconcile the cache rather than roll everything
+      // back. Report how many failed.
+      const results = await Promise.allSettled(
         plops.map((p) => api.patch(`/plantings/${p.id}`, { removedAt: today, version: p.version })),
       )
+      const failed = results.filter((r) => r.status === 'rejected').length
+      if (failed > 0) {
+        throw new Error(`${failed} of ${plops.length} plants couldn't be cleared — refresh and try again.`)
+      }
     },
-    onSuccess: () => qc.invalidateQueries({ queryKey: fullKey(gardenId) }),
-    onError: (err) => toast.error(objectErrorMessage(err, 'Could not clear the bed.')),
+    // Reconcile on success OR partial failure, so the cache matches the server.
+    onSettled: () => qc.invalidateQueries({ queryKey: fullKey(gardenId) }),
+    onError: (err) => toast.error(err instanceof Error ? err.message : 'Could not clear the bed.'),
   })
 }
 
diff --git a/web/src/lib/usePageTitle.ts b/web/src/lib/usePageTitle.ts
index 668a878..d052a94 100644
--- a/web/src/lib/usePageTitle.ts
+++ b/web/src/lib/usePageTitle.ts
@@ -1,13 +1,10 @@
 import { useEffect } from 'react'
 
-/** Set the document title for the current route (suffixed with · pansy), and
- *  restore the previous title on unmount. */
+/** Set the document title for the current route (suffixed with · pansy). No
+ *  restore-on-unmount: each route sets its own title, so restoring the previous
+ *  one would race and clobber the incoming route's title on a fast navigation. */
 export function usePageTitle(title: string): void {
   useEffect(() => {
-    const prev = document.title
     document.title = title ? `${title} · pansy` : 'pansy'
-    return () => {
-      document.title = prev
-    }
   }, [title])
 }
diff --git a/web/src/pages/GardenEditorPage.tsx b/web/src/pages/GardenEditorPage.tsx
index 220ecdc..e688beb 100644
--- a/web/src/pages/GardenEditorPage.tsx
+++ b/web/src/pages/GardenEditorPage.tsx
@@ -8,7 +8,8 @@ import { PlopInspector } from '@/editor/PlopInspector'
 import { PlantPicker } from '@/editor/PlantPicker'
 import { Palette } from '@/editor/Palette'
 import { ClearBedModal } from '@/editor/ClearBedModal'
-import { kindDef } from '@/editor/kinds'
+import { EditorHint } from '@/editor/EditorHint'
+import { objectDisplayName } from '@/editor/kinds'
 import { useEditorStore } from '@/editor/store'
 import type { EditorGarden } from '@/editor/types'
 import { ShareGardenModal } from '@/components/gardens/ShareGardenModal'
@@ -49,6 +50,7 @@ export function GardenEditorPage() {
   const [sharing, setSharing] = useState(false)
   const [clearing, setClearing] = useState(false)
   const nudgeTimer = useRef<number | null>(null)
+  const nudgeFire = useRef<(() => void) | null>(null)
 
   const serverObjects = full.data?.objects
   const objects = useMemo(() => serverObjects?.map(toEditorObject) ?? [], [serverObjects])
@@ -63,6 +65,11 @@ export function GardenEditorPage() {
   const canEdit = gd != null && (gd.ownerId === me.data?.id || gd.myRole === 'editor')
   const isOwner = gd != null && me.data != null && gd.ownerId === me.data.id
 
+  // Latest values for the mount-once nudge keydown handler to read, so its effect
+  // never re-subscribes (which would cancel a pending debounced commit).
+  const nudgeCtx = useRef({ canEdit, objects, plantings, updateObject, updatePlanting })
+  nudgeCtx.current = { canEdit, objects, plantings, updateObject, updatePlanting }
+
   // Adopt the URL's focus and clear transient editor state on entering/switching
   // gardens (and on leaving). `focus` is intentionally read once here, not a dep,
   // so URL syncs below don't retrigger a full reset.
@@ -122,9 +129,12 @@ export function GardenEditorPage() {
 
   // Desktop keyboard nudging: arrows move the selected object/plop 1cm (Shift =
   // 10cm); the PATCH is debounced ~400ms on key-idle so a held key doesn't spam.
-  // Plops nudge in their object's local frame and clamp to its bounds.
+  // Plops nudge in their object's local frame and clamp to its (local) bounds.
+  // Mounted once, reading live values from nudgeCtx so a data refetch can't
+  // re-subscribe and cancel a pending commit; the pending commit is flushed on
+  // unmount, and a fire only commits if its live value is still present (a drag
+  // that cleared it already committed its own PATCH).
   useEffect(() => {
-    if (!canEdit) return
     const DIRS: Record<string, [number, number]> = {
       ArrowUp: [0, -1],
       ArrowDown: [0, 1],
@@ -132,47 +142,54 @@ export function GardenEditorPage() {
       ArrowRight: [1, 0],
     }
     const commitLater = (fire: () => void) => {
+      nudgeFire.current = fire
       if (nudgeTimer.current != null) window.clearTimeout(nudgeTimer.current)
       nudgeTimer.current = window.setTimeout(() => {
-        fire()
         nudgeTimer.current = null
+        const fn = nudgeFire.current
+        nudgeFire.current = null
+        fn?.()
       }, 400)
     }
     function onKey(e: KeyboardEvent) {
+      const { canEdit: canNudge, objects: objs, plantings: plops, updateObject: uo, updatePlanting: up } =
+        nudgeCtx.current
+      if (!canNudge) return
       const dir = DIRS[e.key]
       if (!dir) return
       const el = document.activeElement
       if (el && (el.tagName === 'INPUT' || el.tagName === 'TEXTAREA' || el.tagName === 'SELECT')) return
       const s = useEditorStore.getState()
+      if (s.objectDragging) return // don't fight an active pointer drag
       const step = e.shiftKey ? 10 : 1
       if (s.selectedId != null) {
-        const base = s.liveObject?.id === s.selectedId ? s.liveObject : objects.find((o) => o.id === s.selectedId)
+        const base = s.liveObject?.id === s.selectedId ? s.liveObject : objs.find((o) => o.id === s.selectedId)
         if (!base) return
         e.preventDefault()
-        const next = { ...base, xCm: base.xCm + dir[0] * step, yCm: base.yCm + dir[1] * step }
-        s.setLiveObject(next)
+        s.setLiveObject({ ...base, xCm: base.xCm + dir[0] * step, yCm: base.yCm + dir[1] * step })
         commitLater(() => {
-          updateObject.mutate({ id: next.id, version: next.version, xCm: next.xCm, yCm: next.yCm })
+          const live = useEditorStore.getState().liveObject
+          if (live?.id !== s.selectedId) return // a drag cleared it and committed
+          uo.mutate({ id: live.id, version: live.version, xCm: live.xCm, yCm: live.yCm })
           useEditorStore.getState().setLiveObject(null)
         })
       } else if (s.selectedPlantingId != null) {
         const base =
-          s.livePlanting?.id === s.selectedPlantingId
-            ? s.livePlanting
-            : plantings.find((p) => p.id === s.selectedPlantingId)
+          s.livePlanting?.id === s.selectedPlantingId ? s.livePlanting : plops.find((p) => p.id === s.selectedPlantingId)
         if (!base) return
         e.preventDefault()
-        const obj = objects.find((o) => o.id === base.objectId)
+        const obj = objs.find((o) => o.id === base.objectId)
         let nx = base.xCm + dir[0] * step
         let ny = base.yCm + dir[1] * step
         if (obj) {
           nx = Math.max(-obj.widthCm / 2, Math.min(obj.widthCm / 2, nx))
           ny = Math.max(-obj.heightCm / 2, Math.min(obj.heightCm / 2, ny))
         }
-        const next = { ...base, xCm: nx, yCm: ny }
-        s.setLivePlanting(next)
+        s.setLivePlanting({ ...base, xCm: nx, yCm: ny })
         commitLater(() => {
-          updatePlanting.mutate({ id: next.id, version: next.version, xCm: next.xCm, yCm: next.yCm })
+          const live = useEditorStore.getState().livePlanting
+          if (live?.id !== s.selectedPlantingId) return
+          up.mutate({ id: live.id, version: live.version, xCm: live.xCm, yCm: live.yCm })
           useEditorStore.getState().setLivePlanting(null)
         })
       }
@@ -180,10 +197,16 @@ export function GardenEditorPage() {
     window.addEventListener('keydown', onKey)
     return () => {
       window.removeEventListener('keydown', onKey)
-      if (nudgeTimer.current != null) window.clearTimeout(nudgeTimer.current)
+      if (nudgeTimer.current != null) {
+        window.clearTimeout(nudgeTimer.current)
+        nudgeTimer.current = null
+      }
+      const fn = nudgeFire.current // flush a pending move so it isn't lost
+      nudgeFire.current = null
+      fn?.()
     }
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [canEdit, objects, plantings])
+  }, [])
 
   if (full.isPending) return <p className="p-6 text-sm text-muted">Loading garden…</p>
   if (full.isError)
@@ -243,9 +266,7 @@ export function GardenEditorPage() {
             <button type="button" onClick={exitFocus} className="rounded px-1.5 py-0.5 font-medium text-accent-strong hover:underline">
               ← {garden.name}
             </button>
-            <span className="max-w-[8rem] truncate text-muted">
-              {focusedObject.name || kindDef(focusedObject.kind)?.label || 'Object'}
-            </span>
+            <span className="max-w-[8rem] truncate text-muted">{objectDisplayName(focusedObject)}</span>
             {canEdit &&
               (!focusedObject.plantable ? (
                 <span className="text-xs text-muted">Not plantable</span>
@@ -273,18 +294,10 @@ export function GardenEditorPage() {
 
         {/* Empty-state hints (non-interactive overlays). */}
         {canEdit && focusedObjectId == null && objects.length === 0 && (
-          <div className="pointer-events-none absolute inset-0 flex items-center justify-center p-6 text-center">
-            <p className="max-w-xs rounded-lg bg-surface/85 px-4 py-3 text-sm text-muted shadow-sm backdrop-blur">
-              Pick a shape from the palette, then tap the field to place your first bed.
-            </p>
-          </div>
+          <EditorHint>Pick a shape from the palette, then tap the field to place your first bed.</EditorHint>
         )}
         {canEdit && focusedObject?.plantable && focusedPlops.length === 0 && !armedPlant && (
-          <div className="pointer-events-none absolute inset-x-0 top-16 flex justify-center p-6 text-center">
-            <p className="max-w-xs rounded-lg bg-surface/85 px-4 py-3 text-sm text-muted shadow-sm backdrop-blur">
-              Tap “+ Add plant”, then tap inside the bed to place plops.
-            </p>
-          </div>
+          <EditorHint position="top">Tap “+ Add plant”, then tap inside the bed to place plops.</EditorHint>
         )}
       </div>
 
@@ -330,7 +343,7 @@ export function GardenEditorPage() {
 
       {clearing && focusedObject && (
         <ClearBedModal
-          objectName={focusedObject.name || kindDef(focusedObject.kind)?.label || 'bed'}
+          objectName={objectDisplayName(focusedObject)}
           plops={focusedPlops.map((p) => ({ id: p.id, version: p.version }))}
           gardenId={gid}
           onClose={() => setClearing(false)}