Improve tree editor modal UX: cancel/save and inline node naming

Change 1: Add Cancel button and defer saving until Done is clicked - NodeEditorModal now uses local draft state instead of updating store directly - Cancel button discards changes; Done button commits to store - If editing a brand new node, Cancel deletes it entirely - NodeList tracks isEditingNewNode to pass to modal Change 2: Inline node naming when creating from NodePicker dropdown - Selecting "+ New Decision/Action/Solution" shows inline title input - User enters title before node is created (Enter to create, Escape to cancel) - Node appears in dropdown with human-readable title immediately Change 3: Improved dropdown labels - Format changed from "UUID (UUID...)" to "Title (UUID...)" - Untitled nodes show "Untitled Question" or "Untitled {type}" - Root node shows "Root Question (root)" when empty Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
2026-01-30 01:01:23 -05:00
parent adcaf2f4fe
commit 1d5ba598ca
4 changed files with 244 additions and 81 deletions
--- a/frontend/src/components/tree-editor/NodeEditorModal.tsx
+++ b/frontend/src/components/tree-editor/NodeEditorModal.tsx
@@ -1,3 +1,4 @@
+import { useState, useEffect, useCallback } from 'react'
 import { Modal } from '@/components/common/Modal'
 import { useTreeEditorStore } from '@/store/treeEditorStore'
 import { NodeFormDecision } from './NodeFormDecision'
@@ -8,14 +9,39 @@ import type { TreeStructure } from '@/types'
 interface NodeEditorModalProps {
  node: TreeStructure
  onClose: () => void
+  /** If true, this is a brand new node - cancel will delete it entirely */
+  isNewNode?: boolean
 }

