From f951e07cea9cb2314447c63152181f77fe5fff37 Mon Sep 17 00:00:00 2001 From: Christian Byrne Date: Sun, 21 Sep 2025 17:32:12 -0700 Subject: [PATCH] fix bypass hotkey in vue nodes and fix node data instrumentation setup issue when switching to Vue nodes after initial load (#5715) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixed Vue node keybinding target element ID to enable bypass/pin/collapse hotkeys in both LiteGraph and Vue rendering modes. Also fixed a bug when starting in litegraph mode => switching to Vue nodes without reloading => `graph.onTrigger` is set to `undefined` which interferes with proper setup of node data instrumentation, among other things. ## Changes - **What**: Updated keybinding `targetElementId` from `graph-canvas` to `graph-canvas-container` for node manipulation commands (parent of both the canvas and transform pane -- vue nodes container). - **What**: Added conditional `onTrigger` handler restoration in slot layout sync to prevent Vue node manager conflicts ## Review Focus Event handler precedence between Vue nodes and LiteGraph systems during mode switching, ensuring hotkeys work consistently across rendering modes. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5715-fix-bypass-hotkey-in-vue-nodes-and-fix-node-data-instrumentation-setup-issue-when-switchi-2756d73d3650815c8ec8d5e4d06232e3) by [Unito](https://www.unito.io) --- .../tests/vueNodes/nodeStates/bypass.spec.ts | 49 +++++++++++++++++++ src/constants/coreKeybindings.ts | 10 ++-- .../settings/constants/coreSettings.ts | 2 +- .../core/layout/sync/useSlotLayoutSync.ts | 6 ++- src/scripts/app.ts | 5 +- 5 files changed, 64 insertions(+), 8 deletions(-) create mode 100644 browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts diff --git a/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts b/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts new file mode 100644 index 000000000..c80a86503 --- /dev/null +++ b/browser_tests/tests/vueNodes/nodeStates/bypass.spec.ts @@ -0,0 +1,49 @@ +import { + comfyExpect as expect, + comfyPageFixture as test +} from '../../../fixtures/ComfyPage' + +const BYPASS_HOTKEY = 'Control+b' +const BYPASS_CLASS = /before:bg-bypass\/60/ + +test.describe('Vue Node Bypass', () => { + test.beforeEach(async ({ comfyPage }) => { + await comfyPage.setSetting('Comfy.VueNodes.Enabled', true) + await comfyPage.vueNodes.waitForNodes() + }) + + test('should allow toggling bypass on a selected node with hotkey', async ({ + comfyPage + }) => { + const checkpointNode = comfyPage.page.locator('[data-node-id]').filter({ + hasText: 'Load Checkpoint' + }) + await checkpointNode.getByText('Load Checkpoint').click() + await comfyPage.page.keyboard.press(BYPASS_HOTKEY) + await expect(checkpointNode).toHaveClass(BYPASS_CLASS) + + await comfyPage.page.keyboard.press(BYPASS_HOTKEY) + await expect(checkpointNode).not.toHaveClass(BYPASS_CLASS) + }) + + test('should allow toggling bypass on multiple selected nodes with hotkey', async ({ + comfyPage + }) => { + const checkpointNode = comfyPage.page.locator('[data-node-id]').filter({ + hasText: 'Load Checkpoint' + }) + const ksamplerNode = comfyPage.page.locator('[data-node-id]').filter({ + hasText: 'KSampler' + }) + + await checkpointNode.getByText('Load Checkpoint').click() + await ksamplerNode.getByText('KSampler').click({ modifiers: ['Control'] }) + await comfyPage.page.keyboard.press(BYPASS_HOTKEY) + await expect(checkpointNode).toHaveClass(BYPASS_CLASS) + await expect(ksamplerNode).toHaveClass(BYPASS_CLASS) + + await comfyPage.page.keyboard.press(BYPASS_HOTKEY) + await expect(checkpointNode).not.toHaveClass(BYPASS_CLASS) + await expect(ksamplerNode).not.toHaveClass(BYPASS_CLASS) + }) +}) diff --git a/src/constants/coreKeybindings.ts b/src/constants/coreKeybindings.ts index b4245f789..fe2bde835 100644 --- a/src/constants/coreKeybindings.ts +++ b/src/constants/coreKeybindings.ts @@ -122,14 +122,14 @@ export const CORE_KEYBINDINGS: Keybinding[] = [ key: '.' }, commandId: 'Comfy.Canvas.FitView', - targetElementId: 'graph-canvas' + targetElementId: 'graph-canvas-container' }, { combo: { key: 'p' }, commandId: 'Comfy.Canvas.ToggleSelected.Pin', - targetElementId: 'graph-canvas' + targetElementId: 'graph-canvas-container' }, { combo: { @@ -137,7 +137,7 @@ export const CORE_KEYBINDINGS: Keybinding[] = [ alt: true }, commandId: 'Comfy.Canvas.ToggleSelectedNodes.Collapse', - targetElementId: 'graph-canvas' + targetElementId: 'graph-canvas-container' }, { combo: { @@ -145,7 +145,7 @@ export const CORE_KEYBINDINGS: Keybinding[] = [ ctrl: true }, commandId: 'Comfy.Canvas.ToggleSelectedNodes.Bypass', - targetElementId: 'graph-canvas' + targetElementId: 'graph-canvas-container' }, { combo: { @@ -153,7 +153,7 @@ export const CORE_KEYBINDINGS: Keybinding[] = [ ctrl: true }, commandId: 'Comfy.Canvas.ToggleSelectedNodes.Mute', - targetElementId: 'graph-canvas' + targetElementId: 'graph-canvas-container' }, { combo: { diff --git a/src/platform/settings/constants/coreSettings.ts b/src/platform/settings/constants/coreSettings.ts index d592a92f0..4adf2db9d 100644 --- a/src/platform/settings/constants/coreSettings.ts +++ b/src/platform/settings/constants/coreSettings.ts @@ -595,7 +595,7 @@ export const CORE_SETTINGS: SettingParams[] = [ migrateDeprecatedValue: (value: any[]) => { return value.map((keybinding) => { if (keybinding['targetSelector'] === '#graph-canvas') { - keybinding['targetElementId'] = 'graph-canvas' + keybinding['targetElementId'] = 'graph-canvas-container' } return keybinding }) diff --git a/src/renderer/core/layout/sync/useSlotLayoutSync.ts b/src/renderer/core/layout/sync/useSlotLayoutSync.ts index 618d1857f..281199e8b 100644 --- a/src/renderer/core/layout/sync/useSlotLayoutSync.ts +++ b/src/renderer/core/layout/sync/useSlotLayoutSync.ts @@ -134,7 +134,11 @@ export function useSlotLayoutSync() { restoreHandlers = () => { graph.onNodeAdded = origNodeAdded || undefined graph.onNodeRemoved = origNodeRemoved || undefined - graph.onTrigger = origTrigger || undefined + // Only restore onTrigger if Vue nodes are not active + // Vue node manager sets its own onTrigger handler + if (!LiteGraph.vueNodesMode) { + graph.onTrigger = origTrigger || undefined + } graph.onAfterChange = origAfterChange || undefined } diff --git a/src/scripts/app.ts b/src/scripts/app.ts index 43722f9b4..cd085eead 100644 --- a/src/scripts/app.ts +++ b/src/scripts/app.ts @@ -596,7 +596,10 @@ export class ComfyApp { const keybindingStore = useKeybindingStore() const keybinding = keybindingStore.getKeybinding(keyCombo) - if (keybinding && keybinding.targetElementId === 'graph-canvas') { + if ( + keybinding && + keybinding.targetElementId === 'graph-canvas-container' + ) { useCommandStore().execute(keybinding.commandId) this.graph.change()