[backport 1.25] [feat] Make hotkey for exiting subgraphs configurable in user keybindings (#4914)

Co-authored-by: Christian Byrne <cbyrne@comfy.org>
Co-authored-by: github-actions <github-actions@github.com>

browser_tests/tests/subgraph.spec.ts

@@ -528,4 +528,103 @@ test.describe('Subgraph Operations', () => {
       expect(finalCount).toBe(parentCount)
     })
   })
+
+  test.describe('Navigation Hotkeys', () => {
+    test.beforeEach(async ({ comfyPage }) => {
+      await comfyPage.setSetting('Comfy.UseNewMenu', 'Top')
+    })
+
+    test('Navigation hotkey can be customized', async ({ comfyPage }) => {
+      await comfyPage.loadWorkflow('basic-subgraph')
+      await comfyPage.nextFrame()
+
+      // Change the Exit Subgraph keybinding from Escape to Alt+Q
+      await comfyPage.setSetting('Comfy.Keybinding.NewBindings', [
+        {
+          commandId: 'Comfy.Graph.ExitSubgraph',
+          combo: {
+            key: 'q',
+            ctrl: false,
+            alt: true,
+            shift: false
+          }
+        }
+      ])
+
+      await comfyPage.setSetting('Comfy.Keybinding.UnsetBindings', [
+        {
+          commandId: 'Comfy.Graph.ExitSubgraph',
+          combo: {
+            key: 'Escape',
+            ctrl: false,
+            alt: false,
+            shift: false
+          }
+        }
+      ])
+
+      // Reload the page
+      await comfyPage.page.reload()
+      await comfyPage.page.waitForTimeout(1024)
+
+      // Navigate into subgraph
+      const subgraphNode = await comfyPage.getNodeRefById('2')
+      await subgraphNode.navigateIntoSubgraph()
+      await comfyPage.page.waitForSelector(SELECTORS.breadcrumb)
+
+      // Verify we're in a subgraph
+      expect(await isInSubgraph(comfyPage)).toBe(true)
+
+      // Test that Escape no longer exits subgraph
+      await comfyPage.page.keyboard.press('Escape')
+      await comfyPage.nextFrame()
+      if (!(await isInSubgraph(comfyPage))) {
+        throw new Error('Not in subgraph')
+      }
+
+      // Test that Alt+Q now exits subgraph
+      await comfyPage.page.keyboard.press('Alt+q')
+      await comfyPage.nextFrame()
+      expect(await isInSubgraph(comfyPage)).toBe(false)
+    })
+
+    test('Escape prioritizes closing dialogs over exiting subgraph', async ({
+      comfyPage
+    }) => {
+      await comfyPage.loadWorkflow('basic-subgraph')
+      await comfyPage.nextFrame()
+
+      const subgraphNode = await comfyPage.getNodeRefById('2')
+      await subgraphNode.navigateIntoSubgraph()
+      await comfyPage.page.waitForSelector(SELECTORS.breadcrumb)
+
+      // Verify we're in a subgraph
+      if (!(await isInSubgraph(comfyPage))) {
+        throw new Error('Not in subgraph')
+      }
+
+      // Open settings dialog using hotkey
+      await comfyPage.page.keyboard.press('Control+,')
+      await comfyPage.page.waitForSelector('.settings-container', {
+        state: 'visible'
+      })
+
+      // Press Escape - should close dialog, not exit subgraph
+      await comfyPage.page.keyboard.press('Escape')
+      await comfyPage.nextFrame()
+
+      // Dialog should be closed
+      await expect(
+        comfyPage.page.locator('.settings-container')
+      ).not.toBeVisible()
+
+      // Should still be in subgraph
+      expect(await isInSubgraph(comfyPage)).toBe(true)
+
+      // Press Escape again - now should exit subgraph
+      await comfyPage.page.keyboard.press('Escape')
+      await comfyPage.nextFrame()
+      expect(await isInSubgraph(comfyPage)).toBe(false)
+    })
+  })
 })

src/components/breadcrumb/SubgraphBreadcrumb.vue

@@ -32,7 +32,6 @@
 </template>
 
 <script setup lang="ts">
-import { useEventListener } from '@vueuse/core'
 import Breadcrumb from 'primevue/breadcrumb'
 import type { MenuItem } from 'primevue/menuitem'
 import { computed, onUpdated, ref, watch } from 'vue'
