From a89fa5a78411aceda67f24363f5533f56d8be1d4 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Mon, 15 Dec 2025 07:39:18 -0800 Subject: [PATCH] fix: "convert to subgraph" not shown in context menu if subgraph inside the selection context (#7470) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/Comfy-Org/ComfyUI_frontend/issues/7453. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7470-fix-convert-to-subgraph-not-shown-in-context-menu-if-subgraph-inside-the-selection-con-2c96d73d36508146a475e8d39c64183c) by [Unito](https://www.unito.io) --- src/composables/graph/useMoreOptionsMenu.ts | 7 +- .../graph/useSelectionMenuOptions.test.ts | 106 ++++++++++++++++++ .../graph/useSelectionMenuOptions.ts | 37 ++++-- 3 files changed, 137 insertions(+), 13 deletions(-) create mode 100644 src/composables/graph/useSelectionMenuOptions.test.ts diff --git a/src/composables/graph/useMoreOptionsMenu.ts b/src/composables/graph/useMoreOptionsMenu.ts index 29d6fec91..e4f1224da 100644 --- a/src/composables/graph/useMoreOptionsMenu.ts +++ b/src/composables/graph/useMoreOptionsMenu.ts @@ -177,7 +177,12 @@ export function useMoreOptionsMenu() { } // Section 5: Subgraph operations - options.push(...getSubgraphOptions(hasSubgraphsSelected)) + options.push( + ...getSubgraphOptions({ + hasSubgraphs: hasSubgraphsSelected, + hasMultipleSelection: hasMultipleNodes.value + }) + ) // Section 6: Multiple nodes operations if (hasMultipleNodes.value) { diff --git a/src/composables/graph/useSelectionMenuOptions.test.ts b/src/composables/graph/useSelectionMenuOptions.test.ts new file mode 100644 index 000000000..8600e85ee --- /dev/null +++ b/src/composables/graph/useSelectionMenuOptions.test.ts @@ -0,0 +1,106 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import { useSelectionMenuOptions } from '@/composables/graph/useSelectionMenuOptions' + +const subgraphMocks = vi.hoisted(() => ({ + convertToSubgraph: vi.fn(), + unpackSubgraph: vi.fn(), + addSubgraphToLibrary: vi.fn(), + createI18nMock: vi.fn(() => ({ + global: { + t: vi.fn(), + te: vi.fn(), + d: vi.fn() + } + })) +})) + +vi.mock('vue-i18n', () => ({ + useI18n: () => ({ + t: (key: string) => key + }), + createI18n: subgraphMocks.createI18nMock +})) + +vi.mock('@/composables/graph/useSelectionOperations', () => ({ + useSelectionOperations: () => ({ + copySelection: vi.fn(), + duplicateSelection: vi.fn(), + deleteSelection: vi.fn(), + renameSelection: vi.fn() + }) +})) + +vi.mock('@/composables/graph/useNodeArrangement', () => ({ + useNodeArrangement: () => ({ + alignOptions: [{ localizedName: 'align-left', icon: 'align-left' }], + distributeOptions: [{ localizedName: 'distribute', icon: 'distribute' }], + applyAlign: vi.fn(), + applyDistribute: vi.fn() + }) +})) + +vi.mock('@/composables/graph/useSubgraphOperations', () => ({ + useSubgraphOperations: () => ({ + convertToSubgraph: subgraphMocks.convertToSubgraph, + unpackSubgraph: subgraphMocks.unpackSubgraph, + addSubgraphToLibrary: subgraphMocks.addSubgraphToLibrary + }) +})) + +vi.mock('@/composables/graph/useFrameNodes', () => ({ + useFrameNodes: () => ({ + frameNodes: vi.fn() + }) +})) + +describe('useSelectionMenuOptions - subgraph options', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('returns only convert option when no subgraphs are selected', () => { + const { getSubgraphOptions } = useSelectionMenuOptions() + const options = getSubgraphOptions({ + hasSubgraphs: false, + hasMultipleSelection: true + }) + + expect(options).toHaveLength(1) + expect(options[0]?.label).toBe('contextMenu.Convert to Subgraph') + expect(options[0]?.action).toBe(subgraphMocks.convertToSubgraph) + }) + + it('includes convert, add to library, and unpack when subgraphs are selected', () => { + const { getSubgraphOptions } = useSelectionMenuOptions() + const options = getSubgraphOptions({ + hasSubgraphs: true, + hasMultipleSelection: true + }) + const labels = options.map((option) => option.label) + + expect(labels).toContain('contextMenu.Convert to Subgraph') + expect(labels).toContain('contextMenu.Add Subgraph to Library') + expect(labels).toContain('contextMenu.Unpack Subgraph') + + const convertOption = options.find( + (option) => option.label === 'contextMenu.Convert to Subgraph' + ) + expect(convertOption?.action).toBe(subgraphMocks.convertToSubgraph) + }) + + it('hides convert option when only a single subgraph is selected', () => { + const { getSubgraphOptions } = useSelectionMenuOptions() + const options = getSubgraphOptions({ + hasSubgraphs: true, + hasMultipleSelection: false + }) + + const labels = options.map((option) => option.label) + expect(labels).not.toContain('contextMenu.Convert to Subgraph') + expect(labels).toEqual([ + 'contextMenu.Add Subgraph to Library', + 'contextMenu.Unpack Subgraph' + ]) + }) +}) diff --git a/src/composables/graph/useSelectionMenuOptions.ts b/src/composables/graph/useSelectionMenuOptions.ts index 55e4ec109..e896261a5 100644 --- a/src/composables/graph/useSelectionMenuOptions.ts +++ b/src/composables/graph/useSelectionMenuOptions.ts @@ -63,9 +63,29 @@ export function useSelectionMenuOptions() { } ] - const getSubgraphOptions = (hasSubgraphs: boolean): MenuOption[] => { + const getSubgraphOptions = ({ + hasSubgraphs, + hasMultipleSelection + }: { + hasSubgraphs: boolean + hasMultipleSelection: boolean + }): MenuOption[] => { + const convertOption: MenuOption = { + label: t('contextMenu.Convert to Subgraph'), + icon: 'icon-[lucide--shrink]', + action: convertToSubgraph, + badge: BadgeVariant.NEW + } + + const options: MenuOption[] = [] + const showConvertOption = !hasSubgraphs || hasMultipleSelection + + if (showConvertOption) { + options.push(convertOption) + } + if (hasSubgraphs) { - return [ + options.push( { label: t('contextMenu.Add Subgraph to Library'), icon: 'icon-[lucide--folder-plus]', @@ -76,17 +96,10 @@ export function useSelectionMenuOptions() { icon: 'icon-[lucide--expand]', action: unpackSubgraph } - ] - } else { - return [ - { - label: t('contextMenu.Convert to Subgraph'), - icon: 'icon-[lucide--shrink]', - action: convertToSubgraph, - badge: BadgeVariant.NEW - } - ] + ) } + + return options } const getMultipleNodesOptions = (): MenuOption[] => {