diff --git a/browser_tests/fixtures/ComfyPage.ts b/browser_tests/fixtures/ComfyPage.ts index 6369d654cb..67d59dab69 100644 --- a/browser_tests/fixtures/ComfyPage.ts +++ b/browser_tests/fixtures/ComfyPage.ts @@ -787,106 +787,178 @@ export class ComfyPage { } /** - * Right-clicks on a subgraph input slot to open the context menu. - * Must be called when inside a subgraph. - * - * This method uses the actual slot positions from the subgraph.inputs array, - * which contain the correct coordinates for each input slot. These positions - * are different from the visual node positions and are specifically where - * the slots are rendered on the input node. + * Core helper method for interacting with subgraph I/O slots. + * Handles both input/output slots and both right-click/double-click actions. * - * @param inputName Optional name of the specific input slot to target (e.g., 'text'). - * If not provided, tries all available input slots until one works. - * @returns Promise that resolves when the context menu appears + * @param slotType - 'input' or 'output' + * @param action - 'rightClick' or 'doubleClick' + * @param slotName - Optional specific slot name to target + * @private */ - async rightClickSubgraphInputSlot(inputName?: string): Promise { - const foundSlot = await this.page.evaluate(async (targetInputName) => { - const app = window['app'] - const currentGraph = app.canvas.graph - - // Check if we're in a subgraph - if (currentGraph.constructor.name !== 'Subgraph') { - throw new Error( - 'Not in a subgraph - this method only works inside subgraphs' - ) - } - - // Get the input node - const inputNode = currentGraph.inputNode - if (!inputNode) { - throw new Error('No input node found in subgraph') - } - - // Get available inputs - const inputs = currentGraph.inputs - if (!inputs || inputs.length === 0) { - throw new Error('No input slots found in subgraph') - } - - // Filter to specific input if requested - const inputsToTry = targetInputName - ? inputs.filter((inp) => inp.name === targetInputName) - : inputs + private async interactWithSubgraphSlot( + slotType: 'input' | 'output', + action: 'rightClick' | 'doubleClick', + slotName?: string + ): Promise { + const foundSlot = await this.page.evaluate( + async (params) => { + const { slotType, action, targetSlotName } = params + const app = window['app'] + const currentGraph = app.canvas.graph - if (inputsToTry.length === 0) { - throw new Error( - targetInputName - ? `Input slot '${targetInputName}' not found` - : 'No input slots available to try' - ) - } + // Check if we're in a subgraph + if (currentGraph.constructor.name !== 'Subgraph') { + throw new Error( + 'Not in a subgraph - this method only works inside subgraphs' + ) + } - // Try right-clicking on each input slot position until one works - for (const input of inputsToTry) { - if (!input.pos) continue + // Get the appropriate node and slots + const node = + slotType === 'input' + ? currentGraph.inputNode + : currentGraph.outputNode + const slots = + slotType === 'input' ? currentGraph.inputs : currentGraph.outputs - const testX = input.pos[0] - const testY = input.pos[1] + if (!node) { + throw new Error(`No ${slotType} node found in subgraph`) + } - // Create a right-click event at the input slot position - const rightClickEvent = { - canvasX: testX, - canvasY: testY, - button: 2, // Right mouse button - preventDefault: () => {}, - stopPropagation: () => {} + if (!slots || slots.length === 0) { + throw new Error(`No ${slotType} slots found in subgraph`) } - // Trigger the input node's right-click handler - if (inputNode.onPointerDown) { - inputNode.onPointerDown( - rightClickEvent, - app.canvas.pointer, - app.canvas.linkConnector + // Filter slots based on target name and action type + const slotsToTry = targetSlotName + ? slots.filter((slot) => slot.name === targetSlotName) + : action === 'rightClick' + ? slots + : [slots[0]] // Right-click tries all, double-click uses first + + if (slotsToTry.length === 0) { + throw new Error( + targetSlotName + ? `${slotType} slot '${targetSlotName}' not found` + : `No ${slotType} slots available to try` ) } - // Wait briefly for menu to appear - await new Promise((resolve) => setTimeout(resolve, 100)) + // Handle the interaction based on action type + if (action === 'rightClick') { + // Right-click: try each slot until one works + for (const slot of slotsToTry) { + if (!slot.pos) continue + + const event = { + canvasX: slot.pos[0], + canvasY: slot.pos[1], + button: 2, // Right mouse button + preventDefault: () => {}, + stopPropagation: () => {} + } + + if (node.onPointerDown) { + node.onPointerDown( + event, + app.canvas.pointer, + app.canvas.linkConnector + ) + } + + // Wait briefly for menu to appear + await new Promise((resolve) => setTimeout(resolve, 100)) + + // Check if context menu appeared + const menuExists = document.querySelector('.litemenu-entry') + if (menuExists) { + return { + success: true, + slotName: slot.name, + x: slot.pos[0], + y: slot.pos[1] + } + } + } + } else if (action === 'doubleClick') { + // Double-click: use first slot with bounding rect center + const slot = slotsToTry[0] + if (!slot.boundingRect) { + throw new Error(`${slotType} slot bounding rect not found`) + } + + const rect = slot.boundingRect + const testX = rect[0] + rect[2] / 2 // x + width/2 + const testY = rect[1] + rect[3] / 2 // y + height/2 + + const event = { + canvasX: testX, + canvasY: testY, + button: 0, // Left mouse button + preventDefault: () => {}, + stopPropagation: () => {} + } + + if (node.onPointerDown) { + node.onPointerDown( + event, + app.canvas.pointer, + app.canvas.linkConnector + ) + + // Trigger double-click + if (app.canvas.pointer.onDoubleClick) { + app.canvas.pointer.onDoubleClick(event) + } + } + + // Wait briefly for dialog to appear + await new Promise((resolve) => setTimeout(resolve, 200)) - // Check if litegraph context menu appeared - const menuExists = document.querySelector('.litemenu-entry') - if (menuExists) { - return { success: true, inputName: input.name, x: testX, y: testY } + return { success: true, slotName: slot.name, x: testX, y: testY } } - } - return { success: false } - }, inputName) + return { success: false } + }, + { slotType, action, targetSlotName: slotName } + ) if (!foundSlot.success) { + const actionText = + action === 'rightClick' ? 'open context menu for' : 'double-click' throw new Error( - inputName - ? `Could not open context menu for input slot '${inputName}'` - : 'Could not find any input slot position to right-click' + slotName + ? `Could not ${actionText} ${slotType} slot '${slotName}'` + : `Could not find any ${slotType} slot to ${actionText}` ) } - // Wait for the litegraph context menu to be visible - await this.page.waitForSelector('.litemenu-entry', { - state: 'visible', - timeout: 5000 - }) + // Wait for the appropriate UI element to appear + if (action === 'rightClick') { + await this.page.waitForSelector('.litemenu-entry', { + state: 'visible', + timeout: 5000 + }) + } else { + await this.nextFrame() + } + } + + /** + * Right-clicks on a subgraph input slot to open the context menu. + * Must be called when inside a subgraph. + * + * This method uses the actual slot positions from the subgraph.inputs array, + * which contain the correct coordinates for each input slot. These positions + * are different from the visual node positions and are specifically where + * the slots are rendered on the input node. + * + * @param inputName Optional name of the specific input slot to target (e.g., 'text'). + * If not provided, tries all available input slots until one works. + * @returns Promise that resolves when the context menu appears + */ + async rightClickSubgraphInputSlot(inputName?: string): Promise { + return this.interactWithSubgraphSlot('input', 'rightClick', inputName) } /** @@ -900,93 +972,31 @@ export class ComfyPage { * @returns Promise that resolves when the context menu appears */ async rightClickSubgraphOutputSlot(outputName?: string): Promise { - const foundSlot = await this.page.evaluate(async (targetOutputName) => { - const app = window['app'] - const currentGraph = app.canvas.graph - - // Check if we're in a subgraph - if (currentGraph.constructor.name !== 'Subgraph') { - throw new Error( - 'Not in a subgraph - this method only works inside subgraphs' - ) - } - - // Get the output node - const outputNode = currentGraph.outputNode - if (!outputNode) { - throw new Error('No output node found in subgraph') - } - - // Get available outputs - const outputs = currentGraph.outputs - if (!outputs || outputs.length === 0) { - throw new Error('No output slots found in subgraph') - } - - // Filter to specific output if requested - const outputsToTry = targetOutputName - ? outputs.filter((out) => out.name === targetOutputName) - : outputs - - if (outputsToTry.length === 0) { - throw new Error( - targetOutputName - ? `Output slot '${targetOutputName}' not found` - : 'No output slots available to try' - ) - } - - // Try right-clicking on each output slot position until one works - for (const output of outputsToTry) { - if (!output.pos) continue - - const testX = output.pos[0] - const testY = output.pos[1] - - // Create a right-click event at the output slot position - const rightClickEvent = { - canvasX: testX, - canvasY: testY, - button: 2, // Right mouse button - preventDefault: () => {}, - stopPropagation: () => {} - } - - // Trigger the output node's right-click handler - if (outputNode.onPointerDown) { - outputNode.onPointerDown( - rightClickEvent, - app.canvas.pointer, - app.canvas.linkConnector - ) - } - - // Wait briefly for menu to appear - await new Promise((resolve) => setTimeout(resolve, 100)) - - // Check if litegraph context menu appeared - const menuExists = document.querySelector('.litemenu-entry') - if (menuExists) { - return { success: true, outputName: output.name, x: testX, y: testY } - } - } - - return { success: false } - }, outputName) + return this.interactWithSubgraphSlot('output', 'rightClick', outputName) + } - if (!foundSlot.success) { - throw new Error( - outputName - ? `Could not open context menu for output slot '${outputName}'` - : 'Could not find any output slot position to right-click' - ) - } + /** + * Double-clicks on a subgraph input slot to rename it. + * Must be called when inside a subgraph. + * + * @param inputName Optional name of the specific input slot to target (e.g., 'text'). + * If not provided, tries the first available input slot. + * @returns Promise that resolves when the rename dialog appears + */ + async doubleClickSubgraphInputSlot(inputName?: string): Promise { + return this.interactWithSubgraphSlot('input', 'doubleClick', inputName) + } - // Wait for the litegraph context menu to be visible - await this.page.waitForSelector('.litemenu-entry', { - state: 'visible', - timeout: 5000 - }) + /** + * Double-clicks on a subgraph output slot to rename it. + * Must be called when inside a subgraph. + * + * @param outputName Optional name of the specific output slot to target. + * If not provided, tries the first available output slot. + * @returns Promise that resolves when the rename dialog appears + */ + async doubleClickSubgraphOutputSlot(outputName?: string): Promise { + return this.interactWithSubgraphSlot('output', 'doubleClick', outputName) } /** diff --git a/browser_tests/tests/subgraph.spec.ts b/browser_tests/tests/subgraph.spec.ts index 16321e2299..2bc4d97115 100644 --- a/browser_tests/tests/subgraph.spec.ts +++ b/browser_tests/tests/subgraph.spec.ts @@ -155,6 +155,182 @@ test.describe('Subgraph Operations', () => { expect(newInputName).toBe(RENAMED_INPUT_NAME) expect(newInputName).not.toBe(initialInputLabel) }) + + test('Can rename input slots via double-click', async ({ comfyPage }) => { + await comfyPage.loadWorkflow('basic-subgraph') + + const subgraphNode = await comfyPage.getNodeRefById('2') + await subgraphNode.navigateIntoSubgraph() + + const initialInputLabel = await comfyPage.page.evaluate(() => { + const graph = window['app'].canvas.graph + return graph.inputs?.[0]?.label || null + }) + + await comfyPage.doubleClickSubgraphInputSlot(initialInputLabel) + + await comfyPage.page.waitForSelector(SELECTORS.promptDialog, { + state: 'visible' + }) + await comfyPage.page.fill(SELECTORS.promptDialog, RENAMED_INPUT_NAME) + await comfyPage.page.keyboard.press('Enter') + + // Force re-render + await comfyPage.canvas.click({ position: { x: 100, y: 100 } }) + await comfyPage.nextFrame() + + const newInputName = await comfyPage.page.evaluate(() => { + const graph = window['app'].canvas.graph + return graph.inputs?.[0]?.label || null + }) + + expect(newInputName).toBe(RENAMED_INPUT_NAME) + expect(newInputName).not.toBe(initialInputLabel) + }) + + test('Can rename output slots via double-click', async ({ comfyPage }) => { + await comfyPage.loadWorkflow('basic-subgraph') + + const subgraphNode = await comfyPage.getNodeRefById('2') + await subgraphNode.navigateIntoSubgraph() + + const initialOutputLabel = await comfyPage.page.evaluate(() => { + const graph = window['app'].canvas.graph + return graph.outputs?.[0]?.label || null + }) + + await comfyPage.doubleClickSubgraphOutputSlot(initialOutputLabel) + + await comfyPage.page.waitForSelector(SELECTORS.promptDialog, { + state: 'visible' + }) + const renamedOutputName = 'renamed_output' + await comfyPage.page.fill(SELECTORS.promptDialog, renamedOutputName) + await comfyPage.page.keyboard.press('Enter') + + // Force re-render + await comfyPage.canvas.click({ position: { x: 100, y: 100 } }) + await comfyPage.nextFrame() + + const newOutputName = await comfyPage.page.evaluate(() => { + const graph = window['app'].canvas.graph + return graph.outputs?.[0]?.label || null + }) + + expect(newOutputName).toBe(renamedOutputName) + expect(newOutputName).not.toBe(initialOutputLabel) + }) + + test('Right-click context menu still works alongside double-click', async ({ + comfyPage + }) => { + await comfyPage.loadWorkflow('basic-subgraph') + + const subgraphNode = await comfyPage.getNodeRefById('2') + await subgraphNode.navigateIntoSubgraph() + + const initialInputLabel = await comfyPage.page.evaluate(() => { + const graph = window['app'].canvas.graph + return graph.inputs?.[0]?.label || null + }) + + // Test that right-click still works for renaming + await comfyPage.rightClickSubgraphInputSlot(initialInputLabel) + await comfyPage.clickLitegraphContextMenuItem('Rename Slot') + + await comfyPage.page.waitForSelector(SELECTORS.promptDialog, { + state: 'visible' + }) + const rightClickRenamedName = 'right_click_renamed' + await comfyPage.page.fill(SELECTORS.promptDialog, rightClickRenamedName) + await comfyPage.page.keyboard.press('Enter') + + // Force re-render + await comfyPage.canvas.click({ position: { x: 100, y: 100 } }) + await comfyPage.nextFrame() + + const newInputName = await comfyPage.page.evaluate(() => { + const graph = window['app'].canvas.graph + return graph.inputs?.[0]?.label || null + }) + + expect(newInputName).toBe(rightClickRenamedName) + expect(newInputName).not.toBe(initialInputLabel) + }) + + test('Can double-click on slot label text to rename', async ({ + comfyPage + }) => { + await comfyPage.loadWorkflow('basic-subgraph') + + const subgraphNode = await comfyPage.getNodeRefById('2') + await subgraphNode.navigateIntoSubgraph() + + const initialInputLabel = await comfyPage.page.evaluate(() => { + const graph = window['app'].canvas.graph + return graph.inputs?.[0]?.label || null + }) + + // Use direct pointer event approach to double-click on label + await comfyPage.page.evaluate(() => { + const app = window['app'] + const graph = app.canvas.graph + const input = graph.inputs?.[0] + + if (!input?.labelPos) { + throw new Error('Could not get label position for testing') + } + + // Use labelPos for more precise clicking on the text + const testX = input.labelPos[0] + const testY = input.labelPos[1] + + const leftClickEvent = { + canvasX: testX, + canvasY: testY, + button: 0, // Left mouse button + preventDefault: () => {}, + stopPropagation: () => {} + } + + const inputNode = graph.inputNode + if (inputNode?.onPointerDown) { + inputNode.onPointerDown( + leftClickEvent, + app.canvas.pointer, + app.canvas.linkConnector + ) + + // Trigger double-click if pointer has the handler + if (app.canvas.pointer.onDoubleClick) { + app.canvas.pointer.onDoubleClick(leftClickEvent) + } + } + }) + + // Wait for dialog to appear + await comfyPage.page.waitForTimeout(200) + await comfyPage.nextFrame() + + await comfyPage.page.waitForSelector(SELECTORS.promptDialog, { + state: 'visible' + }) + const labelClickRenamedName = 'label_click_renamed' + await comfyPage.page.fill(SELECTORS.promptDialog, labelClickRenamedName) + await comfyPage.page.keyboard.press('Enter') + + // Force re-render + await comfyPage.canvas.click({ position: { x: 100, y: 100 } }) + await comfyPage.nextFrame() + + const newInputName = await comfyPage.page.evaluate(() => { + const graph = window['app'].canvas.graph + return graph.inputs?.[0]?.label || null + }) + + expect(newInputName).toBe(labelClickRenamedName) + expect(newInputName).not.toBe(initialInputLabel) + }) }) test.describe('Subgraph Creation and Deletion', () => { diff --git a/src/lib/litegraph/src/subgraph/SubgraphIONodeBase.ts b/src/lib/litegraph/src/subgraph/SubgraphIONodeBase.ts index 856136b0f6..8073ee6821 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphIONodeBase.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphIONodeBase.ts @@ -170,6 +170,21 @@ export abstract class SubgraphIONodeBase< } } + /** + * Handles double-click on an IO slot to rename it. + * @param slot The slot that was double-clicked. + * @param event The event that triggered the double-click. + */ + protected handleSlotDoubleClick( + slot: TSlot, + event: CanvasPointerEvent + ): void { + // Only allow renaming non-empty slots + if (slot !== this.emptySlot) { + this.#promptForSlotRename(slot, event) + } + } + /** * Shows the context menu for an IO slot. * @param slot The slot to show the context menu for. @@ -239,16 +254,7 @@ export abstract class SubgraphIONodeBase< // Rename the slot case 'rename': if (slot !== this.emptySlot) { - this.subgraph.canvasAction((c) => - c.prompt( - 'Slot name', - slot.name, - (newName: string) => { - if (newName) this.renameSlot(slot, newName) - }, - event - ) - ) + this.#promptForSlotRename(slot, event) } break } @@ -256,6 +262,24 @@ export abstract class SubgraphIONodeBase< this.subgraph.setDirtyCanvas(true) } + /** + * Prompts the user to rename a slot. + * @param slot The slot to rename. + * @param event The event that triggered the rename. + */ + #promptForSlotRename(slot: TSlot, event: CanvasPointerEvent): void { + this.subgraph.canvasAction((c) => + c.prompt( + 'Slot name', + slot.name, + (newName: string) => { + if (newName) this.renameSlot(slot, newName) + }, + event + ) + ) + } + /** Arrange the slots in this node. */ arrange(): void { const { minWidth, roundedRadius } = SubgraphIONodeBase diff --git a/src/lib/litegraph/src/subgraph/SubgraphInputNode.ts b/src/lib/litegraph/src/subgraph/SubgraphInputNode.ts index 5bdfd6fa35..d46e946545 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphInputNode.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphInputNode.ts @@ -4,7 +4,6 @@ import { LLink } from '@/lib/litegraph/src/LLink' import type { RerouteId } from '@/lib/litegraph/src/Reroute' import type { LinkConnector } from '@/lib/litegraph/src/canvas/LinkConnector' import { SUBGRAPH_INPUT_ID } from '@/lib/litegraph/src/constants' -import { Rectangle } from '@/lib/litegraph/src/infrastructure/Rectangle' import type { DefaultConnectionColors, INodeInputSlot, @@ -51,18 +50,17 @@ export class SubgraphInputNode // Left-click handling for dragging connections if (e.button === 0) { for (const slot of this.allSlots) { - const slotBounds = Rectangle.fromCentre( - slot.pos, - slot.boundingRect.height - ) - - if (slotBounds.containsXy(e.canvasX, e.canvasY)) { + // Check if click is within the full slot area (including label) + if (slot.boundingRect.containsXy(e.canvasX, e.canvasY)) { pointer.onDragStart = () => { linkConnector.dragNewFromSubgraphInput(this.subgraph, this, slot) } pointer.onDragEnd = (eUp) => { linkConnector.dropLinks(this.subgraph, eUp) } + pointer.onDoubleClick = () => { + this.handleSlotDoubleClick(slot, e) + } pointer.finally = () => { linkConnector.reset(true) } diff --git a/src/lib/litegraph/src/subgraph/SubgraphOutputNode.ts b/src/lib/litegraph/src/subgraph/SubgraphOutputNode.ts index 6cce8e60d1..51fca97ef4 100644 --- a/src/lib/litegraph/src/subgraph/SubgraphOutputNode.ts +++ b/src/lib/litegraph/src/subgraph/SubgraphOutputNode.ts @@ -4,7 +4,6 @@ import type { LLink } from '@/lib/litegraph/src/LLink' import type { RerouteId } from '@/lib/litegraph/src/Reroute' import type { LinkConnector } from '@/lib/litegraph/src/canvas/LinkConnector' import { SUBGRAPH_OUTPUT_ID } from '@/lib/litegraph/src/constants' -import { Rectangle } from '@/lib/litegraph/src/infrastructure/Rectangle' import type { DefaultConnectionColors, INodeInputSlot, @@ -51,18 +50,17 @@ export class SubgraphOutputNode // Left-click handling for dragging connections if (e.button === 0) { for (const slot of this.allSlots) { - const slotBounds = Rectangle.fromCentre( - slot.pos, - slot.boundingRect.height - ) - - if (slotBounds.containsXy(e.canvasX, e.canvasY)) { + // Check if click is within the full slot area (including label) + if (slot.boundingRect.containsXy(e.canvasX, e.canvasY)) { pointer.onDragStart = () => { linkConnector.dragNewFromSubgraphOutput(this.subgraph, this, slot) } pointer.onDragEnd = (eUp) => { linkConnector.dropLinks(this.subgraph, eUp) } + pointer.onDoubleClick = () => { + this.handleSlotDoubleClick(slot, e) + } pointer.finally = () => { linkConnector.reset(true) }