mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-19 22:34:15 +00:00
fix: sync node.imgs for legacy context menu in Vue Nodes mode (#8143)
## 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 - Add `syncLegacyNodeImgs` store method to sync loaded image elements to `node.imgs` - Call sync on image load in ImagePreview component - Simplify mask editor handling to call composable directly ## Technical Details - Only runs when `vueNodesMode` is enabled (no impact on legacy mode) - Reuses already-loaded `<img>` element from Vue (no duplicate network requests) - Store owns the sync logic, component just hands off the element Supersedes #7416 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8143-fix-sync-node-imgs-for-legacy-context-menu-in-Vue-Nodes-mode-2ec6d73d365081c59d42cd1722779b61) by [Unito](https://www.unito.io) --------- Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -0,0 +1,55 @@
|
||||
import { expect } from '@playwright/test'
|
||||
|
||||
import { comfyPageFixture as test } from '../../../../fixtures/ComfyPage'
|
||||
|
||||
test.describe('Vue Nodes Image Preview', () => {
|
||||
test.beforeEach(async ({ comfyPage }) => {
|
||||
await comfyPage.setSetting('Comfy.VueNodes.Enabled', true)
|
||||
await comfyPage.loadWorkflow('widgets/load_image_widget')
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
})
|
||||
|
||||
async function loadImageOnNode(
|
||||
comfyPage: Awaited<
|
||||
ReturnType<(typeof test)['info']>
|
||||
>['fixtures']['comfyPage']
|
||||
) {
|
||||
const loadImageNode = (await comfyPage.getNodeRefsByType('LoadImage'))[0]
|
||||
const { x, y } = await loadImageNode.getPosition()
|
||||
|
||||
await comfyPage.dragAndDropFile('image64x64.webp', {
|
||||
dropPosition: { x, y }
|
||||
})
|
||||
|
||||
const imagePreview = comfyPage.page.locator('.image-preview')
|
||||
await expect(imagePreview).toBeVisible()
|
||||
await expect(imagePreview.locator('img')).toBeVisible()
|
||||
await expect(imagePreview).toContainText('x')
|
||||
|
||||
return imagePreview
|
||||
}
|
||||
|
||||
test('opens mask editor from image preview button', async ({ comfyPage }) => {
|
||||
const imagePreview = await loadImageOnNode(comfyPage)
|
||||
|
||||
await imagePreview.locator('[role="img"]').hover()
|
||||
await comfyPage.page.getByLabel('Edit or mask image').click()
|
||||
|
||||
await expect(comfyPage.page.locator('.mask-editor-dialog')).toBeVisible()
|
||||
})
|
||||
|
||||
test('shows image context menu options', async ({ comfyPage }) => {
|
||||
await loadImageOnNode(comfyPage)
|
||||
|
||||
const nodeHeader = comfyPage.vueNodes.getNodeByTitle('Load Image')
|
||||
await nodeHeader.click()
|
||||
await nodeHeader.click({ button: 'right' })
|
||||
|
||||
const contextMenu = comfyPage.page.locator('.p-contextmenu')
|
||||
await expect(contextMenu).toBeVisible()
|
||||
await expect(contextMenu.getByText('Open Image')).toBeVisible()
|
||||
await expect(contextMenu.getByText('Copy Image')).toBeVisible()
|
||||
await expect(contextMenu.getByText('Save Image')).toBeVisible()
|
||||
await expect(contextMenu.getByText('Open in Mask Editor')).toBeVisible()
|
||||
})
|
||||
})
|
||||
@@ -40,7 +40,6 @@
|
||||
<!-- Main Image -->
|
||||
<img
|
||||
v-if="!imageError"
|
||||
ref="currentImageEl"
|
||||
:src="currentImageUrl"
|
||||
:alt="imageAltText"
|
||||
class="block size-full object-contain pointer-events-none contain-size"
|
||||
@@ -128,8 +127,8 @@ import { computed, ref, watch } from 'vue'
|
||||
import { useI18n } from 'vue-i18n'
|
||||
|
||||
import { downloadFile } from '@/base/common/downloadUtil'
|
||||
import { useMaskEditor } from '@/composables/maskeditor/useMaskEditor'
|
||||
import { app } from '@/scripts/app'
|
||||
import { useCommandStore } from '@/stores/commandStore'
|
||||
import { useNodeOutputStore } from '@/stores/imagePreviewStore'
|
||||
|
||||
interface ImagePreviewProps {
|
||||
@@ -142,7 +141,7 @@ interface ImagePreviewProps {
|
||||
const props = defineProps<ImagePreviewProps>()
|
||||
|
||||
const { t } = useI18n()
|
||||
const commandStore = useCommandStore()
|
||||
const maskEditor = useMaskEditor()
|
||||
const nodeOutputStore = useNodeOutputStore()
|
||||
|
||||
const actionButtonClass =
|
||||
@@ -156,7 +155,6 @@ const actualDimensions = ref<string | null>(null)
|
||||
const imageError = ref(false)
|
||||
const showLoader = ref(false)
|
||||
|
||||
const currentImageEl = ref<HTMLImageElement>()
|
||||
const imageWrapperEl = ref<HTMLDivElement>()
|
||||
|
||||
const { start: startDelayedLoader, stop: stopDelayedLoader } = useTimeoutFn(
|
||||
@@ -209,6 +207,10 @@ const handleImageLoad = (event: Event) => {
|
||||
if (img.naturalWidth && img.naturalHeight) {
|
||||
actualDimensions.value = `${img.naturalWidth} x ${img.naturalHeight}`
|
||||
}
|
||||
|
||||
if (props.nodeId) {
|
||||
nodeOutputStore.syncLegacyNodeImgs(props.nodeId, img, currentIndex.value)
|
||||
}
|
||||
}
|
||||
|
||||
const handleImageError = () => {
|
||||
@@ -218,19 +220,11 @@ const handleImageError = () => {
|
||||
actualDimensions.value = null
|
||||
}
|
||||
|
||||
// In vueNodes mode, we need to set them manually before opening the mask editor.
|
||||
const setupNodeForMaskEditor = () => {
|
||||
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)
|
||||
}
|
||||
|
||||
const handleEditMask = () => {
|
||||
setupNodeForMaskEditor()
|
||||
void commandStore.execute('Comfy.MaskEditor.OpenMaskEditor')
|
||||
if (!props.nodeId) return
|
||||
const node = app.rootGraph?.getNodeById(Number(props.nodeId))
|
||||
if (!node) return
|
||||
maskEditor.openMaskEditor(node)
|
||||
}
|
||||
|
||||
const handleDownload = () => {
|
||||
|
||||
@@ -3,6 +3,7 @@ import { setActivePinia } from 'pinia'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import type { LGraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
|
||||
import type { ExecutedWsMessage } from '@/schemas/apiSchema'
|
||||
import { app } from '@/scripts/app'
|
||||
import { useNodeOutputStore } from '@/stores/imagePreviewStore'
|
||||
@@ -13,20 +14,25 @@ vi.mock('@/utils/litegraphUtil', () => ({
|
||||
isVideoNode: vi.fn()
|
||||
}))
|
||||
|
||||
const mockGetNodeById = vi.fn()
|
||||
|
||||
vi.mock('@/scripts/app', () => ({
|
||||
app: {
|
||||
getPreviewFormatParam: vi.fn(() => '&format=test_webp'),
|
||||
rootGraph: {
|
||||
getNodeById: (...args: unknown[]) => mockGetNodeById(...args)
|
||||
},
|
||||
nodeOutputs: {} as Record<string, unknown>,
|
||||
nodePreviewImages: {} as Record<string, string[]>
|
||||
}
|
||||
}))
|
||||
|
||||
const createMockNode = (overrides: Partial<LGraphNode> = {}): LGraphNode =>
|
||||
const createMockNode = (overrides: Record<string, unknown> = {}): LGraphNode =>
|
||||
({
|
||||
id: 1,
|
||||
type: 'TestNode',
|
||||
...overrides
|
||||
}) as LGraphNode
|
||||
}) as unknown as LGraphNode
|
||||
|
||||
const createMockOutputs = (
|
||||
images?: ExecutedWsMessage['output']['images']
|
||||
@@ -216,3 +222,89 @@ describe('imagePreviewStore getPreviewParam', () => {
|
||||
expect(vi.mocked(app).getPreviewFormatParam).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
||||
describe('imagePreviewStore syncLegacyNodeImgs', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createTestingPinia({ stubActions: false }))
|
||||
vi.clearAllMocks()
|
||||
LiteGraph.vueNodesMode = false
|
||||
})
|
||||
|
||||
it('should not sync when vueNodesMode is disabled', () => {
|
||||
const store = useNodeOutputStore()
|
||||
const mockNode = createMockNode({ id: 1 })
|
||||
const mockImg = document.createElement('img')
|
||||
|
||||
mockGetNodeById.mockReturnValue(mockNode)
|
||||
|
||||
store.syncLegacyNodeImgs(1, mockImg, 0)
|
||||
|
||||
expect(mockNode.imgs).toBeUndefined()
|
||||
expect(mockNode.imageIndex).toBeUndefined()
|
||||
})
|
||||
|
||||
it('should sync node.imgs when vueNodesMode is enabled', () => {
|
||||
LiteGraph.vueNodesMode = true
|
||||
const store = useNodeOutputStore()
|
||||
const mockNode = createMockNode({ id: 1 })
|
||||
const mockImg = document.createElement('img')
|
||||
|
||||
mockGetNodeById.mockReturnValue(mockNode)
|
||||
|
||||
store.syncLegacyNodeImgs(1, mockImg, 0)
|
||||
|
||||
expect(mockNode.imgs).toEqual([mockImg])
|
||||
expect(mockNode.imageIndex).toBe(0)
|
||||
})
|
||||
|
||||
it('should sync with correct activeIndex', () => {
|
||||
LiteGraph.vueNodesMode = true
|
||||
const store = useNodeOutputStore()
|
||||
const mockNode = createMockNode({ id: 42 })
|
||||
const mockImg = document.createElement('img')
|
||||
|
||||
mockGetNodeById.mockReturnValue(mockNode)
|
||||
|
||||
store.syncLegacyNodeImgs(42, mockImg, 3)
|
||||
|
||||
expect(mockNode.imgs).toEqual([mockImg])
|
||||
expect(mockNode.imageIndex).toBe(3)
|
||||
})
|
||||
|
||||
it('should handle string nodeId', () => {
|
||||
LiteGraph.vueNodesMode = true
|
||||
const store = useNodeOutputStore()
|
||||
const mockNode = createMockNode({ id: 123 })
|
||||
const mockImg = document.createElement('img')
|
||||
|
||||
mockGetNodeById.mockReturnValue(mockNode)
|
||||
|
||||
store.syncLegacyNodeImgs('123', mockImg, 0)
|
||||
|
||||
expect(mockGetNodeById).toHaveBeenCalledWith(123)
|
||||
expect(mockNode.imgs).toEqual([mockImg])
|
||||
})
|
||||
|
||||
it('should not throw when node is not found', () => {
|
||||
LiteGraph.vueNodesMode = true
|
||||
const store = useNodeOutputStore()
|
||||
const mockImg = document.createElement('img')
|
||||
|
||||
mockGetNodeById.mockReturnValue(undefined)
|
||||
|
||||
expect(() => store.syncLegacyNodeImgs(999, mockImg, 0)).not.toThrow()
|
||||
})
|
||||
|
||||
it('should default activeIndex to 0', () => {
|
||||
LiteGraph.vueNodesMode = true
|
||||
const store = useNodeOutputStore()
|
||||
const mockNode = createMockNode({ id: 1 })
|
||||
const mockImg = document.createElement('img')
|
||||
|
||||
mockGetNodeById.mockReturnValue(mockNode)
|
||||
|
||||
store.syncLegacyNodeImgs(1, mockImg)
|
||||
|
||||
expect(mockNode.imageIndex).toBe(0)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -3,6 +3,7 @@ import { defineStore } from 'pinia'
|
||||
import { ref } from 'vue'
|
||||
|
||||
import type { LGraphNode, SubgraphNode } from '@/lib/litegraph/src/litegraph'
|
||||
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
|
||||
import { useWorkflowStore } from '@/platform/workflow/management/stores/workflowStore'
|
||||
import type {
|
||||
ExecutedWsMessage,
|
||||
@@ -394,6 +395,32 @@ export const useNodeOutputStore = defineStore('nodeOutput', () => {
|
||||
revokeAllPreviews()
|
||||
}
|
||||
|
||||
/**
|
||||
* Sync legacy node.imgs property for backwards compatibility.
|
||||
*
|
||||
* In Vue Nodes mode, legacy systems (Copy Image, Open Image, Save Image,
|
||||
* Open in Mask Editor) rely on `node.imgs` containing HTMLImageElement
|
||||
* references. Since Vue handles image rendering, we need to sync the
|
||||
* already-loaded element from the Vue component to the node.
|
||||
*
|
||||
* @param nodeId - The node ID
|
||||
* @param element - The loaded HTMLImageElement from the Vue component
|
||||
* @param activeIndex - The current image index (for multi-image outputs)
|
||||
*/
|
||||
function syncLegacyNodeImgs(
|
||||
nodeId: string | number,
|
||||
element: HTMLImageElement,
|
||||
activeIndex: number = 0
|
||||
) {
|
||||
if (!LiteGraph.vueNodesMode) return
|
||||
|
||||
const node = app.rootGraph?.getNodeById(Number(nodeId))
|
||||
if (!node) return
|
||||
|
||||
node.imgs = [element]
|
||||
node.imageIndex = activeIndex
|
||||
}
|
||||
|
||||
return {
|
||||
// Getters
|
||||
getNodeOutputs,
|
||||
@@ -407,6 +434,7 @@ export const useNodeOutputStore = defineStore('nodeOutput', () => {
|
||||
setNodePreviewsByExecutionId,
|
||||
setNodePreviewsByNodeId,
|
||||
updateNodeImages,
|
||||
syncLegacyNodeImgs,
|
||||
|
||||
// Cleanup
|
||||
revokePreviewsByExecutionId,
|
||||
|
||||
Reference in New Issue
Block a user