fix: tree DnD — correct cycle detection, reactive indicators, canonical reload

Backend:
- Fix MoveNode validation: wouldCreateCycle walks from newParentID up
  toward root, rejects if nodeID is encountered (parent into descendant)
- Allow moving descendant to ancestor (C into A) and child to root
- Add isContainerType validation for new parent
- Add 8 tests covering all scenarios + duplicate ID invariant

Frontend TreeNode.svelte:
- Build parent map from full tree (not just loaded children)
- canDrop uses parent map for cycle detection
- Reactive drop-valid/drop-invalid CSS via pre-computed dropAllowed map
- Keyed {#each nodes as node (node.id)} for correct identity tracking
- Auto-expand container on 600ms drag-over hover
- Proper dragleave detection (ignore transitions to child elements)
- Clean up state on dragend

Frontend App.svelte:
- reloadTreePreservingExpanded: fresh roots + children (no patching)
- Root area visual drop indicator (dashed outline)
- dragleave handler for root area

Clean up stale GUI dist assets
This commit is contained in:
mirivlad 2026-06-03 05:27:20 +08:00
parent 8cbf23a74d
commit 23b3d07071
19 changed files with 451 additions and 72 deletions

View File

@ -49,14 +49,21 @@ func (a *App) ListWorkspaceChildren(parentID string) ([]NodeDTO, error) {
func filterContainers(dtos []NodeDTO) []NodeDTO {
out := make([]NodeDTO, 0, len(dtos))
for _, d := range dtos {
switch d.Type {
case "case", "client", "project", "folder", "document", "recipe":
if isContainerType(d.Type) {
out = append(out, d)
}
}
return out
}
func isContainerType(typ string) bool {
switch typ {
case "case", "client", "project", "folder", "document", "recipe":
return true
}
return false
}
func (a *App) ListChildren(parentID string) ([]NodeDTO, error) {
list, err := a.nodes.ListChildren(parentID, false)
if err != nil {
@ -532,15 +539,21 @@ func (a *App) renameNoteFileNode(nodeID string, n *nodes.Node, newTitle, seg str
return nil
}
func (a *App) isDescendant(ancestorID, nodeID string) error {
if nodeID == "" || ancestorID == "" {
// wouldCreateCycle checks that moving nodeID into newParentID does not create a cycle.
// It walks from newParentID up toward root: if nodeID is encountered, then newParentID
// is inside nodeID's subtree, and the move would create a cycle.
func (a *App) wouldCreateCycle(nodeID, newParentID string) error {
if nodeID == "" {
return fmt.Errorf("node ID is empty")
}
if newParentID == "" {
return nil
}
current := nodeID
current := newParentID
depth := 0
for current != "" && depth < 1000 {
if current == ancestorID {
return fmt.Errorf("cannot move a node into its own descendant")
if current == nodeID {
return fmt.Errorf("cannot move a node into itself or its descendant")
}
n, err := a.nodes.Get(current)
if err != nil || n.ParentID == nil {
@ -553,16 +566,23 @@ func (a *App) isDescendant(ancestorID, nodeID string) error {
}
func (a *App) MoveNode(nodeID, newParentID string) error {
node, err := a.nodes.GetActive(nodeID)
if err != nil {
return err
if nodeID == "" {
return fmt.Errorf("node ID is required")
}
// Prevent moving node into its own descendant
if newParentID != "" {
if err := a.isDescendant(newParentID, nodeID); err != nil {
return err
node, err := a.nodes.GetActive(nodeID)
if err != nil {
return fmt.Errorf("node not found: %w", err)
}
// Cannot move into itself
if nodeID == newParentID {
return fmt.Errorf("cannot move a node into itself")
}
// Cannot move into descendant (would create cycle)
if err := a.wouldCreateCycle(nodeID, newParentID); err != nil {
return err
}
isFolderLike := node.Type != nodes.TypeNote && node.Type != nodes.TypeFile
@ -574,6 +594,9 @@ func (a *App) MoveNode(nodeID, newParentID string) error {
if err != nil {
return fmt.Errorf("new parent not found: %w", err)
}
if !isContainerType(parent.Type) {
return fmt.Errorf("target %q is not a container (type: %s)", parent.Title, parent.Type)
}
}
// Compute new title (name conflict resolution) but don't commit yet.

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

File diff suppressed because one or more lines are too long

View File

@ -16,8 +16,8 @@
background: #13131f;
}
</style>
<script type="module" crossorigin src="/assets/main-C54_blTe.js"></script>
<link rel="stylesheet" crossorigin href="/assets/main-CtXutc3Z.css">
<script type="module" crossorigin src="/assets/main-BH7waEiY.js"></script>
<link rel="stylesheet" crossorigin href="/assets/main-BzI_Zj56.css">
</head>
<body>
<div id="app"></div>

View File

@ -0,0 +1,255 @@
package main
import (
"strings"
"testing"
"verstak/internal/core/nodes"
)
// setupTree creates a simple tree: A -> B -> C for MoveNode testing.
func setupTree(t *testing.T) (*App, *NodeDTO, *NodeDTO, *NodeDTO) {
t.Helper()
app, _ := setupTestApp(t)
a, err := app.CreateNodeFromTemplate("", "A", "folder.default")
if err != nil {
t.Fatalf("create A: %v", err)
}
b, err := app.CreateNodeFromTemplate(a.ID, "B", "folder.default")
if err != nil {
t.Fatalf("create B: %v", err)
}
c, err := app.CreateNodeFromTemplate(b.ID, "C", "folder.default")
if err != nil {
t.Fatalf("create C: %v", err)
}
return app, a, b, c
}
// listTree returns a flat mapping of all visible node IDs in the workspace.
func listTreeIDs(t *testing.T, app *App) map[string]int {
t.Helper()
roots, err := app.ListWorkspaceTree()
if err != nil {
t.Fatalf("ListWorkspaceTree: %v", err)
}
ids := make(map[string]int)
var walk func(parentID string)
walk = func(parentID string) {
var nodes []NodeDTO
var err error
if parentID == "" {
nodes = roots
} else {
nodes, err = app.ListWorkspaceChildren(parentID)
if err != nil {
return
}
}
for _, n := range nodes {
ids[n.ID]++
if n.HasChildren {
walk(n.ID)
}
}
}
walk("")
return ids
}
func TestMoveNode_DescendantToAncestor(t *testing.T) {
app, a, b, c := setupTree(t)
// Move C into A — must succeed
err := app.MoveNode(c.ID, a.ID)
if err != nil {
t.Fatalf("MoveNode(C, A): %v", err)
}
// Verify C's parent is A
nc, err := app.nodes.GetActive(c.ID)
if err != nil {
t.Fatalf("GetActive(C): %v", err)
}
if nc.ParentID == nil || *nc.ParentID != a.ID {
t.Errorf("C parent_id = %v, want %s", nc.ParentID, a.ID)
}
// C should no longer be a child of B
bKids, _ := app.nodes.ListChildren(b.ID, false)
for _, child := range bKids {
if child.ID == c.ID {
t.Error("C still appears under B after move")
}
}
// C should be a child of A
aKids, _ := app.nodes.ListChildren(a.ID, false)
found := false
for _, child := range aKids {
if child.ID == c.ID {
found = true
break
}
}
if !found {
t.Error("C not found under A after move")
}
}
func TestMoveNode_ChildToRoot(t *testing.T) {
app, a, b, _ := setupTree(t)
// Move B to root — must succeed
err := app.MoveNode(b.ID, "")
if err != nil {
t.Fatalf("MoveNode(B, root): %v", err)
}
// Verify B has no parent
nb, err := app.nodes.GetActive(b.ID)
if err != nil {
t.Fatalf("GetActive(B): %v", err)
}
if nb.ParentID != nil {
t.Errorf("B parent_id = %v, want nil", nb.ParentID)
}
// B should NOT be under A
aKids, _ := app.nodes.ListChildren(a.ID, false)
for _, child := range aKids {
if child.ID == b.ID {
t.Error("B still appears under A after move to root")
}
}
// B should appear in roots
roots, _ := app.nodes.ListRoots(false)
found := false
for _, root := range roots {
if root.ID == b.ID {
found = true
break
}
}
if !found {
t.Error("B not found in roots after move")
}
}
func TestMoveNode_ParentIntoChild_Rejected(t *testing.T) {
app, a, b, _ := setupTree(t)
err := app.MoveNode(a.ID, b.ID)
if err == nil {
t.Fatal("MoveNode(A, B): expected error, got nil")
}
if !strings.Contains(err.Error(), "descendant") && !strings.Contains(err.Error(), "cycle") {
t.Errorf("MoveNode(A, B): unexpected error: %v", err)
}
// Verify A's parent unchanged
na, _ := app.nodes.GetActive(a.ID)
if na.ParentID != nil {
t.Errorf("A parent_id changed to %v after rejected move", *na.ParentID)
}
}
func TestMoveNode_ParentIntoDeepDescendant_Rejected(t *testing.T) {
app, a, _, c := setupTree(t)
err := app.MoveNode(a.ID, c.ID)
if err == nil {
t.Fatal("MoveNode(A, C): expected error, got nil")
}
if !strings.Contains(err.Error(), "descendant") && !strings.Contains(err.Error(), "cycle") {
t.Errorf("MoveNode(A, C): unexpected error: %v", err)
}
}
func TestMoveNode_IntoSelf_Rejected(t *testing.T) {
app, a, _, _ := setupTree(t)
err := app.MoveNode(a.ID, a.ID)
if err == nil {
t.Fatal("MoveNode(A, A): expected error, got nil")
}
if !strings.Contains(err.Error(), "into itself") {
t.Errorf("MoveNode(A, A): unexpected error: %v", err)
}
}
func TestMoveNode_NoDuplicateIDs(t *testing.T) {
app, a, b, c := setupTree(t)
// Initial check: no duplicates
ids := listTreeIDs(t, app)
for id, count := range ids {
if count > 1 {
t.Errorf("Duplicate ID %q found %d times before move", id, count)
}
}
// Move C into A
if err := app.MoveNode(c.ID, a.ID); err != nil {
t.Fatalf("MoveNode(C, A): %v", err)
}
ids = listTreeIDs(t, app)
for id, count := range ids {
if count > 1 {
t.Errorf("Duplicate ID %q found %d times after C→A move", id, count)
}
}
// Move B to root
if err := app.MoveNode(b.ID, ""); err != nil {
t.Fatalf("MoveNode(B, root): %v", err)
}
ids = listTreeIDs(t, app)
for id, count := range ids {
if count > 1 {
t.Errorf("Duplicate ID %q found %d times after B→root move", id, count)
}
}
}
func TestMoveNode_FsPathUpdated(t *testing.T) {
app, _, _, c := setupTree(t)
ncBefore, _ := app.nodes.GetActive(c.ID)
origPath := ncBefore.FsPath
// Move C to root — fs_path should change
if err := app.MoveNode(c.ID, ""); err != nil {
t.Fatalf("MoveNode(C, root): %v", err)
}
nc, _ := app.nodes.GetActive(c.ID)
if nc.FsPath == origPath {
t.Errorf("C fs_path unchanged after move to root: %q", nc.FsPath)
}
}
func TestMoveNode_NonContainerRejected(t *testing.T) {
app, a, _, _ := setupTree(t)
// Create a root-level note (not inside A's subtree)
note, err := app.nodes.Create(nil, nodes.TypeNote, "Test Note", 0, "", "")
if err != nil {
t.Fatalf("create note: %v", err)
}
err = app.MoveNode(a.ID, note.ID)
if err == nil {
t.Fatal("MoveNode(A, note): expected error for non-container, got nil")
}
if !strings.Contains(err.Error(), "not a container") {
t.Errorf("MoveNode(A, note): unexpected error: %v", err)
}
}

View File

@ -729,6 +729,11 @@
function handleDragOverRoot(e) {
e.preventDefault()
e.dataTransfer.dropEffect = 'move'
e.currentTarget.classList.add('drop-valid')
}
function handleDragLeaveRoot(e) {
e.currentTarget.classList.remove('drop-valid')
}
// ===== Node operations from context menu =====
@ -778,11 +783,30 @@
async function reloadTreePreservingExpanded() {
const expandedIds = Object.keys(expanded).filter(id => expanded[id])
// Build new tree from scratch: fresh roots + fresh children for each expanded node
const newTree = await wailsCall('ListWorkspaceTree') || []
for (const nodeId of expandedIds) {
const children = await wailsCall('ListWorkspaceChildren', nodeId) || []
setNodeChildren(workspaceTree, nodeId, children)
const newChildren = children.map(c => ({ ...c })) // plain copies
addChildrenToTree(newTree, nodeId, newChildren)
}
workspaceTree = [...workspaceTree]
workspaceTree = newTree
}
function addChildrenToTree(tree, nodeId, children) {
for (const node of tree) {
if (node.id === nodeId) {
node.children = children
node.has_children = children.length > 0
return true
}
if (node.children) {
const found = addChildrenToTree(node.children, nodeId, children)
if (found) return true
}
}
return false
}
async function refreshParentNode(nodeId) {
@ -1175,6 +1199,7 @@
{#if workspaceTree.length > 0}
<div class="workspace-tree-area"
on:dragover|preventDefault={handleDragOverRoot}
on:dragleave={handleDragLeaveRoot}
on:drop={handleDropRoot}>
<TreeNode
nodes={workspaceTree}
@ -1853,6 +1878,7 @@
.nav-item:hover { background: #222233; }
.nav-item.selected { background: #2a2a4a; color: #fff; font-weight: 500; }
.workspace-tree-area { min-height: 32px; }
.workspace-tree-area.drop-valid { outline: 2px dashed #4ade80; outline-offset: -2px; background: rgba(74, 222, 128, 0.05); }
.nav-empty { padding: 8px 20px; color: #555; font-size: 12px; }
.nav-label-row { font-size: 10px; text-transform: uppercase; letter-spacing: 0.5px; color: #666; padding: 4px 20px; margin-bottom: 4px; display: flex; align-items: center; justify-content: space-between; }
.nav-add-btn { background: none; border: none; color: #666; cursor: pointer; font-size: 16px; padding: 0 4px; font-family: inherit; line-height: 1; }

View File

@ -5,16 +5,27 @@
export let expanded = {}
export let selectedNodeId = ''
export let level = 0
export let onSelect
export let onToggle
export let onContextMenu
export let onDrop
export let onSelect = undefined
export let onToggle = undefined
export let onContextMenu = undefined
export let onDrop = undefined
let autoExpandTimers = {}
let scrollInterval = null
let draggedNodeId = ''
let dragOverNodeId = ''
const CONTAINER_TYPES = ['folder', 'project', 'client', 'document', 'recipe', 'case']
import { onDestroy } from 'svelte'
onDestroy(() => {
for (const key of Object.keys(autoExpandTimers)) {
clearTimeout(autoExpandTimers[key])
}
if (scrollInterval) clearInterval(scrollInterval)
})
function iconKind(node) {
if (node.type === 'client' || node.template_id === 'client.default') return 'client'
if (node.type === 'project' || node.template_id === 'project.default') return 'project'
@ -42,21 +53,53 @@
return node.has_children === true
}
function canDrop(target, draggedId) {
// Build a flat parent map by walking the full tree recursively.
// Key: node.id, Value: parent node id (or '' for roots)
function buildParentMap(allNodes) {
const map = {}
function walk(list, parentId) {
for (const n of list) {
map[n.id] = parentId
if (n.children) {
walk(n.children, n.id)
}
}
}
walk(allNodes, '')
return map
}
// Check if dropping draggedId onto target is allowed.
// Uses a parent map to detect cycles: walk from target up;
// if we hit draggedId, it would be a cycle.
function canDrop(target, draggedId, parentMap) {
if (!target || !draggedId) return false
if (draggedId === target.id) return false
if (!isContainer(target)) return false
if (isDescendant(target, draggedId)) return false
// Walk from target up the parent chain; if we reach draggedId, reject (cycle).
let cur = target.id
while (cur) {
if (cur === draggedId) return false
cur = parentMap[cur] || ''
}
return true
}
function isDescendant(node, ancestorId) {
if (!node.children) return false
for (const child of node.children) {
if (child.id === ancestorId) return true
if (isDescendant(child, ancestorId)) return true
// Flatten all nodes recursively for parent map construction
function flattenTree(list) {
const result = []
function walk(items) {
for (const n of items) {
result.push(n)
if (n.children) {
walk(n.children)
}
return false
}
}
walk(list)
return result
}
function getDraggedId(e) {
@ -67,21 +110,25 @@
e.stopPropagation()
e.dataTransfer.effectAllowed = 'move'
e.dataTransfer.setData('text/plain', node.id)
draggedNodeId = node.id
}
function handleDragOver(e, node) {
const id = getDraggedId(e)
if (!canDrop(node, id)) return
e.preventDefault()
e.stopPropagation()
e.dataTransfer.dropEffect = 'move'
e.currentTarget.classList.add('drop-valid')
if (dragOverNodeId !== node.id) {
dragOverNodeId = node.id
}
if (shouldShowToggle(node) && !expanded[node.id] && !autoExpandTimers[node.id]) {
autoExpandTimers[node.id] = setTimeout(() => {
if (onToggle) onToggle(node.id)
delete autoExpandTimers[node.id]
}, 500)
}, 600)
}
const area = e.currentTarget.closest('.workspace-tree-area')
if (area) {
const areaRect = area.getBoundingClientRect()
@ -96,12 +143,18 @@
}
}
function handleDragLeave(e) {
e.currentTarget.classList.remove('drop-valid')
const nodeId = e.currentTarget.dataset.nodeId
if (nodeId && autoExpandTimers[nodeId]) {
clearTimeout(autoExpandTimers[nodeId])
delete autoExpandTimers[nodeId]
function handleDragLeave(e, node) {
// Only clear if we're actually leaving this row, not moving to a child element
const related = e.relatedTarget
if (related && e.currentTarget.contains(related)) {
return
}
if (dragOverNodeId === node.id) {
dragOverNodeId = ''
}
if (autoExpandTimers[node.id]) {
clearTimeout(autoExpandTimers[node.id])
delete autoExpandTimers[node.id]
}
if (scrollInterval) { clearInterval(scrollInterval); scrollInterval = null }
}
@ -109,15 +162,34 @@
function handleDrop(e, node) {
e.preventDefault()
e.stopPropagation()
e.currentTarget.classList.remove('drop-valid')
if (autoExpandTimers[node.id]) {
clearTimeout(autoExpandTimers[node.id])
delete autoExpandTimers[node.id]
}
if (scrollInterval) { clearInterval(scrollInterval); scrollInterval = null }
const draggedId = getDraggedId(e)
if (!canDrop(node, draggedId)) return
if (onDrop) onDrop(draggedId, node.id)
const allNodes = flattenTree(nodes)
const parentMap = buildParentMap(allNodes)
const id = getDraggedId(e)
if (!canDrop(node, id, parentMap)) {
dragOverNodeId = ''
draggedNodeId = ''
return
}
if (onDrop) onDrop(id, node.id)
dragOverNodeId = ''
draggedNodeId = ''
}
function handleDragEnd() {
dragOverNodeId = ''
draggedNodeId = ''
for (const key of Object.keys(autoExpandTimers)) {
clearTimeout(autoExpandTimers[key])
delete autoExpandTimers[key]
}
if (scrollInterval) { clearInterval(scrollInterval); scrollInterval = null }
}
function handleRowClick(e, node) {
@ -133,11 +205,31 @@
e.stopPropagation()
if (shouldShowToggle(node) && onToggle) onToggle(node.id)
}
function computeDropInfo(allNodes, draggedId, pMap) {
const info = {}
function walk(list) {
for (const n of list) {
info[n.id] = canDrop(n, draggedId, pMap)
if (n.children) walk(n.children)
}
}
walk(allNodes)
return info
}
$: flatNodes = flattenTree(nodes)
$: parentMap = buildParentMap(flatNodes)
$: dropAllowed = computeDropInfo(flatNodes, draggedNodeId, parentMap)
</script>
{#each nodes as node}
<svelte:window on:dragend={handleDragEnd}/>
{#each nodes as node (node.id)}
<div class="tree-item"
class:selected={selectedNodeId === node.id}
class:drop-valid={dragOverNodeId === node.id && dropAllowed[node.id]}
class:drop-invalid={dragOverNodeId === node.id && !dropAllowed[node.id]}
style="padding-left: {level * 16 + 4}px"
draggable="true"
on:dragstart={(e) => handleDragStart(e, node)}
@ -191,6 +283,11 @@
outline: 1px solid #4ade80;
outline-offset: -1px;
}
.tree-item.drop-invalid {
background: #3a1a1a;
outline: 1px solid #ff6b6b;
outline-offset: -1px;
}
.tree-toggle {
background: none;
border: none;