feat: improve drag-and-drop functionality and fix counter layout

frontend/src/App.css

@@ -415,6 +415,37 @@ html, body, #root {
   outline-offset: 8px;
 }
 
+/* container grid */
+.counters-grid {
+  display: grid;
+  gap: 16px;
+  grid-template-columns: repeat(auto-fit, minmax(260px, 1fr));
+  max-width: var(--max-width);
+  margin: 0 auto;
+  justify-content: center;
+  align-items: start;
+}
+
+/* keep each card centered in its cell and center its inner content */
+.counters-grid > .counter-card {
+  justify-self: center;
+  width: 100%;
+  max-width: 460px;
+  box-sizing: border-box;
+  padding: 12px;
+  display: flex;
+  align-items: center;
+  justify-content: center;
+}
+
+/* ensure the actual counter element stays centered and doesn't stretch awkwardly */
+.counter-card .counter {
+  width: 100%;
+  max-width: 420px;
+  margin: 0 auto;
+  display: flex; 
+}
+
 /* ---------------------------
    Utilities / footer
    --------------------------- */

frontend/src/App.tsx

@@ -1,4 +1,4 @@
-import React, { useEffect, useState, useCallback } from 'react'
+import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'
 import Counter from './components/Counter'
 import CreateCounter from './components/CreateCounter'
 import { apiUrl } from './utils/api'
@@ -6,6 +6,13 @@ import './App.css'
 
 type CounterRecord = { id: number; value: number; name?: string; imageUrl?: string; position?: number }
 
