Skip to content

Commit 0d8bc21

Browse files
Myesterychristian-byrne
authored andcommitted
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)
1 parent 01e533e commit 0d8bc21

File tree

7 files changed

+149
-46
lines changed

7 files changed

+149
-46
lines changed

src/components/graph/GraphCanvas.vue

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
<template v-if="comfyAppReady">
6363
<TitleEditor />
6464
<SelectionToolbox v-if="selectionToolboxEnabled" />
65+
<NodeOptions />
6566
<!-- Render legacy DOM widgets only when Vue nodes are disabled -->
6667
<DomWidgets v-if="!shouldRenderVueNodes" />
6768
</template>
@@ -87,6 +88,7 @@ import GraphCanvasMenu from '@/components/graph/GraphCanvasMenu.vue'
8788
import NodeTooltip from '@/components/graph/NodeTooltip.vue'
8889
import SelectionToolbox from '@/components/graph/SelectionToolbox.vue'
8990
import TitleEditor from '@/components/graph/TitleEditor.vue'
91+
import NodeOptions from '@/components/graph/selectionToolbox/NodeOptions.vue'
9092
import NodeSearchboxPopover from '@/components/searchbox/NodeSearchBoxPopover.vue'
9193
import SideToolbar from '@/components/sidebar/SideToolbar.vue'
9294
import SecondRowWorkflowTabs from '@/components/topbar/SecondRowWorkflowTabs.vue'

src/components/graph/SelectionToolbox.spec.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -480,13 +480,6 @@ describe('SelectionToolbox', () => {
480480
} as any)
481481
})
482482

