diff --git a/canvas/src/components/Canvas.tsx b/canvas/src/components/Canvas.tsx index 5d13f00a..16c299cb 100644 --- a/canvas/src/components/Canvas.tsx +++ b/canvas/src/components/Canvas.tsx @@ -195,9 +195,9 @@ function CanvasInner() { {/* Screen-reader live region: announces workspace count on canvas load or change */}
- {nodes.filter((n) => !n.data.parentId).length === 0 + {nodes.filter((n) => !n.parentId).length === 0 ? "No workspaces on canvas" - : `${nodes.filter((n) => !n.data.parentId).length} workspace${nodes.filter((n) => !n.data.parentId).length !== 1 ? "s" : ""} on canvas`} + : `${nodes.filter((n) => !n.parentId).length} workspace${nodes.filter((n) => !n.parentId).length !== 1 ? "s" : ""} on canvas`}
{nodes.length === 0 && } diff --git a/canvas/src/components/canvas/useDragHandlers.ts b/canvas/src/components/canvas/useDragHandlers.ts index ee2b711d..9f7e8f26 100644 --- a/canvas/src/components/canvas/useDragHandlers.ts +++ b/canvas/src/components/canvas/useDragHandlers.ts @@ -9,6 +9,8 @@ import { import { useCanvasStore, type WorkspaceNodeData } from "@/store/canvas"; import { clampChildIntoParent, shouldDetach } from "./dragUtils"; +type WorkspaceNode = Node; + export interface PendingNestState { nodeId: string; targetId: string | null; @@ -25,6 +27,8 @@ interface DragHandlers { cancelNest: () => void; } + + /** * Encapsulates every drag gesture on the canvas: * @@ -48,7 +52,6 @@ export function useDragHandlers(): DragHandlers { const isDescendant = useCanvasStore((s) => s.isDescendant); const { getInternalNode } = useReactFlow(); - const dragStartParentRef = useRef(null); const dragModifiersRef = useRef<{ alt: boolean; meta: boolean }>({ alt: false, meta: false, @@ -98,9 +101,8 @@ export function useDragHandlers(): DragHandlers { [getInternalNode, isDescendant], ); - const onNodeDragStart: OnNodeDrag> = useCallback( - (event, node) => { - dragStartParentRef.current = (node.data as WorkspaceNodeData).parentId; + const onNodeDragStart: OnNodeDrag = useCallback( + (event) => { dragModifiersRef.current = { alt: event.altKey, meta: event.metaKey || event.ctrlKey, @@ -109,7 +111,7 @@ export function useDragHandlers(): DragHandlers { [], ); - const onNodeDrag: OnNodeDrag> = useCallback( + const onNodeDrag: OnNodeDrag = useCallback( (event, node) => { dragModifiersRef.current = { alt: event.altKey, @@ -129,13 +131,13 @@ export function useDragHandlers(): DragHandlers { [findDropTarget, getInternalNode, setDragOverNode], ); - const onNodeDragStop: OnNodeDrag> = useCallback( + const onNodeDragStop: OnNodeDrag = useCallback( (event, node) => { const { dragOverNodeId, nodes: allNodes } = useCanvasStore.getState(); setDragOverNode(null); - const nodeName = (node.data as WorkspaceNodeData).name; - const currentParentId = (node.data as WorkspaceNodeData).parentId; + const nodeName = node.data.name; + const currentParentId = node.data.parentId; const altHeld = event.altKey || dragModifiersRef.current.alt; const forceDetach = event.metaKey || event.ctrlKey || dragModifiersRef.current.meta; @@ -188,16 +190,21 @@ export function useDragHandlers(): DragHandlers { const confirmNest = useCallback(() => { if (!pendingNest) return; + // Close the dialog before dispatching the async store action so a + // second drag can't kick off a competing batch while this one is + // still mid-flight. The store actions surface their own errors via + // showToast, so `void` is the right pattern here. + const pending = pendingNest; + setPendingNest(null); const state = useCanvasStore.getState(); if ( state.selectedNodeIds.size > 1 && - state.selectedNodeIds.has(pendingNest.nodeId) + state.selectedNodeIds.has(pending.nodeId) ) { - batchNest(Array.from(state.selectedNodeIds), pendingNest.targetId); + void batchNest(Array.from(state.selectedNodeIds), pending.targetId); } else { - nestNode(pendingNest.nodeId, pendingNest.targetId); + void nestNode(pending.nodeId, pending.targetId); } - setPendingNest(null); }, [pendingNest, nestNode, batchNest]); const cancelNest = useCallback(() => setPendingNest(null), []); diff --git a/canvas/src/store/__tests__/canvas.test.ts b/canvas/src/store/__tests__/canvas.test.ts index 72c2735e..a98340ff 100644 --- a/canvas/src/store/__tests__/canvas.test.ts +++ b/canvas/src/store/__tests__/canvas.test.ts @@ -952,3 +952,78 @@ describe("bumpZOrder", () => { expect(afterZ).toEqual(beforeZ); }); }); + +// ---------- batchNest ---------- + +describe("batchNest", () => { + beforeEach(() => { + (global.fetch as ReturnType).mockClear(); + // Scenario: two root nodes (a, b) and one nested under a (a-child). + // Tests below re-parent various subsets into `target`. + useCanvasStore.getState().hydrate([ + makeWS({ id: "target", name: "Target", x: 1000, y: 0 }), + makeWS({ id: "a", name: "A", x: 0, y: 0 }), + makeWS({ id: "b", name: "B", x: 200, y: 0 }), + makeWS({ id: "a-child", name: "A/Child", parent_id: "a", x: 50, y: 50 }), + ]); + }); + + it("re-parents every selected root into the target via one PATCH each", async () => { + const mock = global.fetch as ReturnType; + mock.mockImplementation(() => + Promise.resolve({ ok: true, json: () => Promise.resolve({}) } as Response), + ); + // Clear any PATCHes that hydrate's computeAutoLayout may have fired + // (auto-positioned workspaces trigger a savePosition → PATCH). + mock.mockClear(); + await useCanvasStore.getState().batchNest(["a", "b"], "target"); + const nodes = useCanvasStore.getState().nodes; + expect(nodes.find((n) => n.id === "a")!.data.parentId).toBe("target"); + expect(nodes.find((n) => n.id === "b")!.data.parentId).toBe("target"); + // Every PATCH fired by batchNest should target /workspaces/ + // and carry `parent_id: "target"` plus absolute x,y. One per root. + const nestPatchCalls = mock.mock.calls.filter((c) => { + const init = c[1] as RequestInit | undefined; + if (init?.method !== "PATCH") return false; + const body = init.body ? JSON.parse(init.body as string) : {}; + return body.parent_id === "target"; + }); + expect(nestPatchCalls).toHaveLength(2); + for (const call of nestPatchCalls) { + const body = JSON.parse((call[1] as RequestInit).body as string); + expect(body.x).toBeTypeOf("number"); + expect(body.y).toBeTypeOf("number"); + } + }); + + it("filters out selected descendants so a subtree moves intact", async () => { + // User selects both A AND its child A/Child, then drags into target. + // Intent: move the A subtree — A/Child stays under A, not target. + (global.fetch as ReturnType).mockImplementation(() => + Promise.resolve({ ok: true, json: () => Promise.resolve({}) } as Response), + ); + await useCanvasStore.getState().batchNest(["a", "a-child"], "target"); + const nodes = useCanvasStore.getState().nodes; + expect(nodes.find((n) => n.id === "a")!.data.parentId).toBe("target"); + // The descendant is NOT independently re-parented; its parent is still A. + expect(nodes.find((n) => n.id === "a-child")!.data.parentId).toBe("a"); + }); + + it("rolls back only the nodes whose PATCH rejected", async () => { + // Reject the PATCH for `a`, accept the one for `b`. + (global.fetch as ReturnType).mockImplementation((url: string) => { + if (typeof url === "string" && url.endsWith("/workspaces/a")) { + return Promise.reject(new Error("network")); + } + return Promise.resolve({ + ok: true, + json: () => Promise.resolve({}), + } as Response); + }); + await useCanvasStore.getState().batchNest(["a", "b"], "target"); + const nodes = useCanvasStore.getState().nodes; + // `a` rolled back to its original parent (null), `b` stayed committed. + expect(nodes.find((n) => n.id === "a")!.data.parentId).toBeNull(); + expect(nodes.find((n) => n.id === "b")!.data.parentId).toBe("target"); + }); +}); diff --git a/canvas/src/store/canvas-topology.ts b/canvas/src/store/canvas-topology.ts index e66ac438..9ae97b4c 100644 --- a/canvas/src/store/canvas-topology.ts +++ b/canvas/src/store/canvas-topology.ts @@ -252,22 +252,25 @@ export function buildNodesAndEdges( const pa = absPos.get(ws.parent_id!)!; position = { x: abs.x - pa.x, y: abs.y - pa.y }; - // Auto-rescue on load: if the child's stored relative position - // would render it outside the parent's current bounding box, drop - // it into the next default grid slot. This fixes three real - // failure modes at once: (1) legacy rows written before nesting - // existed, whose absolute coords have no relation to the parent; - // (2) org-imports at (0, 0); (3) a child whose parent was later - // resized smaller. Dragging a child past the edge after load is - // still the way to un-nest — that's handled separately in - // Canvas.onNodeDragStop with the hysteresis check. - const psize = parentSize.get(ws.parent_id!)!; - const outside = + // Auto-rescue on load — only when the stored relative position + // is clearly impossible for an intentionally-placed child: + // negative coords (child would render to the LEFT or ABOVE its + // parent) or absurdly distant (child would render further than + // MAX_PLAUSIBLE_OFFSET from the parent's origin). Both signal + // legacy data written before nesting existed, where the child's + // absolute canvas coords have no relation to the parent that was + // later assigned to it. Children past the right/bottom edge by + // a small margin are left alone — the user may have resized the + // parent larger on a previous session, and silent teleportation + // on every reload would undo their layout. The manual "Arrange + // Children" context command stays available for cleanup. + const MAX_PLAUSIBLE_OFFSET = 8000; + const corrupted = position.x < 0 || position.y < 0 || - position.x + CHILD_DEFAULT_WIDTH > psize.width || - position.y + CHILD_DEFAULT_HEIGHT > psize.height; - if (outside) { + position.x > MAX_PLAUSIBLE_OFFSET || + position.y > MAX_PLAUSIBLE_OFFSET; + if (corrupted) { const idx = nextChildIndex.get(ws.parent_id!) ?? 0; nextChildIndex.set(ws.parent_id!, idx + 1); position = defaultChildSlot(idx); diff --git a/canvas/src/store/canvas.ts b/canvas/src/store/canvas.ts index 10213b3a..6652c9ae 100644 --- a/canvas/src/store/canvas.ts +++ b/canvas/src/store/canvas.ts @@ -6,6 +6,7 @@ import { type NodeChange, } from "@xyflow/react"; import { api } from "@/lib/api"; +import { showToast } from "@/components/Toaster"; import type { WorkspaceData, WSMessage } from "./socket"; import { handleCanvasEvent } from "./canvas-events"; import { @@ -326,8 +327,31 @@ export const useCanvasStore = create((set, get) => ({ batchNest: async (nodeIds, targetId) => { if (nodeIds.length === 0) return; - if (nodeIds.length === 1) { - await get().nestNode(nodeIds[0], targetId); + // Selection-roots filter: if the user selected both A and A's + // descendant B and dragged the pair into T, the intent is "move + // the subtree" — B should stay under A, not become a sibling of + // A under T. Drop every selected node whose ancestor is also + // selected; those will follow their ancestor via React Flow's + // parent-of binding automatically. + const selectedSet = new Set(nodeIds); + const { nodes: before, edges: beforeEdges } = get(); + const byId = new Map(before.map((n) => [n.id, n])); + const rootsOnly: string[] = []; + for (const id of nodeIds) { + let cursor = byId.get(id)?.data.parentId ?? null; + let hasSelectedAncestor = false; + while (cursor) { + if (selectedSet.has(cursor)) { + hasSelectedAncestor = true; + break; + } + cursor = byId.get(cursor)?.data.parentId ?? null; + } + if (!hasSelectedAncestor) rootsOnly.push(id); + } + if (rootsOnly.length === 0) return; + if (rootsOnly.length === 1) { + await get().nestNode(rootsOnly[0], targetId); return; } // Batch path: do all state math against one snapshot so every @@ -338,8 +362,6 @@ export const useCanvasStore = create((set, get) => ({ // in flight at once. For a typical 3-5 node selection on a // ~200ms link this drops the perceived re-parent latency from // ~2s to ~200ms. - const { nodes: before, edges: beforeEdges } = get(); - const byId = new Map(before.map((n) => [n.id, n])); const absOf = (id: string | null | undefined): { x: number; y: number } => { let sum = { x: 0, y: 0 }; @@ -376,7 +398,7 @@ export const useCanvasStore = create((set, get) => ({ const plan: Plan[] = []; const movedIds = new Set(); // Filter out nodes that would be invalid targets / no-ops. - for (const id of nodeIds) { + for (const id of rootsOnly) { const dragged = byId.get(id); if (!dragged) continue; const currentParentId = dragged.data.parentId; @@ -487,6 +509,17 @@ export const useCanvasStore = create((set, get) => ({ }; }), }); + // Surface the partial failure — silent rollback would otherwise + // leave the canvas in a state the user can't explain ("I dragged + // 5 cards, 3 moved and 2 snapped back?"). + const failedNames = rolledBack + .map((id) => byId.get(id)?.data.name) + .filter(Boolean) + .join(", "); + showToast( + `Could not re-parent ${rolledBack.length} of ${plan.length} workspace${plan.length === 1 ? "" : "s"}${failedNames ? `: ${failedNames}` : ""}`, + "error", + ); } }, diff --git a/workspace-server/internal/handlers/workspace_crud.go b/workspace-server/internal/handlers/workspace_crud.go index c1c87556..b991cb2f 100644 --- a/workspace-server/internal/handlers/workspace_crud.go +++ b/workspace-server/internal/handlers/workspace_crud.go @@ -188,6 +188,14 @@ func (h *WorkspaceHandler) Update(c *gin.Context) { log.Printf("Update parent_id error for %s: %v", id, err) } } + if collapsed, ok := body["collapsed"]; ok { + // `collapsed` is the canvas UI-only flag that hides descendants + // in the tree view (WorkspaceNode renders the parent as header- + // only). Persisting it here so the state survives reload. + if _, err := db.DB.ExecContext(ctx, `UPDATE workspaces SET collapsed = $2, updated_at = now() WHERE id = $1`, id, collapsed); err != nil { + log.Printf("Update collapsed error for %s: %v", id, err) + } + } if runtime, ok := body["runtime"]; ok { if _, err := db.DB.ExecContext(ctx, `UPDATE workspaces SET runtime = $2, updated_at = now() WHERE id = $1`, id, runtime); err != nil { log.Printf("Update runtime error for %s: %v", id, err)