Browse Source

fix: address code review findings across undo/redo, video wiring, and validation

- 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 <noreply@anthropic.com>
handoff-20260429-1057
shrimbly 4 months ago
parent
commit
5b0825e50b
  1. 1
      src/components/KeyboardShortcutsDialog.tsx
  2. 26
      src/components/WorkflowCanvas.tsx
  3. 19
      src/components/nodes/VideoInputNode.tsx
  4. 2
      src/store/execution/executeNode.ts
  5. 12
      src/store/utils/executionUtils.ts
  6. 17
      src/store/workflowStore.ts

1
src/components/KeyboardShortcutsDialog.tsx

@ -37,6 +37,7 @@ const shortcutGroups: ShortcutGroup[] = [
{ keys: ["Shift", "L"], description: "Add LLM Text node" }, { keys: ["Shift", "L"], description: "Add LLM Text node" },
{ keys: ["Shift", "A"], description: "Add Annotation node" }, { keys: ["Shift", "A"], description: "Add Annotation node" },
{ keys: ["Shift", "T"], description: "Add Audio node" }, { keys: ["Shift", "T"], description: "Add Audio node" },
{ keys: ["Shift", "Y"], description: "Add Video Input node" },
{ keys: ["Shift", "R"], description: "Add Array node" }, { keys: ["Shift", "R"], description: "Add Array node" },
], ],
}, },

26
src/components/WorkflowCanvas.tsx

@ -535,7 +535,7 @@ export function WorkflowCanvas() {
if (!targetNode) return false; if (!targetNode) return false;
const targetNodeType = targetNode.type; 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" // For output node, we allow video even though its handle is typed as "image"
// because output node can display both images and videos // because output node can display both images and videos
return true; return true;
@ -1172,7 +1172,11 @@ export function WorkflowCanvas() {
sourceHandleIdForNewNode = "text"; sourceHandleIdForNewNode = "text";
} }
} else if (handleType === "video") { } 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 // VideoStitch has dynamic video-N inputs and a video output
targetHandleId = "video-0"; targetHandleId = "video-0";
sourceHandleIdForNewNode = "video"; sourceHandleIdForNewNode = "video";
@ -1420,17 +1424,15 @@ export function WorkflowCanvas() {
return; return;
} }
// Handle undo (Ctrl/Cmd + Z) // Handle undo (Ctrl/Cmd + Z) and redo (Ctrl/Cmd + Shift + Z)
if ((event.ctrlKey || event.metaKey) && event.key === "z" && !event.shiftKey) { // Normalize key to lowercase — Shift makes event.key uppercase ("Z" instead of "z")
if ((event.ctrlKey || event.metaKey) && event.key.toLowerCase() === "z") {
event.preventDefault(); event.preventDefault();
undo(); if (event.shiftKey) {
return; redo();
} } else {
undo();
// Handle redo (Ctrl/Cmd + Shift + Z) }
if ((event.ctrlKey || event.metaKey) && event.key === "z" && event.shiftKey) {
event.preventDefault();
redo();
return; return;
} }

19
src/components/nodes/VideoInputNode.tsx

@ -11,6 +11,7 @@ type VideoInputNodeType = Node<VideoInputNodeData, "videoInput">;
const MAX_FILE_SIZE = 200 * 1024 * 1024; // 200MB const MAX_FILE_SIZE = 200 * 1024 * 1024; // 200MB
const ACCEPTED_FORMATS = "video/mp4,video/webm,video/quicktime"; const ACCEPTED_FORMATS = "video/mp4,video/webm,video/quicktime";
const ACCEPTED_MIME_TYPES = ACCEPTED_FORMATS.split(",");
function formatDuration(seconds: number): string { function formatDuration(seconds: number): string {
const mins = Math.floor(seconds / 60); const mins = Math.floor(seconds / 60);
@ -31,7 +32,7 @@ export function VideoInputNode({ id, data, selected }: NodeProps<VideoInputNodeT
const file = e.target.files?.[0]; const file = e.target.files?.[0];
if (!file) return; if (!file) return;
if (!file.type.match(/^video\//)) { if (!ACCEPTED_MIME_TYPES.includes(file.type)) {
alert("Unsupported format. Use MP4, WebM, or QuickTime video files."); alert("Unsupported format. Use MP4, WebM, or QuickTime video files.");
return; return;
} }
@ -41,13 +42,15 @@ export function VideoInputNode({ id, data, selected }: NodeProps<VideoInputNodeT
return; return;
} }
// Extract metadata using a temporary video element pointing at the original file
const metadataUrl = URL.createObjectURL(file);
const video = document.createElement("video");
video.preload = "metadata";
const reader = new FileReader(); const reader = new FileReader();
reader.onload = (event) => { reader.onload = (event) => {
const base64 = event.target?.result as string; 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 = () => { video.onloadedmetadata = () => {
updateNodeData(id, { updateNodeData(id, {
video: base64, video: base64,
@ -57,7 +60,7 @@ export function VideoInputNode({ id, data, selected }: NodeProps<VideoInputNodeT
duration: video.duration, duration: video.duration,
dimensions: { width: video.videoWidth, height: video.videoHeight }, dimensions: { width: video.videoWidth, height: video.videoHeight },
}); });
URL.revokeObjectURL(video.src); URL.revokeObjectURL(metadataUrl);
}; };
video.onerror = () => { video.onerror = () => {
// Still load the file even if metadata extraction fails // Still load the file even if metadata extraction fails
@ -69,11 +72,9 @@ export function VideoInputNode({ id, data, selected }: NodeProps<VideoInputNodeT
duration: null, duration: null,
dimensions: null, dimensions: null,
}); });
URL.revokeObjectURL(video.src); URL.revokeObjectURL(metadataUrl);
}; };
// Use blob URL for metadata extraction to avoid base64 parsing overhead video.src = metadataUrl;
const blob = new Blob([Uint8Array.from(atob(base64.split(",")[1]), c => c.charCodeAt(0))], { type: file.type });
video.src = URL.createObjectURL(blob);
}; };
reader.readAsDataURL(file); reader.readAsDataURL(file);
}, },

