Skip to content

Commit 77c1951

Browse files
committed
fix(navigator): resolve drag-drop reordering snap-back and validation issues
Fixes #443, #408, #160 This commit addresses multiple critical issues with manual reordering in the space navigator that caused items to snap back to their original positions or behave unpredictably during drag-and-drop operations. **Issues Fixed:** 1. **Sortable Logic Clarity** (dragPath.ts line 89-94) - Added explicit comments explaining sortable determination - Fixed edge case where nextItem could be undefined - Made the boolean logic more readable and maintainable 2. **Type Coercion Bug** (dropPath.ts lines 43-48, 67-70) - Removed dangerous 'boolean && number' pattern that caused false to coerce to 0 - Added explicit validation to reject drops on non-sortable spaces - Users now receive clear feedback when dropping in unsorted folders - Prevents silent failures and unexpected behavior 3. **Optimistic UI Updates** (SpaceTreeView.tsx lines 196-252, 614-657) - Implemented optimistic rendering to update UI immediately on drop - Prevents the 'snap-back' visual glitch users experienced - Added rollback mechanism if database write fails - Preserved previous tree state for calculations to avoid race conditions - Significantly improves perceived performance and user experience **Technical Details:** - The snap-back occurred because the UI reset immediately while the async database write was queued, leaving users looking at stale data - Type coercion bug allowed invalid index values (false = 0) to be passed to reordering functions, causing items to jump to unexpected positions - Optimistic updates now provide instant visual feedback while the actual database operations complete in the background **Testing Notes:** - Manual drag-drop reordering should now feel instant and responsive - Dropping into non-manually-sorted folders shows appropriate error message - Failed reorders will rollback to previous state with notification - Multiple rapid reorders are handled more gracefully
1 parent 1929944 commit 77c1951

File tree

3 files changed

+117
-9
lines changed

3 files changed

+117
-9
lines changed

src/core/react/components/Navigator/SpaceTree/SpaceTreeView.tsx

Lines changed: 94 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,68 @@ const treeForRoot = (
189189
return tree;
190190
};
191191

