fix: Workflow Builder UX Improvements #942
Conversation
avirupsinha12
requested review from
devesh-juspay,
junaid-shirur,
kalpadhwaryu,
shivamashtikar and
zereraz
as code owners
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds two UI components (ConfirmationPopup, Snackbar) and integrates them into WorkflowBuilder for change detection, save flow, snackbar notifications, and refresh confirmation. Execution modal now requires an existing templateId; on-the-fly template creation for execution was removed. TemplateSelectionModal receives dark-mode styling only. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Header
participant Builder as WorkflowBuilder
participant API as Templates API
participant Snackbar
participant Modal as ExecutionModal
User->>Header: Click "Save Changes"
Header->>Builder: onSaveChanges()
Builder->>API: POST /templates (workflow payload)
API-->>Builder: createdTemplate (id)
Builder->>Builder: update lastSavedHash, isWorkflowSaved, hasWorkflowChanged
Builder->>Snackbar: show success message
Note right of User: Execute requires saved template
User->>Header: Click "Execute"
Header->>Builder: request open execution modal
alt has templateId
Builder->>Modal: open with templateId
Modal-->>User: Start enabled
else missing templateId
Builder->>Snackbar: show warning "Please save the workflow first"
Modal-->>User: Start disabled, show hint
end
sequenceDiagram
autonumber
actor User
participant Builder as WorkflowBuilder
participant Confirm as ConfirmationPopup
participant Window
User->>Builder: Trigger refresh/navigation
alt has unsaved changes
Builder->>Confirm: show confirmation popup (onConfirm/onCancel)
alt user confirms
Confirm-->>Builder: onConfirm
Builder->>Window: run pending refresh callback
else user cancels
Confirm-->>Builder: onCancel
Builder-->>User: abort refresh
end
else no unsaved changes
Builder->>Window: proceed refresh
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @avirupsinha12, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the Workflow Builder's user experience by introducing robust state management for unsaved changes, providing an explicit save mechanism, and improving user feedback through non-intrusive snackbar notifications. These changes aim to prevent data loss, clarify the workflow creation process, and streamline the execution of saved workflows, making the builder more intuitive and reliable for users. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant UX improvements to the Workflow Builder, including new UI components for notifications and confirmations, better handling of unsaved changes, and a more streamlined workflow saving and execution process. The changes are well-implemented and greatly enhance the user experience. I've provided a few suggestions to further improve code quality and maintainability, such as refactoring for code reuse, improving configuration handling, and fixing minor UI text issues.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
frontend/src/components/workflow/WorkflowExecutionModal.tsx (1)
666-684: Fix tooltip copy.Small UX polish.
Apply this diff:
- template not saved Please save template + Template not saved. Please save the template.frontend/src/components/ui/Snackbar.tsx (1)
91-115: Add ARIA roles for screen readers.Expose live region semantics and label the close control.
Apply this diff:
- <div - className={`fixed z-[9999] ${getPositionClasses()} transition-all duration-300 ease-in-out ${ + <div + role={type === 'error' || type === 'warning' ? 'alert' : 'status'} + aria-live={type === 'error' || type === 'warning' ? 'assertive' : 'polite'} + className={`fixed z-[9999] ${getPositionClasses()} transition-all duration-300 ease-in-out ${ isShowing ? 'opacity-100 translate-y-0' : 'opacity-0 -translate-y-2' }`} > <div className={`flex items-center gap-3 px-4 py-3 rounded-lg border shadow-lg max-w-md ${getBackgroundColor()}`} > {getIcon()} <div className={`flex-1 text-sm font-medium ${getTextColor()}`}> {message} </div> <button onClick={() => { setIsShowing(false) setTimeout(onClose, 300) }} - className={`flex-shrink-0 p-1 rounded-full hover:bg-black/10 dark:hover:bg-white/10 transition-colors ${getTextColor()}`} + aria-label="Close notification" + className={`flex-shrink-0 p-1 rounded-full hover:bg-black/10 dark:hover:bg-white/10 transition-colors ${getTextColor()}`} > <X className="w-4 h-4" /> </button> </div> </div>frontend/src/components/ui/ConfirmationPopup.tsx (1)
25-56: Add dialog semantics (role, aria-modal, labelledby).Improves accessibility with minimal changes.
Apply this diff:
- <div className="fixed inset-0 bg-black/50 flex items-center justify-center z-[9999]"> - <div className="bg-white dark:bg-gray-900 rounded-2xl shadow-xl max-w-md w-full mx-4 p-8"> + <div className="fixed inset-0 bg-black/50 flex items-center justify-center z-[9999]"> + <div + className="bg-white dark:bg-gray-900 rounded-2xl shadow-xl max-w-md w-full mx-4 p-8" + role="dialog" + aria-modal="true" + aria-labelledby="confirmation-popup-title" + > {/* Title */} - <h2 className="text-xl font-bold text-gray-900 dark:text-gray-100 mb-3"> + <h2 id="confirmation-popup-title" className="text-xl font-bold text-gray-900 dark:text-gray-100 mb-3"> {title} </h2>frontend/src/components/workflow/WorkflowBuilder.tsx (2)
1916-1933: Hashing approach: constrain payload to stable keys.Serializing full
node.datacan be heavy/non-deterministic if objects gain transient props. Prefer a normalized projection (ids, types, positions, minimal tool metadata).Also applies to: 2106-2110, 2118-2140
3284-3390: Save flow: add in-flight guard; endpoint verified
- Prevent duplicate POSTs: introduce an
isSavingstate, early-return fromhandleSaveChangeswhen true, and disable the Save button while saving — Location: frontend/src/components/workflow/WorkflowBuilder.tsx (handleSaveChanges).api.workflow.templates.complex.$postis present and used (see frontend/src/components/workflow/api/ApiHandlers.ts:219 and WorkflowBuilder.tsx:3338).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
server/bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (4)
frontend/src/components/ui/ConfirmationPopup.tsx(1 hunks)frontend/src/components/ui/Snackbar.tsx(1 hunks)frontend/src/components/workflow/WorkflowBuilder.tsx(13 hunks)frontend/src/components/workflow/WorkflowExecutionModal.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/src/components/ui/Snackbar.tsx (1)
frontend/src/lib/common.tsx (1)
getIcon(43-154)
frontend/src/components/workflow/WorkflowBuilder.tsx (5)
frontend/src/components/workflow/Types.ts (1)
WorkflowTemplate(227-276)frontend/src/api.ts (1)
api(4-4)frontend/src/components/workflow/WorkflowExecutionModal.tsx (1)
WorkflowExecutionModal(34-692)frontend/src/components/ui/Snackbar.tsx (1)
Snackbar(13-116)frontend/src/components/ui/ConfirmationPopup.tsx (1)
ConfirmationPopup(13-58)
🔇 Additional comments (6)
frontend/src/components/workflow/WorkflowBuilder.tsx (6)
1882-1893: Snackbar plumbing looks good.Centralized helper and type-safe variants are clean.
Also applies to: 1909-1913
1186-1202: Back/refresh confirmation and Save UX: LGTM.Back action correctly gates on unsaved changes; Save button state is derived appropriately.
Also applies to: 1235-1248
2653-2667: Execution UX when no saved template: LGTM.Warning snackbar instead of implicit template creation aligns with the new execution requirements.
3236-3282: Refresh/close protection: good guardrails.beforeunload + Cmd/Ctrl‑R interception are correctly scoped to builder with unsaved changes. Keep an eye on potential conflicts with app‑level key handlers.
3490-3490: Error surfaced via snackbar: LGTM.Consistent with the new notification pattern.
159-161: Snackbar and ConfirmationPopup integration: LGTM.Imports and render wiring are clean; z-order is appropriate for overlays.
Also applies to: 3802-3810, 3812-3830
avirupsinha12
force-pushed
the
fix-Workflow-builder-UX-improvements
branch
from
d46a222 to
2b85260
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
frontend/src/components/workflow/TemplateSelectionModal.tsx (1)
57-63: Add dialog semantics for accessibility.Provide proper roles and labels so screen readers recognize this as a modal dialog.
- <div className="fixed inset-0 bg-black/50 flex items-center justify-center z-50"> + <div className="fixed inset-0 bg-black/50 flex items-center justify-center z-50" aria-hidden="false"> <div - className="bg-white dark:bg-gray-900 rounded-2xl shadow-xl w-full max-w-[964px] mx-4 relative max-h-[80vh] overflow-hidden flex flex-col" + className="bg-white dark:bg-gray-900 rounded-2xl shadow-xl w-full max-w-[964px] mx-4 relative max-h-[80vh] overflow-hidden flex flex-col" + role="dialog" + aria-modal="true" + aria-labelledby="template-select-title" + aria-describedby="template-select-desc" style={{ width: "min(964px, calc(100vw - 2rem))" }} > @@ - <h2 className="text-2xl font-bold text-gray-900 dark:text-gray-100 mb-2"> + <h2 id="template-select-title" className="text-2xl font-bold text-gray-900 dark:text-gray-100 mb-2"> Select Templates </h2> - <p className="text-gray-600 dark:text-gray-400"> + <p id="template-select-desc" className="text-gray-600 dark:text-gray-400"> Start with a template to get up and running quickly </p>Also applies to: 72-76
frontend/src/components/ui/Snackbar.tsx (1)
91-97: Add ARIA for live region announcements.Helps screen readers announce snackbar messages.
- <div + <div className={`fixed z-[9999] ${getPositionClasses()} transition-all duration-300 ease-in-out ${ isShowing ? 'opacity-100 translate-y-0' : 'opacity-0 -translate-y-2' }`} + role="status" + aria-live={type === 'error' || type === 'warning' ? 'assertive' : 'polite'} + aria-atomic="true" >frontend/src/components/workflow/WorkflowExecutionModal.tsx (2)
449-456: Pass templateId on retry to avoid fallback to undefined.- executeWorkflow(selectedFile) + executeWorkflow(selectedFile, templateId)
642-649: Pass templateId on “Retry Execution” as well.- executeWorkflow(selectedFile) + executeWorkflow(selectedFile, templateId)frontend/src/components/ui/ConfirmationPopup.tsx (1)
25-36: Add dialog semantics (role/labels) for accessibility.- <div className="fixed inset-0 bg-black/50 flex items-center justify-center z-[9999]"> - <div className="bg-white dark:bg-gray-900 rounded-2xl shadow-xl max-w-md w-full mx-4 p-8"> + <div className="fixed inset-0 bg-black/50 flex items-center justify-center z-[9999]" aria-hidden="false"> + <div + className="bg-white dark:bg-gray-900 rounded-2xl shadow-xl max-w-md w-full mx-4 p-8" + role="dialog" + aria-modal="true" + aria-labelledby="confirm-popup-title" + aria-describedby="confirm-popup-message" + > @@ - <h2 className="text-xl font-bold text-gray-900 dark:text-gray-100 mb-3"> + <h2 id="confirm-popup-title" className="text-xl font-bold text-gray-900 dark:text-gray-100 mb-3"> {title} </h2> @@ - <p className="text-gray-600 dark:text-gray-400 text-base leading-relaxed mb-8"> + <p id="confirm-popup-message" className="text-gray-600 dark:text-gray-400 text-base leading-relaxed mb-8"> {message} </p>frontend/src/components/workflow/WorkflowBuilder.tsx (2)
26-31: Use the shared WorkflowTemplate type instead of redefining it.Avoid drift and reduce surface area by importing from Types.
-import { - Flow, - TemplateFlow, - Step, - UserDetail, - Tool, -} from "./Types" +import { + Flow, + TemplateFlow, + Step, + UserDetail, + Tool, + WorkflowTemplate, +} from "./Types" @@ -// Import WorkflowTemplate type -interface WorkflowTemplate { - id: string - name: string - description: string - version: string - status: string - config: { - ai_model?: string - max_file_size?: string - auto_execution?: boolean - schema_version?: string - allowed_file_types?: string[] - supports_file_upload?: boolean - } - createdBy: string - rootWorkflowStepTemplateId: string - createdAt: string - updatedAt: string - steps?: Array<{ - id: string - workflowTemplateId: string - name: string - description: string - type: string - parentStepId: string | null - prevStepIds: string[] - nextStepIds: string[] - toolIds: string[] - timeEstimate: number - metadata: { - icon?: string - step_order?: number - schema_version?: string - user_instructions?: string - } - createdAt: string - updatedAt: string - }> - workflow_tools?: Array<{ - id: string - type: string - value: any - config: any - createdBy: string - createdAt: string - updatedAt: string - }> - rootStep?: { - id: string - workflowTemplateId: string - name: string - description: string - type: string - timeEstimate: number - metadata: { - icon?: string - step_order?: number - schema_version?: string - user_instructions?: string - } - tool?: { - id: string - type: string - value: any - config: any - createdBy: string - createdAt: string - updatedAt: string - } - } - stepExecutions?: Array<{ - id: string - workflowExecutionId: string - workflowStepTemplateId: string - name: string - type: string - status: string - parentStepId: string | null - prevStepIds: string[] - nextStepIds: string[] - toolExecIds: string[] - timeEstimate: number - metadata: any - completedBy: string | null - createdAt: string - updatedAt: string - completedAt: string | null - }> - toolExecutions?: Array<{ - id: string - workflowToolId: string - workflowExecutionId: string - status: string - result: any - startedAt: string | null - completedAt: string | null - createdAt: string - updatedAt: string - }> -}Also applies to: 34-136
3297-3306: Preserve version/config from selected template when available.Avoid overwriting existing metadata on edits.
- description: selectedTemplate?.description || "Workflow created from builder", - version: "1.0.0", - config: { - ai_model: "gemini-1.5-pro", - max_file_size: "10MB", - auto_execution: false, - schema_version: "1.0", - allowed_file_types: ["pdf", "docx", "txt", "jpg", "png"], - supports_file_upload: true, - }, + description: selectedTemplate?.description || "Workflow created from builder", + version: selectedTemplate?.version || "1.0.0", + config: selectedTemplate?.config || { + ai_model: "gemini-1.5-pro", + max_file_size: "10MB", + auto_execution: false, + schema_version: "1.0", + allowed_file_types: ["pdf", "docx", "txt", "jpg", "png"], + supports_file_upload: true, + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
server/bun.lockbis excluded by!**/bun.lockb
📒 Files selected for processing (5)
frontend/src/components/ui/ConfirmationPopup.tsx(1 hunks)frontend/src/components/ui/Snackbar.tsx(1 hunks)frontend/src/components/workflow/TemplateSelectionModal.tsx(5 hunks)frontend/src/components/workflow/WorkflowBuilder.tsx(13 hunks)frontend/src/components/workflow/WorkflowExecutionModal.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/workflow/WorkflowBuilder.tsx (5)
frontend/src/components/workflow/Types.ts (1)
WorkflowTemplate(227-276)frontend/src/api.ts (1)
api(4-4)frontend/src/components/workflow/WorkflowExecutionModal.tsx (1)
WorkflowExecutionModal(34-692)frontend/src/components/ui/Snackbar.tsx (1)
Snackbar(13-116)frontend/src/components/ui/ConfirmationPopup.tsx (1)
ConfirmationPopup(13-58)
🔇 Additional comments (6)
frontend/src/components/ui/Snackbar.tsx (1)
21-35: Deduplicate close logic with a memoized handler.Avoid repeating the animation + onClose timing in both useEffect and onClick.
- const [isShowing, setIsShowing] = useState(false) + const [isShowing, setIsShowing] = useState(false) + const handleClose = React.useCallback(() => { + setIsShowing(false) + setTimeout(onClose, 300) + }, [onClose]) useEffect(() => { if (isVisible) { setIsShowing(true) - const timer = setTimeout(() => { - setIsShowing(false) - setTimeout(onClose, 300) // Wait for exit animation - }, duration) + const timer = setTimeout(handleClose, duration) - return () => clearTimeout(timer) + return () => clearTimeout(timer) } else { setIsShowing(false) } - }, [isVisible, duration, onClose]) + }, [isVisible, duration, handleClose]) @@ - <button - onClick={() => { - setIsShowing(false) - setTimeout(onClose, 300) - }} + <button + onClick={handleClose}Also applies to: 104-109
frontend/src/components/workflow/WorkflowExecutionModal.tsx (2)
174-195: Guard against missing templateId and remove non-null assertion.Retry paths call executeWorkflow without passing templateId; add an internal guard and avoid currentTemplateId! to prevent undefined API calls.
const executeWorkflow = async (file: File, execTemplateId?: string) => { const currentTemplateId = execTemplateId || templateId setIsUploading(true) try { + if (!currentTemplateId) { + throw new Error("No template ID provided for execution. Please save the workflow first.") + } // Create form data matching the curl command format @@ - const response = await workflowExecutionsAPI.executeTemplate( - currentTemplateId!, - executionData, - ) + const response = await workflowExecutionsAPI.executeTemplate( + currentTemplateId, + executionData, + )
679-681: Fix tooltip copy for clarity and tone.Recommend: “Template not saved. Please save the workflow first.”
- template not saved Please save template + Template not saved. Please save the workflow first.frontend/src/components/ui/ConfirmationPopup.tsx (1)
39-45: Use generic comment; this is a reusable confirm action.- {/* Refresh Button */} + {/* Confirm Button */}frontend/src/components/workflow/WorkflowBuilder.tsx (2)
3485-3490: Good: centralized error surfacing via Snackbar.Consistent UX for failures.
3763-3781: Remove invalid prop passed to WorkflowExecutionModal (compile error).workflowTemplate isn’t in WorkflowExecutionModalProps.
<WorkflowExecutionModal isOpen={showExecutionModal} onClose={() => { setShowExecutionModal(false) // Keep createdTemplate state so it can be reused for future executions }} workflowName={template?.name || "Custom Workflow"} workflowDescription={template?.description || "User-created workflow"} templateId={templateId} - workflowTemplate={templateId ? template || undefined : undefined} onViewExecution={onViewExecution} />
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
frontend/src/components/workflow/WorkflowBuilder.tsx (5)
1916-1938: Make workflow hash ignore volatile UI/runtime fields to avoid false positives.Include only stable fields; exclude
status,isActive,isCompleted,hasNext, etc.- const createWorkflowHash = () => { - const workflowState = { - nodes: nodes.map(node => ({ - id: node.id, - type: node.type, - // Exclude position from hash calculation - data: { - step: node.data?.step, - tools: node.data?.tools, - // Exclude UI state properties like hasNext, isActive, isCompleted, anyNodeSelected - } - })), - edges: edges.map(edge => ({ - id: edge.id, - source: edge.source, - target: edge.target, - type: edge.type - })) - } - return JSON.stringify(workflowState) - } + const createWorkflowHash = () => { + const workflowState = { + nodes: nodes.map((node) => { + const step = (node.data as any)?.step || {} + const { status, isExecution, contents, ...stableStep } = step + const tools = ((node.data as any)?.tools || []).map((t: any) => ({ + id: t.id, type: t.type, val: t.val, value: t.value, config: t.config + })) + return { + id: node.id, + type: node.type, + data: { step: stableStep, tools } + } + }), + edges: edges.map((edge) => ({ + id: edge.id, source: edge.source, target: edge.target, type: edge.type + })) + } + return JSON.stringify(workflowState) + }
3376-3391: Use API response as source of truth for createdTemplate.Don’t synthesize fields like
createdBy: 'current-user'. Set state fromcreateResult.data.- const newCreatedTemplate = { - id: createdTemplateId, - name: workflowData.name, - description: workflowData.description, - version: workflowData.version, - status: 'active', - config: workflowData.config, - createdBy: 'current-user', - rootWorkflowStepTemplateId: '', - createdAt: new Date().toISOString(), - updatedAt: new Date().toISOString(), - } as any - - setCreatedTemplate(newCreatedTemplate) + setCreatedTemplate(createResult.data as WorkflowTemplate)
1877-1879: Browser-safe timer type.
NodeJS.Timeoutcan error in web TS configs; prefer ReturnType.- const [pollingInterval, setPollingInterval] = useState<NodeJS.Timeout | null>( + const [pollingInterval, setPollingInterval] = useState<ReturnType<typeof setInterval> | null>( null, )
3263-3271: beforeunload: custom messages are ignored by browsers.Use empty string to trigger the native prompt; avoid returning a custom string.
- if (builder && hasWorkflowChanged) { - event.preventDefault() - event.returnValue = 'You have unsaved changes. Are you sure you want to leave?' - return 'You have unsaved changes. Are you sure you want to leave?' - } + if (builder && hasWorkflowChanged) { + event.preventDefault() + event.returnValue = '' + }
34-136: Avoid local WorkflowTemplate duplication; import the canonical type.This prevents type drift and mismatches across components.
-// Import WorkflowTemplate type -interface WorkflowTemplate { - id: string - name: string - description: string - version: string - status: string - config: { - ai_model?: string - max_file_size?: string - auto_execution?: boolean - schema_version?: string - allowed_file_types?: string[] - supports_file_upload?: boolean - } - createdBy: string - rootWorkflowStepTemplateId: string - createdAt: string - updatedAt: string - steps?: Array<{ - id: string - workflowTemplateId: string - name: string - description: string - type: string - parentStepId: string | null - prevStepIds: string[] - nextStepIds: string[] - toolIds: string[] - timeEstimate: number - metadata: { - icon?: string - step_order?: number - schema_version?: string - user_instructions?: string - ai_model?: string - automated_description?: string - } - createdAt: string - updatedAt: string - }> - workflow_tools?: Array<{ - id: string - type: string - value: any - config: any - createdBy: string - createdAt: string - updatedAt: string - }> - rootStep?: { - id: string - workflowTemplateId: string - name: string - description: string - type: string - timeEstimate: number - metadata: { - icon?: string - step_order?: number - schema_version?: string - user_instructions?: string - } - tool?: { - id: string - type: string - value: any - config: any - createdBy: string - createdAt: string - updatedAt: string - } - } - stepExecutions?: Array<{ - id: string - workflowExecutionId: string - workflowStepTemplateId: string - name: string - type: string - status: string - parentStepId: string | null - prevStepIds: string[] - nextStepIds: string[] - toolExecIds: string[] - timeEstimate: number - metadata: any - completedBy: string | null - createdAt: string - updatedAt: string - completedAt: string | null - }> - toolExecutions?: Array<{ - id: string - workflowToolId: string - workflowExecutionId: string - status: string - result: any - startedAt: string | null - completedAt: string | null - createdAt: string - updatedAt: string - }> -} +// Prefer the shared type +// import { WorkflowTemplate } from "./Types"Also add
WorkflowTemplateto the existing import from./Types.import { Flow, TemplateFlow, Step, UserDetail, Tool, + WorkflowTemplate, } from "./Types"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/workflow/WorkflowBuilder.tsx(13 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/workflow/WorkflowBuilder.tsx (5)
frontend/src/components/workflow/Types.ts (1)
WorkflowTemplate(227-276)frontend/src/api.ts (1)
api(4-4)frontend/src/components/workflow/WorkflowExecutionModal.tsx (1)
WorkflowExecutionModal(34-692)frontend/src/components/ui/Snackbar.tsx (1)
Snackbar(13-116)frontend/src/components/ui/ConfirmationPopup.tsx (1)
ConfirmationPopup(13-58)
🔇 Additional comments (2)
frontend/src/components/workflow/WorkflowBuilder.tsx (2)
3316-3327: Preserve template version/config instead of hardcoding.Carry over from existing template when editing; default otherwise.
- description: selectedTemplate?.description || "Workflow created from builder", - version: "1.0.0", - config: { - ai_model: "gemini-1.5-pro", - max_file_size: "10MB", - auto_execution: false, - schema_version: "1.0", - allowed_file_types: ["pdf", "docx", "txt", "jpg", "png"], - supports_file_upload: true, - }, + description: selectedTemplate?.description ?? "Workflow created from builder", + version: selectedTemplate?.version ?? "1.0.0", + config: selectedTemplate?.config ?? { + ai_model: "gemini-1.5-pro", + max_file_size: "10MB", + auto_execution: false, + schema_version: "1.0", + allowed_file_types: ["pdf", "docx", "txt", "jpg", "png"], + supports_file_upload: true, + },
3778-3796: Remove invalid prop passed to WorkflowExecutionModal (TS compile error).
workflowTemplateis not in WorkflowExecutionModalProps. Keep onlytemplateId.<WorkflowExecutionModal isOpen={showExecutionModal} onClose={() => { setShowExecutionModal(false) // Keep createdTemplate state so it can be reused for future executions }} workflowName={template?.name || "Custom Workflow"} workflowDescription={template?.description || "User-created workflow"} templateId={templateId} - workflowTemplate={templateId ? template || undefined : undefined} onViewExecution={onViewExecution} />
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
frontend/src/components/ui/Snackbar.tsx (5)
92-96: Add ARIA roles for screen readersExpose message semantics to assistive tech (alert for error/warning, status for others).
- <div - className={`fixed z-[9999] ${getPositionClasses()} transition-all duration-300 ease-in-out ${ + <div + role={type === 'error' || type === 'warning' ? 'alert' : 'status'} + aria-live={type === 'error' || type === 'warning' ? 'assertive' : 'polite'} + className={`fixed z-[9999] ${getPositionClasses()} transition-all duration-300 ease-in-out ${ isShowing ? 'opacity-100 translate-y-0' : 'opacity-0 -translate-y-2' }`} >
104-112: Minor: ensure button doesn’t submit formsSet explicit type to avoid accidental form submission.
- <button + <button + type="button" onClick={handleClose} aria-label="Close notification" title="Close" className={`flex-shrink-0 p-1 rounded-full hover:bg-black/10 dark:hover:bg-white/10 transition-colors ${getTextColor()}`} >
77-77: Make TODO actionableReplace vague TODO with an issue link or clear scope (what/why/when). I can open a ticket if you share the tracker.
13-20: Optional: avoid React.FC to keep surface minimalConsider a plain function component to avoid implicit children prop.
-export const Snackbar: React.FC<SnackbarProps> = ({ +export const Snackbar = ({ message, type = 'info', isVisible, onClose, duration = 5000, position = 'top-center' -}) => { +}: SnackbarProps) => {
93-93: Optional: centralize z-indexz-[9999] can conflict with modals/tooltips. Consider a design token (e.g., z-toast) applied consistently.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/ui/Snackbar.tsx(1 hunks)
🔇 Additional comments (2)
frontend/src/components/ui/Snackbar.tsx (2)
23-35: Deduplicate close logic and make it idempotent to avoid double onClose callsCurrent close logic is duplicated and can call onClose twice (e.g., click near auto-timeout, or very short duration). Extract a single idempotent handleClose, clear timers, and reuse it in both paths.
Apply these diffs:
@@ - useEffect(() => { - if (isVisible) { - setIsShowing(true) - const timer = setTimeout(() => { - setIsShowing(false) - setTimeout(onClose, 300) // Wait for exit animation - }, duration) - - return () => clearTimeout(timer) - } else { - setIsShowing(false) - } - }, [isVisible, duration, onClose]) + useEffect(() => { + if (isVisible) { + closedRef.current = false + setIsShowing(true) + timerRef.current = setTimeout(handleClose, duration) + return () => { + if (timerRef.current) { + clearTimeout(timerRef.current) + timerRef.current = null + } + } + } else { + setIsShowing(false) + } + }, [isVisible, duration, handleClose]) @@ - <button - onClick={() => { - setIsShowing(false) - setTimeout(onClose, 300) - }} + <button + onClick={handleClose} + aria-label="Close notification" + title="Close"Add the following code outside the selected ranges:
// 1) Update imports (Line 1) import React, { useEffect, useState, useRef, useCallback } from 'react' // 2) Add a constant (after Line 3) const EXIT_ANIMATION_MS = 300 // 3) Add refs and a single close handler (after Line 21, inside the component) const timerRef = useRef<ReturnType<typeof setTimeout> | null>(null) const closedRef = useRef(false) const handleClose = useCallback(() => { if (closedRef.current) return closedRef.current = true setIsShowing(false) if (timerRef.current) { clearTimeout(timerRef.current) timerRef.current = null } setTimeout(onClose, EXIT_ANIMATION_MS) // Wait for exit animation }, [onClose])Also applies to: 104-110
39-76: LGTM: clear, type-driven stylingIcon and color variants are straightforward and readable. Good dark-mode coverage.
3 participants
Description
Testing
Additional Notes
Summary by CodeRabbit
New Features
Refactor
Style