@@ -98,18 +97,6 @@ const home = computed(() => ({
   }
 }))
 
-// Escape exits from the current subgraph.
-useEventListener(document, 'keydown', (event) => {
-  if (event.key === 'Escape') {
-    const canvas = useCanvasStore().getCanvas()
-    if (!canvas.graph) throw new TypeError('Canvas has no graph')
-
-    canvas.setGraph(
-      navigationStore.navigationStack.at(-2) ?? canvas.graph.rootGraph
-    )
-  }
-})
-
 // Check for overflow on breadcrumb items and collapse/expand the breadcrumb to fit
 let overflowObserver: ReturnType<typeof useOverflowObserver> | undefined
 watch(breadcrumbElement, (el) => {

src/composables/useCoreCommands.ts

@@ -23,6 +23,7 @@ import { useExecutionStore } from '@/stores/executionStore'
 import { useCanvasStore, useTitleEditorStore } from '@/stores/graphStore'
 import { useQueueSettingsStore, useQueueStore } from '@/stores/queueStore'
 import { useSettingStore } from '@/stores/settingStore'
+import { useSubgraphNavigationStore } from '@/stores/subgraphNavigationStore'
 import { useToastStore } from '@/stores/toastStore'
 import { type ComfyWorkflow, useWorkflowStore } from '@/stores/workflowStore'
 import { useBottomPanelStore } from '@/stores/workspace/bottomPanelStore'
@@ -805,6 +806,21 @@ export function useCoreCommands(): ComfyCommand[] {
       function: () => {
         bottomPanelStore.togglePanel('shortcuts')
       }
+    },
+    {
+      id: 'Comfy.Graph.ExitSubgraph',
+      icon: 'pi pi-arrow-up',
+      label: 'Exit Subgraph',
+      versionAdded: '1.20.1',
+      function: () => {
+        const canvas = useCanvasStore().getCanvas()
+        const navigationStore = useSubgraphNavigationStore()
+        if (!canvas.graph) return
+
+        canvas.setGraph(
+          navigationStore.navigationStack.at(-2) ?? canvas.graph.rootGraph
+        )
+      }
     }
   ]
 

src/constants/coreKeybindings.ts

@@ -190,5 +190,11 @@ export const CORE_KEYBINDINGS: Keybinding[] = [
       key: 'k'
     },
     commandId: 'Workspace.ToggleBottomPanel.Shortcuts'
+  },
+  {
+    combo: {
+      key: 'Escape'
+    },
+    commandId: 'Comfy.Graph.ExitSubgraph'
   }
 ]

src/locales/en/commands.json

@@ -119,6 +119,9 @@
   "Comfy_Graph_ConvertToSubgraph": {
     "label": "Convert Selection to Subgraph"
   },
+  "Comfy_Graph_ExitSubgraph": {
+    "label": "Exit Subgraph"
+  },
   "Comfy_Graph_FitGroupToContents": {
     "label": "Fit Group To Contents"
   },

src/locales/en/main.json

@@ -974,6 +974,7 @@
     "Export (API)": "Export (API)",
     "Give Feedback": "Give Feedback",
     "Convert Selection to Subgraph": "Convert Selection to Subgraph",
+    "Exit Subgraph": "Exit Subgraph",
     "Fit Group To Contents": "Fit Group To Contents",
     "Group Selected Nodes": "Group Selected Nodes",
     "Convert selected nodes to group node": "Convert selected nodes to group node",

src/locales/es/commands.json

@@ -119,6 +119,9 @@
   "Comfy_Graph_ConvertToSubgraph": {
     "label": "Convertir selección en subgrafo"
   },
+  "Comfy_Graph_ExitSubgraph": {
+    "label": "Salir de subgrafo"
+  },
   "Comfy_Graph_FitGroupToContents": {
     "label": "Ajustar grupo al contenido"
   },

src/locales/es/main.json

@@ -765,6 +765,7 @@
     "Desktop User Guide": "Guía de usuario de escritorio",
     "Duplicate Current Workflow": "Duplicar flujo de trabajo actual",
     "Edit": "Editar",
+    "Exit Subgraph": "Salir de subgrafo",
     "Export": "Exportar",
     "Export (API)": "Exportar (API)",
     "Fit Group To Contents": "Ajustar grupo a contenidos",

src/locales/fr/commands.json

@@ -119,6 +119,9 @@
   "Comfy_Graph_ConvertToSubgraph": {
     "label": "Convertir la sélection en sous-graphe"
   },