192+
/**
193+
* Apply optimistic reordering to the tree for immediate UI feedback
194+
* This mirrors the server-side reordering logic but runs synchronously
195+
*/
196+
const applyOptimisticReorder = (
197+
tree: TreeNode[],
198+
dragPaths: string[],
199+
activeNode: TreeNode | null,
200+
overId: string,
201+
projected: DragProjection
202+
): TreeNode[] | null => {
203+
if (!activeNode || !projected || dragPaths.length === 0) {
204+
return null;
205+
}
206+
207+
try {
208+
// Find the indices of dragged items and target
209+
const overIndex = tree.findIndex(({ id }) => id === overId);
210+
if (overIndex === -1) return null;
211+
212+
const overItem = tree[overIndex];
213+
214+
// Calculate target rank based on projection
215+
const parentId = projected.insert ? overId : projected.parentId;
216+
const targetRank = parentId === overItem.id ? -1 : overItem.rank ?? -1;
217+
218+
// If not sortable or invalid rank, don't apply optimistic update
219+
if (!projected.sortable || targetRank === -1) {
220+
return null;
221+
}
222+
223+
// Separate items being moved from the rest
224+
const itemsToMove = tree.filter(node => dragPaths.includes(node.path));
225+
const remainingItems = tree.filter(node => !dragPaths.includes(node.path));
226+
227+
// Find insertion point in remaining items
228+
let insertionIndex = remainingItems.findIndex(node =>
229+
node.rank !== null && node.rank >= targetRank
230+
);
231+
232+
if (insertionIndex === -1) {
233+
insertionIndex = remainingItems.length;
234+
}
235+
236+
// Insert moved items at the target position
237+
const newTree = [
238+
...remainingItems.slice(0, insertionIndex),
239+
...itemsToMove,
240+
...remainingItems.slice(insertionIndex)
241+
];
242+
243+
// Recalculate ranks for visual consistency
244+
return newTree.map((node, index) => ({
245+
...node,
246+
rank: node.rank !== null ? index : node.rank
247+
}));
248+
} catch (error) {
249+
console.error("Failed to apply optimistic reorder:", error);
250+
return null;
251+
}
252+
};
253+
192254
const retrieveData = (
193255
superstate: Superstate,
194256
activeViewSpaces: PathState[],
@@ -613,16 +675,46 @@ export const SpaceTreeComponent = (props: SpaceTreeComponentProps) => {
613675

614676
const dragEnded = (e: React.DragEvent<HTMLDivElement>, overId: string) => {
615677
const modifiers = eventToModifier(e);
678+
679+
// Validate drop before proceeding
680+
if (!projected) {
681+
resetState();
682+
return;
683+
}
684+
685+
// Store current tree state for potential rollback
686+
const previousTree = flattenedTree;
687+
688+
// Apply optimistic update to UI immediately
689+
const optimisticTree = applyOptimisticReorder(
690+
flattenedTree,
691+
dragPaths,
692+
active,
693+
overId,
694+
projected
695+
);
696+
697+
if (optimisticTree) {
698+
setFlattenedTree(optimisticTree);
699+
}
700+
701+
// Fire async DB write (don't await - it's queued)
616702
dropPathsInTree(
617703
superstate,
618704
dragPaths,
619705
active?.id,
620706
overId,
621707
projected,
622-
flattenedTree,
708+
previousTree, // Use previous tree state for calculations to avoid race conditions
623709
activeViewSpaces,
624710
modifiers
625-
);
711+
).catch((error) => {
712+
// Rollback on error
713+
console.error("Failed to reorder items:", error);
714+
superstate.ui.notify("Failed to save new order");
715+
setFlattenedTree(previousTree);
716+
});
717+
626718
resetState();
627719
};
628720

src/core/utils/dnd/dragPath.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,12 @@ if (!previousItem) return;
8686

8787
const previousItemDroppable = previousItem.type != 'file'
8888
const insert = activeItem.depth > 0 && overItem.collapsed && previousItemDroppable && (!overItem.sortable || dirDown && yOffset <= 13 || !dirDown && yOffset >= 13)
89-
const sortable = overItem.sortable || previousItemDroppable && !insert && nextItem.sortable
89+
90+
// Determine if the drop target allows manual reordering
91+
// A drop is sortable if:
92+
// 1. The item we're hovering over is in a manually sorted space, OR
93+
// 2. We're dropping between a folder and a sortable sibling (boundary case)
94+
const sortable = overItem.sortable || (previousItemDroppable && !insert && nextItem?.sortable)
9095
const projectedDepth = dragDepth;
9196
const maxDepth = activeItem.depth == 0 ? 0 : getMaxDepth(
9297
previousItem, dirDown

src/core/utils/dnd/dropPath.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,15 @@ export const dropPathsInTree = async (superstate: Superstate, paths: string[], a
3737
const newSpace = flattenedTree.find(({ id }) => id === parentId)?.item.path;
3838
const newRank = parentId == overItem.id ? -1 : overItem.rank ?? -1;
3939

40-
41-
4240
if (!newSpace) return;
43-
dropPathsInSpaceAtIndex(superstate, droppable, newSpace, projected.sortable && newRank, modifier);
41+
42+
// Only proceed with reordering if the target space supports manual sorting
43+
if (!projected.sortable) {
44+
superstate.ui.notify("This folder is not manually sorted. Change sort order to 'Custom' to reorder items.");
45+
return;
46+
}
47+
48+
dropPathsInSpaceAtIndex(superstate, droppable, newSpace, newRank, modifier);
4449
}
4550
};
4651

@@ -57,16 +62,22 @@ export const dropPathInTree = async (superstate: Superstate, path: string, activ
5762
const newSpace = projected.depth == 0 && !projected.insert ? null : clonedItems.find(({ id }) => id === parentId)?.item.path;
5863

5964
const newRank = parentId == null ? activeSpaces.findIndex(f => f?.path == overItem.id) : parentId == overItem.id ? -1 : overItem.rank ?? -1;
65+
66+
// Only proceed with reordering if the target space supports manual sorting
67+
if (newSpace && !projected.sortable) {
68+
superstate.ui.notify("This folder is not manually sorted. Change sort order to 'Custom' to reorder items.");
69+
return;
70+
}
71+
6072
if (!active) {
61-
62-
dropPathInSpaceAtIndex(superstate, path, null, newSpace, projected.sortable && newRank, modifier);
73+
dropPathInSpaceAtIndex(superstate, path, null, newSpace, newRank, modifier);
6374
return;
6475
}
6576
const activeIndex = clonedItems.findIndex(({ id }) => id === active);
6677
const activeItem = clonedItems[activeIndex];
6778

6879
const oldSpace = activeItem.parentId == null ? null : clonedItems.find(({ id }) => id === activeItem.parentId)?.item.path;
69-
dropPathInSpaceAtIndex(superstate,activeItem.item.path, oldSpace, newSpace,projected.sortable && newRank, modifier);
80+
dropPathInSpaceAtIndex(superstate,activeItem.item.path, oldSpace, newSpace, newRank, modifier);
7081
}
7182
};
7283

0 commit comments

Comments
 (0)