mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-05 15:40:10 +00:00
[feat] sync node library when subgraph titles change
When a subgraph node is renamed, the node library sidebar now immediately reflects the updated display name. This ensures consistency between the canvas, breadcrumb navigation, and node library. Key changes: - Add updateNodeDefDisplayName method to nodeDefStore with reference preservation - Update TitleEditor to call nodeDefStore for subgraph title changes - Organize subgraph browser tests into dedicated folder - Add comprehensive unit test coverage for nodeDefStore
This commit is contained in:
@@ -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)
|
||||
})
|
||||
})
|
||||
|
||||
54
browser_tests/tests/subgraph/subgraphPersistence.spec.ts
Normal file
54
browser_tests/tests/subgraph/subgraphPersistence.spec.ts
Normal file
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
})
|
||||
|
||||
142
tests-ui/tests/store/nodeDefStore.test.ts
Normal file
142
tests-ui/tests/store/nodeDefStore.test.ts
Normal file
@@ -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<typeof useNodeDefStore>
|
||||
|
||||
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()
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user