Skip to content

fix: sync node.imgs on image load for VueNodes context menu #7742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jtydhr88 wants to merge 2 commits into from
Closed

fix: sync node.imgs on image load for VueNodes context menu #7742

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d7123f
fix: sync node.imgs on image load for VueNodes context menu
jtydhr88 Dec 23, 2025
b0e7883
[automated] Update test expectations
invalid-email-address Dec 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified ...mobileBaseline.spec.ts-snapshots/mobile-settings-dialog-mobile-chrome-linux.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
15 changes: 12 additions & 3 deletions src/renderer/extensions/vueNodes/components/ImagePreview.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,8 @@ const handleImageLoad = (event: Event) => {
if (img.naturalWidth && img.naturalHeight) {
actualDimensions.value = `${img.naturalWidth} x ${img.naturalHeight}`
}

syncNodeImgs()
}

const handleImageError = () => {
Expand All @@ -207,14 +209,21 @@ const handleImageError = () => {
actualDimensions.value = null
}

// In vueNodes mode, we need to set them manually before opening the mask editor.
const setupNodeForMaskEditor = () => {
// Sync node.imgs so context menu can detect this as an image node
const syncNodeImgs = () => {
if (!props.nodeId || !currentImageEl.value) return
const node = app.rootGraph?.getNodeById(props.nodeId)
if (!node) return
node.imageIndex = currentIndex.value
node.imgs = [currentImageEl.value]
app.canvas?.select(node)
}
Comment on lines +212 to +219
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Well-structured extraction for reusability.

Good separation of concerns—syncNodeImgs updates node state without coupling to canvas selection. This makes the logic reusable in both image load and mask editor setup paths.

Optional: Return the node to avoid duplicate lookups

Currently, setupNodeForMaskEditor calls syncNodeImgs() and then separately calls getNodeById again. You could have syncNodeImgs return the node to eliminate the duplicate lookup:

 const syncNodeImgs = () => {
   if (!props.nodeId || !currentImageEl.value) return
   const node = app.rootGraph?.getNodeById(props.nodeId)
   if (!node) return
   node.imageIndex = currentIndex.value
   node.imgs = [currentImageEl.value]
+  return node
 }

Then update setupNodeForMaskEditor accordingly (see next comment).

📝 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
// Sync node.imgs so context menu can detect this as an image node
const syncNodeImgs = () => {
if (!props.nodeId || !currentImageEl.value) return
const node = app.rootGraph?.getNodeById(props.nodeId)
if (!node) return
node.imageIndex = currentIndex.value
node.imgs = [currentImageEl.value]
app.canvas?.select(node)
}
// Sync node.imgs so context menu can detect this as an image node
const syncNodeImgs = () => {
if (!props.nodeId || !currentImageEl.value) return
const node = app.rootGraph?.getNodeById(props.nodeId)
if (!node) return
node.imageIndex = currentIndex.value
node.imgs = [currentImageEl.value]
return node
}
🤖 Prompt for AI Agents 
In src/renderer/extensions/vueNodes/components/ImagePreview.vue around lines 212
to 219, update syncNodeImgs so it returns the node it finds (or null/undefined)
after syncing node.imageIndex and node.imgs, and then modify
setupNodeForMaskEditor to call syncNodeImgs and use the returned node instead of
calling app.rootGraph.getNodeById(props.nodeId) again; ensure you preserve the
existing early returns and types (return null/undefined when nothing found) and
update any caller logic to handle the returned value.


// In vueNodes mode, we need to set them manually before opening the mask editor.
const setupNodeForMaskEditor = () => {
syncNodeImgs()
if (!props.nodeId) return
const node = app.rootGraph?.getNodeById(props.nodeId)
if (node) app.canvas?.select(node)
}
Comment on lines +221 to 227
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Optional: Streamline to use returned node.

If you adopt the optional refactor from the previous comment (having syncNodeImgs return the node), you can simplify this function:

Proposed simplification 
 const setupNodeForMaskEditor = () => {
-  syncNodeImgs()
-  if (!props.nodeId) return
-  const node = app.rootGraph?.getNodeById(props.nodeId)
+  const node = syncNodeImgs()
   if (node) app.canvas?.select(node)
 }

This eliminates the duplicate getNodeById call and clarifies that we're selecting the same node we just synchronized. However, the current approach is also clear and maintainable, so this is purely optional.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/renderer/extensions/vueNodes/components/ImagePreview.vue around lines 221
to 227, streamline setupNodeForMaskEditor by using the node returned from
syncNodeImgs (after adopting the optional refactor) instead of calling
getNodeById again: have syncNodeImgs return the node (or null), call const node
= syncNodeImgs(); if (!props.nodeId || !node) return; then call
app.canvas?.select(node). This removes the duplicate getNodeById lookup and
ensures you select the exact node that was synchronized.


const handleEditMask = () => {
Expand Down