mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-03-06 13:40:25 +00:00
[feat] Replace NodeOptions with PrimeVue ContextMenu (#7114)
## Summary - Add `NodeContextMenu.vue` using PrimeVue ContextMenu component with native submenu support - Rename `SubmenuPopover.vue` to `ColorPickerMenu.vue` (specialized for color picker) - Delete old components: `NodeOptions.vue`, `MenuOptionItem.vue`, `useSubmenuPositioning.ts` - Wire up context menu converter in `useMoreOptionsMenu.ts` - Update tests to use hover instead of click for submenus ## Dependencies **This PR depends on #7113** - the context menu converter infrastructure PR. It should be merged after that PR. ## Benefits - Native PrimeVue submenu support with proper keyboard navigation - Constrained menu dimensions with overflow scrolling (max-h-[80vh]) - Cleaner component architecture with ~280 fewer lines of code - Better separation: ColorPickerMenu handles only the custom color picker UI ## Test plan - [x] Typecheck passes - [x] Lint passes - [x] Knip passes - [ ] Browser tests for submenu interactions pass - [ ] Manual testing of node context menu ## Screenshots (Menu UI should look the same, with improved submenu behavior) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-7114-feat-Replace-NodeOptions-with-PrimeVue-ContextMenu-2be6d73d365081fda576fd691175eacf) by [Unito](https://www.unito.io) --------- Co-authored-by: GitHub Action <action@github.com>
This commit is contained in:
committed by
GitHub
parent
8d7dd9ed67
commit
e21f43f398
@@ -2,8 +2,13 @@ import { computed, ref } from 'vue'
|
||||
import type { Ref } from 'vue'
|
||||
|
||||
import type { LGraphGroup } from '@/lib/litegraph/src/litegraph'
|
||||
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
|
||||
import { isLGraphGroup } from '@/utils/litegraphUtil'
|
||||
|
||||
import {
|
||||
buildStructuredMenu,
|
||||
convertContextMenuToOptions
|
||||
} from './contextMenuConverter'
|
||||
import { useGroupMenuOptions } from './useGroupMenuOptions'
|
||||
import { useImageMenuOptions } from './useImageMenuOptions'
|
||||
import { useNodeMenuOptions } from './useNodeMenuOptions'
|
||||
@@ -43,15 +48,10 @@ 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
|
||||
) {
|
||||
export function toggleNodeOptions(event: Event) {
|
||||
if (nodeOptionsInstance?.toggle) {
|
||||
nodeOptionsInstance.toggle(event, element, clickedFromToolbox)
|
||||
nodeOptionsInstance.toggle(event)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -59,11 +59,7 @@ export function toggleNodeOptions(
|
||||
* Hide the node options popover
|
||||
*/
|
||||
interface NodeOptionsInstance {
|
||||
toggle: (
|
||||
event: Event,
|
||||
element: HTMLElement,
|
||||
clickedFromToolbox: boolean
|
||||
) => void
|
||||
toggle: (event: Event) => void
|
||||
hide: () => void
|
||||
isOpen: Ref<boolean>
|
||||
}
|
||||
@@ -78,6 +74,19 @@ export function registerNodeOptionsInstance(
|
||||
nodeOptionsInstance = instance
|
||||
}
|
||||
|
||||
/**
|
||||
* Mark menu options as coming from Vue hardcoded menu
|
||||
*/
|
||||
function markAsVueOptions(options: MenuOption[]): MenuOption[] {
|
||||
return options.map((opt) => {
|
||||
// Don't mark dividers or category labels
|
||||
if (opt.type === 'divider' || opt.type === 'category') {
|
||||
return opt
|
||||
}
|
||||
return { ...opt, source: 'vue' }
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Composable for managing the More Options menu configuration
|
||||
* Refactored to use smaller, focused composables for better maintainability
|
||||
@@ -95,10 +104,11 @@ export function useMoreOptionsMenu() {
|
||||
computeSelectionFlags
|
||||
} = useSelectionState()
|
||||
|
||||
const canvasStore = useCanvasStore()
|
||||
|
||||
const { getImageMenuOptions } = useImageMenuOptions()
|
||||
const {
|
||||
getNodeInfoOption,
|
||||
getAdjustSizeOption,
|
||||
getNodeVisualOptions,
|
||||
getPinOption,
|
||||
getBypassOption,
|
||||
@@ -106,16 +116,13 @@ export function useMoreOptionsMenu() {
|
||||
} = useNodeMenuOptions()
|
||||
const {
|
||||
getFitGroupToNodesOption,
|
||||
getGroupShapeOptions,
|
||||
getGroupColorOptions,
|
||||
getGroupModeOptions
|
||||
} = useGroupMenuOptions()
|
||||
const {
|
||||
getBasicSelectionOptions,
|
||||
getSubgraphOptions,
|
||||
getMultipleNodesOptions,
|
||||
getDeleteOption,
|
||||
getAlignmentOptions
|
||||
getMultipleNodesOptions
|
||||
} = useSelectionMenuOptions()
|
||||
|
||||
const hasSubgraphs = hasSubgraphsComputed
|
||||
@@ -142,85 +149,107 @@ export function useMoreOptionsMenu() {
|
||||
? selectedGroups[0]
|
||||
: null
|
||||
const hasSubgraphsSelected = hasSubgraphs.value
|
||||
|
||||
// For single node selection, also get LiteGraph menu items to merge
|
||||
const litegraphOptions: MenuOption[] = []
|
||||
if (
|
||||
selectedNodes.value.length === 1 &&
|
||||
!groupContext &&
|
||||
canvasStore.canvas
|
||||
) {
|
||||
try {
|
||||
const node = selectedNodes.value[0]
|
||||
const rawItems = canvasStore.canvas.getNodeMenuOptions(node)
|
||||
// Don't apply structuring yet - we'll do it after merging with Vue options
|
||||
litegraphOptions.push(
|
||||
...convertContextMenuToOptions(rawItems, node, false)
|
||||
)
|
||||
} catch (error) {
|
||||
console.error('Error getting LiteGraph menu items:', error)
|
||||
}
|
||||
}
|
||||
|
||||
const options: MenuOption[] = []
|
||||
|
||||
// Section 1: Basic selection operations (Rename, Copy, Duplicate)
|
||||
options.push(...getBasicSelectionOptions())
|
||||
const basicOps = getBasicSelectionOptions()
|
||||
options.push(...basicOps)
|
||||
options.push({ type: 'divider' })
|
||||
|
||||
// Section 2: Node Info & Size Adjustment
|
||||
if (nodeDef.value) {
|
||||
options.push(getNodeInfoOption(showNodeHelp))
|
||||
// Section 2: Node actions (Run Branch, Pin, Bypass, Mute)
|
||||
if (hasOutputNodesSelected.value) {
|
||||
const runBranch = getRunBranchOption()
|
||||
options.push(runBranch)
|
||||
}
|
||||
if (!groupContext) {
|
||||
const pin = getPinOption(states, bump)
|
||||
const bypass = getBypassOption(states, bump)
|
||||
options.push(pin)
|
||||
options.push(bypass)
|
||||
}
|
||||
|
||||
if (groupContext) {
|
||||
options.push(getFitGroupToNodesOption(groupContext))
|
||||
} else {
|
||||
options.push(getAdjustSizeOption())
|
||||
const groupModes = getGroupModeOptions(groupContext, bump)
|
||||
options.push(...groupModes)
|
||||
}
|
||||
options.push({ type: 'divider' })
|
||||
|
||||
// Section 3: Collapse/Shape/Color
|
||||
if (groupContext) {
|
||||
// Group context: Shape, Color, Divider
|
||||
options.push(getGroupShapeOptions(groupContext, bump))
|
||||
options.push(getGroupColorOptions(groupContext, bump))
|
||||
options.push({ type: 'divider' })
|
||||
} else {
|
||||
// Node context: Expand/Minimize, Shape, Color, Divider
|
||||
options.push(...getNodeVisualOptions(states, bump))
|
||||
options.push({ type: 'divider' })
|
||||
}
|
||||
|
||||
// Section 4: Image operations (if image node)
|
||||
if (hasImageNode.value && selectedNodes.value.length > 0) {
|
||||
options.push(...getImageMenuOptions(selectedNodes.value[0]))
|
||||
}
|
||||
|
||||
// Section 5: Subgraph operations
|
||||
// Section 3: Structure operations (Convert to Subgraph, Frame selection, Minimize Node)
|
||||
options.push(
|
||||
...getSubgraphOptions({
|
||||
hasSubgraphs: hasSubgraphsSelected,
|
||||
hasMultipleSelection: hasMultipleNodes.value
|
||||
})
|
||||
)
|
||||
|
||||
// Section 6: Multiple nodes operations
|
||||
if (hasMultipleNodes.value) {
|
||||
options.push(...getMultipleNodesOptions())
|
||||
}
|
||||
|
||||
// Section 7: Divider
|
||||
options.push({ type: 'divider' })
|
||||
|
||||
// Section 8: Pin/Unpin (non-group only)
|
||||
if (!groupContext) {
|
||||
options.push(getPinOption(states, bump))
|
||||
}
|
||||
|
||||
// Section 9: Alignment (if multiple nodes)
|
||||
if (hasMultipleNodes.value) {
|
||||
options.push(...getAlignmentOptions())
|
||||
}
|
||||
|
||||
// Section 10: Mode operations
|
||||
if (groupContext) {
|
||||
// Group mode operations
|
||||
options.push(...getGroupModeOptions(groupContext, bump))
|
||||
options.push(getFitGroupToNodesOption(groupContext))
|
||||
} else {
|
||||
// Bypass option for nodes
|
||||
options.push(getBypassOption(states, bump))
|
||||
// Node context: Expand/Minimize
|
||||
const visualOptions = getNodeVisualOptions(states, bump)
|
||||
if (visualOptions.length > 0) {
|
||||
options.push(visualOptions[0]) // Expand/Minimize (index 0)
|
||||
}
|
||||
}
|
||||
|
||||
// Section 11: Run Branch (if output nodes)
|
||||
if (hasOutputNodesSelected.value) {
|
||||
options.push(getRunBranchOption())
|
||||
}
|
||||
|
||||
// Section 12: Final divider and Delete
|
||||
options.push({ type: 'divider' })
|
||||
options.push(getDeleteOption())
|
||||
|
||||
return options
|
||||
// Section 4: Node properties (Node Info, Shape, Color)
|
||||
if (nodeDef.value) {
|
||||
options.push(getNodeInfoOption(showNodeHelp))
|
||||
}
|
||||
if (groupContext) {
|
||||
options.push(getGroupColorOptions(groupContext, bump))
|
||||
} else {
|
||||
// Add shape and color options
|
||||
const visualOptions = getNodeVisualOptions(states, bump)
|
||||
if (visualOptions.length > 1) {
|
||||
options.push(visualOptions[1]) // Shape (index 1)
|
||||
}
|
||||
if (visualOptions.length > 2) {
|
||||
options.push(visualOptions[2]) // Color (index 2)
|
||||
}
|
||||
}
|
||||
options.push({ type: 'divider' })
|
||||
|
||||
// Section 5: Image operations (if image node)
|
||||
if (hasImageNode.value && selectedNodes.value.length > 0) {
|
||||
options.push(...getImageMenuOptions(selectedNodes.value[0]))
|
||||
options.push({ type: 'divider' })
|
||||
}
|
||||
// Section 6 & 7: Extensions and Delete are handled by buildStructuredMenu
|
||||
|
||||
// Mark all Vue options with source
|
||||
const markedVueOptions = markAsVueOptions(options)
|
||||
|
||||
if (litegraphOptions.length > 0) {
|
||||
// Merge: LiteGraph options first, then Vue options (Vue will win in dedup)
|
||||
const merged = [...litegraphOptions, ...markedVueOptions]
|
||||
return buildStructuredMenu(merged)
|
||||
}
|
||||
// For other cases, structure the Vue options
|
||||
const result = buildStructuredMenu(markedVueOptions)
|
||||
return result
|
||||
})
|
||||
|
||||
// Computed property to get only menu items with submenus
|
||||
|
||||
@@ -73,6 +73,7 @@ export function useNodeMenuOptions() {
|
||||
icon: 'icon-[lucide--palette]',
|
||||
hasSubmenu: true,
|
||||
submenu: colorSubmenu.value,
|
||||
isColorPicker: true,
|
||||
action: () => {}
|
||||
}
|
||||
]
|
||||
@@ -96,7 +97,7 @@ export function useNodeMenuOptions() {
|
||||
label: states.bypassed
|
||||
? t('contextMenu.Remove Bypass')
|
||||
: t('contextMenu.Bypass'),
|
||||
icon: states.bypassed ? 'icon-[lucide--zap-off]' : 'icon-[lucide--ban]',
|
||||
icon: 'icon-[lucide--redo-dot]',
|
||||
shortcut: 'Ctrl+B',
|
||||
action: () => {
|
||||
toggleNodeBypass()
|
||||
|
||||
@@ -1,163 +0,0 @@
|
||||
import { nextTick } from 'vue'
|
||||
|
||||
import type { MenuOption } from './useMoreOptionsMenu'
|
||||
|
||||
/**
|
||||
* Composable for handling submenu positioning logic
|
||||
*/
|
||||
export function useSubmenuPositioning() {
|
||||
/**
|
||||
* Toggle submenu visibility with proper positioning
|
||||
* @param option - Menu option with submenu
|
||||
* @param event - Click event
|
||||
* @param submenu - PrimeVue Popover reference
|
||||
* @param currentSubmenu - Currently open submenu name
|
||||
* @param menuOptionsWithSubmenu - All menu options with submenus
|
||||
* @param submenuRefs - References to all submenu popovers
|
||||
*/
|
||||
const toggleSubmenu = async (
|
||||
option: MenuOption,
|
||||
event: Event,
|
||||
submenu: any, // Component instance with show/hide methods
|
||||
currentSubmenu: { value: string | null },
|
||||
menuOptionsWithSubmenu: MenuOption[],
|
||||
submenuRefs: Record<string, any> // Component instances
|
||||
): Promise<void> => {
|
||||
if (!option.label || !option.hasSubmenu) return
|
||||
|
||||
// Check if this submenu is currently open
|
||||
const isCurrentlyOpen = currentSubmenu.value === option.label
|
||||
|
||||
// Hide all submenus first
|
||||
menuOptionsWithSubmenu.forEach((opt) => {
|
||||
const sm = submenuRefs[`submenu-${opt.label}`]
|
||||
if (sm) {
|
||||
sm.hide()
|
||||
}
|
||||
})
|
||||
currentSubmenu.value = null
|
||||
|
||||
// If it wasn't open before, show it now
|
||||
if (!isCurrentlyOpen) {
|
||||
currentSubmenu.value = option.label
|
||||
await nextTick()
|
||||
|
||||
const menuItem = event.currentTarget as HTMLElement
|
||||
const menuItemRect = menuItem.getBoundingClientRect()
|
||||
|
||||
// Find the parent popover content element that contains this menu item
|
||||
const mainPopoverContent = menuItem.closest(
|
||||
'[data-pc-section="content"]'
|
||||
) as HTMLElement
|
||||
|
||||
if (mainPopoverContent) {
|
||||
const mainPopoverRect = mainPopoverContent.getBoundingClientRect()
|
||||
|
||||
// Create a temporary positioned element as the target
|
||||
const tempTarget = createPositionedTarget(
|
||||
mainPopoverRect.right + 8,
|
||||
menuItemRect.top,
|
||||
`submenu-target-${option.label}`
|
||||
)
|
||||
|
||||
// Create event using the temp target
|
||||
const tempEvent = createMouseEvent(
|
||||
mainPopoverRect.right + 8,
|
||||
menuItemRect.top
|
||||
)
|
||||
|
||||
// Show submenu relative to temp target
|
||||
submenu.show(tempEvent, tempTarget)
|
||||
|
||||
// Clean up temp target after a delay
|
||||
cleanupTempTarget(tempTarget, 100)
|
||||
} else {
|
||||
// Fallback: position to the right of the menu item
|
||||
const tempTarget = createPositionedTarget(
|
||||
menuItemRect.right + 8,
|
||||
menuItemRect.top,
|
||||
`submenu-fallback-target-${option.label}`
|
||||
)
|
||||
|
||||
// Create event using the temp target
|
||||
const tempEvent = createMouseEvent(
|
||||
menuItemRect.right + 8,
|
||||
menuItemRect.top
|
||||
)
|
||||
|
||||
// Show submenu relative to temp target
|
||||
submenu.show(tempEvent, tempTarget)
|
||||
|
||||
// Clean up temp target after a delay
|
||||
cleanupTempTarget(tempTarget, 100)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a temporary positioned DOM element for submenu targeting
|
||||
*/
|
||||
const createPositionedTarget = (
|
||||
left: number,
|
||||
top: number,
|
||||
id: string
|
||||
): HTMLElement => {
|
||||
const tempTarget = document.createElement('div')
|
||||
tempTarget.style.position = 'absolute'
|
||||
tempTarget.style.left = `${left}px`
|
||||
tempTarget.style.top = `${top}px`
|
||||
tempTarget.style.width = '1px'
|
||||
tempTarget.style.height = '1px'
|
||||
tempTarget.style.pointerEvents = 'none'
|
||||
tempTarget.style.visibility = 'hidden'
|
||||
tempTarget.id = id
|
||||
|
||||
document.body.appendChild(tempTarget)
|
||||
return tempTarget
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a mouse event with specific coordinates
|
||||
*/
|
||||
const createMouseEvent = (clientX: number, clientY: number): MouseEvent => {
|
||||
return new MouseEvent('click', {
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
clientX,
|
||||
clientY
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* Clean up temporary target element after delay
|
||||
*/
|
||||
const cleanupTempTarget = (target: HTMLElement, delay: number): void => {
|
||||
setTimeout(() => {
|
||||
if (target.parentNode) {
|
||||
target.parentNode.removeChild(target)
|
||||
}
|
||||
}, delay)
|
||||
}
|
||||
|
||||
/**
|
||||
* Hide all submenus
|
||||
*/
|
||||
const hideAllSubmenus = (
|
||||
menuOptionsWithSubmenu: MenuOption[],
|
||||
submenuRefs: Record<string, any>, // Component instances
|
||||
currentSubmenu: { value: string | null }
|
||||
): void => {
|
||||
menuOptionsWithSubmenu.forEach((option) => {
|
||||
const submenu = submenuRefs[`submenu-${option.label}`]
|
||||
if (submenu) {
|
||||
submenu.hide()
|
||||
}
|
||||
})
|
||||
currentSubmenu.value = null
|
||||
}
|
||||
|
||||
return {
|
||||
toggleSubmenu,
|
||||
hideAllSubmenus
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user