483-
it('should still show MoreOptions when no items selected', () => {
484-
canvasStore.selectedItems = []
485-
const wrapper = mountComponent()
486-
487-
expect(wrapper.find('.more-options').exists()).toBe(true)
488-
})
489-
490483
it('should hide most buttons when no items selected', () => {
491484
canvasStore.selectedItems = []
492485
const wrapper = mountComponent()

src/components/graph/SelectionToolbox.vue

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
:command="command"
3939
/>
4040
<ExecuteButton v-if="showExecute" />
41-
<MoreOptions />
41+
<NodeOptionsButton />
4242
</Panel>
4343
</Transition>
4444
</div>
@@ -68,7 +68,7 @@ import { useExtensionService } from '@/services/extensionService'
6868
import { type ComfyCommandImpl, useCommandStore } from '@/stores/commandStore'
6969
7070
import FrameNodes from './selectionToolbox/FrameNodes.vue'
71-
import MoreOptions from './selectionToolbox/MoreOptions.vue'
71+
import NodeOptionsButton from './selectionToolbox/NodeOptionsButton.vue'
7272
import VerticalDivider from './selectionToolbox/VerticalDivider.vue'
7373
7474
const commandStore = useCommandStore()

src/components/graph/selectionToolbox/MoreOptions.vue renamed to src/components/graph/selectionToolbox/NodeOptions.vue

Lines changed: 45 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,5 @@
11
<template>
2-
<div class="relative inline-flex items-center">
3-
<Button
4-
ref="buttonRef"
5-
v-tooltip.top="{
6-
value: $t('g.moreOptions'),
7-
showDelay: 1000
8-
}"
9-
data-testid="more-options-button"
10-
text
11-
class="h-8 w-8 px-0"
12-
severity="secondary"
13-
@click="toggle"
14-
>
15-
<i-lucide:more-vertical class="w-4 h-4" />
16-
</Button>
17-
2+
<div>
183
<Popover
194
ref="popover"
205
:append-to="'body'"
@@ -51,7 +36,6 @@
5136

5237
<script setup lang="ts">
5338
import { useRafFn } from '@vueuse/core'
54-
import Button from 'primevue/button'
5539
import Popover from 'primevue/popover'
5640
import { computed, onMounted, onUnmounted, ref, watch } from 'vue'
5741
@@ -64,6 +48,7 @@ import {
6448
import {
6549
type MenuOption,
6650
type SubMenuOption,
51+
registerNodeOptionsInstance,
6752
useMoreOptionsMenu
6853
} from '@/composables/graph/useMoreOptionsMenu'
6954
import { useSubmenuPositioning } from '@/composables/graph/useSubmenuPositioning'
@@ -74,7 +59,8 @@ import MenuOptionItem from './MenuOptionItem.vue'
7459
import SubmenuPopover from './SubmenuPopover.vue'
7560
7661
const popover = ref<InstanceType<typeof Popover>>()
77-
const buttonRef = ref<InstanceType<typeof Button> | HTMLElement | null>(null)
62+
const targetElement = ref<HTMLElement | null>(null)
63+
const isTriggeredByToolbox = ref<boolean>(true)
7864
// Track open state ourselves so we can restore after drag/move
7965
const isOpen = ref(false)
8066
const wasOpenBeforeHide = ref(false)
@@ -90,11 +76,6 @@ const canvasInteractions = useCanvasInteractions()
9076
const minimap = useMinimap()
9177
const containerStyles = minimap.containerStyles
9278
93-
function getButtonEl(): HTMLElement | null {
94-
const el = (buttonRef.value as any)?.$el || buttonRef.value
95-
return el instanceof HTMLElement ? el : null
96-
}
97-
9879
let lastLogTs = 0
9980
const LOG_INTERVAL = 120 // ms
10081
let overlayElCache: HTMLElement | null = null
@@ -109,7 +90,7 @@ function resolveOverlayEl(): HTMLElement | null {
10990
return direct
11091
}
11192
// Fallback: try to locate a recent popover root near the button (same z-index class + absolute)
112-
const btn = getButtonEl()
93+
const btn = targetElement.value
11394
if (btn) {
11495
const candidates = Array.from(
11596
document.querySelectorAll('div.absolute.z-50')
@@ -132,20 +113,24 @@ function resolveOverlayEl(): HTMLElement | null {
132113
133114
const repositionPopover = () => {
134115
if (!isOpen.value) return
135-
const btn = getButtonEl()
116+
const btn = targetElement.value
136117
const overlayEl = resolveOverlayEl()
137118
if (!btn || !overlayEl) return
138119
const rect = btn.getBoundingClientRect()
139120
const marginY = 8 // tailwind mt-2 ~ 0.5rem = 8px
140-
const left = rect.left + rect.width / 2
141-
const top = rect.bottom + marginY
121+
const left = isTriggeredByToolbox.value
122+
? rect.left + rect.width / 2
123+
: rect.right - rect.width / 4
124+
const top = isTriggeredByToolbox.value
125+
? rect.bottom + marginY
126+
: rect.top - marginY - 6
142127
try {
143128
overlayEl.style.position = 'fixed'
144129
overlayEl.style.left = `${left}px`
145130
overlayEl.style.top = `${top}px`
146131
overlayEl.style.transform = 'translate(-50%, 0)'
147132
} catch (e) {
148-
console.warn('[MoreOptions] Failed to set overlay style', e)
133+
console.warn('[NodeOptions] Failed to set overlay style', e)
149134
return
150135
}
151136
const now = performance.now()
@@ -156,9 +141,16 @@ const repositionPopover = () => {
156141
157142
const { resume: startSync, pause: stopSync } = useRafFn(repositionPopover)
158143
159-
function openPopover(triggerEvent?: Event): boolean {
160-
const el = getButtonEl()
144+
function openPopover(
145+
triggerEvent?: Event,
146+
element?: HTMLElement,
147+
clickedFromToolbox?: boolean
148+
): boolean {
149+
const el = element || targetElement.value
161150
if (!el || !el.isConnected) return false
151+
targetElement.value = el
152+
if (clickedFromToolbox !== undefined)
153+
isTriggeredByToolbox.value = clickedFromToolbox
162154
bump()
163155
popover.value?.show(triggerEvent ?? new Event('reopen'), el)
164156
isOpen.value = true
@@ -191,7 +183,7 @@ function attemptRestore() {
191183
if (isOpen.value) return
192184
if (!wasOpenBeforeHide.value && !moreOptionsRestorePending.value) return
193185
// Try immediately
194-
if (openPopover(new Event('reopen'))) {
186+
if (openPopover(new Event('reopen'), targetElement.value || undefined)) {
195187
wasOpenBeforeHide.value = false
196188
restoreAttempts = 0
197189
return
@@ -202,13 +194,24 @@ function attemptRestore() {
202194
requestAnimationFrame(() => attemptRestore())
203195
}
204196
205-
const toggle = (event: Event) => {
197+
const toggle = (
198+
event: Event,
199+
element?: HTMLElement,
200+
clickedFromToolbox?: boolean
201+
) => {
206202
if (isOpen.value) closePopover('manual')
207-
else openPopover(event)
203+
else openPopover(event, element, clickedFromToolbox)
208204
}
209205
210206
const hide = (reason: HideReason = 'manual') => closePopover(reason)
211207
208+
// Export functions for external triggering
209+
defineExpose({
210+
toggle,
211+
hide,
212+
isOpen
213+
})
214+
212215
const hideAll = () => {
213216
hideAllSubmenus(
214217
menuOptionsWithSubmenu.value,
@@ -305,12 +308,21 @@ watch(
305308
)
306309
307310
onMounted(() => {
311+
// Register this instance globally
312+
registerNodeOptionsInstance({
313+
toggle,
314+
hide,
315+
isOpen
316+
})
317+
308318
if (moreOptionsRestorePending.value && !isOpen.value) {
309319
requestAnimationFrame(() => attemptRestore())
310320
}
311321
})
312322
313323
onUnmounted(() => {
314324
stopSync()
325+
// Unregister on unmount
326+
registerNodeOptionsInstance(null)
315327
})
316328
</script>
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<template>
2+
<Button
3+
ref="buttonRef"
4+
v-tooltip.top="{
5+
value: $t('g.moreOptions'),
6+
showDelay: 1000
7+
}"
8+
data-testid="more-options-button"
9+
text
10+
class="h-8 w-8 px-0"
11+
severity="secondary"
12+
@click="handleClick"
13+
>
14+
<i-lucide:more-vertical class="w-4 h-4" />
15+
</Button>
16+
</template>
17+
18+
<script setup lang="ts">
19+
import Button from 'primevue/button'
20+
import { ref } from 'vue'
21+
22+
import { toggleNodeOptions } from '@/composables/graph/useMoreOptionsMenu'
23+
24+
const buttonRef = ref<InstanceType<typeof Button> | null>(null)
25+
26+
const handleClick = (event: Event) => {
27+
const el = (buttonRef.value as any)?.$el || buttonRef.value
28+
const buttonEl = el instanceof HTMLElement ? el : null
29+
if (buttonEl) {
30+
toggleNodeOptions(event, buttonEl, true)
31+
}
32+
}
33+
</script>

src/composables/graph/useMoreOptionsMenu.ts

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { computed, ref } from 'vue'
1+
import { type Ref, computed, ref } from 'vue'
22

33
import type { LGraphGroup } from '@/lib/litegraph/src/litegraph'
44
import { isLGraphGroup } from '@/utils/litegraphUtil'
@@ -32,6 +32,47 @@ export enum BadgeVariant {
3232
DEPRECATED = 'deprecated'
3333
}
3434

35+
// Global singleton for NodeOptions component reference
36+
let nodeOptionsInstance: null | NodeOptionsInstance = null
37+
38+
/**
39+
* Toggle the node options popover
40+
* @param event - The trigger event
41+
* @param element - The target element (button) that triggered the popover
42+
*/
43+
export function toggleNodeOptions(
44+
event: Event,
45+
element: HTMLElement,
46+
clickedFromToolbox: boolean = false
47+
) {
48+
if (nodeOptionsInstance?.toggle) {
49+
nodeOptionsInstance.toggle(event, element, clickedFromToolbox)
50+
}
51+
}
52+
53+
/**
54+
* Hide the node options popover
55+
*/
56+
interface NodeOptionsInstance {
57+
toggle: (
58+
event: Event,
59+
element: HTMLElement,
60+
clickedFromToolbox: boolean
61+
) => void
62+
hide: () => void
63+
isOpen: Ref<boolean>
64+
}
65+
66+
/**
67+
* Register the NodeOptions component instance
68+
* @param instance - The NodeOptions component instance
69+
*/
70+
export function registerNodeOptionsInstance(
71+
instance: null | NodeOptionsInstance
72+
) {
73+
nodeOptionsInstance = instance
74+
}
75+
3576
/**
3677
* Composable for managing the More Options menu configuration
3778
* Refactored to use smaller, focused composables for better maintainability
@@ -181,6 +222,7 @@ export function useMoreOptionsMenu() {
181222
menuOptions,
182223
menuOptionsWithSubmenu,
183224
bump,
184-
hasSubgraphs
225+
hasSubgraphs,
226+
registerNodeOptionsInstance
185227
}
186228
}

src/renderer/extensions/vueNodes/components/LGraphNode.vue

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
]"
4141
v-bind="pointerHandlers"
4242
@wheel="handleWheel"
43+
@contextmenu="handleContextMenu"
4344
>
4445
<div class="flex items-center">
4546
<template v-if="isCollapsed">
@@ -135,6 +136,7 @@ import { storeToRefs } from 'pinia'
135136
import { computed, inject, onErrorCaptured, onMounted, provide, ref } from 'vue'
136137
137138
import type { VueNodeData } from '@/composables/graph/useGraphNodeManager'
139+
import { toggleNodeOptions } from '@/composables/graph/useMoreOptionsMenu'
138140
import { useErrorHandling } from '@/composables/useErrorHandling'
139141
import { LiteGraph } from '@/lib/litegraph/src/litegraph'
140142
import { useCanvasStore } from '@/renderer/core/canvas/canvasStore'
@@ -175,8 +177,12 @@ const {
175177
readonly = false
176178
} = defineProps<LGraphNodeProps>()
177179
178-
const { handleNodeCollapse, handleNodeTitleUpdate, handleNodeSelect } =
179-
useNodeEventHandlers()
180+
const {
181+
handleNodeCollapse,
182+
handleNodeTitleUpdate,
183+
handleNodeSelect,
184+
handleNodeRightClick
185+
} = useNodeEventHandlers()
180186
181187
useVueElementTracking(() => nodeData.id, 'node')
182188
@@ -235,6 +241,21 @@ const { pointerHandlers, isDragging, dragStyle } = useNodePointerInteractions(
235241
handleNodeSelect
236242
)
237243
244+
// Handle right-click context menu
245+
const handleContextMenu = (event: MouseEvent) => {
246+
event.preventDefault()
247+
event.stopPropagation()
248+
249+
// First handle the standard right-click behavior (selection)
250+
handleNodeRightClick(event as PointerEvent, nodeData)
251+
252+
// Show the node options menu at the cursor position
253+
const targetElement = event.currentTarget as HTMLElement
254+
if (targetElement) {
255+
toggleNodeOptions(event, targetElement, false)
256+
}
257+
}
258+
238259
onMounted(() => {
239260
if (size.value && transformState?.camera) {
240261
const scale = transformState.camera.z

0 commit comments

Comments
 (0)