+  "Comfy_Graph_ExitSubgraph": {
+    "label": "Quitter le sous-graphe"
+  },
   "Comfy_Graph_FitGroupToContents": {
     "label": "Ajuster le groupe au contenu"
   },

src/locales/fr/main.json

@@ -765,6 +765,7 @@
     "Desktop User Guide": "Guide de l'utilisateur de bureau",
     "Duplicate Current Workflow": "Dupliquer le flux de travail actuel",
     "Edit": "Éditer",
+    "Exit Subgraph": "Quitter le sous-graphe",
     "Export": "Exporter",
     "Export (API)": "Exporter (API)",
     "Fit Group To Contents": "Ajuster le groupe au contenu",

src/locales/ja/commands.json

@@ -119,6 +119,9 @@
   "Comfy_Graph_ConvertToSubgraph": {
     "label": "選択範囲をサブグラフに変換"
   },
+  "Comfy_Graph_ExitSubgraph": {
+    "label": "サブグラフを終了"
+  },
   "Comfy_Graph_FitGroupToContents": {
     "label": "グループを内容に合わせて調整"
   },

src/locales/ja/main.json

@@ -765,6 +765,7 @@
     "Desktop User Guide": "デスクトップユーザーガイド",
     "Duplicate Current Workflow": "現在のワークフローを複製",
     "Edit": "編集",
+    "Exit Subgraph": "サブグラフを終了",
     "Export": "エクスポート",
     "Export (API)": "エクスポート (API)",
     "Fit Group To Contents": "グループを内容に合わせる",

src/locales/ko/commands.json

@@ -119,6 +119,9 @@
   "Comfy_Graph_ConvertToSubgraph": {
     "label": "선택 영역을 서브그래프로 변환"
   },
+  "Comfy_Graph_ExitSubgraph": {
+    "label": "서브그래프 종료"
+  },
   "Comfy_Graph_FitGroupToContents": {
     "label": "그룹을 내용에 맞게 맞추기"
   },

src/locales/ko/main.json

@@ -765,6 +765,7 @@
     "Desktop User Guide": "데스크톱 사용자 가이드",
     "Duplicate Current Workflow": "현재 워크플로 복제",
     "Edit": "편집",
+    "Exit Subgraph": "서브그래프 종료",
     "Export": "내보내기",
     "Export (API)": "내보내기 (API)",
     "Fit Group To Contents": "그룹을 내용에 맞게 조정",

src/locales/ru/commands.json

@@ -119,6 +119,9 @@
   "Comfy_Graph_ConvertToSubgraph": {
     "label": "Преобразовать выделенное в подграф"
   },
+  "Comfy_Graph_ExitSubgraph": {
+    "label": "Выйти из подграфа"
+  },
   "Comfy_Graph_FitGroupToContents": {
     "label": "Подогнать группу к содержимому"
   },

src/locales/ru/main.json

@@ -765,6 +765,7 @@
     "Desktop User Guide": "Руководство пользователя для настольных ПК",
     "Duplicate Current Workflow": "Дублировать текущий рабочий процесс",
     "Edit": "Редактировать",
+    "Exit Subgraph": "Выйти из подграфа",
     "Export": "Экспортировать",
     "Export (API)": "Экспорт (API)",
     "Fit Group To Contents": "Подогнать группу под содержимое",

src/locales/zh-TW/commands.json

@@ -119,6 +119,9 @@
   "Comfy_Graph_ConvertToSubgraph": {
     "label": "將選取內容轉換為子圖"
   },
+  "Comfy_Graph_ExitSubgraph": {
+    "label": "離開子圖"
+  },
   "Comfy_Graph_FitGroupToContents": {
     "label": "調整群組以符合內容"
   },

src/locales/zh-TW/main.json

@@ -765,6 +765,7 @@
     "Desktop User Guide": "桌面應用程式使用指南",
     "Duplicate Current Workflow": "複製目前工作流程",
     "Edit": "編輯",
+    "Exit Subgraph": "離開子圖",
     "Export": "匯出",
     "Export (API)": "匯出（API）",
     "Fit Group To Contents": "群組貼合內容",

src/locales/zh/commands.json

@@ -119,6 +119,9 @@
   "Comfy_Graph_ConvertToSubgraph": {
     "label": "将选区转换为子图"
   },
