fix: sync node.imgs for legacy context menu in Vue Nodes mode#8143
Draft
christian-byrne wants to merge 2 commits intomainfrom
Draft
fix: sync node.imgs for legacy context menu in Vue Nodes mode#8143christian-byrne wants to merge 2 commits intomainfrom
christian-byrne wants to merge 2 commits intomainfrom
Conversation
Add syncLegacyNodeImgs store method to sync loaded image elements to node.imgs for backwards compatibility with legacy systems (Copy Image, Open Image, Save Image, Open in Mask Editor). - Only runs when vueNodesMode is enabled - Reuses already-loaded img element from Vue component (no duplicate loading) - Store owns the sync logic, component just hands off the element - Simplify mask editor handling to call composable directly Fixes missing context menu options on SaveImage vue node. Amp-Thread-ID: https://ampcode.com/threads/T-019bba3e-0ad8-754a-bd50-5cf17165d5a6 Co-authored-by: Amp <amp@ampcode.com>
dosubot
bot
added
the
size:L
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes refactor image preview handling by introducing a new syncLegacyNodeImgs function for backward-compatibility with legacy Vue nodes, replace commandStore with useMaskEditor hook for mask editor operations, and add corresponding test coverage for the new functionality. Changes
Sequence DiagramsequenceDiagram
participant ImagePreview as ImagePreview.vue
participant Handler as Event Handler
participant Store as nodeOutputStore
participant Graph as LiteGraph
participant Mask as useMaskEditor
rect rgba(200, 150, 255, 0.5)
Note over ImagePreview,Graph: Image Load Flow with Legacy Sync
ImagePreview->>ImagePreview: Image element loads
ImagePreview->>Handler: handleImageLoad(nodeId)
Handler->>Store: syncLegacyNodeImgs(nodeId, element)
alt vueNodesMode enabled
Store->>Graph: rootGraph.getNodeById(nodeId)
Graph-->>Store: Node object
Store->>Store: node.imgs = [element]<br/>node.imageIndex = activeIndex
end
end
rect rgba(100, 200, 255, 0.5)
Note over ImagePreview,Mask: Mask Editor Opening Flow
ImagePreview->>ImagePreview: User clicks edit mask
ImagePreview->>Handler: handleEditMask()
Handler->>Mask: useMaskEditor().openMaskEditor(node)
Mask->>Mask: Initialize mask editor with node
end
Possibly related PRs
Suggested reviewers
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 Build Status✅ Build completed successfully! ⏰ Completed at: 01/20/2026, 11:08:51 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
🎭 Playwright Tests: ❌ FailedResults: 505 passed, 1 failed, 1 flaky, 8 skipped (Total: 515) ❌ Failed Tests📊 Browser Reports
|
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.4 kB (baseline 22.4 kB) • ⚪ 0 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 1.02 MB (baseline 1.02 MB) • 🟢 -219 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 80.7 kB (baseline 80.7 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 9 added / 9 removed Panels & Settings — 430 kB (baseline 430 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 8 added / 8 removed User & Accounts — 3.94 kB (baseline 3.94 kB) • ⚪ 0 BAuthentication, profile, and account management bundles
Status: 3 added / 3 removed Editors & Dialogs — 2.8 kB (baseline 2.8 kB) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 2 added / 2 removed UI Components — 32.8 kB (baseline 32.8 kB) • ⚪ 0 BReusable component library chunks
Status: 5 added / 5 removed Data & Services — 3.04 MB (baseline 3.04 MB) • 🔴 +294 BStores, services, APIs, and repositories
Status: 7 added / 7 removed Utilities & Hooks — 18.7 kB (baseline 18.7 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Status: 4 added / 4 removed Vendor & Third-Party — 10.4 MB (baseline 10.4 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 6.25 MB (baseline 6.25 MB) • ⚪ 0 BBundles that do not match a named category
Status: 25 added / 25 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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/stores/imagePreviewStore.test.ts (1)
26-31: Prefer Partial for mock typing.The current cast hides incompleteness; using
Partial<LGraphNode>makes the intent explicit and aligns with test typing conventions.♻️ Suggested adjustment
-const createMockNode = (overrides: Record<string, unknown> = {}): LGraphNode => - ({ - id: 1, - type: 'TestNode', - ...overrides - }) as LGraphNode +const createMockNode = (overrides: Partial<LGraphNode> = {}): LGraphNode => + ({ + id: 1, + type: 'TestNode', + ...overrides + }) as Partial<LGraphNode> as LGraphNodeBased on learnings, prefer
Partial<Interface>casts for incomplete mock objects.
🤖 Fix all issues with AI agents
In `@src/renderer/extensions/vueNodes/components/ImagePreview.vue`:
- Around line 214-218: Hoist the call to the composable out of the event handler
by invoking useMaskEditor() once in the component setup and storing its return
(e.g. const maskEditor = useMaskEditor()) so handleEditMask uses
maskEditor.openMaskEditor(node) instead of calling useMaskEditor() inside the
handler; update the setup block to create maskEditor and ensure handleEditMask
references that variable to avoid missing component instance/injection timing
issues.
Comment on lines
214
to
+218
| const handleEditMask = () => { | ||
| setupNodeForMaskEditor() | ||
| void commandStore.execute('Comfy.MaskEditor.OpenMaskEditor') | ||
| if (!props.nodeId) return | ||
| const node = app.rootGraph?.getNodeById(Number(props.nodeId)) | ||
| if (!node) return | ||
| useMaskEditor().openMaskEditor(node) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Hoist useMaskEditor() to setup to avoid instance/timing pitfalls.
Calling composables inside event handlers can miss the active component instance if the composable uses injection. Safer to instantiate once in setup and reuse.
♻️ Suggested refactor
-const nodeOutputStore = useNodeOutputStore()
+const nodeOutputStore = useNodeOutputStore()
+const maskEditor = useMaskEditor()
...
- useMaskEditor().openMaskEditor(node)
+ maskEditor.openMaskEditor(node)🤖 Prompt for AI Agents
In `@src/renderer/extensions/vueNodes/components/ImagePreview.vue` around lines
214 - 218, Hoist the call to the composable out of the event handler by invoking
useMaskEditor() once in the component setup and storing its return (e.g. const
maskEditor = useMaskEditor()) so handleEditMask uses
maskEditor.openMaskEditor(node) instead of calling useMaskEditor() inside the
handler; update the setup block to create maskEditor and ensure handleEditMask
references that variable to avoid missing component instance/injection timing
issues.
christian-byrne
added a commit
that referenced
this pull request
Jan 31, 2026
## Summary Fix for COM-14110: Preview image does not display new outputs in vue-nodes. ## Problem The merge logic in `setOutputsByLocatorId` updated `app.nodeOutputs` but returned early without updating the reactive `nodeOutputs.value` ref. This caused Vue components to never receive merged output updates because only the non-reactive `app.nodeOutputs` was being updated. ## Solution Added `nodeOutputs.value[nodeLocatorId] = existingOutput` after the merge loop, before the return statement. ## Testing - Added 2 unit tests covering the merge behavior - All 4076 existing unit tests pass - Typechecks pass - Lint passes ## Notes - Related open PRs touching same files: #8143, #8366 - potential minor conflicts possible Fixes COM-14110 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8479-fix-update-reactive-ref-after-merge-in-imagePreviewStore-2f86d73d365081f1a145fa5a9782515f) by [Unito](https://www.unito.io) Co-authored-by: Amp <amp@ampcode.com>
github-actions bot
pushed a commit
that referenced
this pull request
Jan 31, 2026
## Summary Fix for COM-14110: Preview image does not display new outputs in vue-nodes. ## Problem The merge logic in `setOutputsByLocatorId` updated `app.nodeOutputs` but returned early without updating the reactive `nodeOutputs.value` ref. This caused Vue components to never receive merged output updates because only the non-reactive `app.nodeOutputs` was being updated. ## Solution Added `nodeOutputs.value[nodeLocatorId] = existingOutput` after the merge loop, before the return statement. ## Testing - Added 2 unit tests covering the merge behavior - All 4076 existing unit tests pass - Typechecks pass - Lint passes ## Notes - Related open PRs touching same files: #8143, #8366 - potential minor conflicts possible Fixes COM-14110 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8479-fix-update-reactive-ref-after-merge-in-imagePreviewStore-2f86d73d365081f1a145fa5a9782515f) by [Unito](https://www.unito.io) Co-authored-by: Amp <amp@ampcode.com>
github-actions bot
pushed a commit
that referenced
this pull request
Jan 31, 2026
## Summary Fix for COM-14110: Preview image does not display new outputs in vue-nodes. ## Problem The merge logic in `setOutputsByLocatorId` updated `app.nodeOutputs` but returned early without updating the reactive `nodeOutputs.value` ref. This caused Vue components to never receive merged output updates because only the non-reactive `app.nodeOutputs` was being updated. ## Solution Added `nodeOutputs.value[nodeLocatorId] = existingOutput` after the merge loop, before the return statement. ## Testing - Added 2 unit tests covering the merge behavior - All 4076 existing unit tests pass - Typechecks pass - Lint passes ## Notes - Related open PRs touching same files: #8143, #8366 - potential minor conflicts possible Fixes COM-14110 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8479-fix-update-reactive-ref-after-merge-in-imagePreviewStore-2f86d73d365081f1a145fa5a9782515f) by [Unito](https://www.unito.io) Co-authored-by: Amp <amp@ampcode.com>
DrJKL
pushed a commit
that referenced
this pull request
Jan 31, 2026
## Summary Fix for COM-14110: Preview image does not display new outputs in vue-nodes. ## Problem The merge logic in `setOutputsByLocatorId` updated `app.nodeOutputs` but returned early without updating the reactive `nodeOutputs.value` ref. This caused Vue components to never receive merged output updates because only the non-reactive `app.nodeOutputs` was being updated. ## Solution Added `nodeOutputs.value[nodeLocatorId] = existingOutput` after the merge loop, before the return statement. ## Testing - Added 2 unit tests covering the merge behavior - All 4076 existing unit tests pass - Typechecks pass - Lint passes ## Notes - Related open PRs touching same files: #8143, #8366 - potential minor conflicts possible Fixes COM-14110 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8479-fix-update-reactive-ref-after-merge-in-imagePreviewStore-2f86d73d365081f1a145fa5a9782515f) by [Unito](https://www.unito.io) Co-authored-by: Amp <amp@ampcode.com>
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
1 participant
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
Fixes missing "Copy Image", "Open Image", "Save Image", and "Open in Mask Editor" context menu options on SaveImage nodes when Vue Nodes mode is enabled.
Changes
syncLegacyNodeImgsstore method to sync loaded image elements tonode.imgsTechnical Details
vueNodesModeis enabled (no impact on legacy mode)<img>element from Vue (no duplicate network requests)Supersedes #7416
┆Issue is synchronized with this Notion page by Unito