Polish: imperial, clear-bed, keyboard nudging, empty states (#18) #37
@@ -4,6 +4,7 @@
|
|||||||
<meta charset="UTF-8" />
|
<meta charset="UTF-8" />
|
||||||
<meta name="viewport" content="width=device-width, initial-scale=1.0, viewport-fit=cover" />
|
<meta name="viewport" content="width=device-width, initial-scale=1.0, viewport-fit=cover" />
|
||||||
<meta name="color-scheme" content="light dark" />
|
<meta name="color-scheme" content="light dark" />
|
||||||
|
<link rel="icon" href="data:image/svg+xml,<svg xmlns='http://www.w3.org/2000/svg' viewBox='0 0 100 100'><text y='.9em' font-size='90'>%F0%9F%8C%B1</text></svg>" />
|
||||||
<title>pansy</title>
|
<title>pansy</title>
|
||||||
<meta name="description" content="Self-hostable garden planner — plan beds, containers, and plops of plants at real scale." />
|
<meta name="description" content="Self-hostable garden planner — plan beds, containers, and plops of plants at real scale." />
|
||||||
</head>
|
</head>
|
||||||
|
|||||||
@@ -0,0 +1,20 @@
|
|||||||
|
import { Link } from '@tanstack/react-router'
|
||||||
|
import { buttonClasses } from '@/components/ui/Button'
|
||||||
|
import { usePageTitle } from '@/lib/usePageTitle'
|
||||||
|
|
||||||
|
/** The router's catch-all for unknown paths. */
|
||||||
|
export function NotFound() {
|
||||||
|
usePageTitle('Not found')
|
||||||
|
return (
|
||||||
|
<div className="mx-auto flex min-h-[60vh] w-full max-w-sm flex-col items-center justify-center gap-4 text-center">
|
||||||
|
<p className="text-5xl" aria-hidden>
|
||||||
|
🌱
|
||||||
|
</p>
|
||||||
|
<h1 className="text-lg font-semibold text-fg">Page not found</h1>
|
||||||
|
<p className="text-sm text-muted">That page doesn't exist — the link may be wrong or the page moved.</p>
|
||||||
|
<Link to="/gardens" className={buttonClasses('primary')}>
|
||||||
|
|
|||||||
|
Back to gardens
|
||||||
|
</Link>
|
||||||
|
</div>
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -0,0 +1,43 @@
|
|||||||
|
import { Modal } from '@/components/ui/Modal'
|
||||||
|
import { Button } from '@/components/ui/Button'
|
||||||
|
import { useClearObject } from '@/lib/objects'
|
||||||
|
|
||||||
|
/** Confirm clearing every active plop from a focused bed (soft-remove — the rows
|
||||||
|
* are kept with removed_at, so history survives). */
|
||||||
|
export function ClearBedModal({
|
||||||
|
objectName,
|
||||||
|
plops,
|
||||||
|
gardenId,
|
||||||
|
onClose,
|
||||||
|
}: {
|
||||||
|
objectName: string
|
||||||
|
plops: { id: number; version: number }[]
|
||||||
|
gardenId: number
|
||||||
|
onClose: () => void
|
||||||
|
}) {
|
||||||
|
const clear = useClearObject(gardenId)
|
||||||
|
const n = plops.length
|
||||||
|
return (
|
||||||
|
<Modal title="Clear bed" onClose={onClose} busy={clear.isPending}>
|
||||||
|
<div className="flex flex-col gap-4">
|
||||||
|
<p className="text-sm text-muted">
|
||||||
|
Remove all <span className="font-medium text-fg">{n}</span> {n === 1 ? 'plant' : 'plants'} from{' '}
|
||||||
|
<span className="font-medium text-fg">{objectName}</span>? They're marked removed but kept in history.
|
||||||
|
</p>
|
||||||
|
<div className="flex justify-end gap-2">
|
||||||
|
<Button type="button" variant="ghost" onClick={onClose} disabled={clear.isPending}>
|
||||||
|
Cancel
|
||||||
|
</Button>
|
||||||
|
<Button
|
||||||
|
type="button"
|
||||||
|
variant="danger"
|
||||||
|
disabled={clear.isPending}
|
||||||
|
onClick={() => clear.mutate(plops, { onSuccess: onClose })}
|
||||||
|
gitea-actions
commented
🟠 Clear-bed button not disabled for empty plops array, causing no-op confirm error-handling · flagged by 1 model
🪰 Gadfly · advisory 🟠 **Clear-bed button not disabled for empty plops array, causing no-op confirm**
_error-handling · flagged by 1 model_
- **Unchecked empty-array guard in `ClearBedModal`** — `web/src/editor/ClearBedModal.tsx:35` If `plops` is empty (possible if the caller races with a concurrent delete or a stale focused-object ID), `clear.mutate([])` fires a no-op mutation that succeeds instantly and closes the modal. It does not crash, but it silently consumes a user confirmation for nothing. More critically, `useClearObject` loops over the array with `Promise.all`; an empty array resolves immediately, making the UX misleading…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
>
|
||||||
|
{clear.isPending ? 'Clearing…' : 'Clear bed'}
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</Modal>
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -37,6 +37,9 @@ export const PlopMarker = memo(function PlopMarker({
|
|||||||
const showIcon = scale >= SEMANTIC_FAR && !!plant?.icon
|
const showIcon = scale >= SEMANTIC_FAR && !!plant?.icon
|
||||||
const showText = scale >= SEMANTIC_NEAR && !!plant
|
const showText = scale >= SEMANTIC_NEAR && !!plant
|
||||||
const count = plop.count ?? (plant ? computeDerivedCount(plop.radiusCm, plant.spacingCm) : plop.derivedCount)
|
const count = plop.count ?? (plant ? computeDerivedCount(plop.radiusCm, plant.spacingCm) : plop.derivedCount)
|
||||||
|
// Keep the tap target at least ~44px across even when the plop draws tiny at
|
||||||
|
// low zoom (fill=transparent still receives pointer events, unlike fill=none).
|
||||||
|
const hitR = Math.max(r, 22 / scale)
|
||||||
|
|
||||||
function down(e: PointerEvent) {
|
function down(e: PointerEvent) {
|
||||||
e.stopPropagation()
|
e.stopPropagation()
|
||||||
@@ -45,6 +48,7 @@ export const PlopMarker = memo(function PlopMarker({
|
|||||||
|
|
||||||
return (
|
return (
|
||||||
<g transform={`translate(${plop.xCm} ${plop.yCm})`} onPointerDown={down} style={{ cursor: 'pointer' }}>
|
<g transform={`translate(${plop.xCm} ${plop.yCm})`} onPointerDown={down} style={{ cursor: 'pointer' }}>
|
||||||
|
<circle cx={0} cy={0} r={hitR} fill="transparent" />
|
||||||
<circle
|
<circle
|
||||||
cx={0}
|
cx={0}
|
||||||
cy={0}
|
cy={0}
|
||||||
|
|||||||
@@ -263,6 +263,23 @@ export function useUpdatePlanting(gardenId: number) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Clear a bed: soft-remove every active plop in an object (a loop of PATCHes;
|
||||||
|
* a bulk ClearObject endpoint arrives with the agent seam, #19). Invalidates
|
||||||
|
* once at the end. Pass the object's active plops (id + current version). */
|
||||||
|
export function useClearObject(gardenId: number) {
|
||||||
|
gitea-actions
commented
🔴 useClearObject lacks optimistic update and partial-failure handling correctness, error-handling, maintainability · flagged by 4 models
🪰 Gadfly · advisory 🔴 **useClearObject lacks optimistic update and partial-failure handling**
_correctness, error-handling, maintainability · flagged by 4 models_
- **`web/src/lib/objects.ts:269-281` — `useClearObject` has no optimistic update and no partial-failure handling** `Promise.all` patches each plop individually. If one request fails, `onError` fires but `onSuccess` does not, so the cache is never invalidated. Any plops that were successfully soft-removed on the server still appear in the UI until a manual refresh. There is also no 409 conflict recovery (unlike `useRemovePlanting`). **Fix:** Add an `onMutate` that filters the plops from the cache…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
const qc = useQueryClient()
|
||||||
|
return useMutation({
|
||||||
|
mutationFn: async (plops: { id: number; version: number }[]) => {
|
||||||
|
const today = new Date().toISOString().slice(0, 10)
|
||||||
|
await Promise.all(
|
||||||
|
gitea-actions
commented
🔴 Promise.all is all-or-nothing: successfully soft-removed rows are lost from the cache when one PATCH rejects, because onSuccess (the only invalidate) never runs error-handling, performance · flagged by 4 models
🪰 Gadfly · advisory 🔴 **Promise.all is all-or-nothing: successfully soft-removed rows are lost from the cache when one PATCH rejects, because onSuccess (the only invalidate) never runs**
_error-handling, performance · flagged by 4 models_
- **`web/src/lib/objects.ts:274-276` — `Promise.all` is all-or-nothing; partial success is silently lost.** Verified: the mutation only invalidates on `onSuccess` (line 278), so a single rejected PATCH throws away the fact that the others already soft-removed their rows server-side. The cache remains stale for the successfully-removed plops until the next external refetch. Same location as above; the fix (per-row `useRemovePlanting`, or `Promise.allSettled` + reconcile + invalidate-on-failure) c…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
plops.map((p) => api.patch(`/plantings/${p.id}`, { removedAt: today, version: p.version })),
|
||||||
|
)
|
||||||
|
},
|
||||||
|
onSuccess: () => qc.invalidateQueries({ queryKey: fullKey(gardenId) }),
|
||||||
|
gitea-actions
commented
🟠 useClearObject: Promise.all rejection on one PATCH leaves other successful soft-removes uncommitted to the cache — no invalidate/reconciliation on error, so UI misreports partial failures as total failure correctness, error-handling · flagged by 2 models
🪰 Gadfly · advisory 🟠 **useClearObject: Promise.all rejection on one PATCH leaves other successful soft-removes uncommitted to the cache — no invalidate/reconciliation on error, so UI misreports partial failures as total failure**
_correctness, error-handling · flagged by 2 models_
* `web/src/lib/objects.ts:278` — `useClearObject` fires a `Promise.all` of soft-remove PATCHes but has **no optimistic `onMutate`** and only invalidates on `onSuccess`. If one PATCH fails (e.g., version conflict) while others succeed, `onError` fires, the cache is never invalidated, and the UI continues displaying plants that were already removed on the server. A retry will then 409 on the already-removed rows because their versions were bumped by the successful PATCHes. **Fix:** add an `onMutat…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
onError: (err) => toast.error(objectErrorMessage(err, 'Could not clear the bed.')),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
/** Soft-remove ("Remove"): PATCH removed_at to today; the row is kept but leaves
|
/** Soft-remove ("Remove"): PATCH removed_at to today; the row is kept but leaves
|
||||||
* the active list. Distinct from a hard delete. */
|
* the active list. Distinct from a hard delete. */
|
||||||
export function useRemovePlanting(gardenId: number) {
|
export function useRemovePlanting(gardenId: number) {
|
||||||
|
|||||||
@@ -0,0 +1,13 @@
|
|||||||
|
import { useEffect } from 'react'
|
||||||
|
|
||||||
|
/** Set the document title for the current route (suffixed with · pansy), and
|
||||||
|
* restore the previous title on unmount. */
|
||||||
|
export function usePageTitle(title: string): void {
|
||||||
|
useEffect(() => {
|
||||||
|
gitea-actions
commented
🟠 Page title restoration races on rapid route changes, can clobber active title correctness, error-handling, maintainability · flagged by 3 models
🪰 Gadfly · advisory 🟠 **Page title restoration races on rapid route changes, can clobber active title**
_correctness, error-handling, maintainability · flagged by 3 models_
- **`web/src/lib/usePageTitle.ts:6-12` — `usePageTitle` restore-on-unmount can restore a stale route title across SPA navigations (minor).** On mount it captures `prev = document.title` (the *previous* route's title) and restores that exact string on unmount. Navigating Gardens → GardenEditor → Plants: the GardenEditor effect captures `prev = "Gardens · pansy"`; when GardenEditor unmounts it restores `"Gardens · pansy"` instead of a neutral default, and depending on React's effect/cleanup orderi…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
const prev = document.title
|
||||||
|
document.title = title ? `${title} · pansy` : 'pansy'
|
||||||
|
return () => {
|
||||||
|
document.title = prev
|
||||||
|
}
|
||||||
|
}, [title])
|
||||||
|
}
|
||||||
@@ -1,4 +1,4 @@
|
|||||||
import { useEffect, useMemo, useState } from 'react'
|
import { useEffect, useMemo, useRef, useState } from 'react'
|
||||||
import { getRouteApi } from '@tanstack/react-router'
|
import { getRouteApi } from '@tanstack/react-router'
|
||||||
import { Alert } from '@/components/ui/Alert'
|
import { Alert } from '@/components/ui/Alert'
|
||||||
import { Button } from '@/components/ui/Button'
|
import { Button } from '@/components/ui/Button'
|
||||||
@@ -7,13 +7,15 @@ import { Inspector } from '@/editor/Inspector'
|
|||||||
import { PlopInspector } from '@/editor/PlopInspector'
|
import { PlopInspector } from '@/editor/PlopInspector'
|
||||||
import { PlantPicker } from '@/editor/PlantPicker'
|
import { PlantPicker } from '@/editor/PlantPicker'
|
||||||
import { Palette } from '@/editor/Palette'
|
import { Palette } from '@/editor/Palette'
|
||||||
|
import { ClearBedModal } from '@/editor/ClearBedModal'
|
||||||
import { kindDef } from '@/editor/kinds'
|
import { kindDef } from '@/editor/kinds'
|
||||||
import { useEditorStore } from '@/editor/store'
|
import { useEditorStore } from '@/editor/store'
|
||||||
import type { EditorGarden } from '@/editor/types'
|
import type { EditorGarden } from '@/editor/types'
|
||||||
import { ShareGardenModal } from '@/components/gardens/ShareGardenModal'
|
import { ShareGardenModal } from '@/components/gardens/ShareGardenModal'
|
||||||
import { useMe } from '@/lib/auth'
|
import { useMe } from '@/lib/auth'
|
||||||
import { toEditorObject, useGardenFull, useUpdatePlanting } from '@/lib/objects'
|
import { toEditorObject, useGardenFull, useUpdateObject, useUpdatePlanting } from '@/lib/objects'
|
||||||
import { toEditorPlanting } from '@/lib/plantings'
|
import { toEditorPlanting } from '@/lib/plantings'
|
||||||
|
import { usePageTitle } from '@/lib/usePageTitle'
|
||||||
|
|
||||||
const routeApi = getRouteApi('/gardens/$gardenId')
|
const routeApi = getRouteApi('/gardens/$gardenId')
|
||||||
|
|
||||||
@@ -24,6 +26,7 @@ export function GardenEditorPage() {
|
|||||||
const navigate = routeApi.useNavigate()
|
const navigate = routeApi.useNavigate()
|
||||||
const full = useGardenFull(gid)
|
const full = useGardenFull(gid)
|
||||||
const me = useMe()
|
const me = useMe()
|
||||||
|
usePageTitle(full.data?.garden.name ?? 'Garden')
|
||||||
|
|
||||||
const selectedId = useEditorStore((s) => s.selectedId)
|
const selectedId = useEditorStore((s) => s.selectedId)
|
||||||
const select = useEditorStore((s) => s.select)
|
const select = useEditorStore((s) => s.select)
|
||||||
@@ -38,11 +41,14 @@ export function GardenEditorPage() {
|
|||||||
const setLivePlanting = useEditorStore((s) => s.setLivePlanting)
|
const setLivePlanting = useEditorStore((s) => s.setLivePlanting)
|
||||||
|
|
||||||
const updatePlanting = useUpdatePlanting(gid)
|
const updatePlanting = useUpdatePlanting(gid)
|
||||||
|
const updateObject = useUpdateObject(gid)
|
||||||
|
|
||||||
// Which plant-picker flow is open: 'place' arms a plant for repeat placement;
|
// Which plant-picker flow is open: 'place' arms a plant for repeat placement;
|
||||||
// 'change' swaps the selected plop's plant.
|
// 'change' swaps the selected plop's plant.
|
||||||
const [picker, setPicker] = useState<'place' | 'change' | null>(null)
|
const [picker, setPicker] = useState<'place' | 'change' | null>(null)
|
||||||
const [sharing, setSharing] = useState(false)
|
const [sharing, setSharing] = useState(false)
|
||||||
|
const [clearing, setClearing] = useState(false)
|
||||||
|
const nudgeTimer = useRef<number | null>(null)
|
||||||
|
|
||||||
const serverObjects = full.data?.objects
|
const serverObjects = full.data?.objects
|
||||||
const objects = useMemo(() => serverObjects?.map(toEditorObject) ?? [], [serverObjects])
|
const objects = useMemo(() => serverObjects?.map(toEditorObject) ?? [], [serverObjects])
|
||||||
@@ -51,6 +57,12 @@ export function GardenEditorPage() {
|
|||||||
const plants = useMemo(() => full.data?.plants ?? [], [full.data?.plants])
|
const plants = useMemo(() => full.data?.plants ?? [], [full.data?.plants])
|
||||||
const plantsById = useMemo(() => new Map(plants.map((p) => [p.id, p])), [plants])
|
const plantsById = useMemo(() => new Map(plants.map((p) => [p.id, p])), [plants])
|
||||||
|
|
||||||
|
// Role gating, computed before the effects/returns so the nudge handler can use
|
||||||
|
// it. Ownership is the authoritative ownerId==me check.
|
||||||
|
const gd = full.data?.garden
|
||||||
|
const canEdit = gd != null && (gd.ownerId === me.data?.id || gd.myRole === 'editor')
|
||||||
|
gitea-actions
commented
⚪ canEdit and isOwner use inconsistent null-check idioms for the same ownerId check maintainability · flagged by 1 model 🪰 Gadfly · advisory ⚪ **canEdit and isOwner use inconsistent null-check idioms for the same ownerId check**
_maintainability · flagged by 1 model_
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
const isOwner = gd != null && me.data != null && gd.ownerId === me.data.id
|
||||||
|
|
||||||
// Adopt the URL's focus and clear transient editor state on entering/switching
|
// 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,
|
// gardens (and on leaving). `focus` is intentionally read once here, not a dep,
|
||||||
// so URL syncs below don't retrigger a full reset.
|
// so URL syncs below don't retrigger a full reset.
|
||||||
@@ -108,6 +120,71 @@ export function GardenEditorPage() {
|
|||||||
// eslint-disable-next-line react-hooks/exhaustive-deps
|
// eslint-disable-next-line react-hooks/exhaustive-deps
|
||||||
}, [])
|
}, [])
|
||||||
|
|
||||||
|
// 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.
|
||||||
|
useEffect(() => {
|
||||||
|
gitea-actions
commented
🔴 Nudge timer cancelled by re-renders due to unstable useEffect deps correctness, error-handling, performance · flagged by 3 models
🪰 Gadfly · advisory 🔴 **Nudge timer cancelled by re-renders due to unstable useEffect deps**
_correctness, error-handling, performance · flagged by 3 models_
* `web/src/pages/GardenEditorPage.tsx:126-186` — The keyboard-nudge `useEffect` depends on `[canEdit, objects, plantings]`. Both `objects` and `plantings` are fresh arrays created by `useMemo` on every change to the underlying server data (even identical data, because React Query’s structural sharing only preserves subtree references when the data is unchanged, and any concurrent edit or background refetch can still produce new top-level array references). The effect’s cleanup function unconditi…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
if (!canEdit) return
|
||||||
|
const DIRS: Record<string, [number, number]> = {
|
||||||
|
ArrowUp: [0, -1],
|
||||||
|
ArrowDown: [0, 1],
|
||||||
|
ArrowLeft: [-1, 0],
|
||||||
|
ArrowRight: [1, 0],
|
||||||
|
}
|
||||||
|
const commitLater = (fire: () => void) => {
|
||||||
|
gitea-actions
commented
🔴 Keyboard nudge debounce races with drag gestures, can revert drag results correctness · flagged by 1 model
🪰 Gadfly · advisory 🔴 **Keyboard nudge debounce races with drag gestures, can revert drag results**
_correctness · flagged by 1 model_
* **Keyboard nudge debounce races with drag gestures** — `web/src/pages/GardenEditorPage.tsx:134-186` The 400ms debounced PATCH from arrow-key nudging captures the nudged position in a closure (`next`). If the user starts and finishes a drag gesture (object move in `SelectionOverlay` or plop move in `PlopOverlay`) before that timer fires, both PATCHes fly with the same stale `version`. Whichever reaches the server second 409s; depending on network ordering the drag result can be silently lost an…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
if (nudgeTimer.current != null) window.clearTimeout(nudgeTimer.current)
|
||||||
|
nudgeTimer.current = window.setTimeout(() => {
|
||||||
|
fire()
|
||||||
|
nudgeTimer.current = null
|
||||||
|
}, 400)
|
||||||
|
}
|
||||||
|
function onKey(e: KeyboardEvent) {
|
||||||
|
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()
|
||||||
|
const step = e.shiftKey ? 10 : 1
|
||||||
|
if (s.selectedId != null) {
|
||||||
|
gitea-actions
commented
🟡 Object-nudge and plop-nudge branches duplicate the same find-base/compute-next/setLive/commitLater structure maintainability · flagged by 3 models
🪰 Gadfly · advisory 🟡 **Object-nudge and plop-nudge branches duplicate the same find-base/compute-next/setLive/commitLater structure**
_maintainability · flagged by 3 models_
- `web/src/pages/GardenEditorPage.tsx:148-178` — the keyboard-nudge `onKey` handler has two branches (object nudge vs plop nudge) that repeat the same shape: resolve `base` from live-state-or-list, `e.preventDefault()`, compute `next`, call the matching `setLive*`, then `commitLater(() => { mutate(...); resetLive() })`. Confirmed by reading the full effect (lines 126-186) — the only real differences are the source list/live-state pair, the optional bounds clamp for plops, and which mutation hook…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
const base = s.liveObject?.id === s.selectedId ? s.liveObject : objects.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)
|
||||||
|
commitLater(() => {
|
||||||
|
gitea-actions
commented
🔴 Nudge debounce timer can stomp newer liveObject/livePlanting from a subsequent drag gesture error-handling · flagged by 1 model
🪰 Gadfly · advisory 🔴 **Nudge debounce timer can stomp newer liveObject/livePlanting from a subsequent drag gesture**
_error-handling · flagged by 1 model_
- **`web/src/pages/GardenEditorPage.tsx:154` — Debounced nudge timer can clear a newer live gesture** The `commitLater` closure calls `setLiveObject(null)` / `setLivePlanting(null)` when it fires, without checking whether `liveObject`/`livePlanting` is still the nudged entity. If the user starts dragging another object (or another plop) during the 400 ms debounce window, the timer will overwrite that newer live state and snap the dragged item back to its committed position. **Fix:** In the timer…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
updateObject.mutate({ id: next.id, version: next.version, xCm: next.xCm, yCm: next.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)
|
||||||
|
if (!base) return
|
||||||
|
e.preventDefault()
|
||||||
|
const obj = objects.find((o) => o.id === base.objectId)
|
||||||
|
let nx = base.xCm + dir[0] * step
|
||||||
|
let ny = base.yCm + dir[1] * step
|
||||||
|
if (obj) {
|
||||||
|
gitea-actions
commented
🔴 Nudge clamping ignores object rotation, letting plops escape rotated beds error-handling · flagged by 1 model
🪰 Gadfly · advisory 🔴 **Nudge clamping ignores object rotation, letting plops escape rotated beds**
_error-handling · flagged by 1 model_
- **Boundary / off-by-one in nudge clamping for non-square objects** — `web/src/pages/GardenEditorPage.tsx:168-171` Plops are clamped to `[-widthCm/2, widthCm/2]` and `[-heightCm/2, heightCm/2]`. If the object has been rotated (`rotationDeg != 0`), the plop’s local `xCm/yCm` frame is rotated relative to the object’s bounding box; clamping axis-aligned coordinates against the unrotated width/height allows the plop to drift outside the actual bed. **Fix:** apply the inverse rotation to the plop co…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
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)
|
||||||
|
commitLater(() => {
|
||||||
|
updatePlanting.mutate({ id: next.id, version: next.version, xCm: next.xCm, yCm: next.yCm })
|
||||||
|
useEditorStore.getState().setLivePlanting(null)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
window.addEventListener('keydown', onKey)
|
||||||
|
return () => {
|
||||||
|
window.removeEventListener('keydown', onKey)
|
||||||
|
if (nudgeTimer.current != null) window.clearTimeout(nudgeTimer.current)
|
||||||
|
gitea-actions
commented
🔴 Debounced keyboard-nudge commit is discarded (not flushed) on effect cleanup, silently dropping unsaved moves while the canvas keeps showing the phantom position correctness, error-handling, performance · flagged by 2 models
🪰 Gadfly · advisory 🔴 **Debounced keyboard-nudge commit is discarded (not flushed) on effect cleanup, silently dropping unsaved moves while the canvas keeps showing the phantom position**
_correctness, error-handling, performance · flagged by 2 models_
- **`web/src/pages/GardenEditorPage.tsx:186`** — The keyboard nudge `useEffect` lists `objects` and `plantings` in its dependency array. Because `patchFullCache` (called by `useUpdateObject` / `useUpdatePlanting` optimistic updates) returns new array references on every cache mutation, this effect tears down and re-adds the global `keydown` listener repeatedly during normal editing. The previous effect’s cleanup clears `nudgeTimer.current`, so if **any** mutation patches the garden cache while a…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
}
|
||||||
|
// 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.isPending) return <p className="p-6 text-sm text-muted">Loading garden…</p>
|
||||||
if (full.isError)
|
if (full.isError)
|
||||||
return (
|
return (
|
||||||
@@ -124,16 +201,12 @@ export function GardenEditorPage() {
|
|||||||
heightCm: g.heightCm,
|
heightCm: g.heightCm,
|
||||||
unitPref: g.unitPref,
|
unitPref: g.unitPref,
|
||||||
}
|
}
|
||||||
// Role-driven gating. Ownership is the authoritative ownerId==me check (so a
|
|
||||||
// missing my_role can never lock an owner out); edit access is owner or an
|
|
||||||
// editor share.
|
|
||||||
const isOwner = me.data != null && g.ownerId === me.data.id
|
|
||||||
const canEdit = isOwner || g.myRole === 'editor'
|
|
||||||
|
|
||||||
const selectedObject = objects.find((o) => o.id === selectedId) ?? null
|
const selectedObject = objects.find((o) => o.id === selectedId) ?? null
|
||||||
const focusedObject = focusedObjectId != null ? objects.find((o) => o.id === focusedObjectId) ?? null : null
|
const focusedObject = focusedObjectId != null ? objects.find((o) => o.id === focusedObjectId) ?? null : null
|
||||||
// Committed selected plop; PlopInspector applies live drag geometry itself.
|
// Committed selected plop; PlopInspector applies live drag geometry itself.
|
||||||
const selectedPlop = plantings.find((p) => p.id === selectedPlantingId) ?? null
|
const selectedPlop = plantings.find((p) => p.id === selectedPlantingId) ?? null
|
||||||
|
const focusedPlops = focusedObjectId != null ? plantings.filter((p) => p.objectId === focusedObjectId) : []
|
||||||
|
|
||||||
function onPickPlant(plantId: number) {
|
function onPickPlant(plantId: number) {
|
||||||
if (!canEdit) return // defense in depth: viewers can't reach the picker anyway
|
if (!canEdit) return // defense in depth: viewers can't reach the picker anyway
|
||||||
@@ -185,9 +258,34 @@ export function GardenEditorPage() {
|
|||||||
+ Add plant
|
+ Add plant
|
||||||
</Button>
|
</Button>
|
||||||
))}
|
))}
|
||||||
|
{canEdit && focusedObject.plantable && focusedPlops.length > 0 && (
|
||||||
|
<Button
|
||||||
|
variant="ghost"
|
||||||
|
className="px-2 py-1 text-xs text-red-600 dark:text-red-400"
|
||||||
|
onClick={() => setClearing(true)}
|
||||||
|
>
|
||||||
|
Clear ({focusedPlops.length})
|
||||||
|
</Button>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
<GardenCanvas garden={garden} objects={objects} plantings={plantings} plantsById={plantsById} canEdit={canEdit} />
|
<GardenCanvas garden={garden} objects={objects} plantings={plantings} plantsById={plantsById} canEdit={canEdit} />
|
||||||
|
|
||||||
|
{/* Empty-state hints (non-interactive overlays). */}
|
||||||
|
{canEdit && focusedObjectId == null && objects.length === 0 && (
|
||||||
|
gitea-actions
commented
🟡 Empty-state overlay markup duplicated inline; extract a small EmptyHint maintainability · flagged by 2 models
🪰 Gadfly · advisory 🟡 **Empty-state overlay markup duplicated inline; extract a small EmptyHint**
_maintainability · flagged by 2 models_
- **`web/src/pages/GardenEditorPage.tsx:275-288` — empty-state overlay markup is duplicated inline.** Two near-identical `<div className="pointer-events-none absolute ..."><p className="max-w-xs rounded-lg bg-surface/85 px-4 py-3 text-sm text-muted shadow-sm backdrop-blur">…</p></div>` blocks differ only in positioning class and message. A tiny `<EmptyHint className=…>…</EmptyHint>` (or even a shared `className` constant for the `<p>`) would dedupe.
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
<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>
|
||||||
|
)}
|
||||||
|
{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>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
{(selectedObject || selectedPlop) && (
|
{(selectedObject || selectedPlop) && (
|
||||||
@@ -229,6 +327,15 @@ export function GardenEditorPage() {
|
|||||||
)}
|
)}
|
||||||
|
|
||||||
{sharing && <ShareGardenModal garden={g} onClose={() => setSharing(false)} />}
|
{sharing && <ShareGardenModal garden={g} onClose={() => setSharing(false)} />}
|
||||||
|
|
||||||
|
{clearing && focusedObject && (
|
||||||
|
<ClearBedModal
|
||||||
|
objectName={focusedObject.name || kindDef(focusedObject.kind)?.label || 'bed'}
|
||||||
|
gitea-actions
commented
🟡 Focused-object fallback-name expression duplicated with inconsistent literal ('Object' vs 'bed') maintainability · flagged by 1 model 🪰 Gadfly · advisory 🟡 **Focused-object fallback-name expression duplicated with inconsistent literal ('Object' vs 'bed')**
_maintainability · flagged by 1 model_
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
plops={focusedPlops.map((p) => ({ id: p.id, version: p.version }))}
|
||||||
|
gardenId={gid}
|
||||||
|
onClose={() => setClearing(false)}
|
||||||
|
/>
|
||||||
|
)}
|
||||||
</div>
|
</div>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import { LeaveGardenModal } from '@/components/gardens/LeaveGardenModal'
|
|||||||
import { ShareGardenModal } from '@/components/gardens/ShareGardenModal'
|
import { ShareGardenModal } from '@/components/gardens/ShareGardenModal'
|
||||||
import { useMe } from '@/lib/auth'
|
import { useMe } from '@/lib/auth'
|
||||||
import { useGardens, type Garden } from '@/lib/gardens'
|
import { useGardens, type Garden } from '@/lib/gardens'
|
||||||
|
import { usePageTitle } from '@/lib/usePageTitle'
|
||||||
|
|
||||||
// Which modal is open, if any. edit/delete/share/leave carry the target garden.
|
// Which modal is open, if any. edit/delete/share/leave carry the target garden.
|
||||||
type Dialog =
|
type Dialog =
|
||||||
@@ -19,6 +20,7 @@ type Dialog =
|
|||||||
| null
|
| null
|
||||||
|
|
||||||
export function GardensPage() {
|
export function GardensPage() {
|
||||||
|
usePageTitle('Gardens')
|
||||||
const gardens = useGardens()
|
const gardens = useGardens()
|
||||||
const me = useMe()
|
const me = useMe()
|
||||||
const [dialog, setDialog] = useState<Dialog>(null)
|
const [dialog, setDialog] = useState<Dialog>(null)
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ import { TextField } from '@/components/ui/TextField'
|
|||||||
import { API_BASE, errorMessage } from '@/lib/api'
|
import { API_BASE, errorMessage } from '@/lib/api'
|
||||||
import { useLogin, useProviders } from '@/lib/auth'
|
import { useLogin, useProviders } from '@/lib/auth'
|
||||||
import { safeRedirectPath } from '@/lib/redirect'
|
import { safeRedirectPath } from '@/lib/redirect'
|
||||||
|
import { usePageTitle } from '@/lib/usePageTitle'
|
||||||
|
|
||||||
// Server-initiated redirect (not a fetch): the browser navigates to the Go
|
// Server-initiated redirect (not a fetch): the browser navigates to the Go
|
||||||
// handler, which 302s to the IdP.
|
// handler, which 302s to the IdP.
|
||||||
@@ -24,6 +25,7 @@ const callbackErrors: Record<string, string> = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function LoginPage() {
|
export function LoginPage() {
|
||||||
|
usePageTitle('Sign in')
|
||||||
const search = useSearch({ from: '/login' })
|
const search = useSearch({ from: '/login' })
|
||||||
const navigate = useNavigate()
|
const navigate = useNavigate()
|
||||||
const providers = useProviders()
|
const providers = useProviders()
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import { DeletePlantModal } from '@/components/plants/DeletePlantModal'
|
|||||||
import { PlantPicker } from '@/editor/PlantPicker'
|
import { PlantPicker } from '@/editor/PlantPicker'
|
||||||
import { filterPlants, usePlants, type CategoryFilter, type Plant } from '@/lib/plants'
|
import { filterPlants, usePlants, type CategoryFilter, type Plant } from '@/lib/plants'
|
||||||
import type { UnitPref } from '@/lib/units'
|
import type { UnitPref } from '@/lib/units'
|
||||||
|
import { usePageTitle } from '@/lib/usePageTitle'
|
||||||
|
|
||||||
// Which modal is open, if any. edit/duplicate/delete carry the target plant.
|
// Which modal is open, if any. edit/duplicate/delete carry the target plant.
|
||||||
type Dialog =
|
type Dialog =
|
||||||
@@ -32,6 +33,7 @@ function loadUnit(): UnitPref {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function PlantsPage() {
|
export function PlantsPage() {
|
||||||
|
usePageTitle('Plants')
|
||||||
const plants = usePlants()
|
const plants = usePlants()
|
||||||
const [unit, setUnit] = useState<UnitPref>(() => loadUnit())
|
const [unit, setUnit] = useState<UnitPref>(() => loadUnit())
|
||||||
const [query, setQuery] = useState('')
|
const [query, setQuery] = useState('')
|
||||||
|
|||||||
@@ -6,10 +6,12 @@ import { Button } from '@/components/ui/Button'
|
|||||||
import { TextField } from '@/components/ui/TextField'
|
import { TextField } from '@/components/ui/TextField'
|
||||||
import { apiErrorCode, errorMessage } from '@/lib/api'
|
import { apiErrorCode, errorMessage } from '@/lib/api'
|
||||||
import { useProviders, useRegister } from '@/lib/auth'
|
import { useProviders, useRegister } from '@/lib/auth'
|
||||||
|
import { usePageTitle } from '@/lib/usePageTitle'
|
||||||
|
|
||||||
const MIN_PASSWORD = 8
|
const MIN_PASSWORD = 8
|
||||||
|
|
||||||
export function RegisterPage() {
|
export function RegisterPage() {
|
||||||
|
usePageTitle('Create account')
|
||||||
const navigate = useNavigate()
|
const navigate = useNavigate()
|
||||||
const providers = useProviders()
|
const providers = useProviders()
|
||||||
const register = useRegister()
|
const register = useRegister()
|
||||||
|
|||||||
@@ -6,6 +6,7 @@ import {
|
|||||||
} from '@tanstack/react-router'
|
} from '@tanstack/react-router'
|
||||||
import type { QueryClient } from '@tanstack/react-query'
|
import type { QueryClient } from '@tanstack/react-query'
|
||||||
import { AppShell } from '@/components/layout/AppShell'
|
import { AppShell } from '@/components/layout/AppShell'
|
||||||
|
import { NotFound } from '@/components/NotFound'
|
||||||
import { RouteError } from '@/components/RouteError'
|
import { RouteError } from '@/components/RouteError'
|
||||||
import { LoginPage } from '@/pages/LoginPage'
|
import { LoginPage } from '@/pages/LoginPage'
|
||||||
import { RegisterPage } from '@/pages/RegisterPage'
|
import { RegisterPage } from '@/pages/RegisterPage'
|
||||||
@@ -25,6 +26,8 @@ const rootRoute = createRootRouteWithContext<RouterContext>()({
|
|||||||
// A beforeLoad/loader failure that isn't a redirect (e.g. /auth/me errors with
|
// A beforeLoad/loader failure that isn't a redirect (e.g. /auth/me errors with
|
||||||
// a 500 or the network drops) lands here instead of a blank screen.
|
// a 500 or the network drops) lands here instead of a blank screen.
|
||||||
errorComponent: RouteError,
|
errorComponent: RouteError,
|
||||||
|
// Unknown paths render inside the app shell rather than a blank screen.
|
||||||
|
notFoundComponent: NotFound,
|
||||||
})
|
})
|
||||||
|
|
||||||
// requireAuth: resolve the current user (shared cache with useMe); send anyone
|
// requireAuth: resolve the current user (shared cache with useMe); send anyone
|
||||||
|
|||||||
🟡 404 recovery link has no navigation-fallback error handling
error-handling · flagged by 1 model
buttonClasses('primary')on<Link>discards navigation semantics —web/src/components/NotFound.tsx:15Wrapping a TanStack Router<Link>withbuttonClassesapplies the visual style, but ifbuttonClassesever gainstype="button"or other button-specific attributes (it currently does not, but it is styled as a button), the component could inadvertently suppress default link behavior. More relevant to this lens: if the Link fails to resolve (e.g., router mismatch), there is no fa…🪰 Gadfly · advisory