mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-01 05:49:54 +00:00
Right click vue nodes (#5790)
This pull request refactors and improves the "More Options" popover functionality for graph nodes in the UI. The main change is a rename and redesign of the menu component from `MoreOptions` to `NodeOptions`, introducing a global singleton pattern for popover control and enabling context menu support on node right-click. This results in better maintainability, more flexible triggering, and improved user experience. **Node Options popover refactor and global control:** * Renamed and refactored `MoreOptions.vue` to `NodeOptions.vue`, removing the embedded button and exposing imperative methods (`toggle`, `hide`, `isOpen`) for external control. The component now registers/unregisters itself globally via `registerNodeOptionsInstance`. [[1]](diffhunk://#diff-e0dbd5e37efd2c79e7317415455340b0dd150b758077b170a663f67d2453605eL2-R2) [[2]](diffhunk://#diff-e0dbd5e37efd2c79e7317415455340b0dd150b758077b170a663f67d2453605eL203-R197) [[3]](diffhunk://#diff-e0dbd5e37efd2c79e7317415455340b0dd150b758077b170a663f67d2453605eR294-R309) * Added `NodeOptionsButton.vue` as a dedicated button component for triggering the popover, decoupling the button UI from the popover logic. * Implemented a global singleton pattern in `useMoreOptionsMenu.ts` for controlling the `NodeOptions` popover from anywhere, with `toggleNodeOptions` and `registerNodeOptionsInstance` functions. [[1]](diffhunk://#diff-ae87bdb1e06725eb19b8d0fc82ec40a5f8ca831a6632767cc5d214fc903b89b6R35-R65) [[2]](diffhunk://#diff-ae87bdb1e06725eb19b8d0fc82ec40a5f8ca831a6632767cc5d214fc903b89b6L184-R216) **UI integration and event handling improvements:** * Updated `SelectionToolbox.vue` to use the new `NodeOptionsButton` instead of the previous embedded `MoreOptions` button, and added the `NodeOptions` popover to the main `GraphCanvas.vue` template for global accessibility. [[1]](diffhunk://#diff-05d80ee1e28e634dc758394ddf1bfaa8e5ec72a186a6ea2e2b6f5dfba867b264L41-R41) [[2]](diffhunk://#diff-05d80ee1e28e634dc758394ddf1bfaa8e5ec72a186a6ea2e2b6f5dfba867b264L71-R71) [[3]](diffhunk://#diff-aaf17c713f29c6db8ea03efe7fc3483a858982e818a324b23cff89859e71559cR65) [[4]](diffhunk://#diff-aaf17c713f29c6db8ea03efe7fc3483a858982e818a324b23cff89859e71559cR91) * Added right-click context menu support to `LGraphNode.vue`, triggering the node options popover at the cursor position and integrating with node selection logic. [[1]](diffhunk://#diff-a7744614cf842e54416047326db79ad81f7c7ab7bfb66ae2b46f5c73ac7d47f2R45) [[2]](diffhunk://#diff-a7744614cf842e54416047326db79ad81f7c7ab7bfb66ae2b46f5c73ac7d47f2R141) [[3]](diffhunk://#diff-a7744614cf842e54416047326db79ad81f7c7ab7bfb66ae2b46f5c73ac7d47f2L180-R187) [[4]](diffhunk://#diff-a7744614cf842e54416047326db79ad81f7c7ab7bfb66ae2b46f5c73ac7d47f2R249-R263) **Minor improvements and cleanup:** * Updated references and variable names throughout the codebase to reflect the new `NodeOptions` naming and logic. [[1]](diffhunk://#diff-e0dbd5e37efd2c79e7317415455340b0dd150b758077b170a663f67d2453605eL53) [[2]](diffhunk://#diff-e0dbd5e37efd2c79e7317415455340b0dd150b758077b170a663f67d2453605eR50) [[3]](diffhunk://#diff-e0dbd5e37efd2c79e7317415455340b0dd150b758077b170a663f67d2453605eL75-R60) [[4]](diffhunk://#diff-e0dbd5e37efd2c79e7317415455340b0dd150b758077b170a663f67d2453605eL91-L95) [[5]](diffhunk://#diff-e0dbd5e37efd2c79e7317415455340b0dd150b758077b170a663f67d2453605eL110-R90) [[6]](diffhunk://#diff-e0dbd5e37efd2c79e7317415455340b0dd150b758077b170a663f67d2453605eL133-R113) [[7]](diffhunk://#diff-e0dbd5e37efd2c79e7317415455340b0dd150b758077b170a663f67d2453605eL146-R126) [[8]](diffhunk://#diff-e0dbd5e37efd2c79e7317415455340b0dd150b758077b170a663f67d2453605eL157-R140) This refactor makes the node options menu more modular, easier to maintain, and more flexible for future UI improvements. https://github.com/user-attachments/assets/9c2f2556-4544-4e20-9f22-8f485b0ceadc ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-5790-Right-click-vue-nodes-27a6d73d365081a98263c88d2e09e629) by [Unito](https://www.unito.io)
This commit is contained in:
committed by
GitHub
parent
5f7c7ca949
commit
f2e925de62
@@ -62,6 +62,7 @@
|
||||
<template v-if="comfyAppReady">
|
||||
<TitleEditor />
|
||||
<SelectionToolbox v-if="selectionToolboxEnabled" />
|
||||
<NodeOptions />
|
||||
<!-- Render legacy DOM widgets only when Vue nodes are disabled -->
|
||||
<DomWidgets v-if="!shouldRenderVueNodes" />
|
||||
</template>
|
||||
@@ -87,6 +88,7 @@ import GraphCanvasMenu from '@/components/graph/GraphCanvasMenu.vue'
|
||||
import NodeTooltip from '@/components/graph/NodeTooltip.vue'
|
||||
import SelectionToolbox from '@/components/graph/SelectionToolbox.vue'
|
||||
import TitleEditor from '@/components/graph/TitleEditor.vue'
|
||||
import NodeOptions from '@/components/graph/selectionToolbox/NodeOptions.vue'
|
||||
import NodeSearchboxPopover from '@/components/searchbox/NodeSearchBoxPopover.vue'
|
||||
import SideToolbar from '@/components/sidebar/SideToolbar.vue'
|
||||
import SecondRowWorkflowTabs from '@/components/topbar/SecondRowWorkflowTabs.vue'
|
||||
|
||||
@@ -480,13 +480,6 @@ describe('SelectionToolbox', () => {
|
||||
} as any)
|
||||
})
|
||||
|
||||
it('should still show MoreOptions when no items selected', () => {
|
||||
canvasStore.selectedItems = []
|
||||
const wrapper = mountComponent()
|
||||
|
||||
expect(wrapper.find('.more-options').exists()).toBe(true)
|
||||
})
|
||||
|
||||
it('should hide most buttons when no items selected', () => {
|
||||
canvasStore.selectedItems = []
|
||||
const wrapper = mountComponent()
|
||||
|
||||
@@ -38,7 +38,7 @@
|
||||
:command="command"
|
||||
/>
|
||||
<ExecuteButton v-if="showExecute" />
|
||||
<MoreOptions />
|
||||
<NodeOptionsButton />
|
||||
</Panel>
|
||||
</Transition>
|
||||
</div>
|
||||
@@ -68,7 +68,7 @@ import { useExtensionService } from '@/services/extensionService'
|
||||
import { type ComfyCommandImpl, useCommandStore } from '@/stores/commandStore'
|
||||
|
||||
import FrameNodes from './selectionToolbox/FrameNodes.vue'
|
||||
import MoreOptions from './selectionToolbox/MoreOptions.vue'
|
||||
import NodeOptionsButton from './selectionToolbox/NodeOptionsButton.vue'
|
||||
import VerticalDivider from './selectionToolbox/VerticalDivider.vue'
|
||||
|
||||
const commandStore = useCommandStore()
|
||||
|
||||
@@ -1,20 +1,5 @@
|
||||
<template>
|
||||
<div class="relative inline-flex items-center">
|
||||
<Button
|
||||
ref="buttonRef"
|
||||
v-tooltip.top="{
|
||||
value: $t('g.moreOptions'),
|
||||
showDelay: 1000
|
||||
}"
|
||||
data-testid="more-options-button"
|
||||
text
|
||||
class="h-8 w-8 px-0"
|
||||
severity="secondary"
|
||||
@click="toggle"
|
||||
>
|
||||
<i-lucide:more-vertical class="w-4 h-4" />
|
||||
</Button>
|
||||
|
||||
<div>
|
||||
<Popover
|
||||
ref="popover"
|
||||
:append-to="'body'"
|
||||
@@ -51,7 +36,6 @@
|
||||
|
||||
<script setup lang="ts">
|
||||
import { useRafFn } from '@vueuse/core'
|
||||
import Button from 'primevue/button'
|
||||
import Popover from 'primevue/popover'
|
||||
import { computed, onMounted, onUnmounted, ref, watch } from 'vue'
|
||||
|
||||
@@ -64,6 +48,7 @@ import {
|
||||
import {
|
||||
type MenuOption,
|
||||
type SubMenuOption,
|
||||
registerNodeOptionsInstance,
|
||||
useMoreOptionsMenu
|
||||
} from '@/composables/graph/useMoreOptionsMenu'
|
||||
import { useSubmenuPositioning } from '@/composables/graph/useSubmenuPositioning'
|
||||
@@ -74,7 +59,8 @@ import MenuOptionItem from './MenuOptionItem.vue'
|
||||
import SubmenuPopover from './SubmenuPopover.vue'
|
||||
|
||||
const popover = ref<InstanceType<typeof Popover>>()
|
||||
const buttonRef = ref<InstanceType<typeof Button> | HTMLElement | null>(null)
|
||||
const targetElement = ref<HTMLElement | null>(null)
|
||||
const isTriggeredByToolbox = ref<boolean>(true)
|
||||
// Track open state ourselves so we can restore after drag/move
|
||||
const isOpen = ref(false)
|
||||
const wasOpenBeforeHide = ref(false)
|
||||
@@ -90,11 +76,6 @@ const canvasInteractions = useCanvasInteractions()
|
||||
const minimap = useMinimap()
|
||||
const containerStyles = minimap.containerStyles
|
||||
|
||||
function getButtonEl(): HTMLElement | null {
|
||||
const el = (buttonRef.value as any)?.$el || buttonRef.value
|
||||
return el instanceof HTMLElement ? el : null
|
||||
}
|
||||
|
||||
let lastLogTs = 0
|
||||
const LOG_INTERVAL = 120 // ms
|
||||
let overlayElCache: HTMLElement | null = null
|
||||
@@ -109,7 +90,7 @@ function resolveOverlayEl(): HTMLElement | null {
|
||||
return direct
|
||||
}
|
||||
// Fallback: try to locate a recent popover root near the button (same z-index class + absolute)
|
||||
const btn = getButtonEl()
|
||||
const btn = targetElement.value
|
||||
if (btn) {
|
||||
const candidates = Array.from(
|
||||
document.querySelectorAll('div.absolute.z-50')
|
||||
@@ -132,20 +113,24 @@ function resolveOverlayEl(): HTMLElement | null {
|
||||
|
||||
const repositionPopover = () => {
|
||||
if (!isOpen.value) return
|
||||
const btn = getButtonEl()
|
||||
const btn = targetElement.value
|
||||
const overlayEl = resolveOverlayEl()
|
||||
if (!btn || !overlayEl) return
|
||||
const rect = btn.getBoundingClientRect()
|
||||
const marginY = 8 // tailwind mt-2 ~ 0.5rem = 8px
|
||||
const left = rect.left + rect.width / 2
|
||||
const top = rect.bottom + marginY
|
||||
const left = isTriggeredByToolbox.value
|
||||
? rect.left + rect.width / 2
|
||||
: rect.right - rect.width / 4
|
||||
const top = isTriggeredByToolbox.value
|
||||
? rect.bottom + marginY
|
||||
: rect.top - marginY - 6
|
||||
try {
|
||||
overlayEl.style.position = 'fixed'
|
||||
overlayEl.style.left = `${left}px`
|
||||
overlayEl.style.top = `${top}px`
|
||||
overlayEl.style.transform = 'translate(-50%, 0)'
|
||||
} catch (e) {
|
||||
console.warn('[MoreOptions] Failed to set overlay style', e)
|
||||
console.warn('[NodeOptions] Failed to set overlay style', e)
|
||||
return
|
||||
}
|
||||
const now = performance.now()
|
||||
@@ -156,9 +141,16 @@ const repositionPopover = () => {
|
||||
|
||||
const { resume: startSync, pause: stopSync } = useRafFn(repositionPopover)
|
||||
|
||||
function openPopover(triggerEvent?: Event): boolean {
|
||||
const el = getButtonEl()
|
||||
function openPopover(
|
||||
triggerEvent?: Event,
|
||||
element?: HTMLElement,
|
||||
clickedFromToolbox?: boolean
|
||||
): boolean {
|
||||
const el = element || targetElement.value
|
||||
if (!el || !el.isConnected) return false
|
||||
targetElement.value = el
|
||||
if (clickedFromToolbox !== undefined)
|
||||
isTriggeredByToolbox.value = clickedFromToolbox
|
||||
bump()
|
||||
popover.value?.show(triggerEvent ?? new Event('reopen'), el)
|
||||
isOpen.value = true
|
||||
@@ -191,7 +183,7 @@ function attemptRestore() {
|
||||
if (isOpen.value) return
|
||||
if (!wasOpenBeforeHide.value && !moreOptionsRestorePending.value) return
|
||||
// Try immediately
|
||||
if (openPopover(new Event('reopen'))) {
|
||||
if (openPopover(new Event('reopen'), targetElement.value || undefined)) {
|
||||
wasOpenBeforeHide.value = false
|
||||
restoreAttempts = 0
|
||||
return
|
||||
@@ -202,13 +194,24 @@ function attemptRestore() {
|
||||
requestAnimationFrame(() => attemptRestore())
|
||||
}
|
||||
|
||||
const toggle = (event: Event) => {
|
||||
const toggle = (
|
||||
event: Event,
|
||||
element?: HTMLElement,
|
||||
clickedFromToolbox?: boolean
|
||||
) => {
|
||||
if (isOpen.value) closePopover('manual')
|
||||
else openPopover(event)
|
||||
else openPopover(event, element, clickedFromToolbox)
|
||||
}
|
||||
|
||||
const hide = (reason: HideReason = 'manual') => closePopover(reason)
|
||||
|
||||
// Export functions for external triggering
|
||||
defineExpose({
|
||||
toggle,
|
||||
hide,
|
||||
isOpen
|
||||
})
|
||||
|
||||
const hideAll = () => {
|
||||
hideAllSubmenus(
|
||||
menuOptionsWithSubmenu.value,
|
||||
@@ -305,6 +308,13 @@ watch(
|
||||
)
|
||||
|
||||
onMounted(() => {
|
||||
// Register this instance globally
|
||||
registerNodeOptionsInstance({
|
||||
toggle,
|
||||
hide,
|
||||
isOpen
|
||||
})
|
||||
|
||||
if (moreOptionsRestorePending.value && !isOpen.value) {
|
||||
requestAnimationFrame(() => attemptRestore())
|
||||
}
|
||||
@@ -312,5 +322,7 @@ onMounted(() => {
|
||||
|
||||
onUnmounted(() => {
|
||||
stopSync()
|
||||
// Unregister on unmount
|
||||
registerNodeOptionsInstance(null)
|
||||
})
|
||||
</script>
|
||||
33
src/components/graph/selectionToolbox/NodeOptionsButton.vue
Normal file
33
src/components/graph/selectionToolbox/NodeOptionsButton.vue
Normal file
@@ -0,0 +1,33 @@
|
||||
<template>
|
||||
<Button
|
||||
ref="buttonRef"
|
||||
v-tooltip.top="{
|
||||
value: $t('g.moreOptions'),
|
||||
showDelay: 1000
|
||||
}"
|
||||
data-testid="more-options-button"
|
||||
text
|
||||
class="h-8 w-8 px-0"
|
||||
severity="secondary"
|
||||
@click="handleClick"
|
||||
>
|
||||
<i-lucide:more-vertical class="w-4 h-4" />
|
||||
</Button>
|
||||
</template>
|
||||
|
||||
<script setup lang="ts">
|
||||
import Button from 'primevue/button'
|
||||
import { ref } from 'vue'
|
||||
|
||||
import { toggleNodeOptions } from '@/composables/graph/useMoreOptionsMenu'
|
||||
|
||||
const buttonRef = ref<InstanceType<typeof Button> | null>(null)
|
||||
|
||||
const handleClick = (event: Event) => {
|
||||
const el = (buttonRef.value as any)?.$el || buttonRef.value
|
||||
const buttonEl = el instanceof HTMLElement ? el : null
|
||||
if (buttonEl) {
|
||||
toggleNodeOptions(event, buttonEl, true)
|
||||
}
|
||||
}
|
||||
</script>
|
||||
@@ -1,4 +1,4 @@
|
||||
import { computed, ref } from 'vue'
|
||||
import { type Ref, computed, ref } from 'vue'
|
||||
|
||||
import type { LGraphGroup } from '@/lib/litegraph/src/litegraph'
|
||||
import { isLGraphGroup } from '@/utils/litegraphUtil'
|
||||
@@ -32,6 +32,47 @@ export enum BadgeVariant {
|
||||
DEPRECATED = 'deprecated'
|
||||
}
|
||||
|
||||
// Global singleton for NodeOptions component reference
|
||||
let nodeOptionsInstance: null | NodeOptionsInstance = null
|
||||
|
||||
/**
|
||||
* Toggle the node options popover
|
||||
* @param event - The trigger event
|
||||
* @param element - The target element (button) that triggered the popover
|
||||
*/
|
||||
export function toggleNodeOptions(
|
||||
event: Event,
|
||||
element: HTMLElement,
|
||||
clickedFromToolbox: boolean = false
|
||||
) {
|
||||
if (nodeOptionsInstance?.toggle) {
|
||||
nodeOptionsInstance.toggle(event, element, clickedFromToolbox)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Hide the node options popover
|
||||
*/
|
||||
interface NodeOptionsInstance {
|
||||
toggle: (
|
||||
event: Event,
|
||||
element: HTMLElement,
|
||||
clickedFromToolbox: boolean
|
||||
) => void
|
||||
hide: () => void
|
||||
isOpen: Ref<boolean>
|
||||
}
|
||||
|
||||
/**
|
||||
* Register the NodeOptions component instance
|
||||
* @param instance - The NodeOptions component instance
|
||||
*/
|
||||
export function registerNodeOptionsInstance(
|
||||
instance: null | NodeOptionsInstance
|
||||
) {
|
||||
nodeOptionsInstance = instance
|
||||
}
|
||||
|
||||
/**
|
||||
* Composable for managing the More Options menu configuration
|
||||
* Refactored to use smaller, focused composables for better maintainability
|
||||
@@ -181,6 +222,7 @@ export function useMoreOptionsMenu() {
|
||||
menuOptions,
|
||||
menuOptionsWithSubmenu,
|
||||
bump,
|
||||
hasSubgraphs
|
||||
hasSubgraphs,
|
||||
registerNodeOptionsInstance
|
||||
}
|
||||
}
|
||||
|
||||
@@ -40,6 +40,7 @@
|
||||
]"
|
||||
v-bind="pointerHandlers"
|
||||
@wheel="handleWheel"
|
||||
@contextmenu="handleContextMenu"
|
||||
>
|
||||
<div class="flex items-center">
|
||||
<template v-if="isCollapsed">
|
||||
@@ -135,6 +136,7 @@ import { storeToRefs } from 'pinia'
|
||||
import { computed, inject, onErrorCaptured, onMounted, provide, ref } from 'vue'
|
||||
|
||||
import type { VueNodeData } from '@/composables/graph/useGraphNodeManager'
|
||||
import { toggleNodeOptions } from '@/composables/graph/useMoreOptionsMenu'
|
||||
import { useErrorHandling } from '@/composables/useErrorHandling'
|
||||
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
|
||||
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
||||
@@ -175,8 +177,12 @@ const {
|
||||
readonly = false
|
||||
} = defineProps<LGraphNodeProps>()
|
||||
|
||||
const { handleNodeCollapse, handleNodeTitleUpdate, handleNodeSelect } =
|
||||
useNodeEventHandlers()
|
||||
const {
|
||||
handleNodeCollapse,
|
||||
handleNodeTitleUpdate,
|
||||
handleNodeSelect,
|
||||
handleNodeRightClick
|
||||
} = useNodeEventHandlers()
|
||||
|
||||
useVueElementTracking(() => nodeData.id, 'node')
|
||||
|
||||
@@ -235,6 +241,21 @@ const { pointerHandlers, isDragging, dragStyle } = useNodePointerInteractions(
|
||||
handleNodeSelect
|
||||
)
|
||||
|
||||
// Handle right-click context menu
|
||||
const handleContextMenu = (event: MouseEvent) => {
|
||||
event.preventDefault()
|
||||
event.stopPropagation()
|
||||
|
||||
// First handle the standard right-click behavior (selection)
|
||||
handleNodeRightClick(event as PointerEvent, nodeData)
|
||||
|
||||
// Show the node options menu at the cursor position
|
||||
const targetElement = event.currentTarget as HTMLElement
|
||||
if (targetElement) {
|
||||
toggleNodeOptions(event, targetElement, false)
|
||||
}
|
||||
}
|
||||
|
||||
onMounted(() => {
|
||||
if (size.value && transformState?.camera) {
|
||||
const scale = transformState.camera.z
|
||||
|
||||
Reference in New Issue
Block a user