-export function NodeEditorModal({ node, onClose }: NodeEditorModalProps) {
-  const { updateNode, validationErrors } = useTreeEditorStore()
+export function NodeEditorModal({ node, onClose, isNewNode = false }: NodeEditorModalProps) {
+  const { updateNode, deleteNode, validationErrors } = useTreeEditorStore()
  const nodeErrors = validationErrors.filter(e => e.nodeId === node.id)

-  const handleUpdate = (updates: Partial<TreeStructure>) => {
-    updateNode(node.id, updates)
+  // Local draft state - changes are NOT persisted until "Done" is clicked
+  const [draft, setDraft] = useState<TreeStructure>(() => structuredClone(node))
+
+  // Reset draft when node changes (e.g., external update)
+  useEffect(() => {
+    setDraft(structuredClone(node))
+  }, [node.id]) // Only reset when switching to a different node
+
+  const handleUpdate = useCallback((updates: Partial<TreeStructure>) => {
+    setDraft(prev => ({ ...prev, ...updates }))
+  }, [])
+
+  const handleSave = () => {
+    // Commit all draft changes to the store
+    updateNode(node.id, draft)
+    onClose()
+  }
+
+  const handleCancel = () => {
+    if (isNewNode) {
+      // Delete the unsaved new node entirely
+      deleteNode(node.id)
+    }
+    // Discard changes and close (draft is just thrown away)
+    onClose()
  }

  const getTitle = () => {
@@ -32,10 +58,17 @@ export function NodeEditorModal({ node, onClose }: NodeEditorModalProps) {
  }

  const footerContent = (
-    <div className="flex justify-end">
+    <div className="flex justify-end gap-2">
      <button
        type="button"
-        onClick={onClose}
+        onClick={handleCancel}
+        className="rounded-md border border-input bg-background px-4 py-2 text-sm font-medium text-foreground hover:bg-accent"
+      >
+        Cancel
+      </button>
+      <button
+        type="button"
+        onClick={handleSave}
        className="rounded-md bg-primary px-4 py-2 text-sm font-medium text-primary-foreground hover:bg-primary/90"
      >
        Done
@@ -68,15 +101,15 @@ export function NodeEditorModal({ node, onClose }: NodeEditorModalProps) {
        </div>
      )}

-      {/* Type-specific form */}
-      {node.type === 'decision' && (
-        <NodeFormDecision node={node} onUpdate={handleUpdate} />
+      {/* Type-specific form - uses draft state, not the original node */}
+      {draft.type === 'decision' && (
+        <NodeFormDecision node={draft} onUpdate={handleUpdate} />
      )}
-      {node.type === 'action' && (
-        <NodeFormAction node={node} onUpdate={handleUpdate} />
+      {draft.type === 'action' && (
+        <NodeFormAction node={draft} onUpdate={handleUpdate} />
      )}
-      {node.type === 'solution' && (
-        <NodeFormResolution node={node} onUpdate={handleUpdate} />
+      {draft.type === 'solution' && (
+        <NodeFormResolution node={draft} onUpdate={handleUpdate} />
      )}
    </Modal>
  )
--- a/frontend/src/components/tree-editor/NodeList.tsx
+++ b/frontend/src/components/tree-editor/NodeList.tsx
@@ -310,6 +310,7 @@ function NodeListItem({
 export function NodeList() {
  const { treeStructure, addNode, deleteNode, duplicateNode, reorderNodes, findNode } = useTreeEditorStore()
  const [editingNodeId, setEditingNodeId] = useState<string | null>(null)
+  const [isEditingNewNode, setIsEditingNewNode] = useState(false)
  const [addingToParent, setAddingToParent] = useState<string | null>(null)

  // Get the current node from store (will update when store changes)
@@ -327,8 +328,19 @@ export function NodeList() {
  const handleAddNode = (type: NodeType) => {
    const newId = addNode(addingToParent, type)
    setAddingToParent(null)
-    // Open editor for the new node
+    // Open editor for the new node and mark it as new
    setEditingNodeId(newId)
+    setIsEditingNewNode(true)
+  }
+
+  const handleEditExistingNode = (node: TreeStructure) => {
+    setEditingNodeId(node.id)
+    setIsEditingNewNode(false)
+  }
+
+  const handleCloseEditor = () => {
+    setEditingNodeId(null)
+    setIsEditingNewNode(false)
  }

  const handleDragStart = (
@@ -414,7 +426,7 @@ export function NodeList() {
          parentId={null}
          index={0}
          isLast={true}
-          onEdit={(node) => setEditingNodeId(node.id)}
+          onEdit={handleEditExistingNode}
          onDelete={deleteNode}
          onDuplicate={duplicateNode}
          onAddChild={setAddingToParent}
@@ -489,7 +501,8 @@ export function NodeList() {
      {editingNode && (
        <NodeEditorModal
          node={editingNode}
-          onClose={() => setEditingNodeId(null)}
+          onClose={handleCloseEditor}
+          isNewNode={isEditingNewNode}
        />
      )}
    </div>
--- a/frontend/src/components/tree-editor/NodePicker.tsx
+++ b/frontend/src/components/tree-editor/NodePicker.tsx
@@ -1,4 +1,4 @@
-import { useMemo } from 'react'
+import { useMemo, useState, useRef, useEffect } from 'react'
 import { useTreeEditorStore } from '@/store/treeEditorStore'
 import type { NodeType } from '@/types'
 import { cn } from '@/lib/utils'
@@ -16,6 +16,13 @@ const NODE_TYPE_SYMBOLS: Record<NodeType, string> = {
  solution: '✓'   // Checkmark for solution
 }

+// Node type labels for UI
+const NODE_TYPE_LABELS: Record<NodeType, string> = {
+  decision: 'Decision',
+  action: 'Action',
+  solution: 'Solution'
+}
+
 interface NodePickerProps {
  value: string
  onChange: (nodeId: string) => void
@@ -41,9 +48,21 @@ export function NodePicker({
  error,
  onNodeCreated
 }: NodePickerProps) {
-  const { getAvailableTargetNodes, addNode } = useTreeEditorStore()
+  const { getAvailableTargetNodes, addNode, updateNode } = useTreeEditorStore()
  const availableNodes = getAvailableTargetNodes(excludeNodeId)

+  // State for inline node creation
+  const [creatingNodeType, setCreatingNodeType] = useState<NodeType | null>(null)
+  const [newNodeTitle, setNewNodeTitle] = useState('')
+  const titleInputRef = useRef<HTMLInputElement>(null)
+
+  // Focus the title input when creating a new node
+  useEffect(() => {
+    if (creatingNodeType && titleInputRef.current) {
+      titleInputRef.current.focus()
+    }
+  }, [creatingNodeType])
+
  // Group nodes by type
  const groupedNodes = useMemo(() => {
    const decisions = availableNodes.filter(n => n.type === 'decision')
@@ -66,20 +85,54 @@ export function NodePicker({
        return
      }

-      // Create the new node as a child of the parent
-      const newNodeId = addNode(parentNodeId, nodeType)
-
-      // Set this new node as the selected value
-      onChange(newNodeId)
-
-      // Notify parent if callback provided
-      onNodeCreated?.(newNodeId)
+      // Show inline title input instead of immediately creating
+      setCreatingNodeType(nodeType)
+      setNewNodeTitle('')
    } else {
      // Normal selection
      onChange(selectedValue)
    }
  }

+  const handleCreateNode = () => {
+    if (!creatingNodeType || !newNodeTitle.trim()) return
+
+    // Create the new node as a child of the parent
+    const newNodeId = addNode(parentNodeId, creatingNodeType)
+
+    // Set the title/question on the new node
+    if (creatingNodeType === 'decision') {
+      updateNode(newNodeId, { question: newNodeTitle.trim() })
+    } else {
+      updateNode(newNodeId, { title: newNodeTitle.trim() })
+    }
+
+    // Set this new node as the selected value
+    onChange(newNodeId)
+
+    // Notify parent if callback provided
+    onNodeCreated?.(newNodeId)
+
+    // Reset creation state
+    setCreatingNodeType(null)
+    setNewNodeTitle('')
+  }
+
+  const handleCancelCreate = () => {
+    setCreatingNodeType(null)
+    setNewNodeTitle('')
+  }
+
+  const handleKeyDown = (e: React.KeyboardEvent) => {
+    if (e.key === 'Enter' && newNodeTitle.trim()) {
+      e.preventDefault()
+      handleCreateNode()
+    } else if (e.key === 'Escape') {
+      e.preventDefault()
+      handleCancelCreate()
+    }
+  }
+
  // Find the label for the currently selected node
  const selectedNode = availableNodes.find(n => n.id === value)

@@ -90,62 +143,110 @@ export function NodePicker({
          {label}
        </label>
      )}
-      <select
-        value={value || ''}
-        onChange={(e) => handleChange(e.target.value)}
-        className={cn(
-          'block w-full rounded-md border px-3 py-2 text-sm',
-          'bg-background text-foreground',
-          'focus:border-primary focus:outline-none focus:ring-1 focus:ring-primary',
-          error ? 'border-destructive' : 'border-input'
-        )}
-      >
-        <option value="">{placeholder}</option>

-        {/* Create new options */}
-        <optgroup label="Create New Node">
-          <option value={CREATE_DECISION}>+ New Decision (question)</option>
-          <option value={CREATE_ACTION}>+ New Action (task)</option>
-          <option value={CREATE_SOLUTION}>+ New Solution (endpoint)</option>
-        </optgroup>
+      {/* Inline node creation UI */}
+      {creatingNodeType ? (
+        <div className="space-y-2">
+          <div className="flex items-center gap-2 rounded-md border border-primary bg-primary/5 p-2">
+            <span className="text-xs font-medium text-primary">
+              New {NODE_TYPE_LABELS[creatingNodeType]}:
+            </span>
+            <input
+              ref={titleInputRef}
+              type="text"
+              value={newNodeTitle}
+              onChange={(e) => setNewNodeTitle(e.target.value)}
+              onKeyDown={handleKeyDown}
+              placeholder={creatingNodeType === 'decision' ? 'Enter question...' : 'Enter title...'}
+              className={cn(
+                'flex-1 rounded-md border border-input px-2 py-1 text-sm',
+                'bg-background text-foreground placeholder:text-muted-foreground',
+                'focus:border-primary focus:outline-none focus:ring-1 focus:ring-primary'
+              )}
+            />
+          </div>
+          <div className="flex gap-2">
+            <button
+              type="button"
+              onClick={handleCancelCreate}
+              className="flex-1 rounded-md border border-input px-3 py-1.5 text-xs font-medium hover:bg-accent"
+            >
+              Cancel
+            </button>
+            <button
+              type="button"
+              onClick={handleCreateNode}
+              disabled={!newNodeTitle.trim()}
+              className={cn(
+                'flex-1 rounded-md px-3 py-1.5 text-xs font-medium',
+                'bg-primary text-primary-foreground hover:bg-primary/90',
+                'disabled:opacity-50 disabled:cursor-not-allowed'
+              )}
+            >
+              Create
+            </button>
+          </div>
+        </div>
+      ) : (
+        <>
+          <select
+            value={value || ''}
+            onChange={(e) => handleChange(e.target.value)}
+            className={cn(
+              'block w-full rounded-md border px-3 py-2 text-sm',
+              'bg-background text-foreground',
+              'focus:border-primary focus:outline-none focus:ring-1 focus:ring-primary',
+              error ? 'border-destructive' : 'border-input'
+            )}
+          >
+            <option value="">{placeholder}</option>

-        {/* Existing nodes grouped by type */}
-        {groupedNodes.decisions.length > 0 && (
-          <optgroup label="── Decisions ──">
-            {groupedNodes.decisions.map((node) => (
-              <option key={node.id} value={node.id}>
-                {NODE_TYPE_SYMBOLS.decision} {node.label}
-              </option>
-            ))}
-          </optgroup>
-        )}
+            {/* Create new options */}
+            <optgroup label="Create New Node">
+              <option value={CREATE_DECISION}>+ New Decision (question)</option>
+              <option value={CREATE_ACTION}>+ New Action (task)</option>
+              <option value={CREATE_SOLUTION}>+ New Solution (endpoint)</option>
+            </optgroup>

-        {groupedNodes.actions.length > 0 && (
-          <optgroup label="── Actions ──">
-            {groupedNodes.actions.map((node) => (
-              <option key={node.id} value={node.id}>
-                {NODE_TYPE_SYMBOLS.action} {node.label}
-              </option>
-            ))}
-          </optgroup>
-        )}
+            {/* Existing nodes grouped by type */}
+            {groupedNodes.decisions.length > 0 && (
+              <optgroup label="── Decisions ──">
+                {groupedNodes.decisions.map((node) => (
+                  <option key={node.id} value={node.id}>
+                    {NODE_TYPE_SYMBOLS.decision} {node.label}
+                  </option>
+                ))}
+              </optgroup>
+            )}

-        {groupedNodes.solutions.length > 0 && (
-          <optgroup label="── Solutions ──">
-            {groupedNodes.solutions.map((node) => (
-              <option key={node.id} value={node.id}>
-                {NODE_TYPE_SYMBOLS.solution} {node.label}
-              </option>
-            ))}
-          </optgroup>
-        )}
-      </select>
+            {groupedNodes.actions.length > 0 && (
+              <optgroup label="── Actions ──">
+                {groupedNodes.actions.map((node) => (
+                  <option key={node.id} value={node.id}>
+                    {NODE_TYPE_SYMBOLS.action} {node.label}
+                  </option>
+                ))}
+              </optgroup>
+            )}

-      {/* Show what's selected */}
-      {value && selectedNode && (
-        <p className="mt-1 text-xs text-muted-foreground">
-          → {selectedNode.label}
-        </p>
+            {groupedNodes.solutions.length > 0 && (
+              <optgroup label="── Solutions ──">
+                {groupedNodes.solutions.map((node) => (
+                  <option key={node.id} value={node.id}>
+                    {NODE_TYPE_SYMBOLS.solution} {node.label}
+                  </option>
+                ))}
+              </optgroup>
+            )}
+          </select>
+
+          {/* Show what's selected */}
+          {value && selectedNode && (
+            <p className="mt-1 text-xs text-muted-foreground">
+              → {selectedNode.label}
+            </p>
+          )}
+        </>
      )}

      {error && <p className="mt-1 text-xs text-destructive">{error}</p>}
--- a/frontend/src/store/treeEditorStore.ts
+++ b/frontend/src/store/treeEditorStore.ts
@@ -657,16 +657,32 @@ export const useTreeEditorStore = create<TreeEditorState>()(
          .filter(id => id !== excludeNodeId)
          .map(id => {
            const node = findNodeInTree(id, state.treeStructure)
-            let label = id
            let type: NodeType = 'decision'
+            let title = ''
+
            if (node) {
              type = node.type
-              if (node.question) label = node.question.slice(0, 50)
-              else if (node.title) label = node.title.slice(0, 50)
+              if (node.question) title = node.question.slice(0, 50)
+              else if (node.title) title = node.title.slice(0, 50)
            }
+
            // Use short ID format, but 'root' stays as-is
            const shortId = id === 'root' ? 'root' : id.slice(0, 8) + '...'
-            return { id, label: `${label} (${shortId})`, type }
+
+            // Format: "Title (shortId)" or just "(shortId)" if no title
+            // For root without a question, show "Root Question (root)"
+            let label: string
+            if (id === 'root') {
+              label = title ? `${title} (root)` : 'Root Question (root)'
+            } else if (title) {
+              label = `${title} (${shortId})`
+            } else {
+              // No title yet - show placeholder with ID
+              const typeName = type === 'decision' ? 'Untitled Question' : `Untitled ${type}`
+              label = `${typeName} (${shortId})`
+            }
+
+            return { id, label, type }
          })
      }
    })),