From 3e2ff63f7fb87ae04e88d56a05b684ea8336010a Mon Sep 17 00:00:00 2001 From: Molecule AI Core-FE Date: Sat, 9 May 2026 22:19:01 +0000 Subject: [PATCH 1/2] feat(canvas): keyboard-accessible node drag via Arrow keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes canvas audit item: MEDIUM keyboard-accessible node drag. - Arrow keys move the selected node by 10px per press; Shift+Arrow moves by 50px. Position is persisted to the backend via savePosition. - The modal-dialog guard (same pattern as ? shortcut) prevents Arrow keys from moving nodes when a modal like KeyboardShortcutsDialog is open — dialogs own their own arrow semantics. - All shortcuts guarded by the inInput check so Arrow keys still work for text navigation inside inputs/textareas. Changes: - canvas.ts: new moveNode(dx, dy) store action — updates position directly without the grow-parents pass that onNodesChange runs on every drag tick (avoids edge-chase flicker). - useKeyboardShortcuts.ts: Arrow key handler added. - canvas.test.ts: new moveNode unit tests (position update, no-op, savePosition call). - useKeyboardShortcuts.test.tsx: new integration tests for all keyboard shortcuts including the new Arrow key handlers. - canvas-audit-items.md: Keyboard Shortcuts section upgraded to ✅, drag item marked done. - canvas-events.test.ts: fix pre-existing double-}); syntax error. Co-Authored-By: Claude Opus 4.7 --- .../__tests__/useKeyboardShortcuts.test.tsx | 309 ++++++++++++++++++ .../components/canvas/useKeyboardShortcuts.ts | 28 ++ .../src/store/__tests__/canvas-events.test.ts | 1 - canvas/src/store/__tests__/canvas.test.ts | 43 +++ canvas/src/store/canvas.ts | 20 ++ docs/design-system/canvas-audit-items.md | 16 +- 6 files changed, 409 insertions(+), 8 deletions(-) create mode 100644 canvas/src/components/canvas/__tests__/useKeyboardShortcuts.test.tsx diff --git a/canvas/src/components/canvas/__tests__/useKeyboardShortcuts.test.tsx b/canvas/src/components/canvas/__tests__/useKeyboardShortcuts.test.tsx new file mode 100644 index 00000000..cdf34d81 --- /dev/null +++ b/canvas/src/components/canvas/__tests__/useKeyboardShortcuts.test.tsx @@ -0,0 +1,309 @@ +// @vitest-environment jsdom +/** + * Tests for canvas keyboard shortcuts (useKeyboardShortcuts hook). + * + * Covers: Esc, Enter/Shift+Enter, Cmd+]/[, Z, and Arrow keys. + * + * The hook is tested by dispatching KeyboardEvents at the window and + * asserting the resulting store mutations / dispatched events. + */ +import React from "react"; +import { render, cleanup, fireEvent } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { useKeyboardShortcuts } from "../useKeyboardShortcuts"; +import { useCanvasStore } from "@/store/canvas"; + +// ─── Mock store ────────────────────────────────────────────────────────────── + +const mockSavePosition = vi.fn().mockResolvedValue(undefined); + +vi.mock("@/store/canvas", () => ({ + useCanvasStore: Object.assign( + vi.fn((sel) => sel(mockStoreState)), + { + getState: () => mockStoreState, + } + ), +})); + +// Module-level mutable state so tests can mutate between cases +const mockStoreState = { + selectedNodeId: null as string | null, + selectedNodeIds: new Set(), + nodes: [] as Array<{ id: string; position: { x: number; y: number }; data: { parentId?: string | null } }>, + contextMenu: null as { x: number; y: number; nodeId: string } | null, + closeContextMenu: vi.fn(), + selectNode: vi.fn(), + clearSelection: vi.fn(), + bumpZOrder: vi.fn(), + savePosition: mockSavePosition, + moveNode: vi.fn(), +}; + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); + // Reset to default empty state between tests + mockStoreState.selectedNodeId = null; + mockStoreState.selectedNodeIds = new Set(); + mockStoreState.nodes = []; + mockStoreState.contextMenu = null; + mockStoreState.closeContextMenu.mockClear(); + mockStoreState.selectNode.mockClear(); + mockStoreState.clearSelection.mockClear(); + mockStoreState.bumpZOrder.mockClear(); + mockStoreState.moveNode.mockClear(); + mockStoreState.savePosition.mockClear(); +}); + +// ─── Test wrapper ──────────────────────────────────────────────────────────── + +function ShortcutTestComponent() { + useKeyboardShortcuts(); + return
; +} + +function renderWithProvider() { + return render(); +} + +// ─── Tests ─────────────────────────────────────────────────────────────────── + +describe("Esc — deselect / close context menu", () => { + it("closes the context menu when one is open", () => { + mockStoreState.contextMenu = { x: 100, y: 100, nodeId: "n1" }; + renderWithProvider(); + fireEvent.keyDown(window, { key: "Escape" }); + expect(mockStoreState.closeContextMenu).toHaveBeenCalledTimes(1); + }); + + it("clears the batch selection when no context menu is open", () => { + mockStoreState.contextMenu = null; + mockStoreState.selectedNodeIds = new Set(["n1", "n2"]); + renderWithProvider(); + fireEvent.keyDown(window, { key: "Escape" }); + expect(mockStoreState.clearSelection).toHaveBeenCalledTimes(1); + }); + + it("deselects the focused node when no batch selection exists", () => { + mockStoreState.contextMenu = null; + mockStoreState.selectedNodeIds = new Set(); + mockStoreState.selectedNodeId = "n1"; + renderWithProvider(); + fireEvent.keyDown(window, { key: "Escape" }); + expect(mockStoreState.selectNode).toHaveBeenCalledWith(null); + }); +}); + +describe("Enter — hierarchy navigation", () => { + beforeEach(() => { + mockStoreState.selectedNodeId = "n1"; + mockStoreState.nodes = [ + { id: "n1", position: { x: 0, y: 0 }, data: { parentId: null } }, + { id: "n2", position: { x: 100, y: 0 }, data: { parentId: "n1" } }, + { id: "n3", position: { x: 200, y: 0 }, data: { parentId: null } }, + ]; + }); + + it("navigates to the first child on Enter", () => { + renderWithProvider(); + fireEvent.keyDown(window, { key: "Enter" }); + expect(mockStoreState.selectNode).toHaveBeenCalledWith("n2"); + }); + + it("navigates to the parent on Shift+Enter", () => { + mockStoreState.nodes = [ + { id: "n1", position: { x: 0, y: 0 }, data: { parentId: null } }, + { id: "n2", position: { x: 100, y: 0 }, data: { parentId: "n1" } }, + ]; + mockStoreState.selectedNodeId = "n2"; + renderWithProvider(); + fireEvent.keyDown(window, { key: "Enter", shiftKey: true }); + expect(mockStoreState.selectNode).toHaveBeenCalledWith("n1"); + }); + + it("does NOT navigate when no node is selected", () => { + mockStoreState.selectedNodeId = null; + renderWithProvider(); + fireEvent.keyDown(window, { key: "Enter" }); + expect(mockStoreState.selectNode).not.toHaveBeenCalled(); + }); +}); + +describe("Cmd+]/[ — z-order bump", () => { + beforeEach(() => { + mockStoreState.selectedNodeId = "n1"; + }); + + it("bumps z-order forward on Cmd+]", () => { + renderWithProvider(); + fireEvent.keyDown(window, { key: "]", metaKey: true }); + expect(mockStoreState.bumpZOrder).toHaveBeenCalledWith("n1", 1); + }); + + it("bumps z-order backward on Cmd+[", () => { + renderWithProvider(); + fireEvent.keyDown(window, { key: "[", metaKey: true }); + expect(mockStoreState.bumpZOrder).toHaveBeenCalledWith("n1", -1); + }); + + it("uses Ctrl as the modifier key", () => { + renderWithProvider(); + fireEvent.keyDown(window, { key: "]", ctrlKey: true }); + expect(mockStoreState.bumpZOrder).toHaveBeenCalledWith("n1", 1); + }); +}); + +describe("Z — zoom-to-team", () => { + let dispatchedEvents: CustomEvent[] = []; + + beforeEach(() => { + dispatchedEvents = []; + mockStoreState.selectedNodeId = "n1"; + mockStoreState.nodes = [ + { id: "n1", position: { x: 0, y: 0 }, data: { parentId: null } }, + { id: "n2", position: { x: 100, y: 0 }, data: { parentId: "n1" } }, + ]; + window.addEventListener("molecule:zoom-to-team", (e) => { + dispatchedEvents.push(e as CustomEvent); + }); + }); + + afterEach(() => { + window.removeEventListener("molecule:zoom-to-team", () => {}); + }); + + it("dispatches zoom-to-team when the selected node has children", () => { + renderWithProvider(); + fireEvent.keyDown(window, { key: "z" }); + expect(dispatchedEvents).toHaveLength(1); + expect(dispatchedEvents[0].detail.nodeId).toBe("n1"); + }); + + it("does NOT fire when no node is selected", () => { + mockStoreState.selectedNodeId = null; + renderWithProvider(); + fireEvent.keyDown(window, { key: "z" }); + expect(dispatchedEvents).toHaveLength(0); + }); + + it("does NOT fire when the node has no children", () => { + mockStoreState.nodes = [ + { id: "n1", position: { x: 0, y: 0 }, data: { parentId: null } }, + ]; + renderWithProvider(); + fireEvent.keyDown(window, { key: "z" }); + expect(dispatchedEvents).toHaveLength(0); + }); + + it("skips when the target element is an input", () => { + renderWithProvider(); + const input = document.createElement("input"); + document.body.appendChild(input); + fireEvent.keyDown(input, { key: "z" }); + expect(dispatchedEvents).toHaveLength(0); + document.body.removeChild(input); + }); +}); + +describe("Arrow keys — keyboard node movement", () => { + beforeEach(() => { + mockStoreState.selectedNodeId = "n1"; + mockStoreState.nodes = [ + { id: "n1", position: { x: 100, y: 200 }, data: { parentId: null } }, + ]; + }); + + it("moves the selected node down on ArrowDown", () => { + renderWithProvider(); + fireEvent.keyDown(window, { key: "ArrowDown" }); + expect(mockStoreState.moveNode).toHaveBeenCalledWith("n1", 0, 10); + }); + + it("moves the selected node up on ArrowUp", () => { + renderWithProvider(); + fireEvent.keyDown(window, { key: "ArrowUp" }); + expect(mockStoreState.moveNode).toHaveBeenCalledWith("n1", 0, -10); + }); + + it("moves the selected node right on ArrowRight", () => { + renderWithProvider(); + fireEvent.keyDown(window, { key: "ArrowRight" }); + expect(mockStoreState.moveNode).toHaveBeenCalledWith("n1", 10, 0); + }); + + it("moves the selected node left on ArrowLeft", () => { + renderWithProvider(); + fireEvent.keyDown(window, { key: "ArrowLeft" }); + expect(mockStoreState.moveNode).toHaveBeenCalledWith("n1", -10, 0); + }); + + it("moves 50 px when Shift is held", () => { + renderWithProvider(); + fireEvent.keyDown(window, { key: "ArrowDown", shiftKey: true }); + expect(mockStoreState.moveNode).toHaveBeenCalledWith("n1", 0, 50); + }); + + it("does NOT fire when no node is selected", () => { + mockStoreState.selectedNodeId = null; + renderWithProvider(); + fireEvent.keyDown(window, { key: "ArrowDown" }); + expect(mockStoreState.moveNode).not.toHaveBeenCalled(); + }); + + it("skips when the target element is an input", () => { + renderWithProvider(); + const input = document.createElement("input"); + document.body.appendChild(input); + fireEvent.keyDown(input, { key: "ArrowDown" }); + expect(mockStoreState.moveNode).not.toHaveBeenCalled(); + document.body.removeChild(input); + }); + + it("skips when a modal dialog is already open", () => { + renderWithProvider(); + const dialog = document.createElement("div"); + dialog.setAttribute("role", "dialog"); + dialog.setAttribute("aria-modal", "true"); + document.body.appendChild(dialog); + fireEvent.keyDown(window, { key: "ArrowDown" }); + expect(mockStoreState.moveNode).not.toHaveBeenCalled(); + document.body.removeChild(dialog); + }); + + it("prevents default browser scroll on arrow keys", () => { + renderWithProvider(); + const preventDefault = vi.fn(); + fireEvent.keyDown(window, { + key: "ArrowDown", + preventDefault, + }); + expect(preventDefault).toHaveBeenCalled(); + }); +}); + +describe("all shortcuts respect inInput guard", () => { + it("ArrowDown is skipped in an input element", () => { + mockStoreState.selectedNodeId = "n1"; + renderWithProvider(); + const textarea = document.createElement("textarea"); + document.body.appendChild(textarea); + fireEvent.keyDown(textarea, { key: "ArrowDown" }); + expect(mockStoreState.moveNode).not.toHaveBeenCalled(); + document.body.removeChild(textarea); + }); + + it("Enter navigation is skipped in an input element", () => { + mockStoreState.selectedNodeId = "n1"; + mockStoreState.nodes = [ + { id: "n1", position: { x: 0, y: 0 }, data: { parentId: null } }, + { id: "n2", position: { x: 100, y: 0 }, data: { parentId: "n1" } }, + ]; + renderWithProvider(); + const input = document.createElement("input"); + document.body.appendChild(input); + fireEvent.keyDown(input, { key: "Enter" }); + expect(mockStoreState.selectNode).not.toHaveBeenCalled(); + document.body.removeChild(input); + }); +}); diff --git a/canvas/src/components/canvas/useKeyboardShortcuts.ts b/canvas/src/components/canvas/useKeyboardShortcuts.ts index f9f67fd8..68a4e15b 100644 --- a/canvas/src/components/canvas/useKeyboardShortcuts.ts +++ b/canvas/src/components/canvas/useKeyboardShortcuts.ts @@ -14,6 +14,7 @@ import { useCanvasStore } from "@/store/canvas"; * Cmd/Ctrl+] — bump selected node forward in z-order * Cmd/Ctrl+[ — bump selected node backward in z-order * Z — zoom-to-team if the selected node has children + * Arrow keys — move selected node 10px (50px with Shift) */ export function useKeyboardShortcuts() { useEffect(() => { @@ -80,6 +81,33 @@ export function useKeyboardShortcuts() { ); } } + + // Arrow-key node movement — Figma-style keyboard drag for keyboard users. + // 10 px per press, 50 px with Shift held. Only fires when a node + // is selected and the target isn't a form control. + if ( + !inInput && + (e.key === "ArrowUp" || + e.key === "ArrowDown" || + e.key === "ArrowLeft" || + e.key === "ArrowRight") + ) { + const state = useCanvasStore.getState(); + const selectedId = state.selectedNodeId; + if (!selectedId) return; + // Skip when a modal/dialog is already open — dialogs own their own + // arrow-key semantics and shouldn't trigger canvas moves. + if (document.querySelector('[role="dialog"][aria-modal="true"]')) return; + e.preventDefault(); + const step = e.shiftKey ? 50 : 10; + let dx = 0; + let dy = 0; + if (e.key === "ArrowUp") dy = -step; + else if (e.key === "ArrowDown") dy = step; + else if (e.key === "ArrowLeft") dx = -step; + else dx = step; + state.moveNode(selectedId, dx, dy); + } }; window.addEventListener("keydown", handler); return () => window.removeEventListener("keydown", handler); diff --git a/canvas/src/store/__tests__/canvas-events.test.ts b/canvas/src/store/__tests__/canvas-events.test.ts index ddd7d0cc..28874573 100644 --- a/canvas/src/store/__tests__/canvas-events.test.ts +++ b/canvas/src/store/__tests__/canvas-events.test.ts @@ -1012,4 +1012,3 @@ describe("handleCanvasEvent – liveAnnouncement", () => { expect(state.liveAnnouncement ?? "").toBe(""); }); }); -}); diff --git a/canvas/src/store/__tests__/canvas.test.ts b/canvas/src/store/__tests__/canvas.test.ts index 81f13d81..e3410b14 100644 --- a/canvas/src/store/__tests__/canvas.test.ts +++ b/canvas/src/store/__tests__/canvas.test.ts @@ -1181,3 +1181,46 @@ describe("batchNest", () => { expect(nestPatches).toHaveLength(1); }); }); + +// ---------- moveNode ---------- + +describe("moveNode", () => { + beforeEach(() => { + const mock = global.fetch as ReturnType; + mock.mockImplementation(() => + Promise.resolve({ ok: true, json: () => Promise.resolve({}) } as Response), + ); + mock.mockClear(); + }); + + it("updates the node's position by the given delta", () => { + useCanvasStore.getState().hydrate([ + makeWS({ id: "n1", name: "Node 1", x: 100, y: 200 }), + ]); + useCanvasStore.getState().selectNode("n1"); + useCanvasStore.getState().moveNode("n1", 10, -50); + const node = useCanvasStore.getState().nodes.find((n) => n.id === "n1")!; + expect(node.position).toEqual({ x: 110, y: 150 }); + }); + + it("is a no-op when the node does not exist", () => { + useCanvasStore.getState().hydrate([makeWS({ id: "n1", name: "Node 1", x: 0, y: 0 })]); + expect(() => useCanvasStore.getState().moveNode("nonexistent", 10, 10)).not.toThrow(); + }); + + it("calls savePosition with the new absolute coordinates", async () => { + useCanvasStore.getState().hydrate([makeWS({ id: "n1", name: "Node 1", x: 100, y: 200 })]); + useCanvasStore.getState().selectNode("n1"); + const mock = global.fetch as ReturnType; + useCanvasStore.getState().moveNode("n1", 10, 20); + await vi.waitFor(() => { + expect(mock).toHaveBeenCalledWith( + expect.stringContaining("/workspaces/n1"), + expect.objectContaining({ + method: "PATCH", + body: JSON.stringify({ x: 110, y: 220 }), + }), + ); + }); + }); +}); diff --git a/canvas/src/store/canvas.ts b/canvas/src/store/canvas.ts index e2d6e150..38129468 100644 --- a/canvas/src/store/canvas.ts +++ b/canvas/src/store/canvas.ts @@ -165,6 +165,13 @@ interface CanvasState { * this so a drag that pushed a child past the parent edge commits * the parent grow on release (commit-on-release pattern). */ growParentsToFitChildren: () => void; + /** Move a selected node by (dx, dy) in canvas space. Used by keyboard + * arrow-key shortcuts so keyboard users can reposition nodes without a + * mouse. Persists the new position to the backend and skips the + * grow-parents pass that onNodesChange runs on every drag tick + * (avoids the "edge-chase" flicker that commit-on-release is meant to + * prevent). */ + moveNode: (nodeId: string, dx: number, dy: number) => void; /** Re-layout a parent's children to the default 2-column grid. Used * by the "Arrange children" context-menu command so users can rescue * out-of-bounds children on demand — topology no longer does it @@ -1032,6 +1039,19 @@ export const useCanvasStore = create((set, get) => ({ } }, + moveNode: (nodeId, dx, dy) => { + const node = get().nodes.find((n) => n.id === nodeId); + if (!node) return; + set({ + nodes: get().nodes.map((n) => + n.id === nodeId + ? { ...n, position: { x: n.position.x + dx, y: n.position.y + dy } } + : n, + ), + }); + void get().savePosition(nodeId, node.position.x + dx, node.position.y + dy); + }, + savePosition: async (nodeId: string, x: number, y: number) => { try { await api.patch(`/workspaces/${nodeId}`, { x, y }); diff --git a/docs/design-system/canvas-audit-items.md b/docs/design-system/canvas-audit-items.md index e1171443..26996f6d 100644 --- a/docs/design-system/canvas-audit-items.md +++ b/docs/design-system/canvas-audit-items.md @@ -72,10 +72,12 @@ canvas/src/ - **Minimap:** Not present (MiniMap mocked as null in tests) - **Status:** Basic keyboard support via viewport shortcuts -### Keyboard Shortcuts ⚠️ PARTIAL -- Exists in `useKeyboardShortcuts.ts` but no `aria-describedby` on trigger buttons -- No dedicated keyboard shortcut help dialog -- **Gap:** Users can't discover shortcuts visually +### Keyboard Shortcuts ✅ (strong) +- All shortcuts in `useKeyboardShortcuts.ts` with `inInput` guard ✅ +- Global `?` shortcut opens `KeyboardShortcutsDialog` (PR #175) ✅ +- Dialog: portal-based, aria-modal, focus trap, Escape close ✅ +- Arrow keys move selected node 10px (50px with Shift) — keyboard node drag (this PR) ✅ +- Hierarchy navigation (Enter/Shift+Enter), z-order (Cmd+]/[), zoom-to-team (Z) ✅ ### Focus Management ✅ (strong) - Skip link → `#canvas-main` ✅ @@ -83,9 +85,9 @@ canvas/src/ - Focus trap in modals via Radix ✅ - Focus ring: `focus-visible:ring-2 focus-visible:ring-blue-500 focus-visible:ring-offset-2 focus-visible:ring-offset-zinc-950` -### Accessibility Tree ⚠️ PARTIAL +### Accessibility Tree ✅ - Canvas is in accessibility tree (React Flow DOM nodes) -- Node state changes not announced to screen readers (no `aria-live` region) +- Node state changes announced via `aria-live="polite"` region (PR #172) ✅ - Context menus announced via `role="menu"` ✅ ### Context Menus ✅ (strong) @@ -109,7 +111,7 @@ canvas/src/ |----------|------|-------|--------| | ~~HIGH~~ | ~~Screen reader announcements for canvas state changes~~ | ~~Canvas.tsx, canvas-events.ts, canvas.ts~~ | ✅ Done — PR #172 | | MEDIUM | Keyboard shortcut help dialog | useKeyboardShortcuts.ts | ✅ Done (PR #175) | -| MEDIUM | Keyboard-accessible node drag | WorkspaceNode.tsx, useDragHandlers.ts | Not started | +| MEDIUM | Keyboard-accessible node drag | WorkspaceNode.tsx, useDragHandlers.ts | ✅ Done (this PR) | | LOW | Edge anchor keyboard accessibility | A2AEdge.tsx | Not started | | LOW | Node resize keyboard accessibility | WorkspaceNode.tsx (NodeResizer) | Not started | -- 2.45.2 From 2f13fd24a196d4b9ec16e3ecdc0e0f75b80f5ad1 Mon Sep 17 00:00:00 2001 From: Molecule AI Core Platform Lead Date: Sat, 9 May 2026 22:21:47 +0000 Subject: [PATCH 2/2] trigger: re-run sop-tier-check after core-lead approval + main sync -- 2.45.2