diff --git a/browser_tests/tests/selectionToolbox.spec.ts-snapshots/selection-toolbox-single-node-no-border-chromium-linux.png b/browser_tests/tests/selectionToolbox.spec.ts-snapshots/selection-toolbox-single-node-no-border-chromium-linux.png index d013577553..2d65e3cdb1 100644 Binary files a/browser_tests/tests/selectionToolbox.spec.ts-snapshots/selection-toolbox-single-node-no-border-chromium-linux.png and b/browser_tests/tests/selectionToolbox.spec.ts-snapshots/selection-toolbox-single-node-no-border-chromium-linux.png differ diff --git a/browser_tests/tests/selectionToolboxActions.spec.ts b/browser_tests/tests/selectionToolboxActions.spec.ts index ff3c7551e7..6f268b8c54 100644 --- a/browser_tests/tests/selectionToolboxActions.spec.ts +++ b/browser_tests/tests/selectionToolboxActions.spec.ts @@ -58,16 +58,44 @@ test.describe('Selection Toolbox - Button Actions', { tag: '@ui' }, () => { expect(newCount).toBe(initialCount - 1) }) - test('info button opens properties panel', async ({ comfyPage }) => { + test('info button opens the right-side info tab in new menu mode', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Top') + await comfyPage.settings.setSetting('Comfy.NodeLibrary.NewDesign', true) + await comfyPage.settings.setSetting('Comfy.RightSidePanel.IsOpen', false) + const nodeRef = (await comfyPage.nodeOps.getNodeRefsByTitle('KSampler'))[0] await selectNodeWithPan(comfyPage, nodeRef) + await expect(comfyPage.menu.propertiesPanel.root).toBeHidden() const infoButton = comfyPage.page.getByTestId('info-button') await expect(infoButton).toBeVisible() - await infoButton.click({ force: true }) - await comfyPage.nextFrame() + await infoButton.click() - await expect(comfyPage.page.getByTestId('properties-panel')).toBeVisible() + const panel = comfyPage.menu.propertiesPanel.root + await expect(panel).toBeVisible() + await expect(panel.getByTestId('panel-tab-info')).toHaveAttribute( + 'aria-selected', + 'true' + ) + await expect(panel).toContainText('KSampler') + await expect(comfyPage.menu.nodeLibraryTab.selectedTabButton).toBeHidden() + }) + + test('info button is hidden when the new menu is disabled', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.UseNewMenu', 'Disabled') + await comfyPage.settings.setSetting('Comfy.NodeLibrary.NewDesign', false) + + const nodeRef = (await comfyPage.nodeOps.getNodeRefsByTitle('KSampler'))[0] + await selectNodeWithPan(comfyPage, nodeRef) + + await expect(comfyPage.selectionToolbox).toBeVisible() + await expect( + comfyPage.selectionToolbox.getByTestId('info-button') + ).toBeHidden() }) test('convert-to-subgraph button visible with multi-select', async ({ diff --git a/browser_tests/tests/selectionToolboxSubmenus.spec.ts b/browser_tests/tests/selectionToolboxSubmenus.spec.ts index 0b906e68bd..a29f6a0495 100644 --- a/browser_tests/tests/selectionToolboxSubmenus.spec.ts +++ b/browser_tests/tests/selectionToolboxSubmenus.spec.ts @@ -80,14 +80,16 @@ test.describe( throw new Error('Could not open More Options menu - popover not showing') } - test('opens Node Info from More Options menu', async ({ comfyPage }) => { + test('hides Node Info from More Options menu when the new menu is disabled', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.NodeLibrary.NewDesign', false) + await openMoreOptions(comfyPage) - const nodeInfoButton = comfyPage.page.getByText('Node Info', { - exact: true + const nodeInfoButton = comfyPage.page.getByRole('menuitem', { + name: 'Node Info' }) - await expect(nodeInfoButton).toBeVisible() - await nodeInfoButton.click() - await comfyPage.nextFrame() + await expect(nodeInfoButton).toBeHidden() }) test('changes node shape via Shape submenu', async ({ comfyPage }) => { diff --git a/browser_tests/tests/vueNodes/interactions/node/contextMenu.spec.ts b/browser_tests/tests/vueNodes/interactions/node/contextMenu.spec.ts index 48f8a7ab37..928772dc5d 100644 --- a/browser_tests/tests/vueNodes/interactions/node/contextMenu.spec.ts +++ b/browser_tests/tests/vueNodes/interactions/node/contextMenu.spec.ts @@ -83,6 +83,24 @@ test.describe('Vue Node Context Menu', () => { await expect(renamedNode).toBeVisible() }) + test('should open node info in the right side panel via context menu', async ({ + comfyPage + }) => { + await comfyPage.settings.setSetting('Comfy.RightSidePanel.IsOpen', false) + await expect(comfyPage.menu.propertiesPanel.root).toBeHidden() + + await openContextMenu(comfyPage, 'KSampler') + await clickExactMenuItem(comfyPage, 'Node Info') + + const panel = comfyPage.menu.propertiesPanel.root + await expect(panel).toBeVisible() + await expect(panel.getByTestId('panel-tab-info')).toHaveAttribute( + 'aria-selected', + 'true' + ) + await expect(comfyPage.menu.nodeLibraryTab.selectedTabButton).toBeHidden() + }) + test('should copy and paste node via context menu', async ({ comfyPage }) => { diff --git a/src/components/graph/SelectionToolbox.test.ts b/src/components/graph/SelectionToolbox.test.ts index ce879322f4..9136cd7ccc 100644 --- a/src/components/graph/SelectionToolbox.test.ts +++ b/src/components/graph/SelectionToolbox.test.ts @@ -1,6 +1,7 @@ /* eslint-disable testing-library/no-container, testing-library/no-node-access */ +import { createTestingPinia } from '@pinia/testing' import { fireEvent, render } from '@testing-library/vue' -import { createPinia, setActivePinia } from 'pinia' +import { setActivePinia } from 'pinia' import PrimeVue from 'primevue/config' import { beforeEach, describe, expect, it, vi } from 'vitest' import { createI18n } from 'vue-i18n' @@ -29,6 +30,26 @@ function createMockExtensionService(): ReturnType { > } +const { settingGetMock } = vi.hoisted(() => ({ + settingGetMock: vi.fn() +})) + +const defaultSettingValues: Record = { + 'Comfy.UseNewMenu': 'Top', + 'Comfy.NodeLibrary.NewDesign': true, + 'Comfy.Load3D.3DViewerEnable': true +} + +function mockSettingValues(overrides: Record = {}) { + const settingValues = { + ...defaultSettingValues, + ...overrides + } + settingGetMock.mockImplementation( + (key: string): unknown => settingValues[key] ?? null + ) +} + // Mock the composables and services vi.mock('@/renderer/core/canvas/useCanvasInteractions', () => ({ useCanvasInteractions: vi.fn(() => ({ @@ -79,10 +100,7 @@ vi.mock('@/utils/nodeFilterUtil', () => ({ vi.mock('@/platform/settings/settingStore', () => ({ useSettingStore: () => ({ - get: vi.fn((key: string) => { - if (key === 'Comfy.Load3D.3DViewerEnable') return true - return null - }) + get: settingGetMock }) })) @@ -128,7 +146,7 @@ describe('SelectionToolbox', () => { } beforeEach(() => { - setActivePinia(createPinia()) + setActivePinia(createTestingPinia({ createSpy: vi.fn, stubActions: false })) canvasStore = useCanvasStore() nodeDefMock = { type: 'TestNode', @@ -139,6 +157,7 @@ describe('SelectionToolbox', () => { canvasStore.canvas = createMockCanvas() vi.resetAllMocks() + mockSettingValues() }) function renderComponent(props = {}): { container: Element } { @@ -231,6 +250,42 @@ describe('SelectionToolbox', () => { expect(container.querySelector('.info-button')).toBeFalsy() }) + it('should not show info button when legacy menu uses the new node library', () => { + mockSettingValues({ + 'Comfy.UseNewMenu': 'Disabled', + 'Comfy.NodeLibrary.NewDesign': true + }) + canvasStore.selectedItems = [createMockPositionable()] + + const { container } = renderComponent() + + expect(container.querySelector('.info-button')).toBeFalsy() + }) + + it('should not show info button when legacy menu uses the legacy node library', () => { + mockSettingValues({ + 'Comfy.UseNewMenu': 'Disabled', + 'Comfy.NodeLibrary.NewDesign': false + }) + canvasStore.selectedItems = [createMockPositionable()] + + const { container } = renderComponent() + + expect(container.querySelector('.info-button')).toBeFalsy() + }) + + it('should show info button when new menu uses the legacy node library', () => { + mockSettingValues({ + 'Comfy.UseNewMenu': 'Top', + 'Comfy.NodeLibrary.NewDesign': false + }) + canvasStore.selectedItems = [createMockPositionable()] + + const { container } = renderComponent() + + expect(container.querySelector('.info-button')).toBeTruthy() + }) + it('should show color picker for all selections', () => { // Single node selection canvasStore.selectedItems = [createMockPositionable()] diff --git a/src/components/graph/SelectionToolbox.vue b/src/components/graph/SelectionToolbox.vue index ac8263b7df..850a807726 100644 --- a/src/components/graph/SelectionToolbox.vue +++ b/src/components/graph/SelectionToolbox.vue @@ -15,8 +15,8 @@ @wheel="canvasInteractions.forwardEventToCanvas" > - - + + @@ -104,9 +104,8 @@ const { isSingleImageNode, hasAny3DNodeSelected, hasOutputNodesSelected, - nodeDef + canOpenNodeInfo } = useSelectionState() -const showInfoButton = computed(() => !!nodeDef.value) const showColorPicker = computed(() => hasAnySelection.value) const showConvertToSubgraph = computed(() => hasAnySelection.value) diff --git a/src/components/graph/selectionToolbox/InfoButton.test.ts b/src/components/graph/selectionToolbox/InfoButton.test.ts index 8acb9d61cd..04a08a0f9f 100644 --- a/src/components/graph/selectionToolbox/InfoButton.test.ts +++ b/src/components/graph/selectionToolbox/InfoButton.test.ts @@ -1,6 +1,5 @@ import { render, screen } from '@testing-library/vue' import userEvent from '@testing-library/user-event' -import { createPinia, setActivePinia } from 'pinia' import PrimeVue from 'primevue/config' import Tooltip from 'primevue/tooltip' import { beforeEach, describe, expect, it, vi } from 'vitest' @@ -9,19 +8,20 @@ import { createI18n } from 'vue-i18n' import InfoButton from '@/components/graph/selectionToolbox/InfoButton.vue' import Button from '@/components/ui/button/Button.vue' -const { openPanelMock } = vi.hoisted(() => ({ - openPanelMock: vi.fn() +const { openNodeInfoMock, trackUiButtonClickedMock } = vi.hoisted(() => ({ + openNodeInfoMock: vi.fn(), + trackUiButtonClickedMock: vi.fn() })) -vi.mock('@/stores/workspace/rightSidePanelStore', () => ({ - useRightSidePanelStore: () => ({ - openPanel: openPanelMock +vi.mock('@/composables/graph/useSelectionState', () => ({ + useSelectionState: () => ({ + openNodeInfo: openNodeInfoMock }) })) vi.mock('@/platform/telemetry', () => ({ useTelemetry: () => ({ - trackUiButtonClicked: vi.fn() + trackUiButtonClicked: trackUiButtonClickedMock }) })) @@ -39,8 +39,8 @@ describe('InfoButton', () => { }) beforeEach(() => { - setActivePinia(createPinia()) vi.clearAllMocks() + openNodeInfoMock.mockReturnValue(true) }) const renderComponent = () => { @@ -53,12 +53,29 @@ describe('InfoButton', () => { }) } - it('should open the info panel on click', async () => { + const clickNodeInfoButton = async () => { const user = userEvent.setup() + await user.click(screen.getByRole('button', { name: 'Node Info' })) + } + + it('should open the node info panel on click', async () => { renderComponent() - await user.click(screen.getByRole('button', { name: 'Node Info' })) + await clickNodeInfoButton() - expect(openPanelMock).toHaveBeenCalledWith('info') + expect(openNodeInfoMock).toHaveBeenCalled() + expect(trackUiButtonClickedMock).toHaveBeenCalledWith({ + button_id: 'selection_toolbox_node_info_opened' + }) + }) + + it('should not track the click when the node info panel is unavailable', async () => { + openNodeInfoMock.mockReturnValue(false) + renderComponent() + + await clickNodeInfoButton() + + expect(openNodeInfoMock).toHaveBeenCalled() + expect(trackUiButtonClickedMock).not.toHaveBeenCalled() }) }) diff --git a/src/components/graph/selectionToolbox/InfoButton.vue b/src/components/graph/selectionToolbox/InfoButton.vue index d836f3ec5b..2215f3df2a 100644 --- a/src/components/graph/selectionToolbox/InfoButton.vue +++ b/src/components/graph/selectionToolbox/InfoButton.vue @@ -15,18 +15,16 @@ diff --git a/src/composables/graph/useMoreOptionsMenu.ts b/src/composables/graph/useMoreOptionsMenu.ts index 35756c2bde..eeb5ad8429 100644 --- a/src/composables/graph/useMoreOptionsMenu.ts +++ b/src/composables/graph/useMoreOptionsMenu.ts @@ -124,8 +124,8 @@ export function useMoreOptionsMenu() { const { selectedItems, selectedNodes, - nodeDef, - showNodeHelp, + canOpenNodeInfo, + openNodeInfo, hasSubgraphs: hasSubgraphsComputed, hasImageNode, hasOutputNodesSelected, @@ -243,8 +243,8 @@ export function useMoreOptionsMenu() { options.push({ type: 'divider' }) // Section 4: Node properties (Node Info, Shape, Color) - if (nodeDef.value) { - options.push(getNodeInfoOption(showNodeHelp)) + if (canOpenNodeInfo.value) { + options.push(getNodeInfoOption(openNodeInfo)) } if (groupContext) { options.push(getGroupColorOptions(groupContext, bump)) diff --git a/src/composables/graph/useNodeMenuOptions.ts b/src/composables/graph/useNodeMenuOptions.ts index 7e8cdfcf4e..e36aabe751 100644 --- a/src/composables/graph/useNodeMenuOptions.ts +++ b/src/composables/graph/useNodeMenuOptions.ts @@ -111,10 +111,10 @@ export function useNodeMenuOptions() { action: runBranch }) - const getNodeInfoOption = (showNodeHelp: () => void): MenuOption => ({ + const getNodeInfoOption = (openNodeInfo: () => boolean): MenuOption => ({ label: t('contextMenu.Node Info'), icon: 'icon-[lucide--info]', - action: showNodeHelp + action: openNodeInfo }) return { diff --git a/src/composables/graph/useSelectionState.test.ts b/src/composables/graph/useSelectionState.test.ts index 278365435e..ea53a25c04 100644 --- a/src/composables/graph/useSelectionState.test.ts +++ b/src/composables/graph/useSelectionState.test.ts @@ -3,9 +3,11 @@ import { setActivePinia } from 'pinia' import { beforeEach, describe, expect, test, vi } from 'vitest' import { useSelectionState } from '@/composables/graph/useSelectionState' -import { useNodeLibrarySidebarTab } from '@/composables/sidebarTabs/useNodeLibrarySidebarTab' import { LGraphEventMode } from '@/lib/litegraph/src/litegraph' +import { useSettingStore } from '@/platform/settings/settingStore' import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' +import { ComfyNodeDefImpl, useNodeDefStore } from '@/stores/nodeDefStore' +import { useRightSidePanelStore } from '@/stores/workspace/rightSidePanelStore' import { isImageNode, isLGraphNode } from '@/utils/litegraphUtil' import { filterOutputNodes } from '@/utils/nodeFilterUtil' import { @@ -13,11 +15,6 @@ import { createMockPositionable } from '@/utils/__tests__/litegraphTestUtils' -// Mock composables -vi.mock('@/composables/sidebarTabs/useNodeLibrarySidebarTab', () => ({ - useNodeLibrarySidebarTab: vi.fn() -})) - vi.mock('@/utils/litegraphUtil', () => ({ isLGraphNode: vi.fn(), isImageNode: vi.fn() @@ -39,6 +36,45 @@ const mockConnection = { isNode: false } +function createMockNodeDef() { + return new ComfyNodeDefImpl({ + name: 'TestNode', + display_name: 'Test Node', + category: 'test', + input: {}, + output: [], + output_name: [], + output_is_list: [], + output_node: false, + python_module: 'nodes', + description: '' + }) +} + +function selectSingleNodeWithNodeDef(id: number) { + const canvasStore = useCanvasStore() + const nodeDefStore = useNodeDefStore() + + canvasStore.$state.selectedItems = [ + createMockLGraphNode({ id, type: 'TestNode' }) + ] + vi.mocked(nodeDefStore.fromLGraphNode).mockReturnValue(createMockNodeDef()) +} + +function mockSettingValues(overrides: Record = {}) { + const settingStore = useSettingStore() + const settingValues: Record = { + 'Comfy.UseNewMenu': 'Top', + 'Comfy.NodeLibrary.NewDesign': true, + 'Comfy.Load3D.3DViewerEnable': false, + ...overrides + } + + vi.mocked(settingStore.get).mockImplementation( + (key: string): unknown => settingValues[key] + ) +} + describe('useSelectionState', () => { beforeEach(() => { vi.clearAllMocks() @@ -49,14 +85,7 @@ describe('useSelectionState', () => { createSpy: vi.fn }) ) - - // Setup mock composables - vi.mocked(useNodeLibrarySidebarTab).mockReturnValue({ - id: 'node-library-tab', - title: 'Node Library', - type: 'custom', - render: () => null - } as ReturnType) + mockSettingValues() // Setup mock utility functions vi.mocked(isLGraphNode).mockImplementation((item: unknown) => { @@ -187,4 +216,83 @@ describe('useSelectionState', () => { expect(newIsPinned).toBe(false) }) }) + + describe('Node Info', () => { + test('should open the right side info panel for a selected node', () => { + const rightSidePanelStore = useRightSidePanelStore() + selectSingleNodeWithNodeDef(8) + + const { canOpenNodeInfo, openNodeInfo } = useSelectionState() + expect(canOpenNodeInfo.value).toBe(true) + openNodeInfo() + + expect(rightSidePanelStore.openPanel).toHaveBeenCalledWith('info') + }) + + test('should not open the right side panel for multiple selected nodes', () => { + const canvasStore = useCanvasStore() + const rightSidePanelStore = useRightSidePanelStore() + canvasStore.$state.selectedItems = [ + createMockLGraphNode({ id: 9, type: 'TestNode' }), + createMockLGraphNode({ id: 10, type: 'TestNode' }) + ] + + const { canOpenNodeInfo, openNodeInfo } = useSelectionState() + expect(canOpenNodeInfo.value).toBe(false) + openNodeInfo() + + expect(rightSidePanelStore.openPanel).not.toHaveBeenCalled() + }) + + test('should open the right side info panel when new menu uses the legacy node library', () => { + const rightSidePanelStore = useRightSidePanelStore() + mockSettingValues({ + 'Comfy.UseNewMenu': 'Top', + 'Comfy.NodeLibrary.NewDesign': false + }) + selectSingleNodeWithNodeDef(11) + + const { canOpenNodeInfo, openNodeInfo } = useSelectionState() + expect(canOpenNodeInfo.value).toBe(true) + + const didOpen = openNodeInfo() + + expect(didOpen).toBe(true) + expect(rightSidePanelStore.openPanel).toHaveBeenCalledWith('info') + }) + + test('should not open node info when legacy menu uses the new node library', () => { + const rightSidePanelStore = useRightSidePanelStore() + mockSettingValues({ + 'Comfy.UseNewMenu': 'Disabled', + 'Comfy.NodeLibrary.NewDesign': true + }) + selectSingleNodeWithNodeDef(12) + + const { canOpenNodeInfo, openNodeInfo } = useSelectionState() + expect(canOpenNodeInfo.value).toBe(false) + + const didOpen = openNodeInfo() + + expect(didOpen).toBe(false) + expect(rightSidePanelStore.openPanel).not.toHaveBeenCalled() + }) + + test('should not open node info when legacy menu uses the legacy node library', () => { + const rightSidePanelStore = useRightSidePanelStore() + mockSettingValues({ + 'Comfy.UseNewMenu': 'Disabled', + 'Comfy.NodeLibrary.NewDesign': false + }) + selectSingleNodeWithNodeDef(13) + + const { canOpenNodeInfo, openNodeInfo } = useSelectionState() + expect(canOpenNodeInfo.value).toBe(false) + + const didOpen = openNodeInfo() + + expect(didOpen).toBe(false) + expect(rightSidePanelStore.openPanel).not.toHaveBeenCalled() + }) + }) }) diff --git a/src/composables/graph/useSelectionState.ts b/src/composables/graph/useSelectionState.ts index 5848eafb42..efd746d2a2 100644 --- a/src/composables/graph/useSelectionState.ts +++ b/src/composables/graph/useSelectionState.ts @@ -1,14 +1,12 @@ import { storeToRefs } from 'pinia' import { computed } from 'vue' -import { useNodeLibrarySidebarTab } from '@/composables/sidebarTabs/useNodeLibrarySidebarTab' import type { LGraphNode } from '@/lib/litegraph/src/litegraph' import { LGraphEventMode, SubgraphNode } from '@/lib/litegraph/src/litegraph' import { useSettingStore } from '@/platform/settings/settingStore' import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' import { useNodeDefStore } from '@/stores/nodeDefStore' -import { useNodeHelpStore } from '@/stores/workspace/nodeHelpStore' -import { useSidebarTabStore } from '@/stores/workspace/sidebarTabStore' +import { useRightSidePanelStore } from '@/stores/workspace/rightSidePanelStore' import { isImageNode, isLGraphNode, isLoad3dNode } from '@/utils/litegraphUtil' import { filterOutputNodes } from '@/utils/nodeFilterUtil' @@ -25,9 +23,8 @@ export interface NodeSelectionState { export function useSelectionState() { const canvasStore = useCanvasStore() const nodeDefStore = useNodeDefStore() - const sidebarTabStore = useSidebarTabStore() - const nodeHelpStore = useNodeHelpStore() - const { id: nodeLibraryTabId } = useNodeLibrarySidebarTab() + const settingStore = useSettingStore() + const rightSidePanelStore = useRightSidePanelStore() const { selectedItems } = storeToRefs(canvasStore) @@ -64,7 +61,7 @@ export function useSelectionState() { ) const hasAny3DNodeSelected = computed(() => { - const enable3DViewer = useSettingStore().get('Comfy.Load3D.3DViewerEnable') + const enable3DViewer = settingStore.get('Comfy.Load3D.3DViewerEnable') return ( selectedNodes.value.length === 1 && selectedNodes.value.some(isLoad3dNode) && @@ -98,34 +95,24 @@ export function useSelectionState() { const computeSelectionFlags = (): NodeSelectionState => computeSelectionStatesFromNodes(selectedNodes.value) - /** Toggle node help sidebar/panel for the single selected node (if any). */ - const showNodeHelp = () => { - const def = nodeDef.value - if (!def) return + const canOpenNodeInfo = computed( + () => + Boolean(nodeDef.value) && + settingStore.get('Comfy.UseNewMenu') !== 'Disabled' + ) - const isSidebarActive = - sidebarTabStore.activeSidebarTabId === nodeLibraryTabId - const currentHelpNode = nodeHelpStore.currentHelpNode - const isSameNodeHelpOpen = - isSidebarActive && - nodeHelpStore.isHelpOpen && - currentHelpNode?.nodePath === def.nodePath - - if (isSameNodeHelpOpen) { - nodeHelpStore.closeHelp() - sidebarTabStore.toggleSidebarTab(nodeLibraryTabId) - return - } - - if (!isSidebarActive) sidebarTabStore.toggleSidebarTab(nodeLibraryTabId) - nodeHelpStore.openHelp(def) + const openNodeInfo = () => { + if (!canOpenNodeInfo.value) return false + rightSidePanelStore.openPanel('info') + return true } return { selectedItems, selectedNodes, nodeDef, - showNodeHelp, + canOpenNodeInfo, + openNodeInfo, hasAny3DNodeSelected, hasAnySelection, hasSingleSelection,