+  "Comfy_Graph_ExitSubgraph": {
+    "label": "退出子圖"
+  },
   "Comfy_Graph_FitGroupToContents": {
     "label": "适应节点框到内容"
   },

src/locales/zh/main.json

@@ -765,6 +765,7 @@
     "Desktop User Guide": "桌面端用户指南",
     "Duplicate Current Workflow": "复制当前工作流",
     "Edit": "编辑",
+    "Exit Subgraph": "退出子圖",
     "Export": "导出",
     "Export (API)": "导出 (API)",
     "Fit Group To Contents": "适应组内容",

src/services/keybindingService.ts

@@ -1,5 +1,6 @@
 import { CORE_KEYBINDINGS } from '@/constants/coreKeybindings'
 import { useCommandStore } from '@/stores/commandStore'
+import { useDialogStore } from '@/stores/dialogStore'
 import {
   KeyComboImpl,
   KeybindingImpl,
@@ -11,6 +12,7 @@ export const useKeybindingService = () => {
   const keybindingStore = useKeybindingStore()
   const commandStore = useCommandStore()
   const settingStore = useSettingStore()
+  const dialogStore = useDialogStore()
 
   const keybindHandler = async function (event: KeyboardEvent) {
     const keyCombo = KeyComboImpl.fromEvent(event)
@@ -32,6 +34,19 @@ export const useKeybindingService = () => {
 
     const keybinding = keybindingStore.getKeybinding(keyCombo)
     if (keybinding && keybinding.targetElementId !== 'graph-canvas') {
+      // Special handling for Escape key - let dialogs handle it first
+      if (
+        event.key === 'Escape' &&
+        !event.ctrlKey &&
+        !event.altKey &&
+        !event.metaKey
+      ) {
+        // If dialogs are open, don't execute the keybinding - let the dialog handle it
+        if (dialogStore.dialogStack.length > 0) {
+          return
+        }
+      }
+
       // Prevent default browser behavior first, then execute the command
       event.preventDefault()
       await commandStore.execute(keybinding.commandId)

tests-ui/tests/services/keybindingService.escape.test.ts

@@ -0,0 +1,202 @@
+import { createPinia, setActivePinia } from 'pinia'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { CORE_KEYBINDINGS } from '@/constants/coreKeybindings'
+import { useKeybindingService } from '@/services/keybindingService'
+import { useCommandStore } from '@/stores/commandStore'
+import { useDialogStore } from '@/stores/dialogStore'
+import {
+  KeyComboImpl,
+  KeybindingImpl,
+  useKeybindingStore
+} from '@/stores/keybindingStore'
+
+// Mock stores
+vi.mock('@/stores/settingStore', () => ({
+  useSettingStore: vi.fn(() => ({
+    get: vi.fn(() => [])
+  }))
+}))
+
+vi.mock('@/stores/dialogStore', () => ({
+  useDialogStore: vi.fn(() => ({
+    dialogStack: []
+  }))
+}))
+
+describe('keybindingService - Escape key handling', () => {
+  let keybindingService: ReturnType<typeof useKeybindingService>
+  let mockCommandExecute: ReturnType<typeof vi.fn>
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    setActivePinia(createPinia())
+
+    // Mock command store execute
+    mockCommandExecute = vi.fn()
+    const commandStore = useCommandStore()
+    commandStore.execute = mockCommandExecute
+
+    // Reset dialog store mock to empty
+    vi.mocked(useDialogStore).mockReturnValue({
+      dialogStack: []
+    } as any)
+
+    keybindingService = useKeybindingService()
+    keybindingService.registerCoreKeybindings()
+  })
+
+  it('should register Escape key for ExitSubgraph command', () => {
+    const keybindingStore = useKeybindingStore()
+
+    // Check that the Escape keybinding exists in core keybindings
+    const escapeKeybinding = CORE_KEYBINDINGS.find(
+      (kb) =>
+        kb.combo.key === 'Escape' && kb.commandId === 'Comfy.Graph.ExitSubgraph'
+    )
+    expect(escapeKeybinding).toBeDefined()
+
+    // Check that it was registered in the store
+    const registeredBinding = keybindingStore.getKeybinding(
+      new KeyComboImpl({ key: 'Escape' })
+    )
+    expect(registeredBinding).toBeDefined()
+    expect(registeredBinding?.commandId).toBe('Comfy.Graph.ExitSubgraph')
+  })
+
+  it('should execute ExitSubgraph command when Escape is pressed', async () => {
+    const event = new KeyboardEvent('keydown', {
+      key: 'Escape',
+      bubbles: true,
+      cancelable: true
+    })
+
+    // Mock event methods
+    event.preventDefault = vi.fn()
+    event.composedPath = vi.fn(() => [document.body])
+
+    await keybindingService.keybindHandler(event)
+
+    expect(event.preventDefault).toHaveBeenCalled()
+    expect(mockCommandExecute).toHaveBeenCalledWith('Comfy.Graph.ExitSubgraph')
+  })
+
+  it('should not execute command when Escape is pressed with modifiers', async () => {
+    const event = new KeyboardEvent('keydown', {
+      key: 'Escape',
+      ctrlKey: true,
+      bubbles: true,
+      cancelable: true
+    })
+
+    event.preventDefault = vi.fn()
+    event.composedPath = vi.fn(() => [document.body])
+
+    await keybindingService.keybindHandler(event)
+
+    expect(mockCommandExecute).not.toHaveBeenCalled()
+  })
+
+  it('should not execute command when typing in input field', async () => {
+    const inputElement = document.createElement('input')
+    const event = new KeyboardEvent('keydown', {
+      key: 'Escape',
+      bubbles: true,
+      cancelable: true
+    })
+
+    event.preventDefault = vi.fn()
+    event.composedPath = vi.fn(() => [inputElement])
+
+    await keybindingService.keybindHandler(event)
+
+    expect(mockCommandExecute).not.toHaveBeenCalled()
+  })
+
+  it('should close dialogs when no keybinding is registered', async () => {
+    // Remove the Escape keybinding
+    const keybindingStore = useKeybindingStore()
+    keybindingStore.unsetKeybinding(
+      new KeybindingImpl({
+        commandId: 'Comfy.Graph.ExitSubgraph',
+        combo: { key: 'Escape' }
+      })
+    )
+
+    // Create a mock dialog
+    const dialog = document.createElement('dialog')
+    dialog.open = true
+    dialog.close = vi.fn()
+    document.body.appendChild(dialog)
+
+    const event = new KeyboardEvent('keydown', {
+      key: 'Escape',
+      bubbles: true,
+      cancelable: true
+    })
+
+    event.composedPath = vi.fn(() => [document.body])
+
+    await keybindingService.keybindHandler(event)
+
+    expect(dialog.close).toHaveBeenCalled()
+    expect(mockCommandExecute).not.toHaveBeenCalled()
+
+    // Cleanup
+    document.body.removeChild(dialog)
+  })
+
+  it('should allow user to rebind Escape key to different command', async () => {
+    const keybindingStore = useKeybindingStore()
+
+    // Add a user keybinding for Escape to a different command
+    keybindingStore.addUserKeybinding(
+      new KeybindingImpl({
+        commandId: 'Custom.Command',
+        combo: { key: 'Escape' }
+      })
+    )
+
+    const event = new KeyboardEvent('keydown', {
+      key: 'Escape',
+      bubbles: true,
+      cancelable: true
+    })
+
+    event.preventDefault = vi.fn()
+    event.composedPath = vi.fn(() => [document.body])
+
+    await keybindingService.keybindHandler(event)
+
+    expect(event.preventDefault).toHaveBeenCalled()
+    expect(mockCommandExecute).toHaveBeenCalledWith('Custom.Command')
+    expect(mockCommandExecute).not.toHaveBeenCalledWith(
+      'Comfy.Graph.ExitSubgraph'
+    )
+  })
+
+  it('should not execute Escape keybinding when dialogs are open', async () => {
+    // Mock dialog store to have open dialogs
+    vi.mocked(useDialogStore).mockReturnValue({
+      dialogStack: [{ key: 'test-dialog' }]
+    } as any)
+
+    // Re-create keybinding service to pick up new mock
+    keybindingService = useKeybindingService()
+
+    const event = new KeyboardEvent('keydown', {
+      key: 'Escape',
+      bubbles: true,
+      cancelable: true
+    })
+
+    event.preventDefault = vi.fn()
+    event.composedPath = vi.fn(() => [document.body])
+
+    await keybindingService.keybindHandler(event)
+
+    // Should not call preventDefault or execute command
+    expect(event.preventDefault).not.toHaveBeenCalled()
+    expect(mockCommandExecute).not.toHaveBeenCalled()
+  })
+})