Address Gadfly review on the journal UI
Build image / build-and-push (push) Successful in 17s

The prop names were backwards. `canEdit` meant "may delete" and `canWrite` meant
"may rewrite the text" — which is the opposite of how either word reads, on a
component where the distinction is the entire permission model. They are now
canDelete and canRewrite.

An open edit draft never re-synced with its entry, so a refetch after someone
else's edit left a stale draft and a Cancel that restored the wrong text. It now
resyncs when the entry changes underneath, but only while not editing, so a
background refetch can't clobber what's being typed — the version guard is what
catches a genuine collision.

A 409 from an edit had no handling, so the component kept its stale version and
every retry conflicted again: a button that just doesn't work. A conflict now
invalidates, so the fresh version arrives.

Saving an empty edit returned silently, which also reads as a broken button. It
says why nothing happened, and points at Delete if that's what was meant. A
failed delete's error now clears on retry rather than making a second attempt
look like a second failure.

formatObservedAt let out-of-range parts roll over — 2026-13-45 rendering as
February 2027, confidently wrong. It shows the raw string instead.

Also: the scope select guards against a non-numeric value becoming a NaN
objectId and a cryptic 400; the journal total is memoized rather than
spread-and-reduced every render; EditorRail's doc comment no longer calls the
journal "later" now that it's here; and a static class list stopped being split
across two cn() arguments for no reason.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
This commit is contained in:
2026-07-21 01:55:14 -04:00
co-authored by Claude Opus 4.8
parent d94c967310
commit 32cbe36817
4 changed files with 53 additions and 22 deletions
+3 -3
View File
@@ -4,9 +4,9 @@ import { cn } from '@/lib/cn'
/**
* The editor's side rail, and the answer to "four things want one rail".
*
* The inspector, history, and (later) the journal and chat panel all want the
* same strip of screen. Rather than each bolting on its own chrome, they are
* tabs in one rail — which keeps the canvas one width instead of a different
* The inspector, history, journal, and (later) the chat panel all want the same
* strip of screen. Rather than each bolting on its own chrome, they are tabs in
* one rail — which keeps the canvas one width instead of a different
* width per panel, and means adding the journal or chat is adding a tab.
*
* Two constraints shaped it:
+35 -16
View File
@@ -4,7 +4,6 @@ import { Button } from '@/components/ui/Button'
import { TextArea } from '@/components/ui/TextArea'
import { TextField } from '@/components/ui/TextField'
import { errorMessage } from '@/lib/api'
import { cn } from '@/lib/cn'
import {
formatObservedAt,
today,
@@ -55,7 +54,10 @@ export function JournalPanel({
{objects.length > 0 && (
<select
value={scopeObjectId ?? ''}
onChange={(e) => onScopeChange(e.target.value === '' ? null : Number(e.target.value))}
onChange={(e) => {
const id = Number(e.target.value)
onScopeChange(e.target.value === '' || !Number.isFinite(id) ? null : id)
}}
className="max-w-[9rem] truncate rounded-md border border-border bg-surface px-1.5 py-1 text-xs text-fg outline-none focus-visible:ring-2 focus-visible:ring-accent/40"
>
<option value="">Whole garden</option>
@@ -96,8 +98,8 @@ export function JournalPanel({
entry={e}
gardenId={gardenId}
objects={objects}
canEdit={canEdit && (e.authorId === currentUserId || isOwner)}
canWrite={canEdit && e.authorId === currentUserId}
canDelete={canEdit && (e.authorId === currentUserId || isOwner)}
canRewrite={canEdit && e.authorId === currentUserId}
/>
))}
</ol>
@@ -185,16 +187,17 @@ function Entry({
entry,
gardenId,
objects,
canEdit,
canWrite,
canDelete,
canRewrite,
}: {
entry: JournalEntry
gardenId: number
objects: EditorObject[]
/** May remove it: the author, or the garden owner. */
canEdit: boolean
/** May rewrite the text: the author only. */
canWrite: boolean
canDelete: boolean
/** May rewrite the text: the author only — rewriting someone else's
* observation under their name is a different act from removing it. */
canRewrite: boolean
}) {
const update = useUpdateJournalEntry(gardenId)
const del = useDeleteJournalEntry(gardenId)
@@ -203,9 +206,24 @@ function Entry({
const [error, setError] = useState<string | null>(null)
const about = objects.find((o) => o.id === entry.objectId) ?? null
// Re-sync the draft when the entry changes underneath — a refetch after
// someone else's edit, or this entry's own successful save. Skipped while
// editing so a background refetch can't clobber what's being typed; the
// version guard is what catches a genuine collision.
const [syncedBody, setSyncedBody] = useState(entry.body)
if (!editing && entry.body !== syncedBody) {
setSyncedBody(entry.body)
setDraft(entry.body)
}
const save = () => {
const text = draft.trim()
if (!text) return
if (!text) {
// Silence here reads as a broken button; say why nothing happened.
setError('An entry needs some text. Delete it instead if that\'s what you meant.')
return
}
setError(null)
update.mutate(
{ id: entry.id, version: entry.version, body: text },
{
@@ -257,26 +275,27 @@ function Entry({
{entry.plantingId != null && <span className="rounded bg-border/60 px-1 py-px">planting</span>}
</p>
{(canWrite || canEdit) && !editing && (
{(canRewrite || canDelete) && !editing && (
<div className="mt-1 flex justify-end gap-1">
{canWrite && (
{canRewrite && (
<button
type="button"
onClick={() => setEditing(true)}
className={cn('rounded px-1.5 py-0.5 text-xs text-muted outline-none hover:text-fg', 'focus-visible:ring-2 focus-visible:ring-accent/40')}
className="rounded px-1.5 py-0.5 text-xs text-muted outline-none hover:text-fg focus-visible:ring-2 focus-visible:ring-accent/40"
>
Edit
</button>
)}
{canEdit && (
{canDelete && (
<button
type="button"
disabled={del.isPending}
onClick={() =>
onClick={() => {
setError(null) // clear a previous failure so a retry isn't read as another
del.mutate(entry.id, {
onError: (err) => setError(errorMessage(err, "Couldn't delete that note.")),
})
}
}}
className="rounded px-1.5 py-0.5 text-xs text-red-700 outline-none hover:underline focus-visible:ring-2 focus-visible:ring-accent/40 dark:text-red-400"
>
Delete
+10 -2
View File
@@ -6,7 +6,7 @@
import { useInfiniteQuery, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
import { z } from 'zod'
import { api } from './api'
import { ApiError, api } from './api'
export const journalEntrySchema = z.object({
id: z.number(),
@@ -108,6 +108,12 @@ export function useUpdateJournalEntry(gardenId: number) {
return journalEntrySchema.parse(await api.patch(`/journal/${id}`, rest))
},
onSuccess: () => invalidateJournal(qc, gardenId),
onError: (err) => {
// A 409 means the row moved on. Refetch so the component receives the
// current version — otherwise a retry resends the stale one and conflicts
// forever, which reads as a button that simply doesn't work.
if (err instanceof ApiError && err.isConflict) invalidateJournal(qc, gardenId)
},
})
}
@@ -133,6 +139,8 @@ export function today(): string {
* through a Date (which would shift it by the timezone offset). */
export function formatObservedAt(iso: string): string {
const [y, m, d] = iso.split('-').map(Number)
if (!y || !m || !d) return iso
// Out-of-range parts would silently roll over — 2026-13-45 becoming February
// 2027 — so show the raw string instead of a confidently wrong date.
if (!y || !m || !d || m < 1 || m > 12 || d < 1 || d > 31) return iso
return new Date(y, m - 1, d).toLocaleDateString(undefined, { month: 'short', day: 'numeric', year: 'numeric' })
}
+5 -1
View File
@@ -63,6 +63,10 @@ export function GardenEditorPage() {
const journalObjectId = useEditorStore((s) => s.journalObjectId)
const setJournalObjectId = useEditorStore((s) => s.setJournalObjectId)
const journalCounts = useJournalCounts(gid)
const journalTotal = useMemo(
() => [...(journalCounts.data?.values() ?? [])].reduce((a, b) => a + b, 0),
[journalCounts.data],
)
const armedPlant = useEditorStore((s) => s.armedPlant)
const setArmedPlant = useEditorStore((s) => s.setArmedPlant)
@@ -349,7 +353,7 @@ export function GardenEditorPage() {
label: 'Journal',
// The total on the tab is the discoverability half: a garden with a
// season's notes in it shouldn't look identical to an empty one.
badge: [...(journalCounts.data?.values() ?? [])].reduce((a, b) => a + b, 0),
badge: journalTotal,
render: () => (
<JournalPanel
gardenId={gid}