Cleanup: ConfirmModal primitive, drop dead PageStub, full-width editor (#107) #116
@@ -1,13 +0,0 @@
|
|||||||
import type { ReactNode } from 'react'
|
|
||||||
|
|
||||||
/** Placeholder page scaffolding used until each feature issue fills these in. */
|
|
||||||
export function PageStub({ title, children }: { title: string; children?: ReactNode }) {
|
|
||||||
return (
|
|
||||||
<section>
|
|
||||||
<h1 className="text-2xl font-semibold tracking-tight">{title}</h1>
|
|
||||||
<p className="mt-2 text-sm text-muted">
|
|
||||||
{children ?? 'Placeholder — this page arrives in a later issue.'}
|
|
||||||
</p>
|
|
||||||
</section>
|
|
||||||
)
|
|
||||||
}
|
|
||||||
@@ -1,42 +1,22 @@
|
|||||||
import { useState } from 'react'
|
import { ConfirmModal } from '@/components/ui/ConfirmModal'
|
||||||
import { Modal } from '@/components/ui/Modal'
|
|
||||||
import { Alert } from '@/components/ui/Alert'
|
|
||||||
import { Button } from '@/components/ui/Button'
|
|
||||||
import { errorMessage } from '@/lib/api'
|
|
||||||
import { useDeleteGarden, type Garden } from '@/lib/gardens'
|
import { useDeleteGarden, type Garden } from '@/lib/gardens'
|
||||||
|
|
||||||
/** Confirmation dialog for deleting a garden (and everything in it). */
|
/** Confirmation dialog for deleting a garden (and everything in it). */
|
||||||
export function DeleteGardenModal({ garden, onClose }: { garden: Garden; onClose: () => void }) {
|
export function DeleteGardenModal({ garden, onClose }: { garden: Garden; onClose: () => void }) {
|
||||||
const deletion = useDeleteGarden()
|
const deletion = useDeleteGarden()
|
||||||
const [error, setError] = useState<string | null>(null)
|
|
||||||
|
|
||||||
async function onConfirm() {
|
|
||||||
setError(null)
|
|
||||||
try {
|
|
||||||
await deletion.mutateAsync(garden.id)
|
|
||||||
onClose()
|
|
||||||
} catch (err) {
|
|
||||||
setError(errorMessage(err, 'Could not delete the garden.'))
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Modal title="Delete garden" onClose={onClose} busy={deletion.isPending}>
|
<ConfirmModal
|
||||||
<div className="flex flex-col gap-4">
|
title="Delete garden"
|
||||||
<p className="text-sm text-muted">
|
confirmLabel="Delete"
|
||||||
Delete <span className="font-medium text-fg">{garden.name}</span> and everything planned in it?
|
busyLabel="Deleting…"
|
||||||
This can't be undone.
|
errorFallback="Could not delete the garden."
|
||||||
</p>
|
onConfirm={() => deletion.mutateAsync(garden.id)}
|
||||||
{error && <Alert>{error}</Alert>}
|
onClose={onClose}
|
||||||
<div className="flex justify-end gap-2">
|
>
|
||||||
<Button type="button" variant="ghost" onClick={onClose} disabled={deletion.isPending}>
|
<p className="text-sm text-muted">
|
||||||
Cancel
|
Delete <span className="font-medium text-fg">{garden.name}</span> and everything planned in it?
|
||||||
</Button>
|
This can't be undone.
|
||||||
<Button type="button" variant="danger" onClick={onConfirm} disabled={deletion.isPending}>
|
</p>
|
||||||
{deletion.isPending ? 'Deleting…' : 'Delete'}
|
</ConfirmModal>
|
||||||
</Button>
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
</Modal>
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,8 +1,4 @@
|
|||||||
import { useState } from 'react'
|
import { ConfirmModal } from '@/components/ui/ConfirmModal'
|
||||||
import { Modal } from '@/components/ui/Modal'
|
|
||||||
import { Alert } from '@/components/ui/Alert'
|
|
||||||
import { Button } from '@/components/ui/Button'
|
|
||||||
import { errorMessage } from '@/lib/api'
|
|
||||||
import { useMe } from '@/lib/auth'
|
import { useMe } from '@/lib/auth'
|
||||||
import type { Garden } from '@/lib/gardens'
|
import type { Garden } from '@/lib/gardens'
|
||||||
import { useRemoveShare } from '@/lib/shares'
|
import { useRemoveShare } from '@/lib/shares'
|
||||||
@@ -12,36 +8,26 @@ import { useRemoveShare } from '@/lib/shares'
|
|||||||
export function LeaveGardenModal({ garden, onClose }: { garden: Garden; onClose: () => void }) {
|
export function LeaveGardenModal({ garden, onClose }: { garden: Garden; onClose: () => void }) {
|
||||||
const me = useMe()
|
const me = useMe()
|
||||||
const remove = useRemoveShare(garden.id)
|
const remove = useRemoveShare(garden.id)
|
||||||
const [error, setError] = useState<string | null>(null)
|
|
||||||
|
|
||||||
async function onConfirm() {
|
|
||||||
if (!me.data) return
|
|
||||||
setError(null)
|
|
||||||
try {
|
|
||||||
await remove.mutateAsync(me.data.id)
|
|
||||||
onClose()
|
|
||||||
} catch (err) {
|
|
||||||
setError(errorMessage(err, 'Could not leave the garden.'))
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Modal title="Leave garden" onClose={onClose} busy={remove.isPending}>
|
<ConfirmModal
|
||||||
<div className="flex flex-col gap-4">
|
title="Leave garden"
|
||||||
<p className="text-sm text-muted">
|
confirmLabel="Leave"
|
||||||
Leave <span className="font-medium text-fg">{garden.name}</span>? You'll lose access until the owner
|
busyLabel="Leaving…"
|
||||||
shares it with you again.
|
confirmDisabled={!me.data}
|
||||||
</p>
|
errorFallback="Could not leave the garden."
|
||||||
{error && <Alert>{error}</Alert>}
|
onConfirm={async () => {
|
||||||
|
|
|||||||
<div className="flex justify-end gap-2">
|
// The button is disabled without a current user; throw rather than
|
||||||
<Button type="button" variant="ghost" onClick={onClose} disabled={remove.isPending}>
|
// silently resolve (which would close the dialog as if it had worked) if
|
||||||
Cancel
|
// that guard ever drifts.
|
||||||
</Button>
|
if (!me.data) throw new Error('Not signed in.')
|
||||||
<Button type="button" variant="danger" onClick={onConfirm} disabled={remove.isPending || !me.data}>
|
await remove.mutateAsync(me.data.id)
|
||||||
{remove.isPending ? 'Leaving…' : 'Leave'}
|
}}
|
||||||
</Button>
|
onClose={onClose}
|
||||||
</div>
|
>
|
||||||
</div>
|
<p className="text-sm text-muted">
|
||||||
</Modal>
|
Leave <span className="font-medium text-fg">{garden.name}</span>? You'll lose access until the owner
|
||||||
|
shares it with you again.
|
||||||
|
</p>
|
||||||
|
</ConfirmModal>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -39,14 +39,19 @@ export function AppShell() {
|
|||||||
// so the '/gardens' list itself still shows the bar.
|
// so the '/gardens' list itself still shows the bar.
|
||||||
const inEditor = !!matchRoute({ to: '/gardens/$gardenId', fuzzy: false })
|
const inEditor = !!matchRoute({ to: '/gardens/$gardenId', fuzzy: false })
|
||||||
const inPublicGarden = !!matchRoute({ to: '/g/$token', fuzzy: false })
|
const inPublicGarden = !!matchRoute({ to: '/g/$token', fuzzy: false })
|
||||||
const showBottomNav = !!user && !inEditor && !inPublicGarden
|
// A canvas route wants the whole width — the garden editor is squeezed by the
|
||||||
|
// max-w-5xl reading measure the other pages use (#107).
|
||||||
|
const canvasRoute = inEditor || inPublicGarden
|
||||||
|
const showBottomNav = !!user && !canvasRoute
|
||||||
|
|
||||||
const visibleSections = sections.filter((s) => !s.adminOnly || user?.isAdmin)
|
const visibleSections = sections.filter((s) => !s.adminOnly || user?.isAdmin)
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="flex min-h-full flex-col">
|
<div className="flex min-h-full flex-col">
|
||||||
<header className="sticky top-0 z-20 border-b border-border bg-surface/90 backdrop-blur">
|
<header className="sticky top-0 z-20 border-b border-border bg-surface/90 backdrop-blur">
|
||||||
<nav className="mx-auto flex max-w-5xl items-center gap-4 px-4 py-3">
|
{/* The bar matches the content width below: constrained on reading pages,
|
||||||
|
edge-to-edge on the canvas routes so the brand aligns with the editor. */}
|
||||||
|
<nav className={cn('flex items-center gap-4 px-4 py-3', canvasRoute ? '' : 'mx-auto max-w-5xl')}>
|
||||||
|
gitea-actions
commented
⚪ Ternary-to-empty-string vs && idiom used inconsistently for the same canvasRoute condition in one file maintainability · flagged by 1 model
🪰 Gadfly · advisory ⚪ **Ternary-to-empty-string vs && idiom used inconsistently for the same canvasRoute condition in one file**
_maintainability · flagged by 1 model_
- `web/src/components/layout/AppShell.tsx:54` vs `:98` — the `nav` className uses a ternary-to-empty-string (`canvasRoute ? '' : 'mx-auto max-w-5xl'`) while `<main>` (44 lines below) uses a short-circuit (`!canvasRoute && 'mx-auto max-w-5xl'`) for the identical `canvasRoute` condition. `cn()` strips falsy values, so the ternary's `''` branch is unnecessary. Both compile and render identically, but two idioms for the same width-toggle in one file is exactly the inconsistency this PR is otherwise…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
<Link to="/gardens" className="text-lg font-semibold text-accent-strong">
|
<Link to="/gardens" className="text-lg font-semibold text-accent-strong">
|
||||||
🌱 pansy
|
🌱 pansy
|
||||||
</Link>
|
</Link>
|
||||||
@@ -87,7 +92,10 @@ export function AppShell() {
|
|||||||
|
|
||||||
<main
|
<main
|
||||||
className={cn(
|
className={cn(
|
||||||
'mx-auto w-full max-w-5xl flex-1 px-4 py-6',
|
'w-full flex-1 px-4 py-6',
|
||||||
|
// Constrain the reading pages to a comfortable measure; the canvas
|
||||||
|
// routes go edge-to-edge so the garden gets the whole screen.
|
||||||
|
!canvasRoute && 'mx-auto max-w-5xl',
|
||||||
// Clear the fixed bottom bar on mobile so content isn't hidden behind
|
// Clear the fixed bottom bar on mobile so content isn't hidden behind
|
||||||
// it. The 3.5rem must match BottomNav's h-14 (kept adjacent below).
|
// it. The 3.5rem must match BottomNav's h-14 (kept adjacent below).
|
||||||
showBottomNav && 'pb-[calc(3.5rem+env(safe-area-inset-bottom))] md:pb-6',
|
showBottomNav && 'pb-[calc(3.5rem+env(safe-area-inset-bottom))] md:pb-6',
|
||||||
|
|||||||
@@ -1,46 +1,26 @@
|
|||||||
import { useState } from 'react'
|
import { ConfirmModal } from '@/components/ui/ConfirmModal'
|
||||||
import { Modal } from '@/components/ui/Modal'
|
|
||||||
import { Alert } from '@/components/ui/Alert'
|
|
||||||
import { Button } from '@/components/ui/Button'
|
|
||||||
import { errorMessage } from '@/lib/api'
|
|
||||||
import { useDeletePlant, type Plant } from '@/lib/plants'
|
import { useDeletePlant, type Plant } from '@/lib/plants'
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Confirmation dialog for deleting a custom plant. A plant still used by
|
* Confirmation dialog for deleting a custom plant. A plant still used by
|
||||||
* plantings is refused by the server (409 PLANT_IN_USE); we surface that
|
* plantings is refused by the server (409 PLANT_IN_USE); ConfirmModal surfaces
|
||||||
* message inline rather than pretending it worked.
|
* that message inline rather than pretending it worked.
|
||||||
*/
|
*/
|
||||||
export function DeletePlantModal({ plant, onClose }: { plant: Plant; onClose: () => void }) {
|
export function DeletePlantModal({ plant, onClose }: { plant: Plant; onClose: () => void }) {
|
||||||
const deletion = useDeletePlant()
|
const deletion = useDeletePlant()
|
||||||
const [error, setError] = useState<string | null>(null)
|
|
||||||
|
|
||||||
async function onConfirm() {
|
|
||||||
setError(null)
|
|
||||||
try {
|
|
||||||
await deletion.mutateAsync(plant.id)
|
|
||||||
onClose()
|
|
||||||
} catch (err) {
|
|
||||||
setError(errorMessage(err, 'Could not delete the plant.'))
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Modal title="Delete plant" onClose={onClose} busy={deletion.isPending}>
|
<ConfirmModal
|
||||||
<div className="flex flex-col gap-4">
|
title="Delete plant"
|
||||||
<p className="text-sm text-muted">
|
confirmLabel="Delete"
|
||||||
Delete <span className="font-medium text-fg">{plant.name}</span> from your catalog? This can't be
|
busyLabel="Deleting…"
|
||||||
undone.
|
errorFallback="Could not delete the plant."
|
||||||
</p>
|
onConfirm={() => deletion.mutateAsync(plant.id)}
|
||||||
{error && <Alert>{error}</Alert>}
|
onClose={onClose}
|
||||||
<div className="flex justify-end gap-2">
|
>
|
||||||
<Button type="button" variant="ghost" onClick={onClose} disabled={deletion.isPending}>
|
<p className="text-sm text-muted">
|
||||||
Cancel
|
Delete <span className="font-medium text-fg">{plant.name}</span> from your catalog? This can't be
|
||||||
</Button>
|
undone.
|
||||||
<Button type="button" variant="danger" onClick={onConfirm} disabled={deletion.isPending}>
|
</p>
|
||||||
{deletion.isPending ? 'Deleting…' : 'Delete'}
|
</ConfirmModal>
|
||||||
</Button>
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
</Modal>
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,8 +1,4 @@
|
|||||||
import { useState } from 'react'
|
import { ConfirmModal } from '@/components/ui/ConfirmModal'
|
||||||
import { Alert } from '@/components/ui/Alert'
|
|
||||||
import { Button } from '@/components/ui/Button'
|
|
||||||
import { Modal } from '@/components/ui/Modal'
|
|
||||||
import { errorMessage } from '@/lib/api'
|
|
||||||
import { formatQuantity, useDeleteSeedLot, type SeedLot } from '@/lib/seedLots'
|
import { formatQuantity, useDeleteSeedLot, type SeedLot } from '@/lib/seedLots'
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -12,39 +8,23 @@ import { formatQuantity, useDeleteSeedLot, type SeedLot } from '@/lib/seedLots'
|
|||||||
*/
|
*/
|
||||||
export function DeleteSeedLotModal({ lot, onClose }: { lot: SeedLot; onClose: () => void }) {
|
export function DeleteSeedLotModal({ lot, onClose }: { lot: SeedLot; onClose: () => void }) {
|
||||||
const del = useDeleteSeedLot()
|
const del = useDeleteSeedLot()
|
||||||
const [error, setError] = useState<string | null>(null)
|
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Modal title="Retire this seed lot?" onClose={onClose} busy={del.isPending}>
|
<ConfirmModal
|
||||||
<div className="flex flex-col gap-3">
|
title="Retire this seed lot?"
|
||||||
<p className="text-sm text-fg">
|
confirmLabel="Retire lot"
|
||||||
{formatQuantity(lot.quantity)} {lot.unit}
|
busyLabel="Retiring…"
|
||||||
{lot.vendor ? ` from ${lot.vendor}` : ''}
|
errorFallback="Could not retire the lot."
|
||||||
{lot.packedForYear != null ? `, packed for ${lot.packedForYear}` : ''}.
|
onConfirm={() => del.mutateAsync(lot.id)}
|
||||||
</p>
|
onClose={onClose}
|
||||||
<p className="text-sm text-muted">
|
>
|
||||||
Anything planted from it stays exactly where it is — it just stops being attributed to this purchase.
|
<p className="text-sm text-fg">
|
||||||
</p>
|
{formatQuantity(lot.quantity)} {lot.unit}
|
||||||
{error && <Alert>{error}</Alert>}
|
{lot.vendor ? ` from ${lot.vendor}` : ''}
|
||||||
<div className="mt-1 flex justify-end gap-2">
|
{lot.packedForYear != null ? `, packed for ${lot.packedForYear}` : ''}.
|
||||||
<Button type="button" variant="ghost" onClick={onClose} disabled={del.isPending}>
|
</p>
|
||||||
Cancel
|
<p className="text-sm text-muted">
|
||||||
</Button>
|
Anything planted from it stays exactly where it is — it just stops being attributed to this purchase.
|
||||||
<Button
|
</p>
|
||||||
type="button"
|
</ConfirmModal>
|
||||||
variant="danger"
|
|
||||||
disabled={del.isPending}
|
|
||||||
onClick={() =>
|
|
||||||
del.mutate(lot.id, {
|
|
||||||
onSuccess: onClose,
|
|
||||||
onError: (err) => setError(errorMessage(err, 'Could not retire the lot.')),
|
|
||||||
})
|
|
||||||
}
|
|
||||||
>
|
|
||||||
{del.isPending ? 'Retiring…' : 'Retire lot'}
|
|
||||||
</Button>
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
</Modal>
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,79 @@
|
|||||||
|
import { useState, type ReactNode } from 'react'
|
||||||
|
import { Modal } from '@/components/ui/Modal'
|
||||||
|
import { Alert } from '@/components/ui/Alert'
|
||||||
|
import { Button } from '@/components/ui/Button'
|
||||||
|
import { errorMessage } from '@/lib/api'
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A confirm-and-act dialog: a message, then Cancel / Confirm. It owns the shared
|
||||||
|
* shape every confirmation repeated by hand — the busy lock, the inline error on
|
||||||
|
* failure (so a 409 like PLANT_IN_USE is shown, not swallowed), and the footer —
|
||||||
|
* so each caller supplies only its message, its action, and its labels.
|
||||||
|
*
|
||||||
|
* onConfirm runs the action; it resolving closes the dialog, it throwing keeps
|
||||||
|
* the dialog open with the error and re-enables the button for a retry. This is
|
||||||
|
* confirmations only — dialogs with their own inputs (a rename, a form) keep
|
||||||
|
* using Modal directly.
|
||||||
|
*/
|
||||||
|
export function ConfirmModal({
|
||||||
|
title,
|
||||||
|
children,
|
||||||
|
confirmLabel,
|
||||||
|
busyLabel,
|
||||||
|
confirmVariant = 'danger',
|
||||||
|
confirmDisabled = false,
|
||||||
|
errorFallback,
|
||||||
|
onConfirm,
|
||||||
|
onClose,
|
||||||
|
}: {
|
||||||
|
title: string
|
||||||
|
/** The body — what's being confirmed. */
|
||||||
|
children: ReactNode
|
||||||
|
confirmLabel: string
|
||||||
|
/** Label while the action is in flight (e.g. "Deleting…"). */
|
||||||
|
gitea-actions
commented
⚪ confirmVariant 'primary' option is unused by any caller — speculative API surface on a new primitive maintainability · flagged by 1 model
🪰 Gadfly · advisory ⚪ **confirmVariant 'primary' option is unused by any caller — speculative API surface on a new primitive**
_maintainability · flagged by 1 model_
- `web/src/components/layout/AppShell.tsx:54` vs `:98` — the `nav` className uses a ternary-to-empty-string (`canvasRoute ? '' : 'mx-auto max-w-5xl'`) while `<main>` (44 lines below) uses a short-circuit (`!canvasRoute && 'mx-auto max-w-5xl'`) for the identical `canvasRoute` condition. `cn()` strips falsy values, so the ternary's `''` branch is unnecessary. Both compile and render identically, but two idioms for the same width-toggle in one file is exactly the inconsistency this PR is otherwise…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
busyLabel: string
|
||||||
|
confirmVariant?: 'danger' | 'primary'
|
||||||
|
/** Extra guard beyond busy (e.g. nothing to clear, no current user). */
|
||||||
|
confirmDisabled?: boolean
|
||||||
|
/** Message if the action throws something without its own user-facing text. */
|
||||||
|
errorFallback: string
|
||||||
|
onConfirm: () => Promise<unknown>
|
||||||
|
onClose: () => void
|
||||||
|
}) {
|
||||||
|
const [busy, setBusy] = useState(false)
|
||||||
|
const [error, setError] = useState<string | null>(null)
|
||||||
|
|
||||||
|
async function handleConfirm() {
|
||||||
|
setError(null)
|
||||||
|
setBusy(true)
|
||||||
|
try {
|
||||||
|
await onConfirm()
|
||||||
|
onClose()
|
||||||
|
} catch (err) {
|
||||||
|
setError(errorMessage(err, errorFallback))
|
||||||
|
setBusy(false) // keep the dialog open so the message shows and retry works
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return (
|
||||||
|
<Modal title={title} onClose={onClose} busy={busy}>
|
||||||
|
<div className="flex flex-col gap-4">
|
||||||
|
{children}
|
||||||
|
{error && <Alert>{error}</Alert>}
|
||||||
|
<div className="flex justify-end gap-2">
|
||||||
|
<Button type="button" variant="ghost" onClick={onClose} disabled={busy}>
|
||||||
|
Cancel
|
||||||
|
</Button>
|
||||||
|
<Button
|
||||||
|
type="button"
|
||||||
|
variant={confirmVariant}
|
||||||
|
onClick={handleConfirm}
|
||||||
|
disabled={busy || confirmDisabled}
|
||||||
|
>
|
||||||
|
{busy ? busyLabel : confirmLabel}
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
</Modal>
|
||||||
|
)
|
||||||
|
}
|
||||||
@@ -1,5 +1,4 @@
|
|||||||
import { Modal } from '@/components/ui/Modal'
|
import { ConfirmModal } from '@/components/ui/ConfirmModal'
|
||||||
import { Button } from '@/components/ui/Button'
|
|
||||||
import { useClearObject } from '@/lib/objects'
|
import { useClearObject } from '@/lib/objects'
|
||||||
|
|
||||||
/** Confirm clearing every active plop from a focused bed (soft-remove — the rows
|
/** Confirm clearing every active plop from a focused bed (soft-remove — the rows
|
||||||
@@ -19,27 +18,20 @@ export function ClearBedModal({
|
|||||||
}) {
|
}) {
|
||||||
const clear = useClearObject(gardenId)
|
const clear = useClearObject(gardenId)
|
||||||
return (
|
return (
|
||||||
<Modal title="Clear bed" onClose={onClose} busy={clear.isPending}>
|
<ConfirmModal
|
||||||
|
gitea-actions
commented
🟠 Duplicate error notification: ClearBedModal inline error + useClearObject toast on mutation failure correctness · flagged by 2 models
🪰 Gadfly · advisory 🟠 **Duplicate error notification: ClearBedModal inline error + useClearObject toast on mutation failure**
_correctness · flagged by 2 models_
- **`web/src/editor/ClearBedModal.tsx:21`** — The refactor switches `ClearBedModal` from `clear.mutate()` with an `onSuccess` callback to `clear.mutateAsync()` inside `ConfirmModal`. The underlying `useClearObject` hook (`web/src/lib/objects.ts:384`) still defines `onError: (err) => toast.error(...)`. TanStack Query runs `onError` before `mutateAsync` rejects, so a failure now produces **two notifications**: the inline error from `ConfirmModal` *and* a toast from the mutation hook. Previously th…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
<div className="flex flex-col gap-4">
|
title="Clear bed"
|
||||||
<p className="text-sm text-muted">
|
confirmLabel="Clear bed"
|
||||||
Remove all <span className="font-medium text-fg">{plopCount}</span>{' '}
|
busyLabel="Clearing…"
|
||||||
{plopCount === 1 ? 'plant' : 'plants'} from{' '}
|
confirmDisabled={plopCount === 0}
|
||||||
<span className="font-medium text-fg">{objectName}</span>? They're marked removed but kept in history.
|
errorFallback="Could not clear the bed."
|
||||||
|
gitea-actions
commented
🟡 ClearBed now double-reports errors (inline Alert + toast from useClearObject.onError) correctness, error-handling · flagged by 3 models
🪰 Gadfly · advisory 🟡 **ClearBed now double-reports errors (inline Alert + toast from useClearObject.onError)**
_correctness, error-handling · flagged by 3 models_
- **Double error reporting in `ClearBedModal`** — `web/src/editor/ClearBedModal.tsx:26` (via `web/src/components/ui/ConfirmModal.tsx`). `useClearObject` still carries `onError: (err) => toast.error(objectErrorMessage(err, 'Could not clear the bed.'))` (`web/src/lib/objects.ts:384`), so on a clear failure the user now sees both the inline `Alert` that `ConfirmModal` renders *and* the toast from the mutation. This is the only one of the five migrated confirmations whose underlying hook keeps a glo…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
</p>
|
onConfirm={() => clear.mutateAsync(objectId)}
|
||||||
<div className="flex justify-end gap-2">
|
onClose={onClose}
|
||||||
<Button type="button" variant="ghost" onClick={onClose} disabled={clear.isPending}>
|
>
|
||||||
Cancel
|
<p className="text-sm text-muted">
|
||||||
</Button>
|
Remove all <span className="font-medium text-fg">{plopCount}</span>{' '}
|
||||||
<Button
|
{plopCount === 1 ? 'plant' : 'plants'} from{' '}
|
||||||
type="button"
|
<span className="font-medium text-fg">{objectName}</span>? They're marked removed but kept in history.
|
||||||
variant="danger"
|
</p>
|
||||||
disabled={clear.isPending || plopCount === 0}
|
</ConfirmModal>
|
||||||
onClick={() => clear.mutate(objectId, { onSuccess: onClose })}
|
|
||||||
>
|
|
||||||
{clear.isPending ? 'Clearing…' : 'Clear bed'}
|
|
||||||
</Button>
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
</Modal>
|
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -381,7 +381,9 @@ export function useClearObject(gardenId: number) {
|
|||||||
return res.cleared
|
return res.cleared
|
||||||
},
|
},
|
||||||
onSettled: () => qc.invalidateQueries({ queryKey: fullKey(gardenId) }),
|
onSettled: () => qc.invalidateQueries({ queryKey: fullKey(gardenId) }),
|
||||||
onError: (err) => toast.error(objectErrorMessage(err, 'Could not clear the bed.')),
|
// No toast: the only caller (ClearBedModal → ConfirmModal) shows the failure
|
||||||
|
gitea-actions
commented
🟡 ClearBed failure now surfaces both a toast and ConfirmModal's inline alert error-handling · flagged by 1 model
🪰 Gadfly · advisory 🟡 **ClearBed failure now surfaces both a toast and ConfirmModal's inline alert**
_error-handling · flagged by 1 model_
- **`web/src/lib/objects.ts:384` / `web/src/editor/ClearBedModal.tsx`** — `useClearObject` still has its own `onError: (err) => toast.error(...)`. Since `ClearBedModal` now uses `ConfirmModal`, whose `handleConfirm` also catches the same rejection from `clear.mutateAsync(objectId)` and renders it inline (`ConfirmModal.tsx:52-54`), a clear-bed failure now shows both a toast and the modal's inline alert for the same error. None of the other four converted modals' hooks (`useDeleteGarden`, `useDele…
<sub>🪰 Gadfly · advisory</sub>
|
|||||||
|
// inline in the dialog, which is more contextual than a detached toast — and
|
||||||
|
// two of them for one failure is worse than one.
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
🟡 Dialog closes on no-op when me.data is absent instead of staying open
error-handling · flagged by 3 models
LeaveGardenModalcloses on no-op whenme.datais absent.web/src/components/gardens/LeaveGardenModal.tsx:18🪰 Gadfly · advisory