diff --git a/browser_tests/tests/sidebar/nodeLibrary.spec.ts b/browser_tests/tests/sidebar/nodeLibrary.spec.ts index 58f2ae448..69606b797 100644 --- a/browser_tests/tests/sidebar/nodeLibrary.spec.ts +++ b/browser_tests/tests/sidebar/nodeLibrary.spec.ts @@ -336,4 +336,54 @@ test.describe('Node library sidebar', () => { await comfyPage.page.waitForTimeout(1000) expect(await tab.getNode('KSampler (Advanced)').count()).toBe(2) }) + + test('Subgraph node display name updates in library when renamed', async ({ + comfyPage + }) => { + // Load a workflow with subgraphs to populate the node library + await comfyPage.loadWorkflow('nested-subgraph') + await comfyPage.nextFrame() + + const tab = comfyPage.menu.nodeLibraryTab + + // Navigate to subgraph folder in node library + await tab.getFolder('subgraph').click() + await comfyPage.nextFrame() + + // Get initial subgraph node name in the library + const initialSubgraphNodeCount = await tab.getNode('Subgraph Node').count() + expect(initialSubgraphNodeCount).toBeGreaterThan(0) + + // Get the subgraph node by ID (node 10 is the subgraph) + const subgraphNode = await comfyPage.getNodeRefById('10') + const nodePos = await subgraphNode.getPosition() + const nodeSize = await subgraphNode.getSize() + + // Double-click on the title area of the subgraph node to edit + await comfyPage.canvas.dblclick({ + position: { + x: nodePos.x + nodeSize.width / 2, + y: nodePos.y - 10 // Title area is above the node body + }, + delay: 5 + }) + + // Wait for title editor to appear + await expect(comfyPage.page.locator('.node-title-editor')).toBeVisible() + + // Clear existing text and type new title + await comfyPage.page.keyboard.press('Control+a') + const newTitle = 'Renamed Subgraph Node' + await comfyPage.page.keyboard.type(newTitle) + await comfyPage.page.keyboard.press('Enter') + + // Wait a frame for the update to complete + await comfyPage.nextFrame() + + // Verify the node library shows the updated title + expect(await tab.getNode(newTitle).count()).toBeGreaterThan(0) + + // Verify the old node name is no longer in the library + expect(await tab.getNode('Subgraph Node').count()).toBe(0) + }) }) diff --git a/browser_tests/tests/subgraphBreadcrumb.spec.ts b/browser_tests/tests/subgraph/subgraphBreadcrumb.spec.ts similarity index 100% rename from browser_tests/tests/subgraphBreadcrumb.spec.ts rename to browser_tests/tests/subgraph/subgraphBreadcrumb.spec.ts diff --git a/browser_tests/tests/subgraph/subgraphPersistence.spec.ts b/browser_tests/tests/subgraph/subgraphPersistence.spec.ts new file mode 100644 index 000000000..a26644714 --- /dev/null +++ b/browser_tests/tests/subgraph/subgraphPersistence.spec.ts @@ -0,0 +1,54 @@ +import { expect } from '@playwright/test' +import { comfyPageFixture as test } from '@/browser_tests/fixtures/ComfyPage' + +test.describe('Subgraph Persistence', () => { + test('Node library updates when subgraph title changes', async ({ + comfyPage + }) => { + // Load a workflow with subgraphs to populate the node library + await comfyPage.loadWorkflow('nested-subgraph') + await comfyPage.nextFrame() + + const tab = comfyPage.menu.nodeLibraryTab + + // Navigate to subgraph folder in node library + await tab.getFolder('subgraph').click() + await comfyPage.nextFrame() + + // Get initial subgraph node name in the library + const initialSubgraphNodeCount = await tab.getNode('Subgraph Node').count() + expect(initialSubgraphNodeCount).toBeGreaterThan(0) + + // Get the subgraph node by ID (node 10 is the subgraph) + const subgraphNode = await comfyPage.getNodeRefById('10') + const nodePos = await subgraphNode.getPosition() + const nodeSize = await subgraphNode.getSize() + + // Double-click on the title area of the subgraph node to edit + await comfyPage.canvas.dblclick({ + position: { + x: nodePos.x + nodeSize.width / 2, + y: nodePos.y - 10 // Title area is above the node body + }, + delay: 5 + }) + + // Wait for title editor to appear + await expect(comfyPage.page.locator('.node-title-editor')).toBeVisible() + + // Clear existing text and type new title + await comfyPage.page.keyboard.press('Control+a') + const newTitle = 'Renamed Subgraph Node' + await comfyPage.page.keyboard.type(newTitle) + await comfyPage.page.keyboard.press('Enter') + + // Wait a frame for the update to complete + await comfyPage.nextFrame() + + // Verify the node library shows the updated title + expect(await tab.getNode(newTitle).count()).toBeGreaterThan(0) + + // Verify the old node name is no longer in the library + expect(await tab.getNode('Subgraph Node').count()).toBe(0) + }) +}) \ No newline at end of file diff --git a/src/components/graph/TitleEditor.vue b/src/components/graph/TitleEditor.vue index f3e49e77f..5569ccc9c 100644 --- a/src/components/graph/TitleEditor.vue +++ b/src/components/graph/TitleEditor.vue @@ -22,9 +22,11 @@ import EditableText from '@/components/common/EditableText.vue' import { useAbsolutePosition } from '@/composables/element/useAbsolutePosition' import { app } from '@/scripts/app' import { useCanvasStore, useTitleEditorStore } from '@/stores/graphStore' +import { useNodeDefStore } from '@/stores/nodeDefStore' import { useSettingStore } from '@/stores/settingStore' const settingStore = useSettingStore() +const nodeDefStore = useNodeDefStore() const showInput = ref(false) const editedTitle = ref('') @@ -48,6 +50,9 @@ const onEdit = (newValue: string) => { const target = titleEditorStore.titleEditorTarget if (target instanceof LGraphNode && target.isSubgraphNode?.()) { target.subgraph.name = trimmedTitle + + // Also update the node definition display name in the store + nodeDefStore.updateNodeDefDisplayName(target.type, trimmedTitle) } app.graph.setDirtyCanvas(true, true) diff --git a/src/stores/nodeDefStore.ts b/src/stores/nodeDefStore.ts index aaa6d000b..346ed5b02 100644 --- a/src/stores/nodeDefStore.ts +++ b/src/stores/nodeDefStore.ts @@ -1,4 +1,4 @@ -import type { LGraphNode } from '@comfyorg/litegraph' +import { type LGraphNode, LiteGraph } from '@comfyorg/litegraph' import axios from 'axios' import _ from 'lodash' import { defineStore } from 'pinia' @@ -304,6 +304,28 @@ export const useNodeDefStore = defineStore('nodeDef', () => { nodeDefsByName.value[nodeDef.name] = nodeDefImpl nodeDefsByDisplayName.value[nodeDef.display_name] = nodeDefImpl } + function updateNodeDefDisplayName(nodeType: string, newDisplayName: string) { + const existingNodeDef = nodeDefsByName.value[nodeType] + if (!existingNodeDef) return + + // Remove old display name mapping + delete nodeDefsByDisplayName.value[existingNodeDef.display_name] + + // Clone and mutate like bookmark store does to preserve references + const clonedNodeDef = _.clone(existingNodeDef) as any + clonedNodeDef.display_name = newDisplayName + + // Update both mappings + nodeDefsByName.value[nodeType] = clonedNodeDef + nodeDefsByDisplayName.value[newDisplayName] = clonedNodeDef + + // Also update the LiteGraph registered node type's static title + // This is necessary for new instances to use the updated display name + const RegisteredNodeClass = LiteGraph.registered_node_types[nodeType] + if (RegisteredNodeClass) { + RegisteredNodeClass.title = newDisplayName + } + } function fromLGraphNode(node: LGraphNode): ComfyNodeDefImpl | null { // Frontend-only nodes don't have nodeDef // @ts-expect-error Optional chaining used in index @@ -324,6 +346,7 @@ export const useNodeDefStore = defineStore('nodeDef', () => { updateNodeDefs, addNodeDef, + updateNodeDefDisplayName, fromLGraphNode } }) diff --git a/tests-ui/tests/store/nodeDefStore.test.ts b/tests-ui/tests/store/nodeDefStore.test.ts new file mode 100644 index 000000000..99fe305be --- /dev/null +++ b/tests-ui/tests/store/nodeDefStore.test.ts @@ -0,0 +1,142 @@ +import { LiteGraph } from '@comfyorg/litegraph' +import { createPinia, setActivePinia } from 'pinia' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import type { ComfyNodeDef as ComfyNodeDefV1 } from '@/schemas/nodeDefSchema' +import { ComfyNodeDefImpl, useNodeDefStore } from '@/stores/nodeDefStore' + +// Mock LiteGraph +vi.mock('@comfyorg/litegraph', async () => { + const actual = await vi.importActual('@comfyorg/litegraph') + return { + ...actual, + LiteGraph: { + registered_node_types: {} + } + } +}) + +describe('useNodeDefStore', () => { + let store: ReturnType + + beforeEach(() => { + // Set up Pinia + setActivePinia(createPinia()) + // Create fresh store instance + store = useNodeDefStore() + // Clear registered node types + LiteGraph.registered_node_types = {} + }) + + describe('updateNodeDefDisplayName', () => { + const createTestNodeDef = ( + name: string, + displayName: string + ): ComfyNodeDefV1 => ({ + name, + display_name: displayName, + category: 'test', + python_module: 'test_module', + description: 'Test node', + input: { required: {}, optional: {} }, + output: [], + output_node: false + }) + + it('should update display name in nodeDefsByName mapping', () => { + const nodeDef = createTestNodeDef('TestNode', 'Original Display Name') + store.addNodeDef(nodeDef) + + store.updateNodeDefDisplayName('TestNode', 'New Display Name') + + const updatedNodeDef = store.nodeDefsByName['TestNode'] + expect(updatedNodeDef.display_name).toBe('New Display Name') + }) + + it('should update nodeDefsByDisplayName mapping', () => { + const nodeDef = createTestNodeDef('TestNode', 'Original Display Name') + store.addNodeDef(nodeDef) + + store.updateNodeDefDisplayName('TestNode', 'New Display Name') + + // Old display name should be removed + expect( + store.nodeDefsByDisplayName['Original Display Name'] + ).toBeUndefined() + // New display name should be added + expect(store.nodeDefsByDisplayName['New Display Name']).toBeDefined() + expect(store.nodeDefsByDisplayName['New Display Name'].name).toBe( + 'TestNode' + ) + }) + + it('should update the object while preserving clone pattern', () => { + const nodeDef = createTestNodeDef('TestNode', 'Original Display Name') + store.addNodeDef(nodeDef) + + const originalRef = store.nodeDefsByName['TestNode'] + store.updateNodeDefDisplayName('TestNode', 'New Display Name') + const updatedRef = store.nodeDefsByName['TestNode'] + + // Clone creates a new reference but preserves properties + expect(updatedRef).not.toBe(originalRef) + expect(updatedRef.display_name).toBe('New Display Name') + expect(updatedRef.name).toBe('TestNode') + expect(updatedRef.category).toBe('test') + }) + + it('should update LiteGraph registered node type title', () => { + const nodeDef = createTestNodeDef('TestNode', 'Original Display Name') + store.addNodeDef(nodeDef) + + // Simulate a registered LiteGraph node type + const mockNodeClass = { title: 'Original Display Name' } + LiteGraph.registered_node_types['TestNode'] = mockNodeClass as any + + store.updateNodeDefDisplayName('TestNode', 'New Display Name') + + expect(mockNodeClass.title).toBe('New Display Name') + }) + + it('should handle non-existent node type gracefully', () => { + // Should not throw when updating non-existent node + expect(() => { + store.updateNodeDefDisplayName('NonExistentNode', 'New Display Name') + }).not.toThrow() + }) + + it('should handle missing LiteGraph registered type gracefully', () => { + const nodeDef = createTestNodeDef('TestNode', 'Original Display Name') + store.addNodeDef(nodeDef) + + // No registered LiteGraph type + expect(LiteGraph.registered_node_types['TestNode']).toBeUndefined() + + // Should not throw + expect(() => { + store.updateNodeDefDisplayName('TestNode', 'New Display Name') + }).not.toThrow() + + // Store should still be updated + expect(store.nodeDefsByName['TestNode'].display_name).toBe( + 'New Display Name' + ) + }) + + it('should work with ComfyNodeDefImpl instances', () => { + const nodeDef = createTestNodeDef('TestNode', 'Original Display Name') + const nodeDefImpl = new ComfyNodeDefImpl(nodeDef) + + // Manually add to store mappings + store.nodeDefsByName['TestNode'] = nodeDefImpl + store.nodeDefsByDisplayName['Original Display Name'] = nodeDefImpl + + store.updateNodeDefDisplayName('TestNode', 'New Display Name') + + expect(store.nodeDefsByName['TestNode'].display_name).toBe( + 'New Display Name' + ) + expect(store.nodeDefsByDisplayName['New Display Name']).toBeDefined() + }) + }) +}) \ No newline at end of file