From 0801778f6047654e3185de6a970898c519124929 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Fri, 19 Sep 2025 14:19:06 -0700 Subject: [PATCH] feat: Add Vue node subgraph title button and fix subgraph navigation with vue nodes (#5572) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Adds subgraph title button to Vue node headers (matching LiteGraph behavior) - Fixes Vue node lifecycle issues during subgraph navigation and tab switching - Extracts reusable `useSubgraphNavigation` composable with callback-based API - Adds comprehensive tests for subgraph functionality - Ensures proper graph context restoration during tab switches https://github.com/user-attachments/assets/fd4ff16a-4071-4da6-903f-b2be8dd6e672 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5572-feat-Add-Vue-node-subgraph-title-button-with-lifecycle-management-26f6d73d365081bfbd9cfd7d2775e1ef) by [Unito](https://www.unito.io) --------- Co-authored-by: Claude Co-authored-by: DrJKL --- src/components/graph/GraphCanvas.vue | 21 ++ src/composables/auth/useCurrentUser.ts | 10 +- src/composables/graph/useVueNodeLifecycle.ts | 18 +- src/renderer/core/canvas/canvasStore.ts | 29 ++- .../vueNodes/components/LGraphNode.vue | 36 ++- .../vueNodes/components/NodeHeader.vue | 45 +++- src/utils/graphTraversalUtil.ts | 17 ++ .../components/NodeHeader.subgraph.test.ts | 223 ++++++++++++++++++ 8 files changed, 382 insertions(+), 17 deletions(-) create mode 100644 tests-ui/tests/renderer/extensions/vueNodes/components/NodeHeader.subgraph.test.ts diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue index 3abf47813b..6ded0e3f82 100644 --- a/src/components/graph/GraphCanvas.vue +++ b/src/components/graph/GraphCanvas.vue @@ -76,6 +76,7 @@ import { useEventListener, whenever } from '@vueuse/core' import { computed, + nextTick, onMounted, onUnmounted, provide, @@ -182,6 +183,26 @@ const viewportCulling = useViewportCulling( ) const nodeEventHandlers = useNodeEventHandlers(vueNodeLifecycle.nodeManager) +const handleVueNodeLifecycleReset = async () => { + if (isVueNodesEnabled.value) { + vueNodeLifecycle.disposeNodeManagerAndSyncs() + await nextTick() + vueNodeLifecycle.initializeNodeManager() + } +} + +watch(() => canvasStore.currentGraph, handleVueNodeLifecycleReset) + +watch( + () => canvasStore.isInSubgraph, + async (newValue, oldValue) => { + if (oldValue && !newValue) { + useWorkflowStore().updateActiveGraph() + } + await handleVueNodeLifecycleReset() + } +) + const nodePositions = vueNodeLifecycle.nodePositions const nodeSizes = vueNodeLifecycle.nodeSizes const allNodes = viewportCulling.allNodes diff --git a/src/composables/auth/useCurrentUser.ts b/src/composables/auth/useCurrentUser.ts index cf89c1069b..2c70be227a 100644 --- a/src/composables/auth/useCurrentUser.ts +++ b/src/composables/auth/useCurrentUser.ts @@ -1,5 +1,4 @@ -import { whenever } from '@vueuse/core' -import { computed } from 'vue' +import { computed, watch } from 'vue' import { useFirebaseAuthActions } from '@/composables/auth/useFirebaseAuthActions' import { t } from '@/i18n' @@ -39,7 +38,12 @@ export const useCurrentUser = () => { callback(resolvedUserInfo.value) } - const stop = whenever(resolvedUserInfo, callback) + const stop = watch(resolvedUserInfo, (value) => { + if (value) { + callback(value) + } + }) + return () => stop() } diff --git a/src/composables/graph/useVueNodeLifecycle.ts b/src/composables/graph/useVueNodeLifecycle.ts index 54296b900a..b481439dda 100644 --- a/src/composables/graph/useVueNodeLifecycle.ts +++ b/src/composables/graph/useVueNodeLifecycle.ts @@ -57,10 +57,12 @@ export function useVueNodeLifecycle(isVueNodesEnabled: Ref) { const isNodeManagerReady = computed(() => nodeManager.value !== null) const initializeNodeManager = () => { - if (!comfyApp.graph || nodeManager.value) return + // Use canvas graph if available (handles subgraph contexts), fallback to app graph + const activeGraph = comfyApp.canvas?.graph || comfyApp.graph + if (!activeGraph || nodeManager.value) return // Initialize the core node manager - const manager = useGraphNodeManager(comfyApp.graph) + const manager = useGraphNodeManager(activeGraph) nodeManager.value = manager cleanupNodeManager.value = manager.cleanup @@ -71,8 +73,8 @@ export function useVueNodeLifecycle(isVueNodesEnabled: Ref) { nodeSizes.value = manager.nodeSizes detectChangesInRAF.value = manager.detectChangesInRAF - // Initialize layout system with existing nodes - const nodes = comfyApp.graph._nodes.map((node: LGraphNode) => ({ + // Initialize layout system with existing nodes from active graph + const nodes = activeGraph._nodes.map((node: LGraphNode) => ({ id: node.id.toString(), pos: [node.pos[0], node.pos[1]] as [number, number], size: [node.size[0], node.size[1]] as [number, number] @@ -80,7 +82,7 @@ export function useVueNodeLifecycle(isVueNodesEnabled: Ref) { layoutStore.initializeFromLiteGraph(nodes) // Seed reroutes into the Layout Store so hit-testing uses the new path - for (const reroute of comfyApp.graph.reroutes.values()) { + for (const reroute of activeGraph.reroutes.values()) { const [x, y] = reroute.pos const parent = reroute.parentId ?? undefined const linkIds = Array.from(reroute.linkIds) @@ -88,7 +90,7 @@ export function useVueNodeLifecycle(isVueNodesEnabled: Ref) { } // Seed existing links into the Layout Store (topology only) - for (const link of comfyApp.graph._links.values()) { + for (const link of activeGraph._links.values()) { layoutMutations.createLink( link.id, link.origin_id, @@ -142,7 +144,9 @@ export function useVueNodeLifecycle(isVueNodesEnabled: Ref) { // Watch for Vue nodes enabled state changes watch( - () => isVueNodesEnabled.value && Boolean(comfyApp.graph), + () => + isVueNodesEnabled.value && + Boolean(comfyApp.canvas?.graph || comfyApp.graph), (enabled) => { if (enabled) { initializeNodeManager() diff --git a/src/renderer/core/canvas/canvasStore.ts b/src/renderer/core/canvas/canvasStore.ts index 6e09d95a3a..371035c08c 100644 --- a/src/renderer/core/canvas/canvasStore.ts +++ b/src/renderer/core/canvas/canvasStore.ts @@ -1,8 +1,10 @@ +import { useEventListener, whenever } from '@vueuse/core' import { defineStore } from 'pinia' import { type Raw, computed, markRaw, ref, shallowRef } from 'vue' import type { Point, Positionable } from '@/lib/litegraph/src/interfaces' import type { + LGraph, LGraphCanvas, LGraphGroup, LGraphNode @@ -94,6 +96,29 @@ export const useCanvasStore = defineStore('canvas', () => { appScalePercentage.value = Math.round(newScale * 100) } + const currentGraph = shallowRef(null) + const isInSubgraph = ref(false) + + whenever( + () => canvas.value, + (newCanvas) => { + useEventListener( + newCanvas.canvas, + 'litegraph:set-graph', + (event: CustomEvent<{ newGraph: LGraph; oldGraph: LGraph }>) => { + const newGraph = event.detail?.newGraph || app.canvas?.graph + currentGraph.value = newGraph + isInSubgraph.value = Boolean(app.canvas?.subgraph) + } + ) + + useEventListener(newCanvas.canvas, 'subgraph-opened', () => { + isInSubgraph.value = true + }) + }, + { immediate: true } + ) + return { canvas, selectedItems, @@ -105,6 +130,8 @@ export const useCanvasStore = defineStore('canvas', () => { getCanvas, setAppZoomFromPercentage, initScaleSync, - cleanupScaleSync + cleanupScaleSync, + currentGraph, + isInSubgraph } }) diff --git a/src/renderer/extensions/vueNodes/components/LGraphNode.vue b/src/renderer/extensions/vueNodes/components/LGraphNode.vue index 478326c83b..bd6480c38a 100644 --- a/src/renderer/extensions/vueNodes/components/LGraphNode.vue +++ b/src/renderer/extensions/vueNodes/components/LGraphNode.vue @@ -55,6 +55,7 @@ :collapsed="isCollapsed" @collapse="handleCollapse" @update:title="handleTitleUpdate" + @enter-subgraph="handleEnterSubgraph" /> @@ -164,7 +165,10 @@ import type { ExecutedWsMessage } from '@/schemas/apiSchema' import { app } from '@/scripts/app' import { useExecutionStore } from '@/stores/executionStore' import { useNodeOutputStore } from '@/stores/imagePreviewStore' -import { getNodeByLocatorId } from '@/utils/graphTraversalUtil' +import { + getLocatorIdFromNodeData, + getNodeByLocatorId +} from '@/utils/graphTraversalUtil' import { cn } from '@/utils/tailwindUtil' import { useVueElementTracking } from '../composables/useVueNodeResizeTracking' @@ -453,14 +457,36 @@ const handleTitleUpdate = (newTitle: string) => { emit('update:title', nodeData.id, newTitle) } +const handleEnterSubgraph = () => { + const graph = app.graph?.rootGraph || app.graph + if (!graph) { + console.warn('LGraphNode: No graph available for subgraph navigation') + return + } + + const locatorId = getLocatorIdFromNodeData(nodeData) + + const litegraphNode = getNodeByLocatorId(graph, locatorId) + + if (!litegraphNode?.isSubgraphNode() || !('subgraph' in litegraphNode)) { + console.warn('LGraphNode: Node is not a valid subgraph node', litegraphNode) + return + } + + const canvas = app.canvas + if (!canvas || typeof canvas.openSubgraph !== 'function') { + console.warn('LGraphNode: Canvas or openSubgraph method not available') + return + } + + canvas.openSubgraph(litegraphNode.subgraph) +} + const nodeOutputs = useNodeOutputStore() const nodeImageUrls = ref([]) const onNodeOutputsUpdate = (newOutputs: ExecutedWsMessage['output']) => { - // Construct proper locator ID using subgraph ID from VueNodeData - const locatorId = nodeData.subgraphId - ? `${nodeData.subgraphId}:${nodeData.id}` - : nodeData.id + const locatorId = getLocatorIdFromNodeData(nodeData) // Use root graph for getNodeByLocatorId since it needs to traverse from root const rootGraph = app.graph?.rootGraph || app.graph diff --git a/src/renderer/extensions/vueNodes/components/NodeHeader.vue b/src/renderer/extensions/vueNodes/components/NodeHeader.vue index 0c6ebfc207..3bd75024c6 100644 --- a/src/renderer/extensions/vueNodes/components/NodeHeader.vue +++ b/src/renderer/extensions/vueNodes/components/NodeHeader.vue @@ -4,7 +4,7 @@
@@ -36,17 +36,39 @@ @cancel="handleTitleCancel" />
+ + +
+ + + +
diff --git a/src/utils/graphTraversalUtil.ts b/src/utils/graphTraversalUtil.ts index 72f6f37333..4a573ed24c 100644 --- a/src/utils/graphTraversalUtil.ts +++ b/src/utils/graphTraversalUtil.ts @@ -8,6 +8,23 @@ import { parseNodeLocatorId } from '@/types/nodeIdentification' import { isSubgraphIoNode } from './typeGuardUtil' +interface NodeWithId { + id: string | number + subgraphId?: string | null +} + +/** + * Constructs a locator ID from node data with optional subgraph context. + * + * @param nodeData - Node data containing id and optional subgraphId + * @returns The locator ID string + */ +export function getLocatorIdFromNodeData(nodeData: NodeWithId): string { + return nodeData.subgraphId + ? `${nodeData.subgraphId}:${String(nodeData.id)}` + : String(nodeData.id) +} + /** * Parses an execution ID into its component parts. * diff --git a/tests-ui/tests/renderer/extensions/vueNodes/components/NodeHeader.subgraph.test.ts b/tests-ui/tests/renderer/extensions/vueNodes/components/NodeHeader.subgraph.test.ts new file mode 100644 index 0000000000..cca01bce8d --- /dev/null +++ b/tests-ui/tests/renderer/extensions/vueNodes/components/NodeHeader.subgraph.test.ts @@ -0,0 +1,223 @@ +/** + * Tests for NodeHeader subgraph functionality + */ +import { createTestingPinia } from '@pinia/testing' +import { mount } from '@vue/test-utils' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import type { VueNodeData } from '@/composables/graph/useGraphNodeManager' +import NodeHeader from '@/renderer/extensions/vueNodes/components/NodeHeader.vue' +import { getNodeByLocatorId } from '@/utils/graphTraversalUtil' + +// Mock dependencies +vi.mock('@/scripts/app', () => ({ + app: { + graph: null as any + } +})) + +vi.mock('@/utils/graphTraversalUtil', () => ({ + getNodeByLocatorId: vi.fn(), + getLocatorIdFromNodeData: vi.fn((nodeData) => + nodeData.subgraphId + ? `${nodeData.subgraphId}:${String(nodeData.id)}` + : String(nodeData.id) + ) +})) + +vi.mock('@/composables/useErrorHandling', () => ({ + useErrorHandling: () => ({ + toastErrorHandler: vi.fn() + }) +})) + +vi.mock('vue-i18n', () => ({ + useI18n: () => ({ + t: vi.fn((key) => key) + }), + createI18n: vi.fn(() => ({ + global: { + t: vi.fn((key) => key) + } + })) +})) + +vi.mock('@/i18n', () => ({ + st: vi.fn((key) => key), + t: vi.fn((key) => key), + i18n: { + global: { + t: vi.fn((key) => key) + } + } +})) + +describe('NodeHeader - Subgraph Functionality', () => { + // Helper to setup common mocks + const setupMocks = async (isSubgraph = true, hasGraph = true) => { + const { app } = await import('@/scripts/app') + + if (hasGraph) { + ;(app as any).graph = { rootGraph: {} } + } else { + ;(app as any).graph = null + } + + vi.mocked(getNodeByLocatorId).mockReturnValue({ + isSubgraphNode: () => isSubgraph + } as any) + } + + beforeEach(() => { + vi.clearAllMocks() + }) + + const createMockNodeData = ( + id: string, + subgraphId?: string + ): VueNodeData => ({ + id, + title: 'Test Node', + type: 'TestNode', + mode: 0, + selected: false, + executing: false, + subgraphId, + widgets: [], + inputs: [], + outputs: [], + hasErrors: false, + flags: {} + }) + + const createWrapper = (props = {}) => { + return mount(NodeHeader, { + props, + global: { + plugins: [createTestingPinia({ createSpy: vi.fn })], + mocks: { + $t: vi.fn((key: string) => key), + $primevue: { config: {} } + } + } + }) + } + + it('should show subgraph button for subgraph nodes', async () => { + await setupMocks(true) // isSubgraph = true + + const wrapper = createWrapper({ + nodeData: createMockNodeData('test-node-1'), + readonly: false + }) + + await wrapper.vm.$nextTick() + + const subgraphButton = wrapper.find('[data-testid="subgraph-enter-button"]') + expect(subgraphButton.exists()).toBe(true) + }) + + it('should not show subgraph button for regular nodes', async () => { + await setupMocks(false) // isSubgraph = false + + const wrapper = createWrapper({ + nodeData: createMockNodeData('test-node-1'), + readonly: false + }) + + await wrapper.vm.$nextTick() + + const subgraphButton = wrapper.find('[data-testid="subgraph-enter-button"]') + expect(subgraphButton.exists()).toBe(false) + }) + + it('should not show subgraph button in readonly mode', async () => { + await setupMocks(true) // isSubgraph = true + + const wrapper = createWrapper({ + nodeData: createMockNodeData('test-node-1'), + readonly: true + }) + + await wrapper.vm.$nextTick() + + const subgraphButton = wrapper.find('[data-testid="subgraph-enter-button"]') + expect(subgraphButton.exists()).toBe(false) + }) + + it('should emit enter-subgraph event when button is clicked', async () => { + await setupMocks(true) // isSubgraph = true + + const wrapper = createWrapper({ + nodeData: createMockNodeData('test-node-1'), + readonly: false + }) + + await wrapper.vm.$nextTick() + + const subgraphButton = wrapper.find('[data-testid="subgraph-enter-button"]') + await subgraphButton.trigger('click') + + expect(wrapper.emitted('enter-subgraph')).toBeTruthy() + expect(wrapper.emitted('enter-subgraph')).toHaveLength(1) + }) + + it('should handle subgraph context correctly', async () => { + await setupMocks(true) // isSubgraph = true + + const wrapper = createWrapper({ + nodeData: createMockNodeData('test-node-1', 'subgraph-id'), + readonly: false + }) + + await wrapper.vm.$nextTick() + + // Should call getNodeByLocatorId with correct locator ID + expect(vi.mocked(getNodeByLocatorId)).toHaveBeenCalledWith( + expect.anything(), + 'subgraph-id:test-node-1' + ) + + const subgraphButton = wrapper.find('[data-testid="subgraph-enter-button"]') + expect(subgraphButton.exists()).toBe(true) + }) + + it('should handle missing graph gracefully', async () => { + await setupMocks(true, false) // isSubgraph = true, hasGraph = false + + const wrapper = createWrapper({ + nodeData: createMockNodeData('test-node-1'), + readonly: false + }) + + await wrapper.vm.$nextTick() + + const subgraphButton = wrapper.find('[data-testid="subgraph-enter-button"]') + expect(subgraphButton.exists()).toBe(false) + }) + + it('should prevent event propagation on double click', async () => { + await setupMocks(true) // isSubgraph = true + + const wrapper = createWrapper({ + nodeData: createMockNodeData('test-node-1'), + readonly: false + }) + + await wrapper.vm.$nextTick() + + const subgraphButton = wrapper.find('[data-testid="subgraph-enter-button"]') + + // Mock event object + const mockEvent = { + stopPropagation: vi.fn() + } + + // Trigger dblclick event + await subgraphButton.trigger('dblclick', mockEvent) + + // Should prevent propagation (handled by @dblclick.stop directive) + // This is tested by ensuring the component doesn't error and renders correctly + expect(subgraphButton.exists()).toBe(true) + }) +})