Skip to content

Commit 071c2fb

Browse files
committed
fix(DragAndDrop): update drop target logic to correctly identify sibling and child nodes; update test orienting to demand
1 parent 1d3b194 commit 071c2fb

2 files changed

Lines changed: 70 additions & 39 deletions

File tree

Lines changed: 64 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
import { calculateDragState } from './dragLogic';
32
import type { BookmarkUI } from '../types/bookmark';
43

@@ -7,7 +6,6 @@ function assertEquals(actual: any, expected: any, message: string) {
76
console.error(`❌ FAIL: ${message}`);
87
console.error(` Actual: ${JSON.stringify(actual)}`);
98
console.error(` Expected: ${JSON.stringify(expected)}`);
10-
// We throw to fail the test run in CI, but for local debugging console.error is fine if we want to see all
119
throw new Error(message);
1210
} else {
1311
console.log(`✅ PASS: ${message}`);
@@ -23,15 +21,15 @@ const mockNodes: BookmarkUI[] = [
2321

2422
const NODE_HEIGHT = 28;
2523

26-
console.log('--- Starting Drag Logic Tests ---');
24+
console.log('--- Starting Drag Logic Tests (Requirement-Driven) ---');
2725

28-
// Original Functional Tests
2926
// Case 1: Sibling - Below Node 1, Level 1
27+
// Goal: Insert after Node 1, same level.
3028
(() => {
3129
const result = calculateDragState(
3230
NODE_HEIGHT * 0.8,
3331
NODE_HEIGHT,
34-
10, // Close to level 1 (4px)
32+
10, // Close to level 1
3533
'1',
3634
'drag-id',
3735
mockNodes
@@ -41,22 +39,26 @@ console.log('--- Starting Drag Logic Tests ---');
4139
assertEquals(result?.dropTargetLevel, 1, 'Case 1: Level');
4240
})();
4341

44-
// Case 2: Child - Below Node 1, Level 2
42+
// Case 2: Child (New Parent) - Below Node 1, Level 2
43+
// Goal: Make Node 1 a parent. Node 1 currently has NO visible children.
44+
// Action: Must be 'inside' Node 1.
4545
(() => {
4646
const result = calculateDragState(
4747
NODE_HEIGHT * 0.8,
4848
NODE_HEIGHT,
49-
30, // Close to level 2 (24px)
49+
30, // Level 2 indent
5050
'1',
5151
'drag-id',
5252
mockNodes
5353
);
54-
assertEquals(result?.dropTargetId, '2', 'Case 2: Target ID (before next)');
55-
assertEquals(result?.dropPosition, 'before', 'Case 2: Position');
54+
assertEquals(result?.dropTargetId, '1', 'Case 2: Target ID (parent)');
55+
assertEquals(result?.dropPosition, 'inside', 'Case 2: Position');
5656
assertEquals(result?.dropTargetLevel, 2, 'Case 2: Level');
5757
})();
5858

5959
// Case 3: Outdent - Below Node 2-1 (L2), Level 1
60+
// Goal: Insert after the whole Node 2 family (after 2-1), at Level 1.
61+
// Logic: This is effectively inserting after Node 2.
6062
(() => {
6163
const result = calculateDragState(
6264
NODE_HEIGHT * 0.8,
@@ -66,12 +68,13 @@ console.log('--- Starting Drag Logic Tests ---');
6668
'drag-id',
6769
mockNodes
6870
);
69-
assertEquals(result?.dropTargetId, '2', 'Case 3: Target ID (ancestor sibling)');
71+
assertEquals(result?.dropTargetId, '2', 'Case 3: Target ID (Node 2)');
7072
assertEquals(result?.dropPosition, 'after', 'Case 3: Position');
7173
assertEquals(result?.dropTargetLevel, 1, 'Case 3: Level');
7274
})();
7375

7476
// Case 4: Top - Top of Node 1
77+
// Goal: Insert at very top.
7578
(() => {
7679
const result = calculateDragState(
7780
NODE_HEIGHT * 0.1,
@@ -83,41 +86,64 @@ console.log('--- Starting Drag Logic Tests ---');
8386
);
8487
assertEquals(result?.dropTargetId, '1', 'Case 4: Target ID');
8588
assertEquals(result?.dropPosition, 'before', 'Case 4: Position');
86-
assertEquals(result?.dropTargetLevel, 1, 'Case 4: Level');
87-
assertEquals(result?.gapPosition, 'before', 'Case 4: Visual Gap Pos');
8889
})();
8990

90-
console.log('\n--- Level Threshold Tests ---');
91-
92-
// Helper to test level calculation only
93-
function testLevel(mouseX: number, expectedLevel: number, refNodeId: string = '1') {
91+
// Case 5: Child (Existing Parent) - Below Node 2, Level 2
92+
// Goal: Insert between Node 2 and Node 2-1.
93+
// Context: Node 2 is followed immediately by Node 2-1 (its child).
94+
// Action: Insert BEFORE Node 2-1.
95+
(() => {
9496
const result = calculateDragState(
95-
NODE_HEIGHT * 0.8, // Bottom gap
97+
NODE_HEIGHT * 0.8, // Bottom of Node 2
9698
NODE_HEIGHT,
97-
mouseX,
98-
refNodeId,
99+
30, // Level 2
100+
'2',
99101
'drag-id',
100102
mockNodes
101103
);
102-
// Note: Max level depends on refNode.
103-
assertEquals(result?.dropTargetLevel, expectedLevel, `X=${mouseX}px -> Level ${expectedLevel} (Ref: ${refNodeId})`);
104-
}
104+
// Gap is between 2 and 2-1.
105+
// RefNode: 2. NextNode: 2-1.
106+
// Target Level: 2. Ref Level: 1.
107+
// Logic: targetLevel == refLevel + 1.
108+
// Check: NextNode (2-1) is Level 2. So NextNode IS a child.
109+
// Action: before NextNode.
110+
assertEquals(result?.dropTargetId, '2-1', 'Case 5: Target ID (2-1)');
111+
assertEquals(result?.dropPosition, 'before', 'Case 5: Position');
112+
assertEquals(result?.dropTargetLevel, 2, 'Case 5: Level');
113+
})();
105114

106-
// Current Formula: floor((x - 4 + 10) / 20) + 1
107-
// Thresholds:
108-
// x < 14 -> Level 1
109-
// x >= 14 -> Level 2
110-
testLevel(0, 1);
111-
testLevel(13, 1);
112-
testLevel(14, 2);
113-
testLevel(20, 2);
114-
testLevel(33, 2); // 33-4+10 = 39 / 20 = 1.95 -> 1+1=2
115-
testLevel(34, 2); // Max level cap for Node 1 is 2. (34 would be Level 3)
115+
// Case 6: Sibling (Nested) - Below Node 2-1, Level 2
116+
// Goal: Insert after Node 2-1, keeping Level 2.
117+
(() => {
118+
const result = calculateDragState(
119+
NODE_HEIGHT * 0.8,
120+
NODE_HEIGHT,
121+
30, // Level 2
122+
'2-1',
123+
'drag-id',
124+
mockNodes
125+
);
126+
// RefNode: 2-1 (L2). Target Level: 2.
127+
// Logic: Sibling.
128+
assertEquals(result?.dropTargetId, '2-1', 'Case 6: Target ID');
129+
assertEquals(result?.dropPosition, 'after', 'Case 6: Position');
130+
})();
116131

117-
// Deep Nesting (Node 2-1 is Level 2, so can go to Level 3)
118-
testLevel(13, 1, '2-1');
119-
testLevel(14, 2, '2-1');
120-
testLevel(33, 2, '2-1');
121-
testLevel(34, 3, '2-1');
132+
// Case 7: Child (Deepest) - Below Node 2-1, Level 3
133+
// Goal: Make Node 2-1 a parent.
134+
// Action: inside 2-1.
135+
(() => {
136+
const result = calculateDragState(
137+
NODE_HEIGHT * 0.8,
138+
NODE_HEIGHT,
139+
50, // Level 3 (assuming 20px indent steps: 4, 24, 44, 64...)
140+
'2-1',
141+
'drag-id',
142+
mockNodes
143+
);
144+
assertEquals(result?.dropTargetId, '2-1', 'Case 7: Target ID');
145+
assertEquals(result?.dropPosition, 'inside', 'Case 7: Position');
146+
assertEquals(result?.dropTargetLevel, 3, 'Case 7: Level');
147+
})();
122148

123-
console.log('--- All Tests Passed ---');
149+
console.log('--- All Tests Passed ---');

src/lib/drag-drop/dragLogic.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,10 +91,15 @@ export function calculateDragState(
9191
dropTargetId = refNode.id;
9292
dropPosition = 'after';
9393
} else if (targetLevel === refNode.level + 1) {
94-
if (nextNode) {
94+
// Check if nextNode is actually a child of refNode (deeper level)
95+
const isNextChild = nextNode && nextNode.level > refNode.level;
96+
97+
if (isNextChild) {
98+
// Insert before the existing child
9599
dropTargetId = nextNode.id;
96100
dropPosition = 'before';
97101
} else {
102+
// NextNode is a sibling (or null). Append as new child to RefNode.
98103
dropTargetId = refNode.id;
99104
dropPosition = 'inside';
100105
}

0 commit comments

Comments
 (0)