Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[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.
  • Loading branch information
christian-byrne committed Aug 9, 2025
commit a8800b80dc7930659f748f7c06dd3e37a2e7c639
113 changes: 113 additions & 0 deletions browser_tests/tests/subgraph.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
13 changes: 0 additions & 13 deletions src/components/breadcrumb/SubgraphBreadcrumb.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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) => {
Expand Down
16 changes: 16 additions & 0 deletions src/composables/useCoreCommands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
)
}
}
]

Expand Down
6 changes: 6 additions & 0 deletions src/constants/coreKeybindings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,5 +190,11 @@ export const CORE_KEYBINDINGS: Keybinding[] = [
key: 'k'
},
commandId: 'Workspace.ToggleBottomPanel.Shortcuts'
},
{
combo: {
key: 'Escape'
},
commandId: 'Comfy.Graph.ExitSubgraph'
}
]
15 changes: 15 additions & 0 deletions src/services/keybindingService.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { CORE_KEYBINDINGS } from '@/constants/coreKeybindings'
import { useCommandStore } from '@/stores/commandStore'
import { useDialogStore } from '@/stores/dialogStore'
import {
KeyComboImpl,
KeybindingImpl,
Expand All @@ -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)
Expand All @@ -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)
Expand Down
163 changes: 163 additions & 0 deletions tests-ui/tests/composables/useCoreCommands.exitSubgraph.test.ts
Original file line number Diff line number Diff line change
@@ -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')
})
})
Loading
Loading