diff --git a/LESSONS-LEARNED.md b/LESSONS-LEARNED.md index 87553003..f7fd576d 100644 --- a/LESSONS-LEARNED.md +++ b/LESSONS-LEARNED.md @@ -2,7 +2,7 @@ > **Purpose:** This file documents bugs, fixes, and gotchas encountered during development. > **For Claude Code:** Read this file at the start of each session to avoid repeating past mistakes. -> **Last Updated:** January 28, 2026 +> **Last Updated:** January 30, 2026 --- @@ -185,6 +185,51 @@ setEditingNodeId(node.id) --- +### Modal Draft State: Don't Overwrite Store-Managed Fields ⚠️ CRITICAL +**Problem:** Child nodes created while a modal is open disappear when clicking "Done". Tree validation fails with errors about non-existent nodes. + +**Cause:** When using local draft state for Cancel/Done functionality in a modal: +1. Modal opens and clones the entire node (including `children: []`) into local state +2. User creates new child nodes via a dropdown (e.g., NodePicker) - these are added to the **store** +3. User clicks "Done" → modal saves draft back to store +4. The draft's stale `children: []` **overwrites** the store's actual children array +5. Newly created nodes are deleted + +**Symptoms:** +- Child nodes don't appear in the tree after closing modal +- Validation errors: "Option references non-existent node" +- Tree won't save due to validation failures + +**Broken pattern:** +```tsx +// Modal with local draft state +const [draft, setDraft] = useState(() => structuredClone(node)) + +const handleSave = () => { + // BAD: This overwrites children with stale snapshot! + updateNode(node.id, draft) + onClose() +} +``` + +**Solution:** Exclude fields that are managed by other store actions (like `children` managed by `addNode`/`deleteNode`): +```tsx +const handleSave = () => { + // GOOD: Exclude 'children' - it's managed separately by addNode/deleteNode + const { children, ...draftWithoutChildren } = draft + updateNode(node.id, draftWithoutChildren) + onClose() +} +``` + +**General rule:** When a modal uses local draft state, identify which fields are: +- **Editable by the form** → include in draft save +- **Managed by other actions** (addNode, deleteNode, etc.) → exclude from draft save + +**Files affected:** `NodeEditorModal.tsx`, any modal that edits objects with nested collections + +--- + ### Modal Scroll/Overflow with Fixed Header and Footer **Problem:** Modal content extends beyond screen when there's too much content, pushing close button and action buttons off-screen. diff --git a/frontend/src/components/tree-editor/NodeEditorModal.tsx b/frontend/src/components/tree-editor/NodeEditorModal.tsx index c4ad37e3..0b736eb2 100644 --- a/frontend/src/components/tree-editor/NodeEditorModal.tsx +++ b/frontend/src/components/tree-editor/NodeEditorModal.tsx @@ -31,7 +31,10 @@ export function NodeEditorModal({ node, onClose, isNewNode = false }: NodeEditor const handleSave = () => { // Commit all draft changes to the store - updateNode(node.id, draft) + // IMPORTANT: Exclude 'children' from the update - children are managed separately + // by addNode/deleteNode and we don't want to overwrite them with stale draft data + const { children, ...draftWithoutChildren } = draft + updateNode(node.id, draftWithoutChildren) onClose() }