diff --git a/src/components/graph/GraphCanvas.vue b/src/components/graph/GraphCanvas.vue index 67ef64d7445..d5f5a695a55 100644 --- a/src/components/graph/GraphCanvas.vue +++ b/src/components/graph/GraphCanvas.vue @@ -152,6 +152,7 @@ import { useGlobalLitegraph } from '@/composables/useGlobalLitegraph' import { usePaste } from '@/composables/usePaste' import { useVueFeatureFlags } from '@/composables/useVueFeatureFlags' import { LiteGraph } from '@/lib/litegraph/src/litegraph' +import { useLiteGraphContextMenuTracking } from '@/platform/keybindings/raisedSurfaceLiteGraphBridge' import { useLitegraphSettings } from '@/platform/settings/composables/useLitegraphSettings' import { CORE_SETTINGS } from '@/platform/settings/constants/coreSettings' import { useSettingStore } from '@/platform/settings/settingStore' @@ -460,6 +461,7 @@ useNodeBadge() useGlobalLitegraph() useContextMenuTranslation() +useLiteGraphContextMenuTracking() useCopy() usePaste() useWorkflowAutoSave() diff --git a/src/components/graph/NodeContextMenu.vue b/src/components/graph/NodeContextMenu.vue index 6bd285fd8c5..70cf98a27b6 100644 --- a/src/components/graph/NodeContextMenu.vue +++ b/src/components/graph/NodeContextMenu.vue @@ -52,6 +52,7 @@ import type { MenuOption, SubMenuOption } from '@/composables/graph/useMoreOptionsMenu' +import { useRaisedSurface } from '@/platform/keybindings/raisedSurfaceStore' import { useCanvasStore } from '@/renderer/core/canvas/canvasStore' import ColorPickerMenu from './selectionToolbox/ColorPickerMenu.vue' @@ -69,6 +70,8 @@ const isOpen = ref(false) const { menuOptions, bump } = useMoreOptionsMenu() const canvasStore = useCanvasStore() +useRaisedSurface('context-menu', isOpen) + // World position (canvas coordinates) where menu was opened const worldPosition = ref({ x: 0, y: 0 }) diff --git a/src/lib/litegraph/src/ContextMenu.test.ts b/src/lib/litegraph/src/ContextMenu.test.ts new file mode 100644 index 00000000000..9dcf32a75a7 --- /dev/null +++ b/src/lib/litegraph/src/ContextMenu.test.ts @@ -0,0 +1,52 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import { ContextMenu } from '@/lib/litegraph/src/ContextMenu' + +describe('ContextMenu lifecycle events', () => { + let listener: (event: Event) => void + let calls: Event[] + let menus: ContextMenu[] + + beforeEach(() => { + document.body.innerHTML = '' + calls = [] + menus = [] + listener = (event) => calls.push(event) + document.addEventListener('litegraph:contextmenu', listener) + }) + + afterEach(() => { + for (const menu of menus) menu.close() + document.removeEventListener('litegraph:contextmenu', listener) + }) + + it('dispatches an "open" event when a top-level menu is constructed', () => { + menus.push(new ContextMenu(['Item A', 'Item B'], {})) + + expect(calls).toHaveLength(1) + const detail = (calls[0] as CustomEvent).detail + expect(detail.type).toBe('open') + }) + + it('dispatches a "close" event when a top-level menu closes', () => { + const menu = new ContextMenu(['Item A'], {}) + calls.length = 0 + + menu.close() + + expect(calls).toHaveLength(1) + const detail = (calls[0] as CustomEvent).detail + expect(detail.type).toBe('close') + expect(detail.menu).toBe(menu) + }) + + it('does not dispatch lifecycle events for submenus', () => { + const parent = new ContextMenu(['Item A'], {}) + menus.push(parent) + calls.length = 0 + + new ContextMenu(['Sub Item'], { parentMenu: parent }) + + expect(calls).toHaveLength(0) + }) +}) diff --git a/src/lib/litegraph/src/ContextMenu.ts b/src/lib/litegraph/src/ContextMenu.ts index 90450522fe5..284d12199e3 100644 --- a/src/lib/litegraph/src/ContextMenu.ts +++ b/src/lib/litegraph/src/ContextMenu.ts @@ -56,6 +56,7 @@ export class ContextMenu { root: ContextMenuDivElement current_submenu?: ContextMenu lock?: boolean + ownerDocument: Document controller: AbortController = new AbortController() @@ -105,7 +106,13 @@ export class ContextMenu { options.event = undefined } - const root: ContextMenuDivElement = document.createElement('div') + const ownerDocument = + (options.event?.target as Node | null | undefined)?.ownerDocument ?? + document + this.ownerDocument = ownerDocument + + const root: ContextMenuDivElement = + ownerDocument.createElement('div') let classes = 'litegraph litecontextmenu litemenubar-panel' if (options.className) classes += ` ${options.className}` root.className = classes @@ -117,7 +124,7 @@ export class ContextMenu { const eventOptions = { capture: true, signal } if (!this.parentMenu) { - document.addEventListener( + ownerDocument.addEventListener( 'pointerdown', (e) => { if (e.target instanceof Node && !this.containsNode(e.target)) { @@ -126,6 +133,16 @@ export class ContextMenu { }, eventOptions ) + ownerDocument.addEventListener( + 'keydown', + (e) => { + if (e.key !== 'Escape' || e.ctrlKey || e.altKey || e.metaKey) return + this.close() + e.preventDefault() + e.stopPropagation() + }, + eventOptions + ) } // this prevents the default context browser menu to open in case this menu was created when pressing right button @@ -179,13 +196,9 @@ export class ContextMenu { } // insert before checking position - const ownerDocument = (options.event?.target as Node | null | undefined) - ?.ownerDocument - const root_document = ownerDocument || document - - if (root_document.fullscreenElement) - root_document.fullscreenElement.append(root) - else root_document.body.append(root) + if (ownerDocument.fullscreenElement) + ownerDocument.fullscreenElement.append(root) + else ownerDocument.body.append(root) // compute best position let left = options.left || 0 @@ -200,7 +213,7 @@ export class ContextMenu { left = rect.left + rect.width } - const body_rect = document.body.getBoundingClientRect() + const body_rect = ownerDocument.body.getBoundingClientRect() const root_rect = root.getBoundingClientRect() if (body_rect.height == 0) console.error( @@ -219,6 +232,14 @@ export class ContextMenu { if (LiteGraph.context_menu_scaling && options.scale) { root.style.transform = `scale(${Math.round(options.scale * 4) * 0.25})` } + + if (!parent) { + ownerDocument.dispatchEvent( + new CustomEvent('litegraph:contextmenu', { + detail: { type: 'open', menu: this } + }) + ) + } } /** @@ -402,6 +423,13 @@ export class ContextMenu { } } this.current_submenu?.close(e, true) + if (!this.parentMenu) { + this.ownerDocument.dispatchEvent( + new CustomEvent('litegraph:contextmenu', { + detail: { type: 'close', menu: this } + }) + ) + } } /** @deprecated Likely unused, however code search was inconclusive (too many results to check by hand). */ diff --git a/src/platform/keybindings/keybindingService.escape.test.ts b/src/platform/keybindings/keybindingService.escape.test.ts index 42b946a13a5..b235fffdc69 100644 --- a/src/platform/keybindings/keybindingService.escape.test.ts +++ b/src/platform/keybindings/keybindingService.escape.test.ts @@ -7,6 +7,7 @@ import { KeyComboImpl } from '@/platform/keybindings/keyCombo' import { KeybindingImpl } from '@/platform/keybindings/keybinding' import { useKeybindingService } from '@/platform/keybindings/keybindingService' import { useKeybindingStore } from '@/platform/keybindings/keybindingStore' +import { useRaisedSurfaceStore } from '@/platform/keybindings/raisedSurfaceStore' import { useCommandStore } from '@/stores/commandStore' import type { DialogInstance } from '@/stores/dialogStore' import { useDialogStore } from '@/stores/dialogStore' @@ -108,6 +109,31 @@ describe('keybindingService - Escape key handling', () => { expect(mockCommandExecute).not.toHaveBeenCalled() }) + it('should NOT execute Escape keybinding when a raised surface is open', async () => { + const raisedSurfaceStore = useRaisedSurfaceStore() + raisedSurfaceStore.open('context-menu') + + keybindingService = useKeybindingService() + + const event = createKeyboardEvent('Escape') + await keybindingService.keybindHandler(event) + + expect(mockCommandExecute).not.toHaveBeenCalled() + }) + + it('should resume executing Escape keybinding after a raised surface closes', async () => { + const raisedSurfaceStore = useRaisedSurfaceStore() + const id = raisedSurfaceStore.open('context-menu') + raisedSurfaceStore.close(id) + + keybindingService = useKeybindingService() + + const event = createKeyboardEvent('Escape') + await keybindingService.keybindHandler(event) + + expect(mockCommandExecute).toHaveBeenCalledWith('Comfy.Graph.ExitSubgraph') + }) + it('should execute Escape keybinding with modifiers regardless of dialog state', async () => { const dialogStore = useDialogStore() dialogStore.dialogStack.push(createTestDialogInstance('test-dialog')) diff --git a/src/platform/keybindings/keybindingService.ts b/src/platform/keybindings/keybindingService.ts index b11d6831a7d..f78572e1d59 100644 --- a/src/platform/keybindings/keybindingService.ts +++ b/src/platform/keybindings/keybindingService.ts @@ -7,12 +7,14 @@ import { CORE_KEYBINDINGS } from './defaults' import { KeyComboImpl } from './keyCombo' import { KeybindingImpl } from './keybinding' import { useKeybindingStore } from './keybindingStore' +import { useRaisedSurfaceStore } from './raisedSurfaceStore' export function useKeybindingService() { const keybindingStore = useKeybindingStore() const commandStore = useCommandStore() const settingStore = useSettingStore() const dialogStore = useDialogStore() + const raisedSurfaceStore = useRaisedSurfaceStore() async function keybindHandler(event: KeyboardEvent) { const keyCombo = KeyComboImpl.fromEvent(event) @@ -50,7 +52,10 @@ export function useKeybindingService() { !event.altKey && !event.metaKey ) { - if (dialogStore.dialogStack.length > 0) { + if ( + raisedSurfaceStore.isAnyOpen || + dialogStore.dialogStack.length > 0 + ) { return } } diff --git a/src/platform/keybindings/raisedSurfaceLiteGraphBridge.ts b/src/platform/keybindings/raisedSurfaceLiteGraphBridge.ts new file mode 100644 index 00000000000..0f15641c34b --- /dev/null +++ b/src/platform/keybindings/raisedSurfaceLiteGraphBridge.ts @@ -0,0 +1,34 @@ +import { onScopeDispose } from 'vue' + +import { useRaisedSurfaceStore } from './raisedSurfaceStore' + +interface LiteGraphContextMenuEventDetail { + type: 'open' | 'close' + menu: object +} + +export function useLiteGraphContextMenuTracking(): void { + const store = useRaisedSurfaceStore() + const idsByMenu = new Map() + + const handler = (event: Event) => { + const detail = (event as CustomEvent) + .detail + if (detail.type === 'open') { + idsByMenu.set(detail.menu, store.open('context-menu')) + return + } + const id = idsByMenu.get(detail.menu) + if (id !== undefined) { + store.close(id) + idsByMenu.delete(detail.menu) + } + } + + document.addEventListener('litegraph:contextmenu', handler) + onScopeDispose(() => { + document.removeEventListener('litegraph:contextmenu', handler) + for (const id of idsByMenu.values()) store.close(id) + idsByMenu.clear() + }) +} diff --git a/src/platform/keybindings/raisedSurfaceStore.test.ts b/src/platform/keybindings/raisedSurfaceStore.test.ts new file mode 100644 index 00000000000..2590a0b0dcb --- /dev/null +++ b/src/platform/keybindings/raisedSurfaceStore.test.ts @@ -0,0 +1,89 @@ +import { createPinia, setActivePinia } from 'pinia' +import { effectScope, ref } from 'vue' +import { beforeEach, describe, expect, it } from 'vitest' + +import { useRaisedSurface, useRaisedSurfaceStore } from './raisedSurfaceStore' + +describe('raisedSurfaceStore', () => { + beforeEach(() => { + setActivePinia(createPinia()) + }) + + it('is empty by default', () => { + const store = useRaisedSurfaceStore() + expect(store.isAnyOpen).toBe(false) + expect(store.stack).toHaveLength(0) + }) + + it('tracks open/close and reports isAnyOpen', () => { + const store = useRaisedSurfaceStore() + const id = store.open('context-menu') + expect(store.isAnyOpen).toBe(true) + expect(store.stack).toHaveLength(1) + store.close(id) + expect(store.isAnyOpen).toBe(false) + expect(store.stack).toHaveLength(0) + }) + + it('supports concurrent surfaces and closes by id (LIFO not required)', () => { + const store = useRaisedSurfaceStore() + const a = store.open('context-menu') + const b = store.open('popover') + expect(store.stack).toHaveLength(2) + store.close(a) + expect(store.stack).toHaveLength(1) + expect(store.stack[0].kind).toBe('popover') + store.close(b) + expect(store.isAnyOpen).toBe(false) + }) + + it('close is a no-op for unknown ids', () => { + const store = useRaisedSurfaceStore() + store.open('modal') + store.close(Symbol('stale')) + expect(store.stack).toHaveLength(1) + }) +}) + +describe('useRaisedSurface', () => { + beforeEach(() => { + setActivePinia(createPinia()) + }) + + it('opens when reactive flag turns true and closes when it turns false', () => { + const store = useRaisedSurfaceStore() + const isOpen = ref(false) + const scope = effectScope() + scope.run(() => useRaisedSurface('context-menu', isOpen)) + + expect(store.isAnyOpen).toBe(false) + isOpen.value = true + expect(store.isAnyOpen).toBe(true) + isOpen.value = false + expect(store.isAnyOpen).toBe(false) + scope.stop() + }) + + it('does not double-register when flag is toggled true twice', () => { + const store = useRaisedSurfaceStore() + const isOpen = ref(false) + const scope = effectScope() + scope.run(() => useRaisedSurface('context-menu', isOpen)) + + isOpen.value = true + isOpen.value = true + expect(store.stack).toHaveLength(1) + scope.stop() + }) + + it('releases the surface when the owning scope is disposed', () => { + const store = useRaisedSurfaceStore() + const isOpen = ref(true) + const scope = effectScope() + scope.run(() => useRaisedSurface('context-menu', isOpen)) + + expect(store.isAnyOpen).toBe(true) + scope.stop() + expect(store.isAnyOpen).toBe(false) + }) +}) diff --git a/src/platform/keybindings/raisedSurfaceStore.ts b/src/platform/keybindings/raisedSurfaceStore.ts new file mode 100644 index 00000000000..86f81c2f029 --- /dev/null +++ b/src/platform/keybindings/raisedSurfaceStore.ts @@ -0,0 +1,72 @@ +import { defineStore } from 'pinia' +import { computed, onScopeDispose, ref, toValue, watch } from 'vue' +import type { MaybeRefOrGetter } from 'vue' + +/** + * Tracks open "raised surfaces" — popovers, context menus, and top-level + * modals — that should suppress global keybindings while they are open. + * + * UX axiom: standard keybindings work in every context EXCEPT when a raised + * surface is open. Consumers register/unregister surfaces here; the keybinding + * service consults {@link isAnyOpen} as its single source of truth. + */ +type RaisedSurfaceKind = 'context-menu' | 'popover' | 'modal' + +interface RaisedSurfaceEntry { + id: symbol + kind: RaisedSurfaceKind +} + +export const useRaisedSurfaceStore = defineStore('raisedSurface', () => { + const stack = ref([]) + const isAnyOpen = computed(() => stack.value.length > 0) + + function open(kind: RaisedSurfaceKind): symbol { + const id = Symbol(kind) + stack.value.push({ id, kind }) + return id + } + + function close(id: symbol): void { + const index = stack.value.findIndex((entry) => entry.id === id) + if (index !== -1) stack.value.splice(index, 1) + } + + return { stack, isAnyOpen, open, close } +}) + +/** + * Bind a surface's reactive open-state to the raised-surface registry. + * + * @example + * const isOpen = ref(false) + * useRaisedSurface('context-menu', isOpen) + */ +export function useRaisedSurface( + kind: RaisedSurfaceKind, + isOpen: MaybeRefOrGetter +): void { + const store = useRaisedSurfaceStore() + let id: symbol | null = null + + function release() { + if (id !== null) { + store.close(id) + id = null + } + } + + watch( + () => toValue(isOpen), + (open) => { + if (open && id === null) { + id = store.open(kind) + } else if (!open) { + release() + } + }, + { immediate: true, flush: 'sync' } + ) + + onScopeDispose(release) +}