fix: repair tree editor drag-to-reorder (6 bugs) #48

Merged
chihlasm merged 2 commits from fix/tree-editor-drag-reorder into main 2026-02-09 14:50:33 +00:00
2 changed files with 94 additions and 26 deletions
Showing only changes of commit 1381aaae99 - Show all commits

View File

@@ -14,7 +14,7 @@ import {
AlertCircle, AlertCircle,
AlertTriangle AlertTriangle
} from 'lucide-react' } from 'lucide-react'
import { useTreeEditorStore } from '@/store/treeEditorStore' import { useTreeEditorStore, findNodeInTree } from '@/store/treeEditorStore'
import { NodeEditorModal } from './NodeEditorModal' import { NodeEditorModal } from './NodeEditorModal'
import type { TreeStructure, NodeType } from '@/types' import type { TreeStructure, NodeType } from '@/types'
import { cn } from '@/lib/utils' import { cn } from '@/lib/utils'
@@ -36,8 +36,7 @@ interface NodeListItemProps {
onDrop: (e: React.DragEvent, parentId: string | null, index: number) => void onDrop: (e: React.DragEvent, parentId: string | null, index: number) => void
onDragEnd: () => void onDragEnd: () => void
onDragLeave: (e: React.DragEvent) => void onDragLeave: (e: React.DragEvent) => void
dragOverTarget: { parentId: string | null; index: number } | null dragOverTarget: { parentId: string | null; index: number; isValid: boolean } | null
dragSourceParentId: string | null | undefined
/** Array of booleans indicating which ancestor levels should show continuing lines */ /** Array of booleans indicating which ancestor levels should show continuing lines */
ancestorLines?: boolean[] ancestorLines?: boolean[]
} }
@@ -59,7 +58,6 @@ function NodeListItem({
onDragEnd, onDragEnd,
onDragLeave, onDragLeave,
dragOverTarget, dragOverTarget,
dragSourceParentId,
ancestorLines = [] ancestorLines = []
}: NodeListItemProps) { }: NodeListItemProps) {
const { selectedNodeId, selectNode, validationErrors } = useTreeEditorStore() const { selectedNodeId, selectNode, validationErrors } = useTreeEditorStore()
@@ -87,8 +85,8 @@ function NodeListItem({
const isDragTarget = const isDragTarget =
dragOverTarget?.parentId === parentId && dragOverTarget?.parentId === parentId &&
dragOverTarget?.index === index && dragOverTarget?.index === index
(dragSourceParentId === undefined || dragSourceParentId === parentId) const isDragValid = isDragTarget && dragOverTarget?.isValid
const nodeTypeIcons: Record<NodeType, React.ReactNode> = { const nodeTypeIcons: Record<NodeType, React.ReactNode> = {
decision: <HelpCircle className="h-4 w-4" />, decision: <HelpCircle className="h-4 w-4" />,
@@ -152,7 +150,15 @@ function NodeListItem({
<> <>
{/* Drop indicator above */} {/* Drop indicator above */}
{isDragTarget && ( {isDragTarget && (
<div className="h-1 bg-primary rounded-full mx-2" style={{ marginLeft: `${depth * 20 + 8}px` }} /> <div
className={cn(
'h-1 rounded-full mx-2 transition-colors',
isDragValid
? 'bg-primary'
: 'bg-destructive animate-pulse shadow-[0_0_8px_rgba(239,68,68,0.6)]'
)}
style={{ marginLeft: `${depth * 20 + 8}px` }}
/>
)} )}
<div <div
@@ -363,7 +369,6 @@ function NodeListItem({
onDragEnd={onDragEnd} onDragEnd={onDragEnd}
onDragLeave={onDragLeave} onDragLeave={onDragLeave}
dragOverTarget={dragOverTarget} dragOverTarget={dragOverTarget}
dragSourceParentId={dragSourceParentId}
ancestorLines={childAncestorLines} ancestorLines={childAncestorLines}
/> />
) )
@@ -374,13 +379,17 @@ function NodeListItem({
const trailingIndex = node.children!.length const trailingIndex = node.children!.length
const isTrailingTarget = const isTrailingTarget =
dragOverTarget?.parentId === node.id && dragOverTarget?.parentId === node.id &&
dragOverTarget?.index === trailingIndex && dragOverTarget?.index === trailingIndex
(dragSourceParentId === undefined || dragSourceParentId === node.id) const isTrailingValid = isTrailingTarget && dragOverTarget?.isValid
return ( return (
<div <div
className={cn( className={cn(
'h-2 rounded-full mx-2 transition-colors', 'h-2 rounded-full mx-2 transition-colors',
isTrailingTarget ? 'bg-primary' : 'bg-transparent' isTrailingTarget
? isTrailingValid
? 'bg-primary'
: 'bg-destructive animate-pulse shadow-[0_0_8px_rgba(239,68,68,0.6)]'
: 'bg-transparent'
)} )}
style={{ marginLeft: `${(depth + 1) * 20 + 8}px` }} style={{ marginLeft: `${(depth + 1) * 20 + 8}px` }}
onDragOver={(e) => onDragOver(e, node.id, trailingIndex)} onDragOver={(e) => onDragOver(e, node.id, trailingIndex)}
@@ -394,7 +403,7 @@ function NodeListItem({
} }
export function NodeList() { export function NodeList() {
const { treeStructure, addNode, deleteNode, duplicateNode, reorderNodes, findNode } = useTreeEditorStore() const { treeStructure, addNode, deleteNode, duplicateNode, reorderNodes, moveNode, findNode } = useTreeEditorStore()
const [editingNodeId, setEditingNodeId] = useState<string | null>(null) const [editingNodeId, setEditingNodeId] = useState<string | null>(null)
const [isEditingNewNode, setIsEditingNewNode] = useState(false) const [isEditingNewNode, setIsEditingNewNode] = useState(false)
const [addingToParent, setAddingToParent] = useState<string | null>(null) const [addingToParent, setAddingToParent] = useState<string | null>(null)
@@ -409,6 +418,7 @@ export function NodeList() {
const [dragOverTarget, setDragOverTarget] = useState<{ const [dragOverTarget, setDragOverTarget] = useState<{
parentId: string | null parentId: string | null
index: number index: number
isValid: boolean
} | null>(null) } | null>(null)
const handleAddNode = (type: NodeType) => { const handleAddNode = (type: NodeType) => {
@@ -439,6 +449,29 @@ export function NodeList() {
setDragState({ nodeId, parentId, index }) 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 = ( const handleDragOver = (
e: React.DragEvent, e: React.DragEvent,
parentId: string | null, parentId: string | null,
@@ -447,13 +480,11 @@ export function NodeList() {
e.preventDefault() e.preventDefault()
if (!dragState) return 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 if (dragState.nodeId === parentId) return
// Suppress cross-parent drag indicator (not supported yet) const isValid = canDropAt(dragState.nodeId, parentId)
if (dragState.parentId !== parentId) return setDragOverTarget({ parentId, index, isValid })
setDragOverTarget({ parentId, index })
} }
const handleDragLeave = (e: React.DragEvent) => { const handleDragLeave = (e: React.DragEvent) => {
@@ -472,14 +503,20 @@ export function NodeList() {
e.preventDefault() e.preventDefault()
if (!dragState) return if (!dragState) return
const { parentId: sourceParentId, index: sourceIndex } = dragState const { parentId: sourceParentId, index: sourceIndex, nodeId } = dragState
// Only handle reordering within same parent for now // Only execute valid drops
if (sourceParentId === targetParentId && sourceParentId) { if (targetParentId && canDropAt(nodeId, targetParentId)) {
if (sourceParentId === targetParentId) {
// Same parent — use reorderNodes with index adjustment
const adjustedIndex = sourceIndex < targetIndex ? targetIndex - 1 : targetIndex const adjustedIndex = sourceIndex < targetIndex ? targetIndex - 1 : targetIndex
if (sourceIndex !== adjustedIndex) { if (sourceIndex !== adjustedIndex) {
reorderNodes(sourceParentId, sourceIndex, adjustedIndex) reorderNodes(sourceParentId, sourceIndex, adjustedIndex)
} }
} else {
// Cross-parent — use moveNode
moveNode(nodeId, targetParentId, targetIndex)
}
} }
setDragState(null) setDragState(null)
@@ -533,7 +570,6 @@ export function NodeList() {
onDragEnd={handleDragEnd} onDragEnd={handleDragEnd}
onDragLeave={handleDragLeave} onDragLeave={handleDragLeave}
dragOverTarget={dragOverTarget} dragOverTarget={dragOverTarget}
dragSourceParentId={dragState?.parentId}
/> />
</div> </div>

View File

@@ -17,8 +17,8 @@ const DRAFT_STORAGE_KEY = 'tree-editor-draft'
// Helper to generate unique IDs // Helper to generate unique IDs
const generateId = () => crypto.randomUUID() const generateId = () => crypto.randomUUID()
// Helper to find a node in the tree structure // Helper to find a node in the tree structure (exported for drag validation)
const findNodeInTree = ( export const findNodeInTree = (
nodeId: string, nodeId: string,
structure: TreeStructure | null structure: TreeStructure | null
): TreeStructure | null => { ): TreeStructure | null => {
@@ -144,6 +144,7 @@ interface TreeEditorState {
// Actions - Node ordering // Actions - Node ordering
reorderNodes: (parentId: string, fromIndex: number, toIndex: number) => void reorderNodes: (parentId: string, fromIndex: number, toIndex: number) => void
moveNode: (nodeId: string, targetParentId: string, targetIndex: number) => void
reorderOptions: (nodeId: string, fromIndex: number, toIndex: number) => void reorderOptions: (nodeId: string, fromIndex: number, toIndex: number) => void
// Actions - Selection // Actions - Selection
@@ -496,6 +497,37 @@ export const useTreeEditorStore = create<TreeEditorState>()(
get().autoSaveDraft() 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) => { reorderOptions: (nodeId: string, fromIndex: number, toIndex: number) => {
set((state) => { set((state) => {
const node = findNodeInTree(nodeId, state.treeStructure) const node = findNodeInTree(nodeId, state.treeStructure)