App mode - builder toolbar save - 7#9030
Conversation
📝 WalkthroughWalkthroughThese changes introduce a complete builder save-as workflow, including reusable dialog components, a composition hook to orchestrate the save flow, service updates to handle filename and mode options, and corresponding localization strings for the user interface. Changes
Sequence DiagramsequenceDiagram
participant User
participant GraphView
participant useBuilderSave as useBuilderSave Hook
participant dialogStore
participant appModeStore
participant BuilderDialog as Save Dialog Components
participant workflowService
User->>GraphView: Trigger save action
GraphView->>appModeStore: setBuilderSaving(true)
appModeStore-->>useBuilderSave: isBuilderSaving = true (watched)
alt Temporary workflow
useBuilderSave->>useBuilderSave: showSaveDialog()
useBuilderSave->>dialogStore: Show BuilderSaveDialogContent
dialogStore->>BuilderDialog: Render save dialog
BuilderDialog-->>User: Display filename & type options
User->>BuilderDialog: Enter filename & select type
BuilderDialog->>useBuilderSave: onSave(filename, openAsApp)
useBuilderSave->>workflowService: saveWorkflowAs(workflow, {filename, openAsApp})
workflowService->>workflowService: Save file & update state
workflowService-->>useBuilderSave: true (success)
useBuilderSave->>dialogStore: Close save dialog
useBuilderSave->>useBuilderSave: showSuccessDialog()
useBuilderSave->>dialogStore: Show BuilderSaveSuccessDialogContent
dialogStore->>BuilderDialog: Render success dialog
BuilderDialog-->>User: Show success message
User->>BuilderDialog: Click close or view app
BuilderDialog->>useBuilderSave: onClose() or onViewApp()
useBuilderSave->>appModeStore: resetSaving()
else Permanent workflow
useBuilderSave->>workflowService: saveWorkflow(workflow)
workflowService-->>useBuilderSave: Success
useBuilderSave->>useBuilderSave: showSuccessDialog()
useBuilderSave->>appModeStore: resetSaving()
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
🎨 Storybook: ✅ Built — View Storybook |
🎭 Playwright: ✅ 531 passed, 0 failed · 2 flaky📊 Browser Reports
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/builder/BuilderDialog.vue`:
- Around line 13-19: Replace the raw <button> used for closing in
BuilderDialog.vue with the shared Button component
(src/components/ui/button/Button.vue): import and register Button in the
component, render <Button variant="link"> instead of the raw element, pass the
existing :aria-label and the `@click`="$emit('close')" through to Button, keep the
current class overrides (the long class string and the <i class="pi pi-times
size-4" /> child) so styling/behavior remain identical, and ensure the Button
component receives any necessary props or listeners to preserve focus/hover/ring
behavior.
In `@src/components/builder/BuilderSaveDialogContent.vue`:
- Around line 28-58: The save-type options are rendered as raw <button>
elements; replace them with the shared Button component
(src/components/ui/button/Button.vue) so they follow the design system while
preserving behavior—use the Button component where the loop over saveTypeOptions
is rendered, keep the same props/state bindings (openAsApp, option.value,
option.icon, option.title, option.subtitle, itemClasses), preserve ARIA
roles/aria-checked and the click handler by setting `@click` to set openAsApp =
option.value, and apply an appropriate variant (e.g., variant="link" or other
existing variant) and classes to reproduce the layout (including the icon
container, text spans, and conditional check icon) so visual/ARIA behavior
remains unchanged.
In `@src/components/builder/useBuilderSave.ts`:
- Around line 53-65: The save dialog close path leaves the isBuilderSaving flag
true; ensure resetSaving (or the setter that clears isBuilderSaving) is invoked
whenever the dialog is closed or cancelled. Update showSaveDialog to pass
dialogComponentProps.onClose=resetSaving (already present) and confirm
closeSaveDialog and any alternate dialog opening path (the similar block around
lines 107-118) call resetSaving when closing or onCancel; specifically modify
closeSaveDialog (and the other dialog close handler) to call resetSaving so the
isBuilderSaving flag is cleared on both Save completion and when the user
cancels/closes the dialog.
8c87a21 to
7e2c5d9
Compare
36986c0 to
9857cc3
Compare
| autofocus | ||
| type="text" | ||
| class="flex h-10 min-h-8 items-center self-stretch rounded-lg border-none bg-secondary-background pl-4 text-sm text-base-foreground focus:outline-none" | ||
| @keydown.enter="filename.trim() && onSave(filename.trim(), openAsApp)" |
| "saveSuccessAppMessage": "'{name}' has been saved. It will open in App Mode by default from now on.", | ||
| "saveSuccessAppPrompt": "Would you like to view it now?", | ||
| "saveSuccessGraphMessage": "'{name}' has been saved. It will open as a node graph by default.", | ||
| "viewApp": "View app" |
There was a problem hiding this comment.
Potential repeat of prior comment on group app localizations.
In this case, I think all the current location is appropriate, so if it comes up, we can just ctrl+f
28080b1 to
bf04561
Compare
The base branch was changed.
9aa54a9 to
2ccef4a
Compare
📦 Bundle: 4.38 MB gzip 🔴 +1.6 kBDetailsSummary
Category Glance App Entry Points — 17.9 kB (baseline 17.9 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 962 kB (baseline 952 kB) • 🔴 +10.4 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.8 kB (baseline 68.8 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 436 kB (baseline 436 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 10 added / 10 removed User & Accounts — 16 kB (baseline 16 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 5 added / 5 removed Editors & Dialogs — 738 B (baseline 738 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 47 kB (baseline 47 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 2.52 MB (baseline 2.52 MB) • 🔴 +241 BStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 58.3 kB (baseline 58.3 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 13 added / 13 removed Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.62 MB (baseline 7.62 MB) • 🔴 +578 BBundles that do not match a named category
Status: 52 added / 52 removed |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/platform/workflow/core/services/workflowService.ts (1)
96-127:⚠️ Potential issue | 🟠 MajorRoll back
linearModewhen the save is aborted.
openAsAppmutatesapp.rootGraph.extra.linearModebefore overwrite confirmation. If the user cancels or delete fails, the workflow state stays modified even though the save didn’t happen. Capture and restore the previous value on early-return paths.🛠️ Suggested rollback pattern
- if (options.openAsApp !== undefined) { - app.rootGraph.extra ??= {} - app.rootGraph.extra.linearMode = options.openAsApp - workflow.changeTracker?.checkState() - } + const shouldSetLinearMode = options.openAsApp !== undefined + const previousLinearMode = app.rootGraph.extra?.linearMode + const hadExtra = !!app.rootGraph.extra + const restoreLinearMode = () => { + if (!shouldSetLinearMode) return + if (!hadExtra) { + delete app.rootGraph.extra + } else { + app.rootGraph.extra!.linearMode = previousLinearMode + } + workflow.changeTracker?.checkState() + } + if (shouldSetLinearMode) { + app.rootGraph.extra ??= {} + app.rootGraph.extra.linearMode = options.openAsApp + workflow.changeTracker?.checkState() + } @@ - if (res !== true) return false + if (res !== true) { + restoreLinearMode() + return false + } @@ - if (!deleted) return false + if (!deleted) { + restoreLinearMode() + return false + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/platform/workflow/core/services/workflowService.ts` around lines 96 - 127, In saveWorkflowAs capture the previous linearMode before mutating it (e.g., const prevLinearMode = app.rootGraph.extra?.linearMode) when options.openAsApp !== undefined, then set app.rootGraph.extra.linearMode = options.openAsApp; on any early-return paths after the mutation (specifically when the overwrite confirm is canceled: res !== true, and when deleteWorkflow(existingWorkflow, true) returns false) restore app.rootGraph.extra.linearMode = prevLinearMode (or delete the property if prevLinearMode was undefined) so the app state is rolled back when the save is aborted; keep the new value if the save proceeds and completes (saveWorkflow or successful delete+save).
🧹 Nitpick comments (1)
src/components/builder/BuilderSaveSuccessDialogContent.vue (1)
45-50: Destructure props to match the project’s Vue prop style.Use the TS-style destructuring pattern so props follow the repository convention.
As per coding guidelines: "Define props using TypeScript style: `const { prop1, prop2 = default } = defineProps<{ prop1: Type; prop2?: Type }>()`."♻️ Suggested update
-defineProps<{ - workflowName: string - savedAsApp: boolean - onViewApp?: () => void - onClose: () => void -}>() +const { workflowName, savedAsApp, onViewApp, onClose } = defineProps<{ + workflowName: string + savedAsApp: boolean + onViewApp?: () => void + onClose: () => void +}>()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/builder/BuilderSaveSuccessDialogContent.vue` around lines 45 - 50, The props are currently declared via defineProps<{...}>(); refactor to the project's TS-style destructuring pattern: replace the raw defineProps call with a const destructure (e.g., const { workflowName, savedAsApp = false, onViewApp, onClose } = defineProps<{ workflowName: string; savedAsApp?: boolean; onViewApp?: () => void; onClose: () => void }>() ) so the component uses destructured variables (workflowName, savedAsApp, onViewApp, onClose) and provides a default for savedAsApp to match repo conventions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/builder/BuilderSaveDialogContent.vue`:
- Around line 100-112: The saveTypeOptions array is created at setup time and
freezes the i18n strings; change it to a computed property so it recomputes when
locale changes: import computed from 'vue' (if not already), replace the
saveTypeOptions constant with a computed(() => [...]) that constructs the same
objects using t(...) so components referencing saveTypeOptions will see reactive
updates, and ensure any existing usages (e.g., in the template or other setup
code) keep the same name.
---
Outside diff comments:
In `@src/platform/workflow/core/services/workflowService.ts`:
- Around line 96-127: In saveWorkflowAs capture the previous linearMode before
mutating it (e.g., const prevLinearMode = app.rootGraph.extra?.linearMode) when
options.openAsApp !== undefined, then set app.rootGraph.extra.linearMode =
options.openAsApp; on any early-return paths after the mutation (specifically
when the overwrite confirm is canceled: res !== true, and when
deleteWorkflow(existingWorkflow, true) returns false) restore
app.rootGraph.extra.linearMode = prevLinearMode (or delete the property if
prevLinearMode was undefined) so the app state is rolled back when the save is
aborted; keep the new value if the save proceeds and completes (saveWorkflow or
successful delete+save).
---
Nitpick comments:
In `@src/components/builder/BuilderSaveSuccessDialogContent.vue`:
- Around line 45-50: The props are currently declared via defineProps<{...}>();
refactor to the project's TS-style destructuring pattern: replace the raw
defineProps call with a const destructure (e.g., const { workflowName,
savedAsApp = false, onViewApp, onClose } = defineProps<{ workflowName: string;
savedAsApp?: boolean; onViewApp?: () => void; onClose: () => void }>() ) so the
component uses destructured variables (workflowName, savedAsApp, onViewApp,
onClose) and provides a default for savedAsApp to match repo conventions.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/builder/BuilderDialog.vuesrc/components/builder/BuilderSaveDialogContent.vuesrc/components/builder/BuilderSaveSuccessDialogContent.vuesrc/components/builder/useBuilderSave.tssrc/locales/en/main.jsonsrc/platform/workflow/core/services/workflowService.tssrc/views/GraphView.vue
| const saveTypeOptions = [ | ||
| { | ||
| value: true, | ||
| icon: 'icon-[lucide--app-window]', | ||
| title: t('builderToolbar.app'), | ||
| subtitle: t('builderToolbar.appDescription') | ||
| }, | ||
| { | ||
| value: false, | ||
| icon: 'icon-[comfy--workflow]', | ||
| title: t('builderToolbar.nodeGraph'), | ||
| subtitle: t('builderToolbar.nodeGraphDescription') | ||
| } |
There was a problem hiding this comment.
Make saveTypeOptions computed so locale changes stay reactive.
Right now the labels are frozen at setup time. Wrapping the list in computed ensures the i18n strings update if the locale changes while the dialog is open.
🔁 Suggested fix
-import { ref, useId } from 'vue'
+import { computed, ref, useId } from 'vue'
@@
-const saveTypeOptions = [
- {
- value: true,
- icon: 'icon-[lucide--app-window]',
- title: t('builderToolbar.app'),
- subtitle: t('builderToolbar.appDescription')
- },
- {
- value: false,
- icon: 'icon-[comfy--workflow]',
- title: t('builderToolbar.nodeGraph'),
- subtitle: t('builderToolbar.nodeGraphDescription')
- }
-]
+const saveTypeOptions = computed(() => [
+ {
+ value: true,
+ icon: 'icon-[lucide--app-window]',
+ title: t('builderToolbar.app'),
+ subtitle: t('builderToolbar.appDescription')
+ },
+ {
+ value: false,
+ icon: 'icon-[comfy--workflow]',
+ title: t('builderToolbar.nodeGraph'),
+ subtitle: t('builderToolbar.nodeGraphDescription')
+ }
+])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/builder/BuilderSaveDialogContent.vue` around lines 100 - 112,
The saveTypeOptions array is created at setup time and freezes the i18n strings;
change it to a computed property so it recomputes when locale changes: import
computed from 'vue' (if not already), replace the saveTypeOptions constant with
a computed(() => [...]) that constructs the same objects using t(...) so
components referencing saveTypeOptions will see reactive updates, and ensure any
existing usages (e.g., in the template or other setup code) keep the same name.
Summary
Implements save flow for the builder toolbar.
The todo will be done in a future PR once the serailized format is finalized
Screenshots (if applicable)
app.mode.save.mp4
┆Issue is synchronized with this Notion page by Unito