diff --git a/.claude/rules/ui-conventions.md b/.claude/rules/ui-conventions.md index 520cf65..ddd1b8d 100644 --- a/.claude/rules/ui-conventions.md +++ b/.claude/rules/ui-conventions.md @@ -42,3 +42,30 @@ container (the schedule grid scrolls; only modified wheels change its range), and keep the callbacks in the dependency array identity-stable (`useCallback` with functional `setState`) so the listener is not torn down and rebuilt on every render. See `src/components/schedule/schedule-grid.tsx`. + +## Selection state: keep the id, look up the object + +When a view owns a document and a side panel edits one element of it, store +only the element's **id** in state and derive the element from the document: + +```ts +const [selectedId, setSelectedId] = useState(null); +const selected = useMemo( + () => (selectedId ? (doc?.items.find((i) => i.id === selectedId) ?? null) : null), + [doc, selectedId], +); +``` + +Holding a copy of the element (`useState`) forks the truth. Every +edit path that does not go through the panel — a drag on the grid, a keyboard +nudge, an undo, a reload after an external file edit — updates the document +and leaves the copy behind; the panel's next patch then spreads that stale copy +back over the document and silently reverts the earlier edit. The schedule tab +shipped exactly that bug: resize a bar on the grid, then change its kind in the +panel, and the old dates came back (T-0101). + +The same rule applies inside the panel: render fields straight from the `item` +prop rather than seeding a local draft from it. A draft re-seeded by +`useEffect` is the same stale copy one level down. If a field ever does need a +draft (an in-progress text edit that must not round-trip), scope it to that +field and reconcile it explicitly — do not draft the whole element. diff --git a/src/components/schedule/item-editor.tsx b/src/components/schedule/item-editor.tsx index cb362a4..67502de 100644 --- a/src/components/schedule/item-editor.tsx +++ b/src/components/schedule/item-editor.tsx @@ -1,4 +1,3 @@ -import { useEffect, useState } from "react"; import { Trash2 } from "lucide-react"; import { Button } from "@/components/ui/button"; import { DatePicker } from "@/components/ui/date-picker"; @@ -31,6 +30,12 @@ import type { Task } from "@/types"; * `Details` is the element's body — the indented continuation lines under it * in the file. A note shows it on hover in the grid; every other kind shows it * in its tooltip, where it reads as a remark about the element. + * + * The panel holds **no draft state**: every field renders straight from `item`, + * and each edit is a patch on that same object. A local copy re-seeded from the + * prop looks equivalent but is not — a drag on the grid changes the document + * without going through this panel, and the copy would keep serving the dates + * from before the drag until the next edit wrote them back over it. */ interface Props { @@ -54,13 +59,8 @@ function collapseLines(value: string): string { } export function ItemEditor({ item, tasks, onChange, onDelete, onClose }: Props) { - const [draft, setDraft] = useState(item); - - // Re-seed when the grid selects a different element (the panel is reused). - useEffect(() => setDraft(item), [item]); - const commit = (patch: Partial) => { - const next = { ...draft, ...patch }; + const next = { ...item, ...patch }; // A range element cannot end before it starts; pushing the far edge along is // less surprising than rejecting the edit the user just made. if (isRangeKind(next.kind) && next.end < next.start) { @@ -68,7 +68,6 @@ export function ItemEditor({ item, tasks, onChange, onDelete, onClose }: Props) else next.start = next.end; } if (!isRangeKind(next.kind)) next.end = next.start; - setDraft(next); onChange(next); }; @@ -76,9 +75,9 @@ export function ItemEditor({ item, tasks, onChange, onDelete, onClose }: Props) // Width comes from the sidebar column, not from here — see schedule-view.
- {draft.id} + {item.id} commit({ title: collapseLines(e.target.value) })} />