[feat] make Escape key configurable for exiting subgraphs

Fixes #4208 - Escape hotkey is now configurable through the keybinding system.
Added Exit Subgraph command that can be bound to any key combination.
Escape key prioritizes closing dialogs before exiting subgraphs.

Author: christian-byrne

browser_tests/tests/subgraph.spec.ts

@@ -642,4 +642,117 @@ test.describe('Subgraph Operations', () => {
       expect(finalCount).toBe(parentCount)
     })
   })
+
+  test.describe('Configurable Escape Key', () => {
+    test('Escape keybinding can be customized', async ({ comfyPage }) => {
+      await comfyPage.loadWorkflow('subgraph')
+      await comfyPage.nextFrame()
+
+      const subgraphNode = await comfyPage.getNodeRefById('1')
+      await subgraphNode.navigateIntoSubgraph()
+      await comfyPage.page.waitForSelector(SELECTORS.breadcrumb)
+
+      // Verify we're in a subgraph
+      expect(await isInSubgraph(comfyPage)).toBe(true)
+
+      // Open settings dialog
+      await comfyPage.menu.openSettings()
+
+      // Navigate to keybindings
+      await comfyPage.page.click('text=Keybinding')
+
+      // Find the Exit Subgraph keybinding
+      const keybindingRow = comfyPage.page.locator(
+        'tr:has-text("Exit Subgraph")'
+      )
+      await expect(keybindingRow).toBeVisible()
+
+      // Verify default binding is Escape
+      const keyDisplay = keybindingRow.locator('.key-combo-display')
+      await expect(keyDisplay).toHaveText('Escape')
+
+      // Change binding to Alt+Up
+      const editButton = keybindingRow.locator(
+        'button[aria-label="Edit keybinding"]'
+      )
+      await editButton.click()
+
+      // Record new keybinding
+      await comfyPage.page.keyboard.press('Alt+ArrowUp')
+
+      // Save the keybinding
+      const saveButton = keybindingRow.locator(
+        'button[aria-label="Save keybinding"]'
+      )
+      await saveButton.click()
+
+      // Close settings
+      await comfyPage.page.keyboard.press('Escape')
+      await comfyPage.nextFrame()
+
+      // Test that Escape no longer exits subgraph
+      await comfyPage.page.keyboard.press('Escape')
+      await comfyPage.nextFrame()
+      expect(await isInSubgraph(comfyPage)).toBe(true)
+
+      // Test that Alt+Up now exits subgraph
+      await comfyPage.page.keyboard.press('Alt+ArrowUp')
+      await comfyPage.nextFrame()
+      expect(await isInSubgraph(comfyPage)).toBe(false)
+
+      // Reset keybinding to default
+      await comfyPage.menu.openSettings()
+      await comfyPage.page.click('text=Keybinding')
+
+      const resetButton = keybindingRow.locator(
+        'button[aria-label="Reset to default"]'
+      )
+      await resetButton.click()
+
+      await comfyPage.page.keyboard.press('Escape')
+      await comfyPage.nextFrame()
+
+      // Verify Escape works again
+      await subgraphNode.navigateIntoSubgraph()
+      await comfyPage.page.waitForSelector(SELECTORS.breadcrumb)
+      expect(await isInSubgraph(comfyPage)).toBe(true)
+
+      await comfyPage.page.keyboard.press('Escape')
+      await comfyPage.nextFrame()
+      expect(await isInSubgraph(comfyPage)).toBe(false)
+    })
+
+    test('Escape prioritizes closing dialogs over exiting subgraph', async ({
+      comfyPage
+    }) => {
+      await comfyPage.loadWorkflow('subgraph')
+      await comfyPage.nextFrame()
+
+      const subgraphNode = await comfyPage.getNodeRefById('1')
+      await subgraphNode.navigateIntoSubgraph()
+      await comfyPage.page.waitForSelector(SELECTORS.breadcrumb)
+
+      // Verify we're in a subgraph
+      expect(await isInSubgraph(comfyPage)).toBe(true)
+
+      // Open a dialog (settings)
+      await comfyPage.menu.openSettings()
+      await expect(comfyPage.page.locator('dialog[open]')).toBeVisible()
+
+      // 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('dialog[open]')).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/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/composables/useCoreCommands.exitSubgraph.test.ts

@@ -0,0 +1,163 @@
+import { createPinia, setActivePinia } from 'pinia'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { useCoreCommands } from '@/composables/useCoreCommands'
+import { useCanvasStore } from '@/stores/graphStore'
+import { useSubgraphNavigationStore } from '@/stores/subgraphNavigationStore'
+
+// Mock the stores
+vi.mock('@/stores/graphStore')
+vi.mock('@/stores/subgraphNavigationStore')
+
+// Mock other dependencies
+vi.mock('@/scripts/app', () => ({
+  app: {}
+}))
+
+vi.mock('@/scripts/api', () => ({
+  api: {
+    apiURL: vi.fn(() => 'http://localhost:8188')
+  }
+}))
+
+vi.mock('@/stores/firebaseAuthStore', () => ({
+  useFirebaseAuthStore: vi.fn(() => ({}))
+}))
+
+vi.mock('@/composables/auth/useFirebaseAuth', () => ({
+  useFirebaseAuth: vi.fn(() => null)
+}))
+
+vi.mock('firebase/auth', () => ({
+  setPersistence: vi.fn(),
+  browserLocalPersistence: {},
+  onAuthStateChanged: vi.fn()
+}))
+
+vi.mock('@/services/workflowService', () => ({
+  useWorkflowService: vi.fn(() => ({}))
+}))
+
+vi.mock('@/services/dialogService', () => ({
+  useDialogService: vi.fn(() => ({}))
+}))
+
+vi.mock('@/services/litegraphService', () => ({
+  useLitegraphService: vi.fn(() => ({}))
+}))
+
+vi.mock('@/stores/executionStore', () => ({
+  useExecutionStore: vi.fn(() => ({}))
+}))
+
+vi.mock('@/stores/toastStore', () => ({
+  useToastStore: vi.fn(() => ({}))
+}))
+
+vi.mock('@/stores/workflowStore', () => ({
+  useWorkflowStore: vi.fn(() => ({}))
+}))
+
+vi.mock('@/stores/workspace/colorPaletteStore', () => ({
+  useColorPaletteStore: vi.fn(() => ({}))
+}))
+
+vi.mock('@/composables/auth/useFirebaseAuthActions', () => ({
+  useFirebaseAuthActions: vi.fn(() => ({}))
+}))
+
+describe('useCoreCommands - ExitSubgraph', () => {
+  let mockCanvas: any
+  let mockSetGraph: ReturnType<typeof vi.fn>
+  let mockGetCanvas: ReturnType<typeof vi.fn>
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    setActivePinia(createPinia())
+
+    // Mock canvas and its methods
+    mockSetGraph = vi.fn()
+    mockCanvas = {
+      graph: {
+        rootGraph: { id: 'root-graph' }
+      },
+      setGraph: mockSetGraph
+    }
+
+    mockGetCanvas = vi.fn(() => mockCanvas)
+
+    // Mock canvasStore
+    vi.mocked(useCanvasStore).mockReturnValue({
+      getCanvas: mockGetCanvas
+    } as any)
+  })
+
+  it('should exit to parent subgraph when navigation stack has multiple items', () => {
+    // Mock navigation stack with multiple subgraphs
+    const parentSubgraph = { id: 'parent-subgraph' }
+    const currentSubgraph = { id: 'current-subgraph' }
+
+    vi.mocked(useSubgraphNavigationStore).mockReturnValue({
+      navigationStack: [parentSubgraph, currentSubgraph]
+    } as any)
+
+    const commands = useCoreCommands()
+    const exitSubgraphCommand = commands.find(
+      (cmd) => cmd.id === 'Comfy.Graph.ExitSubgraph'
+    )!
+
+    // Execute the command
+    void exitSubgraphCommand.function()
+
+    expect(mockGetCanvas).toHaveBeenCalled()
+    expect(mockSetGraph).toHaveBeenCalledWith(parentSubgraph)
+  })
+
+  it('should exit to root graph when navigation stack has only current subgraph', () => {
+    // Mock navigation stack with only current subgraph
+    const currentSubgraph = { id: 'current-subgraph' }
+
+    vi.mocked(useSubgraphNavigationStore).mockReturnValue({
+      navigationStack: [currentSubgraph]
+    } as any)
+
+    const commands = useCoreCommands()
+    const exitSubgraphCommand = commands.find(
+      (cmd) => cmd.id === 'Comfy.Graph.ExitSubgraph'
+    )!
+
+    // Execute the command
+    void exitSubgraphCommand.function()
+
+    expect(mockGetCanvas).toHaveBeenCalled()
+    expect(mockSetGraph).toHaveBeenCalledWith(mockCanvas.graph.rootGraph)
+  })
+
+  it('should do nothing when canvas.graph is null', () => {
+    // Mock canvas with null graph
+    mockCanvas.graph = null
+
+    const commands = useCoreCommands()
+    const exitSubgraphCommand = commands.find(
+      (cmd) => cmd.id === 'Comfy.Graph.ExitSubgraph'
+    )!
+
+    // Execute the command
+    void exitSubgraphCommand.function()
+
+    expect(mockGetCanvas).toHaveBeenCalled()
+    expect(mockSetGraph).not.toHaveBeenCalled()
+  })
+
+  it('should have correct metadata', () => {
+    const commands = useCoreCommands()
+    const exitSubgraphCommand = commands.find(
+      (cmd) => cmd.id === 'Comfy.Graph.ExitSubgraph'
+    )!
+
+    expect(exitSubgraphCommand).toBeDefined()
+    expect(exitSubgraphCommand.icon).toBe('pi pi-arrow-up')
+    expect(exitSubgraphCommand.label).toBe('Exit Subgraph')
+    expect(exitSubgraphCommand.versionAdded).toBe('1.20.1')
+  })
+})