From 5b0825e50b5e33749079ff1601e7560173439ee7 Mon Sep 17 00:00:00 2001 From: shrimbly Date: Tue, 31 Mar 2026 07:39:00 +1300 Subject: [PATCH] fix: address code review findings across undo/redo, video wiring, and validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix redo shortcut by normalizing event.key to lowercase (Shift makes it "Z") - Flush pendingDataSnapshot in undo/redo instead of discarding it, preserving the pre-edit checkpoint so Cmd+Z doesn't skip states - Recompute dimmedNodeIds after undo/redo to keep dimming in sync - Set deleteCheckpointActive in removeEdge before clearStaleInputImages to prevent split undo entries - Wire videoInput in handleMenuSelect and isValidConnection for video connections - Validate video MIME type against ACCEPTED_FORMATS whitelist instead of broad regex - Use URL.createObjectURL(file) for metadata extraction instead of base64→Blob - Add Shift+Y (Video Input) to keyboard shortcuts dialog - Update executeNode JSDoc to include videoInput as data-source no-op - Clear all *Ref/*Refs fields dynamically in clearNodeImageRefs Co-Authored-By: Claude Opus 4.6 --- src/components/KeyboardShortcutsDialog.tsx | 1 + src/components/WorkflowCanvas.tsx | 26 ++++++++++++---------- src/components/nodes/VideoInputNode.tsx | 19 ++++++++-------- src/store/execution/executeNode.ts | 2 +- src/store/utils/executionUtils.ts | 12 +++++----- src/store/workflowStore.ts | 17 +++++++++++--- 6 files changed, 46 insertions(+), 31 deletions(-) diff --git a/src/components/KeyboardShortcutsDialog.tsx b/src/components/KeyboardShortcutsDialog.tsx index 63c8a43b..8c7e2e7a 100644 --- a/src/components/KeyboardShortcutsDialog.tsx +++ b/src/components/KeyboardShortcutsDialog.tsx @@ -37,6 +37,7 @@ const shortcutGroups: ShortcutGroup[] = [ { keys: ["Shift", "L"], description: "Add LLM Text node" }, { keys: ["Shift", "A"], description: "Add Annotation node" }, { keys: ["Shift", "T"], description: "Add Audio node" }, + { keys: ["Shift", "Y"], description: "Add Video Input node" }, { keys: ["Shift", "R"], description: "Add Array node" }, ], }, diff --git a/src/components/WorkflowCanvas.tsx b/src/components/WorkflowCanvas.tsx index 52d1680f..958b7032 100644 --- a/src/components/WorkflowCanvas.tsx +++ b/src/components/WorkflowCanvas.tsx @@ -535,7 +535,7 @@ export function WorkflowCanvas() { if (!targetNode) return false; const targetNodeType = targetNode.type; - if (targetNodeType === "generateVideo" || targetNodeType === "videoStitch" || targetNodeType === "easeCurve" || targetNodeType === "videoTrim" || targetNodeType === "videoFrameGrab" || targetNodeType === "output" || targetNodeType === "router") { + if (targetNodeType === "generateVideo" || targetNodeType === "videoStitch" || targetNodeType === "easeCurve" || targetNodeType === "videoTrim" || targetNodeType === "videoFrameGrab" || targetNodeType === "videoInput" || targetNodeType === "output" || targetNodeType === "router") { // For output node, we allow video even though its handle is typed as "image" // because output node can display both images and videos return true; @@ -1172,7 +1172,11 @@ export function WorkflowCanvas() { sourceHandleIdForNewNode = "text"; } } else if (handleType === "video") { - if (nodeType === "videoStitch") { + if (nodeType === "videoInput") { + // VideoInput accepts video input and outputs video + targetHandleId = "video"; + sourceHandleIdForNewNode = "video"; + } else if (nodeType === "videoStitch") { // VideoStitch has dynamic video-N inputs and a video output targetHandleId = "video-0"; sourceHandleIdForNewNode = "video"; @@ -1420,17 +1424,15 @@ export function WorkflowCanvas() { return; } - // Handle undo (Ctrl/Cmd + Z) - if ((event.ctrlKey || event.metaKey) && event.key === "z" && !event.shiftKey) { + // Handle undo (Ctrl/Cmd + Z) and redo (Ctrl/Cmd + Shift + Z) + // Normalize key to lowercase — Shift makes event.key uppercase ("Z" instead of "z") + if ((event.ctrlKey || event.metaKey) && event.key.toLowerCase() === "z") { event.preventDefault(); - undo(); - return; - } - - // Handle redo (Ctrl/Cmd + Shift + Z) - if ((event.ctrlKey || event.metaKey) && event.key === "z" && event.shiftKey) { - event.preventDefault(); - redo(); + if (event.shiftKey) { + redo(); + } else { + undo(); + } return; } diff --git a/src/components/nodes/VideoInputNode.tsx b/src/components/nodes/VideoInputNode.tsx index beae53f3..b2861e2e 100644 --- a/src/components/nodes/VideoInputNode.tsx +++ b/src/components/nodes/VideoInputNode.tsx @@ -11,6 +11,7 @@ type VideoInputNodeType = Node; const MAX_FILE_SIZE = 200 * 1024 * 1024; // 200MB const ACCEPTED_FORMATS = "video/mp4,video/webm,video/quicktime"; +const ACCEPTED_MIME_TYPES = ACCEPTED_FORMATS.split(","); function formatDuration(seconds: number): string { const mins = Math.floor(seconds / 60); @@ -31,7 +32,7 @@ export function VideoInputNode({ id, data, selected }: NodeProps { const base64 = event.target?.result as string; - // Extract duration and dimensions using HTML Video element - const video = document.createElement("video"); - video.preload = "metadata"; video.onloadedmetadata = () => { updateNodeData(id, { video: base64, @@ -57,7 +60,7 @@ export function VideoInputNode({ id, data, selected }: NodeProps { // Still load the file even if metadata extraction fails @@ -69,11 +72,9 @@ export function VideoInputNode({ id, data, selected }: NodeProps c.charCodeAt(0))], { type: file.type }); - video.src = URL.createObjectURL(blob); + video.src = metadataUrl; }; reader.readAsDataURL(file); }, diff --git a/src/store/execution/executeNode.ts b/src/store/execution/executeNode.ts index 9657fb82..32d1d792 100644 --- a/src/store/execution/executeNode.ts +++ b/src/store/execution/executeNode.ts @@ -33,7 +33,7 @@ export interface ExecuteNodeOptions { /** * Execute a single node by dispatching to the appropriate executor. * - * Data-source node types (`imageInput`, `audioInput`) are no-ops. + * Data-source node types (`imageInput`, `audioInput`, `videoInput`) are no-ops. */ export async function executeNode( ctx: NodeExecutionContext, diff --git a/src/store/utils/executionUtils.ts b/src/store/utils/executionUtils.ts index aba4225f..5f42f7ba 100644 --- a/src/store/utils/executionUtils.ts +++ b/src/store/utils/executionUtils.ts @@ -129,12 +129,12 @@ export function clearNodeImageRefs(nodes: WorkflowNode[]): WorkflowNode[] { revokeBlobUrl(data.outputVideo as string | undefined); revokeBlobUrl(data.glbUrl as string | undefined); - // Clear all ref fields regardless of node type - delete data.imageRef; - delete data.sourceImageRef; - delete data.outputImageRef; - delete data.inputImageRefs; - delete data.videoRef; + // Clear all ref fields regardless of node type (match any key ending in Ref or Refs) + for (const key of Object.keys(data)) { + if (/Refs?$/.test(key)) { + delete data[key]; + } + } return { ...node, data: data as WorkflowNodeData } as WorkflowNode; }); diff --git a/src/store/workflowStore.ts b/src/store/workflowStore.ts index 188c30ad..0a6d9f08 100644 --- a/src/store/workflowStore.ts +++ b/src/store/workflowStore.ts @@ -579,8 +579,9 @@ const workflowStoreImpl: StateCreator = (set, get) => ({ canRedo: false, undo: () => { - // Discard any pending debounced data snapshot — it's intermediate state + // Flush any pending debounced data snapshot so the pre-edit state is preserved if (pendingDataSnapshot) { + undoManager.push(pendingDataSnapshot); pendingDataSnapshot = null; if (dataChangeTimer) { clearTimeout(dataChangeTimer); @@ -597,13 +598,15 @@ const workflowStoreImpl: StateCreator = (set, get) => ({ edgeStyle: previous.edgeStyle, hasUnsavedChanges: true, }); + get().recomputeDimmedNodes(); } syncUndoFlags(set); }, redo: () => { - // Discard any pending debounced data snapshot + // Flush any pending debounced data snapshot so the pre-edit state is preserved if (pendingDataSnapshot) { + undoManager.push(pendingDataSnapshot); pendingDataSnapshot = null; if (dataChangeTimer) { clearTimeout(dataChangeTimer); @@ -620,6 +623,7 @@ const workflowStoreImpl: StateCreator = (set, get) => ({ edgeStyle: next.edgeStyle, hasUnsavedChanges: true, }); + get().recomputeDimmedNodes(); } syncUndoFlags(set); }, @@ -861,7 +865,14 @@ const workflowStoreImpl: StateCreator = (set, get) => ({ edges: state.edges.filter((edge) => edge.id !== edgeId), hasUnsavedChanges: true, })); - if (removedEdge) clearStaleInputImages([removedEdge], get); + if (removedEdge) { + deleteCheckpointActive = true; + try { + clearStaleInputImages([removedEdge], get); + } finally { + deleteCheckpointActive = false; + } + } get().incrementManualChangeCount(); },