From b2652690248b264045846c952edc8120ecd1c7f5 Mon Sep 17 00:00:00 2001 From: chihlasm Date: Sun, 8 Feb 2026 23:58:48 -0500 Subject: [PATCH 1/2] fix: repair tree editor drag-to-reorder with 6 bug fixes - Grip-only drag initiation (prevents conflict with click-to-select) - onDragEnd on each draggable item (clears ghost state after failed drops) - Trailing drop zone after last child (enables drop-to-last-position) - Suppress cross-parent drag indicators (no misleading visual feedback) - onDragLeave handler to clear drop indicators when cursor exits - Source parent tracking threaded through component tree Co-Authored-By: Claude Opus 4.6 --- .../src/components/tree-editor/NodeList.tsx | 72 +++++++++++++++++-- 1 file changed, 67 insertions(+), 5 deletions(-) diff --git a/frontend/src/components/tree-editor/NodeList.tsx b/frontend/src/components/tree-editor/NodeList.tsx index 0f924bbb..c3487b19 100644 --- a/frontend/src/components/tree-editor/NodeList.tsx +++ b/frontend/src/components/tree-editor/NodeList.tsx @@ -1,4 +1,4 @@ -import { useState } from 'react' +import { useState, useRef } from 'react' import { Plus, Pencil, @@ -34,7 +34,10 @@ interface NodeListItemProps { onDragStart: (e: React.DragEvent, nodeId: string, parentId: string | null, index: number) => void onDragOver: (e: React.DragEvent, parentId: string | null, index: number) => void onDrop: (e: React.DragEvent, parentId: string | null, index: number) => void + onDragEnd: () => void + onDragLeave: (e: React.DragEvent) => void dragOverTarget: { parentId: string | null; index: number } | null + dragSourceParentId: string | null | undefined /** Array of booleans indicating which ancestor levels should show continuing lines */ ancestorLines?: boolean[] } @@ -53,11 +56,15 @@ function NodeListItem({ onDragStart, onDragOver, onDrop, + onDragEnd, + onDragLeave, dragOverTarget, + dragSourceParentId, ancestorLines = [] }: NodeListItemProps) { const { selectedNodeId, selectNode, validationErrors } = useTreeEditorStore() const [isCollapsed, setIsCollapsed] = useState(false) + const gripInitiated = useRef(false) const isSelected = selectedNodeId === node.id const isRootNode = node.id === 'root' const nodeErrors = validationErrors.filter(e => e.nodeId === node.id && e.severity === 'error') @@ -79,7 +86,9 @@ function NodeListItem({ } const isDragTarget = - dragOverTarget?.parentId === parentId && dragOverTarget?.index === index + dragOverTarget?.parentId === parentId && + dragOverTarget?.index === index && + (dragSourceParentId === undefined || dragSourceParentId === parentId) const nodeTypeIcons: Record = { decision: , @@ -148,8 +157,19 @@ function NodeListItem({
onDragStart(e, node.id, parentId, index)} + onDragStart={(e) => { + if (!gripInitiated.current) { + e.preventDefault() + return + } + onDragStart(e, node.id, parentId, index) + }} + onDragEnd={() => { + gripInitiated.current = false + onDragEnd() + }} onDragOver={(e) => onDragOver(e, parentId, index)} + onDragLeave={onDragLeave} onDrop={(e) => onDrop(e, parentId, index)} onClick={() => selectNode(node.id)} className={cn( @@ -187,7 +207,11 @@ function NodeListItem({ {/* Drag handle */} {node.id !== 'root' && ( - + { gripInitiated.current = true }} + onMouseUp={() => { gripInitiated.current = false }} + /> )} {node.id === 'root' &&
} @@ -336,11 +360,35 @@ function NodeListItem({ onDragStart={onDragStart} onDragOver={onDragOver} onDrop={onDrop} + onDragEnd={onDragEnd} + onDragLeave={onDragLeave} dragOverTarget={dragOverTarget} + dragSourceParentId={dragSourceParentId} ancestorLines={childAncestorLines} /> ) })} + + {/* Trailing drop zone — allows dropping after the last child */} + {!isCollapsed && hasChildren && (() => { + const trailingIndex = node.children!.length + const isTrailingTarget = + dragOverTarget?.parentId === node.id && + dragOverTarget?.index === trailingIndex && + (dragSourceParentId === undefined || dragSourceParentId === node.id) + return ( +
onDragOver(e, node.id, trailingIndex)} + onDragLeave={onDragLeave} + onDrop={(e) => onDrop(e, node.id, trailingIndex)} + /> + ) + })()} ) } @@ -402,9 +450,20 @@ export function NodeList() { // Don't allow dropping on itself or its descendants if (dragState.nodeId === parentId) return + // Suppress cross-parent drag indicator (not supported yet) + if (dragState.parentId !== parentId) return + setDragOverTarget({ parentId, index }) } + const handleDragLeave = (e: React.DragEvent) => { + // Only clear if leaving to an element outside the current target + const relatedTarget = e.relatedTarget as HTMLElement | null + if (!relatedTarget || !e.currentTarget.contains(relatedTarget)) { + setDragOverTarget(null) + } + } + const handleDrop = ( e: React.DragEvent, targetParentId: string | null, @@ -441,7 +500,7 @@ export function NodeList() { } return ( -
+

Nodes

-- 2.49.1 From 1381aaae99a0684259c348bc83b0f149e74d0d11 Mon Sep 17 00:00:00 2001 From: chihlasm Date: Mon, 9 Feb 2026 00:13:48 -0500 Subject: [PATCH 2/2] feat: add cross-parent drag-and-drop with validation feedback Enable moving nodes between different parents in the tree editor. Drop targets show blue indicator for valid drops and red pulsing glow for invalid drops (e.g., dropping onto solution nodes or onto descendants of the dragged node). Co-Authored-By: Claude Opus 4.6 --- .../src/components/tree-editor/NodeList.tsx | 84 +++++++++++++------ frontend/src/store/treeEditorStore.ts | 36 +++++++- 2 files changed, 94 insertions(+), 26 deletions(-) diff --git a/frontend/src/components/tree-editor/NodeList.tsx b/frontend/src/components/tree-editor/NodeList.tsx index c3487b19..3ac0e314 100644 --- a/frontend/src/components/tree-editor/NodeList.tsx +++ b/frontend/src/components/tree-editor/NodeList.tsx @@ -14,7 +14,7 @@ import { AlertCircle, AlertTriangle } from 'lucide-react' -import { useTreeEditorStore } from '@/store/treeEditorStore' +import { useTreeEditorStore, findNodeInTree } from '@/store/treeEditorStore' import { NodeEditorModal } from './NodeEditorModal' import type { TreeStructure, NodeType } from '@/types' import { cn } from '@/lib/utils' @@ -36,8 +36,7 @@ interface NodeListItemProps { onDrop: (e: React.DragEvent, parentId: string | null, index: number) => void onDragEnd: () => void onDragLeave: (e: React.DragEvent) => void - dragOverTarget: { parentId: string | null; index: number } | null - dragSourceParentId: string | null | undefined + dragOverTarget: { parentId: string | null; index: number; isValid: boolean } | null /** Array of booleans indicating which ancestor levels should show continuing lines */ ancestorLines?: boolean[] } @@ -59,7 +58,6 @@ function NodeListItem({ onDragEnd, onDragLeave, dragOverTarget, - dragSourceParentId, ancestorLines = [] }: NodeListItemProps) { const { selectedNodeId, selectNode, validationErrors } = useTreeEditorStore() @@ -87,8 +85,8 @@ function NodeListItem({ const isDragTarget = dragOverTarget?.parentId === parentId && - dragOverTarget?.index === index && - (dragSourceParentId === undefined || dragSourceParentId === parentId) + dragOverTarget?.index === index + const isDragValid = isDragTarget && dragOverTarget?.isValid const nodeTypeIcons: Record = { decision: , @@ -152,7 +150,15 @@ function NodeListItem({ <> {/* Drop indicator above */} {isDragTarget && ( -
+
)}
) @@ -374,13 +379,17 @@ function NodeListItem({ const trailingIndex = node.children!.length const isTrailingTarget = dragOverTarget?.parentId === node.id && - dragOverTarget?.index === trailingIndex && - (dragSourceParentId === undefined || dragSourceParentId === node.id) + dragOverTarget?.index === trailingIndex + const isTrailingValid = isTrailingTarget && dragOverTarget?.isValid return (
onDragOver(e, node.id, trailingIndex)} @@ -394,7 +403,7 @@ function NodeListItem({ } export function NodeList() { - const { treeStructure, addNode, deleteNode, duplicateNode, reorderNodes, findNode } = useTreeEditorStore() + const { treeStructure, addNode, deleteNode, duplicateNode, reorderNodes, moveNode, findNode } = useTreeEditorStore() const [editingNodeId, setEditingNodeId] = useState(null) const [isEditingNewNode, setIsEditingNewNode] = useState(false) const [addingToParent, setAddingToParent] = useState(null) @@ -409,6 +418,7 @@ export function NodeList() { const [dragOverTarget, setDragOverTarget] = useState<{ parentId: string | null index: number + isValid: boolean } | null>(null) const handleAddNode = (type: NodeType) => { @@ -439,6 +449,29 @@ export function NodeList() { setDragState({ nodeId, parentId, index }) } + // Check if a node can be dropped at a target location + const canDropAt = (draggedNodeId: string, targetParentId: string | null): boolean => { + if (!treeStructure) return false + + // Can't drop at root level (no parent) + if (targetParentId === null) return false + + // Can't drop onto itself + if (draggedNodeId === targetParentId) return false + + // Can't drop onto a descendant of the dragged node + const draggedNode = findNodeInTree(draggedNodeId, treeStructure) + if (!draggedNode) return false + if (findNodeInTree(targetParentId, draggedNode)) return false + + // Target parent must be able to have children (solution nodes are terminal) + const targetParent = findNodeInTree(targetParentId, treeStructure) + if (!targetParent) return false + if (targetParent.type === 'solution') return false + + return true + } + const handleDragOver = ( e: React.DragEvent, parentId: string | null, @@ -447,13 +480,11 @@ export function NodeList() { e.preventDefault() if (!dragState) return - // Don't allow dropping on itself or its descendants + // Don't show indicator when hovering over self if (dragState.nodeId === parentId) return - // Suppress cross-parent drag indicator (not supported yet) - if (dragState.parentId !== parentId) return - - setDragOverTarget({ parentId, index }) + const isValid = canDropAt(dragState.nodeId, parentId) + setDragOverTarget({ parentId, index, isValid }) } const handleDragLeave = (e: React.DragEvent) => { @@ -472,13 +503,19 @@ export function NodeList() { e.preventDefault() if (!dragState) return - const { parentId: sourceParentId, index: sourceIndex } = dragState + const { parentId: sourceParentId, index: sourceIndex, nodeId } = dragState - // Only handle reordering within same parent for now - if (sourceParentId === targetParentId && sourceParentId) { - const adjustedIndex = sourceIndex < targetIndex ? targetIndex - 1 : targetIndex - if (sourceIndex !== adjustedIndex) { - reorderNodes(sourceParentId, sourceIndex, adjustedIndex) + // Only execute valid drops + if (targetParentId && canDropAt(nodeId, targetParentId)) { + if (sourceParentId === targetParentId) { + // Same parent — use reorderNodes with index adjustment + const adjustedIndex = sourceIndex < targetIndex ? targetIndex - 1 : targetIndex + if (sourceIndex !== adjustedIndex) { + reorderNodes(sourceParentId, sourceIndex, adjustedIndex) + } + } else { + // Cross-parent — use moveNode + moveNode(nodeId, targetParentId, targetIndex) } } @@ -533,7 +570,6 @@ export function NodeList() { onDragEnd={handleDragEnd} onDragLeave={handleDragLeave} dragOverTarget={dragOverTarget} - dragSourceParentId={dragState?.parentId} />
diff --git a/frontend/src/store/treeEditorStore.ts b/frontend/src/store/treeEditorStore.ts index 495e5091..92d310a7 100644 --- a/frontend/src/store/treeEditorStore.ts +++ b/frontend/src/store/treeEditorStore.ts @@ -17,8 +17,8 @@ const DRAFT_STORAGE_KEY = 'tree-editor-draft' // Helper to generate unique IDs const generateId = () => crypto.randomUUID() -// Helper to find a node in the tree structure -const findNodeInTree = ( +// Helper to find a node in the tree structure (exported for drag validation) +export const findNodeInTree = ( nodeId: string, structure: TreeStructure | null ): TreeStructure | null => { @@ -144,6 +144,7 @@ interface TreeEditorState { // Actions - Node ordering reorderNodes: (parentId: string, fromIndex: number, toIndex: number) => void + moveNode: (nodeId: string, targetParentId: string, targetIndex: number) => void reorderOptions: (nodeId: string, fromIndex: number, toIndex: number) => void // Actions - Selection @@ -496,6 +497,37 @@ export const useTreeEditorStore = create()( get().autoSaveDraft() }, + moveNode: (nodeId: string, targetParentId: string, targetIndex: number) => { + set((state) => { + // Find and remove from current parent + const currentParent = findParentNode(nodeId, state.treeStructure) + if (!currentParent?.children) return + + const sourceIndex = currentParent.children.findIndex(c => c.id === nodeId) + if (sourceIndex === -1) return + + const [movedNode] = currentParent.children.splice(sourceIndex, 1) + + // Find target parent and insert + const targetParent = findNodeInTree(targetParentId, state.treeStructure) + if (!targetParent) return + + if (!targetParent.children) { + targetParent.children = [] + } + + // Adjust index if moving within same parent and source was before target + let adjustedIndex = targetIndex + if (currentParent.id === targetParent.id && sourceIndex < targetIndex) { + adjustedIndex = targetIndex - 1 + } + + targetParent.children.splice(adjustedIndex, 0, movedNode) + state.isDirty = true + }) + get().autoSaveDraft() + }, + reorderOptions: (nodeId: string, fromIndex: number, toIndex: number) => { set((state) => { const node = findNodeInTree(nodeId, state.treeStructure) -- 2.49.1