+// small pure helper kept outside the component so it's stable
+const swapInArray = (arr: CounterRecord[], i: number, j: number) => {
+  const copy = arr.slice()
+  ;[copy[i], copy[j]] = [copy[j], copy[i]]
+  return copy
+}
+
 export default function App() {
   const [counters, setCounters] = useState<CounterRecord[]>([])
   const [loading, setLoading] = useState(true)
@@ -13,33 +20,46 @@ export default function App() {
   const [draggingId, setDraggingId] = useState<number | null>(null)
   const [dragOverId, setDragOverId] = useState<number | null>(null)
 
+  // track which modal ids are currently open (idempotent)
+  const [modalOpenSet, setModalOpenSet] = useState<Set<number>>(new Set())
+  const draggingEnabled = useMemo(() => modalOpenSet.size === 0, [modalOpenSet])
+
+  // helper: idempotently add/remove modal id
+  const handleEditingChange = useCallback((id: number, editing: boolean) => {
+    setModalOpenSet((prev) => {
+      const next = new Set(prev)
+      if (editing) next.add(id)
+      else next.delete(id)
+      return next
+    })
+  }, [])
+
+  // mounted ref to avoid setting state on unmounted component
+  const mountedRef = useRef(true)
+  useEffect(() => {
+    mountedRef.current = true
+    return () => {
+      mountedRef.current = false
+    }
+  }, [])
+
   useEffect(() => {
-    let mounted = true
     ;(async () => {
       try {
         const res = await fetch(apiUrl('/api/counters'))
         const list: CounterRecord[] = await res.json()
-        if (!mounted) return
+        if (!mountedRef.current) return
         setCounters(Array.isArray(list) ? list : [])
       } catch (err) {
+        // keep minimal logging here
         console.error('failed to load counters', err)
-        if (mounted) setCounters([])
+        if (mountedRef.current) setCounters([])
       } finally {
-        if (mounted) setLoading(false)
+        if (mountedRef.current) setLoading(false)
       }
     })()
-    return () => {
-      mounted = false
-    }
   }, [])
 
-  // swap two items in an array (keeps array length and only swaps the two indexes)
-  const swapInArray = (arr: CounterRecord[], i: number, j: number) => {
-    const copy = arr.slice()
-    ;[copy[i], copy[j]] = [copy[j], copy[i]]
-    return copy
-  }
-
   const clearDragState = useCallback(() => {
     setDraggingId(null)
     setDragOverId(null)
@@ -121,130 +141,137 @@ export default function App() {
     clearDragState()
   }, [clearDragState])
 
-  const handleChange = async (id: number, next: number) => {
+  const handleChange = useCallback(
-    // enforce non-negative values
+    async (id: number, next: number) => {
-    const safeNext = Math.max(0, next)
+      // enforce non-negative values
+      const safeNext = Math.max(0, next)
 
-    const idx = counters.findIndex((c) => c.id === id)
+      const idx = counters.findIndex((c) => c.id === id)
-    if (idx === -1) return
+      if (idx === -1) return
 
-    const prev = counters[idx].value
+      const prev = counters[idx].value
-    // optimistic update (use safeNext)
+      // optimistic update (use safeNext)
-    setCounters((prevList) => prevList.map((c) => (c.id === id ? { ...c, value: safeNext } : c)))
+      setCounters((prevList) => prevList.map((c) => (c.id === id ? { ...c, value: safeNext } : c)))
-    setSavingIds((s) => {
-      const n = new Set(s)
-      n.add(id)
-      return n
-    })
-
-    try {
-      const res = await fetch(apiUrl(`/api/counters/${id}`), {
-        method: 'PATCH',
-        headers: { 'Content-Type': 'application/json' },
-        body: JSON.stringify({ value: safeNext }),
-      })
-      if (!res.ok) throw new Error('patch failed')
-      const updated: CounterRecord = await res.json().catch(() => null)
-      if (updated) {
-        setCounters((prevList) => prevList.map((c) => (c.id === updated.id ? updated : c)))
-      }
-    } catch (err) {
-      console.error('save failed', err)
-      // revert
-      setCounters((prevList) => prevList.map((c) => (c.id === id ? { ...c, value: prev } : c)))
-    } finally {
       setSavingIds((s) => {
         const n = new Set(s)
-        n.delete(id)
+        n.add(id)
         return n
       })
-    }
-  }
 
-  const handleCreated = (c: CounterRecord) => {
+      try {
+        const res = await fetch(apiUrl(`/api/counters/${id}`), {
+          method: 'PATCH',
+          headers: { 'Content-Type': 'application/json' },
+          body: JSON.stringify({ value: safeNext }),
+        })
+        if (!res.ok) throw new Error('patch failed')
+        const updated: CounterRecord = await res.json().catch(() => null)
+        if (updated) {
+          setCounters((prevList) => prevList.map((c) => (c.id === updated.id ? updated : c)))
+        }
+      } catch (err) {
+        console.error('save failed', err)
+        // revert
+        setCounters((prevList) => prevList.map((c) => (c.id === id ? { ...c, value: prev } : c)))
+      } finally {
+        setSavingIds((s) => {
+          const n = new Set(s)
+          n.delete(id)
+          return n
+        })
+      }
+    },
+    [counters]
+  )
+
+  const handleCreated = useCallback((c: CounterRecord) => {
     setCounters((prev) => [...prev, c])
-  }
+  }, [])
 
-  // onUpdate: upload file (if present), then PATCH name/imageUrl for given id
+  const handleUpdate = useCallback(
-  const handleUpdate = async (id: number, payload: { name?: string; file?: File | null }) => {
+    async (id: number, payload: { name?: string; file?: File | null }) => {
-    setSavingIds((s) => {
-      const n = new Set(s)
-      n.add(id)
-      return n
-    })
-
-    try {
-      let imageUrl: string | null | undefined = undefined
-
-      if (payload.file) {
-        const fd = new FormData()
-        fd.append('file', payload.file)
-        const up = await fetch(apiUrl('/api/upload'), { method: 'POST', body: fd })
-        if (!up.ok) throw new Error('upload failed')
-        const body = await up.json().catch(() => null)
-        imageUrl = body?.url ?? null
-      }
-
-      const patchBody: any = {}
-      if (payload.name !== undefined) patchBody.name = payload.name
-      if (payload.file) patchBody.imageUrl = imageUrl
-
-      if (Object.keys(patchBody).length === 0) {
-        // nothing to do
-        return
-      }
-
-      const res = await fetch(apiUrl(`/api/counters/${id}`), {
-        method: 'PATCH',
-        headers: { 'Content-Type': 'application/json' },
-        body: JSON.stringify(patchBody),
-      })
-      if (!res.ok) throw new Error(`update failed (${res.status})`)
-      const updated: CounterRecord = await res.json().catch(() => null)
-      if (updated) {
-        setCounters((prevList) => prevList.map((c) => (c.id === updated.id ? updated : c)))
-      } else {
-        // best-effort local apply if server didn't return the updated object
-        setCounters((prevList) =>
-          prevList.map((c) => (c.id === id ? { ...c, name: payload.name ?? c.name, imageUrl: imageUrl ?? c.imageUrl } : c))
-        )
-      }
-    } catch (err) {
-      console.error('update failed', err)
-      throw err
-    } finally {
       setSavingIds((s) => {
         const n = new Set(s)
-        n.delete(id)
+        n.add(id)
         return n
       })
-    }
-  }
 
-  // added delete handler
+      try {
-  const handleDelete = async (id: number) => {
+        let imageUrl: string | null | undefined = undefined
-    setSavingIds((s) => {
-      const n = new Set(s)
-      n.add(id)
-      return n
-    })
 
-    try {
+        if (payload.file) {
-      const res = await fetch(apiUrl(`/api/counters/${id}`), { method: 'DELETE' })
+          const fd = new FormData()
-      if (!res.ok) throw new Error(`delete failed (${res.status})`)
+          fd.append('file', payload.file)
-      // remove locally
+          const up = await fetch(apiUrl('/api/upload'), { method: 'POST', body: fd })
-      setCounters((prev) => prev.filter((c) => c.id !== id))
+          if (!up.ok) throw new Error('upload failed')
-    } catch (err) {
+          const body = await up.json().catch(() => null)
-      console.error('delete failed', err)
+          imageUrl = body?.url ?? null
-      throw err
+        }
-    } finally {
+
+        const patchBody: any = {}
+        if (payload.name !== undefined) patchBody.name = payload.name
+        if (payload.file) patchBody.imageUrl = imageUrl
+
+        if (Object.keys(patchBody).length === 0) {
+          // nothing to do
+          return
+        }
+
+        const res = await fetch(apiUrl(`/api/counters/${id}`), {
+          method: 'PATCH',
+          headers: { 'Content-Type': 'application/json' },
+          body: JSON.stringify(patchBody),
+        })
+        if (!res.ok) throw new Error(`update failed (${res.status})`)
+        const updated: CounterRecord = await res.json().catch(() => null)
+        if (updated) {
+          setCounters((prevList) => prevList.map((c) => (c.id === updated.id ? updated : c)))
+        } else {
+          // best-effort local apply if server didn't return the updated object
+          setCounters((prevList) =>
+            prevList.map((c) => (c.id === id ? { ...c, name: payload.name ?? c.name, imageUrl: imageUrl ?? c.imageUrl } : c))
+          )
+        }
+      } catch (err) {
+        console.error('update failed', err)
+        throw err
+      } finally {
+        setSavingIds((s) => {
+          const n = new Set(s)
+          n.delete(id)
+          return n
+        })
+      }
+    },
+    []
+  )
+
+  const handleDelete = useCallback(
+    async (id: number) => {
       setSavingIds((s) => {
         const n = new Set(s)
-        n.delete(id)
+        n.add(id)
         return n
       })
-    }
+
-  }
+      try {
+        const res = await fetch(apiUrl(`/api/counters/${id}`), { method: 'DELETE' })
+        if (!res.ok) throw new Error(`delete failed (${res.status})`)
+        // remove locally
+        setCounters((prev) => prev.filter((c) => c.id !== id))
+      } catch (err) {
+        console.error('delete failed', err)
+        throw err
+      } finally {
+        setSavingIds((s) => {
+          const n = new Set(s)
+          n.delete(id)
+          return n
+        })
+      }
+    },
+    []
+  )
 
   if (loading) return <div className="loading">Loading counters…</div>
 
@@ -267,31 +294,34 @@ export default function App() {
             <p>Create your first counter to begin tracking.</p>
           </div>
         ) : (
-          <div className="counters-grid" style={{ display: 'grid', gap: 16, gridTemplateColumns: 'repeat(auto-fit, minmax(260px, 1fr))' }}>
+          <div className="counters-grid">
-            {counters.map((c) => (
+            {counters.map((c) => {
-              <div
+              return (
-                key={c.id}
+                <div
-                className={`counter-card card ${draggingId === c.id ? 'dragging' : ''} ${dragOverId === c.id ? 'drag-over' : ''}`}
+                  key={c.id}
-                draggable={true}
+                  className={`counter-card card ${draggingId === c.id ? 'dragging' : ''} ${dragOverId === c.id ? 'drag-over' : ''}`}
-                onDragStart={(e) => handleDragStart(e, c.id)}
+                  draggable={draggingEnabled}
-                onDragEnter={(e) => handleDragOver(e, c.id)}
+                  onDragStart={draggingEnabled ? (e) => handleDragStart(e, c.id) : undefined}
-                onDragOver={(e) => handleDragOver(e, c.id)}
+                  onDragEnter={draggingEnabled ? (e) => handleDragOver(e, c.id) : undefined}
-                onDrop={(e) => handleDropOnCard(e, c.id)}
+                  onDragOver={draggingEnabled ? (e) => handleDragOver(e, c.id) : undefined}
-                onDragEnd={handleDragEnd}
+                  onDrop={draggingEnabled ? (e) => handleDropOnCard(e, c.id) : undefined}
-              >
+                  onDragEnd={draggingEnabled ? handleDragEnd : undefined}
-                <Counter
+                >
-                  id={String(c.id)}
+                  <Counter
-                  label={c.name ?? 'Counter'}
+                    id={String(c.id)}
-                  value={c.value}
+                    label={c.name ?? 'Counter'}
-                  min={0}
+                    value={c.value}
-                  isSaving={savingIds.has(c.id)}
+                    min={0}
-                  imageUrl={c.imageUrl}
+                    isSaving={savingIds.has(c.id)}
-                  onChange={(next) => handleChange(c.id, next)}
+                    imageUrl={c.imageUrl}
-                  onUpdate={(payload) => handleUpdate(c.id, payload)}
+                    onChange={(next) => handleChange(c.id, next)}
-                  onDelete={() => handleDelete(c.id)}
+                    onUpdate={(payload) => handleUpdate(c.id, payload)}
-                />
+                    onDelete={() => handleDelete(c.id)}
-              </div>
+                    onEditingChange={(editing: boolean) => handleEditingChange(c.id, editing)}
-            ))}
+                  />
+                </div>
+              )
+            })}
           </div>
         )}
       </main>

frontend/src/components/Counter.tsx

@@ -15,6 +15,7 @@ interface CounterProps {
 
   onUpdate?: (payload: { name?: string; file?: File | null }) => Promise<void> | void
   onDelete?: () => Promise<void> | void    // added delete prop
+  onEditingChange?: (editing: boolean) => void
 }
 
 const clamp = (v: number, min?: number, max?: number) => {
@@ -36,6 +37,7 @@ const Counter: React.FC<CounterProps> = ({
   isSaving = false,
   onUpdate,
   onDelete,
+  onEditingChange,
 }) => {
   const setValue = useCallback(
     (next: number) => {
@@ -97,23 +99,26 @@ const Counter: React.FC<CounterProps> = ({
     setEditName(label ?? '')
     setEditFile(null)
     setEditing(true)
-  }, [label])
+  }, [label, onEditingChange])
 
   const closeEdit = useCallback(() => {
     setEditing(false)
     setEditFile(null)
     setPreviewUrl(null)
-  }, [])
+  }, [onEditingChange])
 
   const handleSave = useCallback(async () => {
     if (!onUpdate) {
+      // close and notify parent
       setEditing(false)
+      onEditingChange?.(false)
       return
     }
     try {
       setSavingLocal(true)
       await onUpdate({ name: editName || undefined, file: editFile })
       setEditing(false)
+      onEditingChange?.(false)
     } catch (err) {
       console.error('update failed', err)
     } finally {
@@ -128,6 +133,7 @@ const Counter: React.FC<CounterProps> = ({
       await onDelete()
       // close modal locally (parent will remove the card)
       setEditing(false)
+      onEditingChange?.(false)
     } catch (err) {
       console.error('delete failed', err)
     } finally {
@@ -135,6 +141,20 @@ const Counter: React.FC<CounterProps> = ({
     }
   }, [onDelete])
 
+  // Call the latest onEditingChange but only re-run when `editing` toggles.
+  // This avoids re-running the effect when the parent's callback identity changes.
+  const onEditingChangeRef = useRef(onEditingChange)
+  useEffect(() => {
+    onEditingChangeRef.current = onEditingChange
+  }, [onEditingChange])
+
+  useEffect(() => {
+    onEditingChangeRef.current?.(editing)
+    return () => {
+      if (editing) onEditingChangeRef.current?.(false)
+    }
+  }, [editing])
+
   return (
     <div
       className={className ?? 'counter'}