mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-04-19 22:09:37 +00:00
fix: allow URI drops to bubble from Vue nodes to document handler (#9463)
## Summary Fix URI drops (e.g. dragging `<img>` thumbnails) onto Vue-rendered nodes by letting unhandled drops bubble to the document-level `text/uri-list` fallback in `app.ts`. ## Changes - **What**: Removed unconditional `.stop` modifier from `@drop` in `LGraphNode.vue`. `stopPropagation()` is now called conditionally — only when `onDragDrop` returns `true` (file drop handled). Made `handleDrop` synchronous since `onDragDrop` returns a plain boolean. ## Review Focus The key insight is that `onDragDrop` (from `useNodeDragAndDrop`) returns `false` synchronously for URI drags (no files in `DataTransfer`), so the event must bubble to reach the document handler that fetches the URI. The original `async` + `await` pattern would have deferred `stopPropagation` past the synchronous propagation phase, so `handleDrop` is now synchronous. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-9463-fix-allow-URI-drops-to-bubble-from-Vue-nodes-to-document-handler-31b6d73d36508196a1b3f17e7e4837a9) by [Unito](https://www.unito.io)
This commit is contained in:
@@ -25,13 +25,15 @@ export class DragDropHelper {
|
||||
url?: string
|
||||
dropPosition?: Position
|
||||
waitForUpload?: boolean
|
||||
preserveNativePropagation?: boolean
|
||||
} = {}
|
||||
): Promise<void> {
|
||||
const {
|
||||
dropPosition = { x: 100, y: 100 },
|
||||
fileName,
|
||||
url,
|
||||
waitForUpload = false
|
||||
waitForUpload = false,
|
||||
preserveNativePropagation = false
|
||||
} = options
|
||||
|
||||
if (!fileName && !url)
|
||||
@@ -43,7 +45,8 @@ export class DragDropHelper {
|
||||
fileType?: string
|
||||
buffer?: Uint8Array | number[]
|
||||
url?: string
|
||||
} = { dropPosition }
|
||||
preserveNativePropagation: boolean
|
||||
} = { dropPosition, preserveNativePropagation }
|
||||
|
||||
if (fileName) {
|
||||
const filePath = this.assetPath(fileName)
|
||||
@@ -115,15 +118,17 @@ export class DragDropHelper {
|
||||
)
|
||||
}
|
||||
|
||||
Object.defineProperty(dropEvent, 'preventDefault', {
|
||||
value: () => {},
|
||||
writable: false
|
||||
})
|
||||
if (!params.preserveNativePropagation) {
|
||||
Object.defineProperty(dropEvent, 'preventDefault', {
|
||||
value: () => {},
|
||||
writable: false
|
||||
})
|
||||
|
||||
Object.defineProperty(dropEvent, 'stopPropagation', {
|
||||
value: () => {},
|
||||
writable: false
|
||||
})
|
||||
Object.defineProperty(dropEvent, 'stopPropagation', {
|
||||
value: () => {},
|
||||
writable: false
|
||||
})
|
||||
}
|
||||
|
||||
targetElement.dispatchEvent(dragOverEvent)
|
||||
targetElement.dispatchEvent(dropEvent)
|
||||
@@ -154,7 +159,10 @@ export class DragDropHelper {
|
||||
|
||||
async dragAndDropURL(
|
||||
url: string,
|
||||
options: { dropPosition?: Position } = {}
|
||||
options: {
|
||||
dropPosition?: Position
|
||||
preserveNativePropagation?: boolean
|
||||
} = {}
|
||||
): Promise<void> {
|
||||
return this.dragAndDropExternalResource({ url, ...options })
|
||||
}
|
||||
|
||||
@@ -67,5 +67,44 @@ test.describe(
|
||||
)
|
||||
})
|
||||
})
|
||||
|
||||
test('Load workflow from URL dropped onto Vue node', async ({
|
||||
comfyPage
|
||||
}) => {
|
||||
const fakeUrl = 'https://example.com/workflow.png'
|
||||
await comfyPage.page.route(fakeUrl, (route) =>
|
||||
route.fulfill({
|
||||
path: comfyPage.assetPath('workflowInMedia/workflow_itxt.png')
|
||||
})
|
||||
)
|
||||
|
||||
await comfyPage.settings.setSetting('Comfy.VueNodes.Enabled', true)
|
||||
await comfyPage.vueNodes.waitForNodes()
|
||||
|
||||
const initialNodeCount = await comfyPage.nodeOps.getGraphNodesCount()
|
||||
|
||||
const node = comfyPage.vueNodes.getNodeByTitle('KSampler')
|
||||
const box = await node.boundingBox()
|
||||
expect(box).not.toBeNull()
|
||||
|
||||
const dropPosition = {
|
||||
x: box!.x + box!.width / 2,
|
||||
y: box!.y + box!.height / 2
|
||||
}
|
||||
|
||||
await comfyPage.dragDrop.dragAndDropURL(fakeUrl, {
|
||||
dropPosition,
|
||||
preserveNativePropagation: true
|
||||
})
|
||||
|
||||
await comfyPage.page.waitForFunction(
|
||||
(prevCount) => window.app!.graph.nodes.length !== prevCount,
|
||||
initialNodeCount,
|
||||
{ timeout: 10000 }
|
||||
)
|
||||
|
||||
const newNodeCount = await comfyPage.nodeOps.getGraphNodesCount()
|
||||
expect(newNodeCount).not.toBe(initialNodeCount)
|
||||
})
|
||||
}
|
||||
)
|
||||
|
||||
@@ -13,9 +13,21 @@ import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
||||
import { setActivePinia } from 'pinia'
|
||||
|
||||
const mockData = vi.hoisted(() => ({
|
||||
mockExecuting: false
|
||||
mockExecuting: false,
|
||||
mockLgraphNode: null as Record<string, unknown> | null
|
||||
}))
|
||||
|
||||
vi.mock('@/utils/graphTraversalUtil', async (importOriginal) => {
|
||||
const actual = (await importOriginal()) as Record<string, unknown>
|
||||
return {
|
||||
...actual,
|
||||
getLocatorIdFromNodeData: vi.fn(() => 'test-node-123'),
|
||||
getNodeByLocatorId: vi.fn(
|
||||
() => mockData.mockLgraphNode ?? { isSubgraphNode: () => false }
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('@/renderer/core/layout/transform/useTransformState', () => {
|
||||
return {
|
||||
useTransformState: () => ({
|
||||
@@ -49,16 +61,6 @@ vi.mock('@/scripts/app', () => ({
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@/utils/graphTraversalUtil', async (importOriginal) => {
|
||||
const actual = (await importOriginal()) as Record<string, unknown>
|
||||
return {
|
||||
...actual,
|
||||
getNodeByLocatorId: vi.fn(() => ({
|
||||
isSubgraphNode: () => false
|
||||
}))
|
||||
}
|
||||
})
|
||||
|
||||
vi.mock('@/composables/useErrorHandling', () => ({
|
||||
useErrorHandling: () => ({
|
||||
toastErrorHandler: vi.fn()
|
||||
@@ -293,4 +295,52 @@ describe('LGraphNode', () => {
|
||||
expect(wrapper.find('[role="button"][aria-label]').exists()).toBe(true)
|
||||
})
|
||||
})
|
||||
|
||||
describe('handleDrop', () => {
|
||||
it('should stop propagation when onDragDrop returns true', async () => {
|
||||
const onDragDrop = vi.fn().mockReturnValue(true)
|
||||
mockData.mockLgraphNode = {
|
||||
onDragDrop,
|
||||
onDragOver: vi.fn(),
|
||||
isSubgraphNode: () => false
|
||||
}
|
||||
|
||||
const wrapper = mountLGraphNode({ nodeData: mockNodeData })
|
||||
|
||||
const parentListener = vi.fn()
|
||||
const parent = wrapper.element.parentElement
|
||||
expect(parent).not.toBeNull()
|
||||
parent!.addEventListener('drop', parentListener)
|
||||
|
||||
wrapper.element.dispatchEvent(
|
||||
new Event('drop', { bubbles: true, cancelable: true })
|
||||
)
|
||||
|
||||
expect(onDragDrop).toHaveBeenCalled()
|
||||
expect(parentListener).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('should not stop propagation when onDragDrop returns false', async () => {
|
||||
const onDragDrop = vi.fn().mockReturnValue(false)
|
||||
mockData.mockLgraphNode = {
|
||||
onDragDrop,
|
||||
onDragOver: vi.fn(),
|
||||
isSubgraphNode: () => false
|
||||
}
|
||||
|
||||
const wrapper = mountLGraphNode({ nodeData: mockNodeData })
|
||||
|
||||
const parentListener = vi.fn()
|
||||
const parent = wrapper.element.parentElement
|
||||
expect(parent).not.toBeNull()
|
||||
parent!.addEventListener('drop', parentListener)
|
||||
|
||||
wrapper.element.dispatchEvent(
|
||||
new Event('drop', { bubbles: true, cancelable: true })
|
||||
)
|
||||
|
||||
expect(onDragDrop).toHaveBeenCalled()
|
||||
expect(parentListener).toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -33,7 +33,7 @@
|
||||
@contextmenu="handleContextMenu"
|
||||
@dragover.prevent="handleDragOver"
|
||||
@dragleave="handleDragLeave"
|
||||
@drop.stop.prevent="handleDrop"
|
||||
@drop.prevent="handleDrop"
|
||||
>
|
||||
<!-- Selection/Execution Outline Overlay -->
|
||||
<AppOutput
|
||||
@@ -827,15 +827,13 @@ function handleDragLeave() {
|
||||
isDraggingOver.value = false
|
||||
}
|
||||
|
||||
async function handleDrop(event: DragEvent) {
|
||||
function handleDrop(event: DragEvent) {
|
||||
isDraggingOver.value = false
|
||||
|
||||
const node = lgraphNode.value
|
||||
if (!node || !node.onDragDrop) {
|
||||
return
|
||||
}
|
||||
if (!node?.onDragDrop) return
|
||||
|
||||
// Forward the drop event to the litegraph node's onDragDrop callback
|
||||
await node.onDragDrop(event)
|
||||
const handled = node.onDragDrop(event)
|
||||
if (handled === true) event.stopPropagation()
|
||||
}
|
||||
</script>
|
||||
|
||||
@@ -1,8 +1,18 @@
|
||||
import { extractFilesFromDragEvent } from '@/utils/eventUtils'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
describe('eventUtils', () => {
|
||||
describe('extractFilesFromDragEvent', () => {
|
||||
let fetchSpy: ReturnType<typeof vi.fn>
|
||||
|
||||
beforeEach(() => {
|
||||
fetchSpy = vi.fn()
|
||||
vi.stubGlobal('fetch', fetchSpy)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.unstubAllGlobals()
|
||||
})
|
||||
it('should return empty array when no dataTransfer', async () => {
|
||||
const actual = await extractFilesFromDragEvent(new FakeDragEvent('drop'))
|
||||
expect(actual).toEqual([])
|
||||
@@ -98,19 +108,56 @@ describe('eventUtils', () => {
|
||||
expect(actual).toEqual([file1, file2])
|
||||
})
|
||||
|
||||
// Skip until we can setup MSW
|
||||
it.skip('should handle drops with URLs', async () => {
|
||||
const urlWithWorkflow = 'https://fakewebsite.notreal/fake_workflow.json'
|
||||
it('should fetch URI and return as File when text/uri-list is present', async () => {
|
||||
const uri = 'https://example.com/api/view?filename=test.png&type=input'
|
||||
const imageBlob = new Blob([new Uint8Array([0x89, 0x50])], {
|
||||
type: 'image/png'
|
||||
})
|
||||
fetchSpy.mockResolvedValue(new Response(imageBlob))
|
||||
|
||||
const dataTransfer = new DataTransfer()
|
||||
dataTransfer.setData('text/uri-list', urlWithWorkflow)
|
||||
dataTransfer.setData('text/x-moz-url', urlWithWorkflow)
|
||||
dataTransfer.setData('text/uri-list', uri)
|
||||
|
||||
const actual = await extractFilesFromDragEvent(
|
||||
new FakeDragEvent('drop', { dataTransfer })
|
||||
)
|
||||
expect(actual.length).toBe(1)
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledOnce()
|
||||
expect(actual).toHaveLength(1)
|
||||
expect(actual[0]).toBeInstanceOf(File)
|
||||
expect(actual[0].type).toBe('image/png')
|
||||
})
|
||||
|
||||
it('should handle text/x-moz-url type', async () => {
|
||||
const uri = 'https://example.com/api/view?filename=test.png&type=input'
|
||||
const imageBlob = new Blob([new Uint8Array([0x89, 0x50])], {
|
||||
type: 'image/png'
|
||||
})
|
||||
fetchSpy.mockResolvedValue(new Response(imageBlob))
|
||||
|
||||
const dataTransfer = new DataTransfer()
|
||||
dataTransfer.setData('text/x-moz-url', uri)
|
||||
|
||||
const actual = await extractFilesFromDragEvent(
|
||||
new FakeDragEvent('drop', { dataTransfer })
|
||||
)
|
||||
|
||||
expect(fetchSpy).toHaveBeenCalledOnce()
|
||||
expect(actual).toHaveLength(1)
|
||||
})
|
||||
|
||||
it('should return empty array when URI fetch fails', async () => {
|
||||
const uri = 'https://example.com/api/view?filename=test.png&type=input'
|
||||
fetchSpy.mockRejectedValue(new TypeError('Failed to fetch'))
|
||||
|
||||
const dataTransfer = new DataTransfer()
|
||||
dataTransfer.setData('text/uri-list', uri)
|
||||
|
||||
const actual = await extractFilesFromDragEvent(
|
||||
new FakeDragEvent('drop', { dataTransfer })
|
||||
)
|
||||
|
||||
expect(actual).toEqual([])
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -20,9 +20,13 @@ export async function extractFilesFromDragEvent(
|
||||
const uri = event.dataTransfer.getData(match)?.split('\n')?.[0]
|
||||
if (!uri) return []
|
||||
|
||||
const response = await fetch(uri)
|
||||
const blob = await response.blob()
|
||||
return [new File([blob], uri, { type: blob.type })]
|
||||
try {
|
||||
const response = await fetch(uri)
|
||||
const blob = await response.blob()
|
||||
return [new File([blob], uri, { type: blob.type })]
|
||||
} catch {
|
||||
return []
|
||||
}
|
||||
}
|
||||
|
||||
export function hasImageType({ type }: File): boolean {
|
||||
|
||||
Reference in New Issue
Block a user