2
src/store/execution/executeNode.ts

@ -33,7 +33,7 @@ export interface ExecuteNodeOptions {
/** /**
* Execute a single node by dispatching to the appropriate executor. * 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( export async function executeNode(
ctx: NodeExecutionContext, ctx: NodeExecutionContext,

12
src/store/utils/executionUtils.ts

@ -129,12 +129,12 @@ export function clearNodeImageRefs(nodes: WorkflowNode[]): WorkflowNode[] {
revokeBlobUrl(data.outputVideo as string | undefined); revokeBlobUrl(data.outputVideo as string | undefined);
revokeBlobUrl(data.glbUrl as string | undefined); revokeBlobUrl(data.glbUrl as string | undefined);
// Clear all ref fields regardless of node type // Clear all ref fields regardless of node type (match any key ending in Ref or Refs)
delete data.imageRef; for (const key of Object.keys(data)) {
delete data.sourceImageRef; if (/Refs?$/.test(key)) {
delete data.outputImageRef; delete data[key];
delete data.inputImageRefs; }
delete data.videoRef; }
return { ...node, data: data as WorkflowNodeData } as WorkflowNode; return { ...node, data: data as WorkflowNodeData } as WorkflowNode;
}); });

17
src/store/workflowStore.ts

@ -579,8 +579,9 @@ const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
canRedo: false, canRedo: false,
undo: () => { 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) { if (pendingDataSnapshot) {
undoManager.push(pendingDataSnapshot);
pendingDataSnapshot = null; pendingDataSnapshot = null;
if (dataChangeTimer) { if (dataChangeTimer) {
clearTimeout(dataChangeTimer); clearTimeout(dataChangeTimer);
@ -597,13 +598,15 @@ const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
edgeStyle: previous.edgeStyle, edgeStyle: previous.edgeStyle,
hasUnsavedChanges: true, hasUnsavedChanges: true,
}); });
get().recomputeDimmedNodes();
} }
syncUndoFlags(set); syncUndoFlags(set);
}, },
redo: () => { redo: () => {
// Discard any pending debounced data snapshot // Flush any pending debounced data snapshot so the pre-edit state is preserved
if (pendingDataSnapshot) { if (pendingDataSnapshot) {
undoManager.push(pendingDataSnapshot);
pendingDataSnapshot = null; pendingDataSnapshot = null;
if (dataChangeTimer) { if (dataChangeTimer) {
clearTimeout(dataChangeTimer); clearTimeout(dataChangeTimer);
@ -620,6 +623,7 @@ const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
edgeStyle: next.edgeStyle, edgeStyle: next.edgeStyle,
hasUnsavedChanges: true, hasUnsavedChanges: true,
}); });
get().recomputeDimmedNodes();
} }
syncUndoFlags(set); syncUndoFlags(set);
}, },
@ -861,7 +865,14 @@ const workflowStoreImpl: StateCreator<WorkflowStore> = (set, get) => ({
edges: state.edges.filter((edge) => edge.id !== edgeId), edges: state.edges.filter((edge) => edge.id !== edgeId),
hasUnsavedChanges: true, hasUnsavedChanges: true,
})); }));
if (removedEdge) clearStaleInputImages([removedEdge], get); if (removedEdge) {
deleteCheckpointActive = true;
try {
clearStaleInputImages([removedEdge], get);
} finally {
deleteCheckpointActive = false;
}
}
get().incrementManualChangeCount(); get().incrementManualChangeCount();
}, },

Loading…
Cancel
Save