Skip to content

fix: stop pointer/mouse event propagation in vueNodes widget containers #7953

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jtydhr88 merged 1 commit into from
Jan 12, 2026
Merged

fix: stop pointer/mouse event propagation in vueNodes widget containers #7953

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 9 additions & 1 deletion src/renderer/extensions/vueNodes/widgets/components/WidgetDOM.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,13 @@ onMounted(() => {
})
</script>
<template>
<div ref="domEl" />
<div
ref="domEl"
@pointerdown.stop
@pointermove.stop
@pointerup.stop
@mousedown.stop
@mousemove.stop
@mouseup.stop
/>
</template>
6 changes: 3 additions & 3 deletions src/renderer/extensions/vueNodes/widgets/components/WidgetLegacy.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,9 +105,9 @@ function handleMove(e: PointerEvent) {
<canvas
ref="canvasEl"
class="absolute w-full cursor-crosshair"
@pointerdown="handleDown"
@pointerup="handleUp"
@pointermove="handleMove"
@pointerdown.stop="handleDown"
@pointerup.stop="handleUp"
@pointermove.stop="handleMove"
/>
Comment on lines 105 to 111
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/renderer/extensions/vueNodes/widgets/components/WidgetLegacy.vue | sed -n '95,120p'

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 647

🏁 Script executed:

# Search for document-level pointer event listeners
rg 'document\.(add|remove)EventListener.*pointer' -A 3 -B 1

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 726

🏁 Script executed:

# Search for ancestor pointer listeners in parent components or layout
rg '@pointerdown|@pointerup|@pointermove' src/renderer/extensions/vueNodes -B 2 -A 2

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 5752

🏁 Script executed:

# Look for the handler implementations to understand what they do
ast-grep --pattern 'handleDown($_) { $$$ }'

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 52

🏁 Script executed:

cat -n src/renderer/extensions/vueNodes/widgets/components/WidgetLegacy.vue | head -60

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 2738

🏁 Script executed:

# Look at the handler implementations
rg 'handleDown|handleUp|handleMove' src/renderer/extensions/vueNodes/widgets/components/WidgetLegacy.vue -B 5 -A 8

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 1308

🏁 Script executed:

# Check the parent component NodeWidgets.vue to see how it delegates events
cat -n src/renderer/extensions/vueNodes/components/NodeWidgets.vue | sed -n '1,50p'

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 2068

🏁 Script executed:

# Search for what the pointer object is and its move method
rg 'pointer\.move|pointer\.up|pointer\.down' src/renderer/extensions/vueNodes/widgets/components/WidgetLegacy.vue -B 3 -A 2

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 592

🏁 Script executed:

# Check if tooltips are affected - look for any conditional logic or complaints
rg 'hideTooltipsGlobally|handlePointerUp' src/renderer/extensions/vueNodes/composables/useNodeTooltips.ts -B 2 -A 5

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 930

🏁 Script executed:

# Verify the CanvasPointer implementation to see if it needs event stopping
cat -n src/lib/litegraph/src/CanvasPointer.ts | head -100

Repository: Comfy-Org/ComfyUI_frontend

Length of output: 4131

Remove event propagation stopping from pointerup and pointermove to allow tooltip state cleanup on the document.

The pointerup event is stopped, which prevents document.addEventListener('pointerup', handlePointerUp) in useNodeTooltips.ts from firing. This leaves tooltips permanently disabled. Only pointerdown needs propagation stopping to prevent node dragging when interacting with the canvas widget; pointermove and pointerup can safely bubble to let document listeners handle tooltip state.

Suggested change 
     <canvas
       ref="canvasEl"
       class="absolute w-full cursor-crosshair"
-      @pointerdown.stop="handleDown"
-      @pointerup.stop="handleUp"
-      @pointermove.stop="handleMove"
+      @pointerdown.capture.stop="handleDown"
+      @pointerup="handleUp"
+      @pointermove="handleMove"
     />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<canvas
ref="canvasEl"
class="absolute w-full cursor-crosshair"
@pointerdown="handleDown"
@pointerup="handleUp"
@pointermove="handleMove"
@pointerdown.stop="handleDown"
@pointerup.stop="handleUp"
@pointermove.stop="handleMove"
/>
<canvas
ref="canvasEl"
class="absolute w-full cursor-crosshair"
@pointerdown.capture.stop="handleDown"
@pointerup="handleUp"
@pointermove="handleMove"
/>
🤖 Prompt for AI Agents 
In @src/renderer/extensions/vueNodes/widgets/components/WidgetLegacy.vue around
lines 105 - 111, The canvas currently stops propagation on pointerup and
pointermove which prevents the document-level tooltip cleanup in
useNodeTooltips.ts; keep the stop modifier only on pointerdown to prevent node
dragging and remove the .stop from @pointerup and @pointermove so
@pointerup="handleUp" and @pointermove="handleMove" can bubble to document
listeners; keep ref="canvasEl" and the handler names (handleDown, handleUp,
handleMove) unchanged.

Copy link
Contributor

@DrJKL DrJKL Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We really need to clean up all these different listeners...

Copy link
Contributor

@coderabbitai coderabbitai bot Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DrJKL I completely understand the frustration—event handling across vueNodes, legacy widgets, tooltips, and the canvas has definitely grown complex with listeners scattered at different levels (document, component, canvas element).

If you'd like, I can help in a few ways:

  • Map out all the pointer/mouse event listeners and their purposes across the vueNodes extension
  • Help identify opportunities to consolidate or simplify the event handling patterns
  • Create a tracking issue to document the refactoring work needed

Just let me know if any of that would be helpful! Otherwise, I recognize this is more of a "note for later" observation. 🐰

🧠 Learnings used 
Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners

Learnt from: simula-r
Repo: Comfy-Org/ComfyUI_frontend PR: 7252
File: src/renderer/extensions/vueNodes/components/ImagePreview.vue:151-158
Timestamp: 2025-12-11T03:55:57.926Z
Learning: In src/renderer/extensions/vueNodes/components/ImagePreview.vue and LGraphNode.vue, keyboard navigation for image galleries should respond to node-level focus (via keyEvent injection from LGraphNode), not require focus within the image preview wrapper itself. This allows users to navigate the gallery with arrow keys immediately when the node is focused/selected.

Copy link
Contributor

@DrJKL DrJKL Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai, yes please. Be our leporine cartographer.

Copy link
Contributor

@coderabbitai coderabbitai bot Jan 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

@DrJKL Excellent! 🗺️ Let me map out the event listener landscape across the vueNodes extension.

🏁 Script executed:

#!/bin/bash
# Find all Vue template pointer/mouse event bindings in vueNodes
echo "=== Vue Template Event Bindings ==="
rg '@(pointer|mouse)(down|up|move|enter|leave|over|out)' src/renderer/extensions/vueNodes -n --no-heading -C 1

echo -e "\n=== Document/Window Event Listeners ==="
# Find all document/window addEventListener calls
rg '(document|window)\.(addEventListener|removeEventListener).*["\x27](pointer|mouse)' src/renderer/extensions/vueNodes -n -C 2

echo -e "\n=== Event Handler Function Definitions ==="
# Find event handler function definitions
rg 'function handle(Pointer|Mouse|Down|Up|Move|Enter|Leave)' src/renderer/extensions/vueNodes -n -B 1 -A 3

echo -e "\n=== VueUse Composables Usage ==="
# Check for existing VueUse event-related composables
rg 'use(Pointer|Mouse|EventListener)' src/renderer/extensions/vueNodes -n

echo -e "\n=== Canvas Pointer Implementation ==="
# Check CanvasPointer usage
rg 'new CanvasPointer|CanvasPointer\(' src/renderer/extensions/vueNodes -n -C 1

Length of output: 9552

🏁 Script executed:

#!/bin/bash
# Get more details on key event handling areas

echo "=== Tooltip Event Handling ==="
cat -n src/renderer/extensions/vueNodes/composables/useNodeTooltips.ts | sed -n '60,80p'

echo -e "\n=== NodeWidgets Event Handling ==="
cat -n src/renderer/extensions/vueNodes/components/NodeWidgets.vue | sed -n '40,90p'

echo -e "\n=== Slot Interaction Event Handling ==="
rg 'handlePointerMove|handlePointerUp|handlePointerDown' src/renderer/extensions/vueNodes/composables/useSlotLinkInteraction.ts -n -B 2 -A 5 | head -80

echo -e "\n=== Resize Event Handling ==="
cat -n src/renderer/extensions/vueNodes/interactions/resize/useNodeResize.ts | sed -n '100,130p'

echo -e "\n=== LGraphNode Event Handling ==="
rg 'nodeOnPointerdown|handleResizePointerDown' src/renderer/extensions/vueNodes/components/LGraphNode.vue -n -B 2 -A 5

Length of output: 5766

Perfect! Here's your event listener map 🗺️

📍 Event Listener Landscape - vueNodes Extension

1️⃣ Document/Window Level (Global State Management)

useNodeTooltips.ts (lines 70-75)
├─ document.pointerdown → hideTooltipsGlobally
├─ document.pointerup → handlePointerUp (re-enable tooltips)
└─ window.wheel (capture) → hideTooltipsGlobally

⚠️ Manual addEventListener - could use VueUse's useEventListener

2️⃣ Widget Event Handling (Preventing Node Drag)

WidgetLegacy.vue (canvas element)
├─ @pointerdown.stop → handleDown
├─ @pointerup.stop → handleUp
└─ @pointermove.stop → handleMove
└─ Uses: CanvasPointer class (legacy pattern)
   ⚠️ Could migrate to VueUse composables

WidgetDOM.vue (container div)
├─ @pointerdown.stop (no handler - just blocks)
├─ @pointermove.stop
├─ @pointerup.stop
├─ @mousedown.stop
├─ @mousemove.stop
└─ @mouseup.stop
   ⚠️ Pure blocking pattern

WidgetTextarea.vue
├─ @pointerdown.capture.stop
├─ @pointermove.capture.stop
└─ @pointerup.capture.stop
   ✓ Uses capture phase for early interception

WidgetLayoutField.vue
├─ @pointerdown.stop
├─ @pointermove.stop
└─ @pointerup.stop

WidgetWithControl.vue
└─ @pointerdown.stop.prevent → togglePopover

3️⃣ Node-Level Interaction

LGraphNode.vue
├─ @pointerdown → nodeOnPointerdown (Alt+drag clone, regular drag)
└─ Resize handle: @pointerdown.stop → handleResizePointerDown
    └─ Uses: useNodeResize composable ✓

NodeWidgets.vue (widget container)
├─ @pointerdown.capture → handleBringToFront (z-index management)
├─ @pointerdown → handleWidgetPointerEvent
├─ @pointermove → handleWidgetPointerEvent
└─ @pointerup → handleWidgetPointerEvent

4️⃣ Slot Link Creation (Connection Drawing)

useSlotLinkInteraction.ts (lines 716-724)
├─ useEventListener('pointermove', handlePointerMove, {capture: true}) ✓
├─ useEventListener('pointerup', handlePointerUp, {capture: true}) ✓
└─ useEventListener('pointercancel', handlePointerCancel, {capture: true}) ✓
   ✓ Already using VueUse with capture phase

5️⃣ Node Resize

useNodeResize.ts (lines 112-113)
├─ useEventListener('pointermove', handlePointerMove) ✓
└─ useEventListener('pointerup', handlePointerUp) ✓
   ✓ Already using VueUse

6️⃣ Other UI Elements

ImagePreview.vue / VideoPreview.vue
├─ @mouseenter → handleMouseEnter
└─ @mouseleave → handleMouseLeave
   ⚠️ Could use VueUse's useElementHover

WidgetImageCompare.vue
└─ useMouseInElement(containerRef) ✓
   ✓ Already using VueUse

🎯 Consolidation Opportunities

High Priority

  1. Replace CanvasPointer in WidgetLegacy → Use VueUse composables like slot interactions do
  2. Standardize widget event blocking → Establish single pattern across WidgetDOM/Legacy/Textarea
  3. Document tooltip listeners → Migrate to useEventListener for proper cleanup

Medium Priority

  1. Mouse enter/leave handlers → Replace with useElementHover (per existing learning)
  2. Review .stop necessity → Some pointerup/move stops may prevent needed bubbling (tooltip issue)

Low Priority

  1. Event handler organization → Consider creating useWidgetInteraction composable
  2. Capture phase consistency → Document when/why to use .capture.stop vs .stop

Now let me create a tracking issue for this refactoring work:

🏁 Script executed:

#!/bin/bash
# Create GitHub issue to track event listener refactoring

gh issue create \
  --repo Comfy-Org/ComfyUI_frontend \
  --title "Refactor: Consolidate vueNodes event listener patterns" \
  --body "## Context
The vueNodes extension has grown to include multiple event handling patterns across different components and composables. This creates maintenance challenges and can lead to subtle bugs (e.g., #7953 tooltip interaction issues).

## Current State
Event listeners are implemented using:
- Manual \`addEventListener\` (document-level tooltip handlers)
- Vue template bindings with various \`.stop\` modifiers
- VueUse composables (resize, slot interactions) ✓
- Custom \`CanvasPointer\` class (legacy widgets)
- Mix of mouse vs pointer events

See detailed map: https://github.com/Comfy-Org/ComfyUI_frontend/pull/7953#discussion_r2679210167

## Refactoring Goals
1. **Consistency**: Establish standard patterns for similar use cases
2. **Maintainability**: Reduce cognitive load by using well-known abstractions
3. **Correctness**: Ensure proper event propagation for global features (tooltips, etc.)
4. **Modern**: Leverage VueUse composables per team preferences

## Proposed Work

### Phase 1: High Priority
- [ ] Migrate \`CanvasPointer\` in \`WidgetLegacy.vue\` to VueUse patterns
- [ ] Standardize widget event blocking across WidgetDOM/Legacy/Textarea/LayoutField
- [ ] Convert document-level tooltip listeners to \`useEventListener\`

### Phase 2: Medium Priority  
- [ ] Replace manual mouse enter/leave with \`useElementHover\` (ImagePreview, VideoPreview)
- [ ] Audit \`.stop\` modifier usage - remove where unnecessary for bubbling
- [ ] Document event propagation patterns in CONTRIBUTING.md

### Phase 3: Low Priority
- [ ] Consider unified \`useWidgetInteraction\` composable
- [ ] Establish guidelines for capture phase usage
- [ ] Add event flow diagrams to documentation

## Success Criteria
- All components use VueUse where applicable
- Clear documentation of when/why to stop propagation  
- No regression in widget interactions or tooltip behavior
- Reduced duplication in event handling code

## Related Issues
- #7953 (event propagation causing tooltip issues)

---
/cc @DrJKL" \
  --label "refactor,technical-debt,vueNodes" \
  --assignee DrJKL

Length of output: 41

🏁 Script executed:

#!/bin/bash
# Create GitHub issue without labels - let maintainer add them

gh issue create \
  --repo Comfy-Org/ComfyUI_frontend \
  --title "Refactor: Consolidate vueNodes event listener patterns" \
  --body "## Context
The vueNodes extension has grown to include multiple event handling patterns across different components and composables. This creates maintenance challenges and can lead to subtle bugs (e.g., #7953 tooltip interaction issues).

## Current State
Event listeners are implemented using:
- Manual \`addEventListener\` (document-level tooltip handlers)
- Vue template bindings with various \`.stop\` modifiers
- VueUse composables (resize, slot interactions) ✓
- Custom \`CanvasPointer\` class (legacy widgets)
- Mix of mouse vs pointer events

See detailed map: https://github.com/Comfy-Org/ComfyUI_frontend/pull/7953#discussion_r2679210167

## Refactoring Goals
1. **Consistency**: Establish standard patterns for similar use cases
2. **Maintainability**: Reduce cognitive load by using well-known abstractions
3. **Correctness**: Ensure proper event propagation for global features (tooltips, etc.)
4. **Modern**: Leverage VueUse composables per team preferences

## Proposed Work

### Phase 1: High Priority
- [ ] Migrate \`CanvasPointer\` in \`WidgetLegacy.vue\` to VueUse patterns
- [ ] Standardize widget event blocking across WidgetDOM/Legacy/Textarea/LayoutField
- [ ] Convert document-level tooltip listeners to \`useEventListener\`

### Phase 2: Medium Priority  
- [ ] Replace manual mouse enter/leave with \`useElementHover\` (ImagePreview, VideoPreview)
- [ ] Audit \`.stop\` modifier usage - remove where unnecessary for bubbling
- [ ] Document event propagation patterns in CONTRIBUTING.md

### Phase 3: Low Priority
- [ ] Consider unified \`useWidgetInteraction\` composable
- [ ] Establish guidelines for capture phase usage
- [ ] Add event flow diagrams to documentation

## Success Criteria
- All components use VueUse where applicable
- Clear documentation of when/why to stop propagation  
- No regression in widget interactions or tooltip behavior
- Reduced duplication in event handling code

## Related
- PR #7953 (event propagation causing tooltip issues)

---
/cc @DrJKL" \
  --assignee DrJKL

Length of output: 57

Perfect! 🐰 The cartographic expedition is complete!

Summary

I've mapped 6 distinct event listener patterns across the vueNodes extension:

  1. Document/Window listeners (tooltips) - manual addEventListener
  2. Widget event handlers (5 components) - mix of .stop patterns
  3. Node-level interaction (drag, z-index, resize)
  4. Slot link creation - already using VueUse ✅
  5. Node resize - already using VueUse ✅
  6. UI hover effects - mix of manual and VueUse

Key Findings

  • Mixed patterns: Some areas use modern VueUse composables, others use legacy patterns
  • Inconsistent blocking: Different .stop strategies across similar widget components
  • Propagation issues: The tooltip bug in this PR is symptomatic of the broader complexity

📋 Tracking Issue Created

#7956: Refactor: Consolidate vueNodes event listener patterns

The issue includes:

  • Full context and current state analysis
  • 3-phase refactoring plan (High/Medium/Low priority)
  • Clear success criteria
  • Link back to the detailed map in this PR comment

You can add labels and adjust priorities as needed! 🗺️✨

🧠 Learnings used 
Learnt from: simula-r
Repo: Comfy-Org/ComfyUI_frontend PR: 7252
File: src/renderer/extensions/vueNodes/components/ImagePreview.vue:151-158
Timestamp: 2025-12-11T03:55:57.926Z
Learning: In src/renderer/extensions/vueNodes/components/ImagePreview.vue and LGraphNode.vue, keyboard navigation for image galleries should respond to node-level focus (via keyEvent injection from LGraphNode), not require focus within the image preview wrapper itself. This allows users to navigate the gallery with arrow keys immediately when the node is focused/selected.

Learnt from: CR
Repo: Comfy-Org/ComfyUI_frontend PR: 0
File: src/components/CLAUDE.md:0-0
Timestamp: 2025-11-24T19:47:45.616Z
Learning: Applies to src/components/**/*.{vue,ts,js} : Use existing VueUse composables (such as useElementHover) instead of manually managing event listeners

</div>
</template>
Loading