Merged
Conversation
Contributor
📝 WalkthroughWalkthroughAdds two builder UI components (popover menu and bottom exit button), moves builder controls into GraphView when in builder mode, and removes the confirmation prompt from appModeStore.exitBuilder. New i18n keys for the menu were added. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Menu as BuilderMenu.vue
participant ExitBtn as BuilderExitButton.vue
participant Dialog as dialogStore
participant Save as useBuilderSave
participant AppMode as appModeStore
User->>Menu: Click "Save App"
Menu->>Save: setSaving(true)
Save-->>Menu: saving state set
Menu-->>User: close popover
User->>Menu: Click "Exit App Builder"
Menu->>Dialog: query open dialogs?
Dialog-->>Menu: no dialogs
Menu->>AppMode: call exitBuilder()
AppMode-->>Menu: mode -> 'graph'
Menu-->>User: builder UI hidden
User->>ExitBtn: Click "Exit" / Press Escape
ExitBtn->>Dialog: query open dialogs?
Dialog-->>ExitBtn: no dialogs
ExitBtn->>AppMode: call exitBuilder()
AppMode-->>ExitBtn: mode -> 'graph'
ExitBtn-->>User: builder UI hidden
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
🎭 Playwright: ✅ 547 passed, 0 failed · 5 flaky📊 Browser Reports
|
🎨 Storybook: ✅ Built — View Storybook |
pythongosssss
force-pushed
the
pysssss/appmode-updates
branch
from
2214f48 to
46a7108
Compare
- add builder exit button to bottom of canvas - add builder menu with save & exit in top left
pythongosssss
force-pushed
the
pysssss/appmode-exit-updates
branch
from
ee5315e to
e85d92e
Compare
📦 Bundle: 4.46 MB gzip 🔴 +675 BDetailsSummary
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 — 1.03 MB (baseline 1.02 MB) • 🔴 +3.57 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 72.1 kB (baseline 72.1 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 435 kB (baseline 435 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 — 736 B (baseline 736 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.55 MB (baseline 2.55 MB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 55.5 kB (baseline 55.5 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 12 added / 12 removed Vendor & Third-Party — 8.84 MB (baseline 8.84 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 7.86 MB (baseline 7.86 MB) • 🔴 +84 BBundles that do not match a named category
Status: 56 added / 56 removed |
⚡ Performance Report
Raw data{
"timestamp": "2026-02-27T21:43:10.626Z",
"gitSha": "d5e3f1442ffe671cbfb0bed5e0a556bf1178ef18",
"branch": "pysssss/appmode-exit-updates",
"measurements": [
{
"name": "canvas-idle",
"durationMs": 2031.090000000006,
"styleRecalcs": 124,
"styleRecalcDurationMs": 24.587000000000003,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 445.413,
"heapDeltaBytes": -2047444
},
{
"name": "canvas-mouse-sweep",
"durationMs": 1844.5690000000354,
"styleRecalcs": 169,
"styleRecalcDurationMs": 51.761,
"layouts": 12,
"layoutDurationMs": 3.878,
"taskDurationMs": 801.431,
"heapDeltaBytes": -3721392
},
{
"name": "dom-widget-clipping",
"durationMs": 583.5329999999885,
"styleRecalcs": 41,
"styleRecalcDurationMs": 13.785,
"layouts": 0,
"layoutDurationMs": 0,
"taskDurationMs": 360.62300000000005,
"heapDeltaBytes": 7949304
}
]
} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/components/builder/BuilderMenu.vue (2)
4-19: Remove redundantaria-labelfrom trigger button.Per repository guidelines, buttons with visible text content (the span with "App Builder") should not have
aria-labelas the visible text already provides the accessible name.♻️ Suggested fix
<button :class=" cn( 'absolute left-4 top-[calc(var(--workflow-tabs-height)+16px)] z-[1000] inline-flex h-10 cursor-pointer items-center gap-2.5 rounded-lg py-2 pr-2 pl-3 shadow-interface transition-colors border-none', 'bg-secondary-background hover:bg-secondary-background-hover', 'data-[state=open]:bg-secondary-background-hover' ) " - :aria-label="t('linearMode.appModeToolbar.appBuilder')" >Based on learnings: "In the ComfyUI_frontend repository, for Vue components, do not add aria-label to buttons that have visible text content."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/builder/BuilderMenu.vue` around lines 4 - 19, Remove the redundant aria-label attribute from the button element in BuilderMenu.vue (the button that currently has :aria-label="t('linearMode.appModeToolbar.appBuilder')"); the visible span that renders {{ t('linearMode.appModeToolbar.appBuilder') }} already provides the accessible name, so delete the :aria-label attribute on that trigger button and ensure no other ARIA duplication remains for the same element.
29-51: Consider using the sharedButtoncomponent for menu items.The menu item buttons use raw
<button>elements. For consistency with the design system, consider using the sharedButtoncomponent with appropriate variants (e.g.,variant="ghost").Based on learnings: "In the ComfyUI_frontend Vue codebase, replace raw HTML elements with the shared Button component."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/builder/BuilderMenu.vue` around lines 29 - 51, Replace the raw <button> elements in BuilderMenu.vue with the shared Button component to match the design system: for the save button, use the Button component with variant="ghost" (or equivalent) and bind :disabled="!hasOutputs", preserve the dynamic classes/hover behavior currently built with cn and still call `@click`="onSave" and show the same icon and label; for the exit button, use Button variant="ghost" with `@click`="onExitBuilder" and the same icon/label and hover styling. Ensure you import the shared Button component and register it in the component, and keep references to hasOutputs, onSave, onExitBuilder, and the cn usage (or move conditional styles into Button props) so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/builder/BuilderMenu.vue`:
- Around line 4-19: Remove the redundant aria-label attribute from the button
element in BuilderMenu.vue (the button that currently has
:aria-label="t('linearMode.appModeToolbar.appBuilder')"); the visible span that
renders {{ t('linearMode.appModeToolbar.appBuilder') }} already provides the
accessible name, so delete the :aria-label attribute on that trigger button and
ensure no other ARIA duplication remains for the same element.
- Around line 29-51: Replace the raw <button> elements in BuilderMenu.vue with
the shared Button component to match the design system: for the save button, use
the Button component with variant="ghost" (or equivalent) and bind
:disabled="!hasOutputs", preserve the dynamic classes/hover behavior currently
built with cn and still call `@click`="onSave" and show the same icon and label;
for the exit button, use Button variant="ghost" with `@click`="onExitBuilder" and
the same icon/label and hover styling. Ensure you import the shared Button
component and register it in the component, and keep references to hasOutputs,
onSave, onExitBuilder, and the cn usage (or move conditional styles into Button
props) so behavior remains identical.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/builder/AppBuilder.vuesrc/components/builder/BuilderExitButton.vuesrc/components/builder/BuilderMenu.vuesrc/locales/en/main.jsonsrc/views/GraphView.vue
💤 Files with no reviewable changes (1)
- src/components/builder/AppBuilder.vue
dosubot
bot
added
the
size:L
AustinMroz
previously approved these changes
Feb 27, 2026
| @@ -0,0 +1,43 @@ | |||
| <template> | |||
| <div | |||
| class="fixed bottom-4 left-1/2 z-[1000] flex -translate-x-1/2 items-center rounded-2xl border border-border-default bg-base-background p-2 shadow-interface" | |||
Collaborator
There was a problem hiding this comment.
Suggested change
| class="fixed bottom-4 left-1/2 z-[1000] flex -translate-x-1/2 items-center rounded-2xl border border-border-default bg-base-background p-2 shadow-interface" | |
| class="fixed bottom-4 left-1/2 z-1000 flex -translate-x-1/2 items-center rounded-2xl border border-border-default bg-base-background p-2 shadow-interface" |
| <button | ||
| :class=" | ||
| cn( | ||
| 'absolute left-4 top-[calc(var(--workflow-tabs-height)+16px)] z-[1000] inline-flex h-10 cursor-pointer items-center gap-2.5 rounded-lg py-2 pr-2 pl-3 shadow-interface transition-colors border-none', |
Collaborator
There was a problem hiding this comment.
Suggested change
| 'absolute left-4 top-[calc(var(--workflow-tabs-height)+16px)] z-[1000] inline-flex h-10 cursor-pointer items-center gap-2.5 rounded-lg py-2 pr-2 pl-3 shadow-interface transition-colors border-none', | |
| 'absolute left-4 top-[calc(var(--workflow-tabs-height)+16px)] z-1000 inline-flex h-10 cursor-pointer items-center gap-2.5 rounded-lg py-2 pr-2 pl-3 shadow-interface transition-colors border-none', |
| ) | ||
| " | ||
| :disabled="!hasOutputs" | ||
| @click="onSave" |
Collaborator
There was a problem hiding this comment.
Super nit, but this doesn't trigger a close of the popover.
Was there a problem with using the @/components/ui/Popover.vue component directly?
Member
Author
There was a problem hiding this comment.
No good reason for it, fixed
With the move to always storing builder selection changes, the confirmation dialogue on builder exit is no longer needed ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9286-Remove-confirmation-from-builder-exit-3146d73d3650815aa1c1dcfb491cc963) by [Unito](https://www.unito.io)
- use popover component - use proper tailwind class
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/BuilderExitButton.vue`:
- Around line 25-37: The Escape handler registered via useEventListener
currently calls onExitBuilder whenever isBuilderMode.value is true and
dialogStack is empty; change it to first check document.activeElement and skip
handling if focus is inside an editable element (e.g., HTMLInputElement,
HTMLTextAreaElement, or any element with contentEditable === "true" or true) or
if the active element is inside a rich-text/editor component (check
element.closest for common editor selectors if needed). Update the listener
logic in the same block where useEventListener is used so the conditions include
this focus guard before calling e.preventDefault(), e.stopPropagation(), and
onExitBuilder; keep the existing checks for isBuilderMode.value and
dialogStore.dialogStack.length.
In `@src/components/builder/BuilderMenu.vue`:
- Around line 4-17: In the BuilderMenu component remove the redundant
:aria-label binding on the visible-text button (the button rendering the hammer
icon and the span with {{ t('linearMode.appModeToolbar.appBuilder') }}) so the
accessible name comes from the visible label instead of a duplicated aria
attribute; simply delete the
:aria-label="t('linearMode.appModeToolbar.appBuilder')" from that button (leave
aria-label usage only for icon-only controls elsewhere).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/components/builder/AppBuilder.vuesrc/components/builder/BuilderExitButton.vuesrc/components/builder/BuilderMenu.vuesrc/components/builder/BuilderToolbar.vuesrc/locales/en/main.json
💤 Files with no reviewable changes (1)
- src/components/builder/AppBuilder.vue
🚧 Files skipped from review as they are similar to previous changes (1)
- src/locales/en/main.json
Comment on lines
+25
to
+37
| useEventListener(window, 'keydown', (e: KeyboardEvent) => { | ||
| if ( | ||
| e.key === 'Escape' && | ||
| !e.ctrlKey && | ||
| !e.altKey && | ||
| !e.metaKey && | ||
| dialogStore.dialogStack.length === 0 && | ||
| isBuilderMode.value | ||
| ) { | ||
| e.preventDefault() | ||
| e.stopPropagation() | ||
| onExitBuilder() | ||
| } |
Contributor
There was a problem hiding this comment.
Guard Escape handling while editing text inputs
The global Escape handler exits builder mode even when focus is in editable fields. With direct exit behavior, this can cause accidental mode exits during text entry.
Suggested fix
useEventListener(window, 'keydown', (e: KeyboardEvent) => {
+ const target = e.target as HTMLElement | null
+ const isEditable =
+ target instanceof HTMLInputElement ||
+ target instanceof HTMLTextAreaElement ||
+ target?.isContentEditable === true
+ if (isEditable) return
+
if (
e.key === 'Escape' &&
!e.ctrlKey &&
!e.altKey &&
!e.metaKey &&
+ !e.shiftKey &&
dialogStore.dialogStack.length === 0 &&
isBuilderMode.value
) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| useEventListener(window, 'keydown', (e: KeyboardEvent) => { | |
| if ( | |
| e.key === 'Escape' && | |
| !e.ctrlKey && | |
| !e.altKey && | |
| !e.metaKey && | |
| dialogStore.dialogStack.length === 0 && | |
| isBuilderMode.value | |
| ) { | |
| e.preventDefault() | |
| e.stopPropagation() | |
| onExitBuilder() | |
| } | |
| useEventListener(window, 'keydown', (e: KeyboardEvent) => { | |
| const target = e.target as HTMLElement | null | |
| const isEditable = | |
| target instanceof HTMLInputElement || | |
| target instanceof HTMLTextAreaElement || | |
| target?.isContentEditable === true | |
| if (isEditable) return | |
| if ( | |
| e.key === 'Escape' && | |
| !e.ctrlKey && | |
| !e.altKey && | |
| !e.metaKey && | |
| !e.shiftKey && | |
| dialogStore.dialogStack.length === 0 && | |
| isBuilderMode.value | |
| ) { | |
| e.preventDefault() | |
| e.stopPropagation() | |
| onExitBuilder() | |
| } | |
| }) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/builder/BuilderExitButton.vue` around lines 25 - 37, The
Escape handler registered via useEventListener currently calls onExitBuilder
whenever isBuilderMode.value is true and dialogStack is empty; change it to
first check document.activeElement and skip handling if focus is inside an
editable element (e.g., HTMLInputElement, HTMLTextAreaElement, or any element
with contentEditable === "true" or true) or if the active element is inside a
rich-text/editor component (check element.closest for common editor selectors if
needed). Update the listener logic in the same block where useEventListener is
used so the conditions include this focus guard before calling
e.preventDefault(), e.stopPropagation(), and onExitBuilder; keep the existing
checks for isBuilderMode.value and dialogStore.dialogStack.length.
AustinMroz
approved these changes
Feb 27, 2026
DrJKL
pushed a commit
that referenced
this pull request
Feb 28, 2026
## Summary - remove exit builder button from right panel - add builder exit button to bottom of canvas - add builder menu with save & exit in top left ## Screenshots (if applicable) <img width="1544" height="998" alt="image" src="https://github.com/user-attachments/assets/f5deadc5-2bf5-4729-b644-2b6a815b9975" /> ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9218-App-builder-exit-updates-3126d73d365081a0bf1adf92e1171060) by [Unito](https://www.unito.io)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Screenshots (if applicable)
┆Issue is synchronized with this Notion page by Unito