Drag vuenodes input #6514
Merged
Drag vuenodes input #6514
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Enable drag-and-drop file uploads for load image, load video, and load audio nodes in VueNodes mode by forwarding drag events to existing litegraph callbacks. - Add drag event handlers (@dragover, @dragleave, @drop) to node container - Add visual feedback (border highlight) when dragging valid files over nodes - Forward drop events to node.onDragDrop callback - Mark canvas as dirty after drop to trigger VueNodes refresh
Create new arrays for batch uploads to ensure Vue detects value changes. When allow_batch is true, spread the output array to create a new reference instead of reusing the same array reference which Vue won't detect as changed.
After uploading audio files, manually invoke the widget callback to ensure VueNodes reactive system detects the value change and updates the UI. Also mark canvas as dirty in the callback to trigger VueNodes refresh.
Add nextTick callback to completely replace node data object after widget updates, ensuring Vue's reactivity system detects changes and re-renders all dependent components.
Pass modelValue as a getter function (() => props.modelValue) instead of the direct prop value to ensure the watcher inside useWidgetValue properly tracks reactive prop changes.
Update useWidgetValue to accept modelValue as either a value or getter function. Normalize input to always use getter function for proper reactivity tracking. This fixes the issue where widget value changes weren't detected because the watcher was watching a static captured value instead of a reactive prop.
dosubot
bot
added
the
size:L
🎭 Playwright Test Results⏰ Completed at: 11/03/2025, 08:36:34 PM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 11/03/2025, 08:22:32 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 3.27 MB (baseline 3.27 MB) • 🔴 +240 BMain entry bundles and manifests
Status: 3 added / 3 removed Graph Workspace — 729 kB (baseline 728 kB) • 🔴 +1.14 kBGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 8.18 kB (baseline 8.18 kB) • ⚪ 0 BTop-level views, pages, and routed surfaces
Status: 1 added / 1 removed Panels & Settings — 293 kB (baseline 293 kB) • ⚪ 0 BConfiguration panels, inspectors, and settings screens
Status: 6 added / 6 removed UI Components — 12.6 kB (baseline 12.6 kB) • ⚪ 0 BReusable component library chunks
Status: 1 added / 1 removed Data & Services — 10.4 kB (baseline 10.4 kB) • ⚪ 0 BStores, services, APIs, and repositories
Status: 1 added / 1 removed Utilities & Hooks — 1.07 kB (baseline 1.07 kB) • ⚪ 0 BHelpers, composables, and utility bundles
Vendor & Third-Party — 5.32 MB (baseline 5.32 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Other — 2.55 MB (baseline 2.55 MB) • ⚪ 0 BBundles that do not match a named category
|
DrJKL
reviewed
Nov 1, 2025
DrJKL
reviewed
Nov 1, 2025
DrJKL
reviewed
Nov 1, 2025
DrJKL
reviewed
Nov 1, 2025
DrJKL
reviewed
Nov 1, 2025
DrJKL
reviewed
Nov 1, 2025
christian-byrne
added
the
claude-review
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
src/renderer/extensions/vueNodes/widgets/composables/useImageUploadWidget.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Comprehensive PR Review
This review is generated by Claude. It may not always be accurate, as with human reviewers. If you believe that any of the comments are invalid or incorrect, please state why for each. For others, please implement the changes in one way or another.
Review Summary
PR: Drag vuenodes input (#6514)
Impact: 94 additions, 20 deletions across 6 files
Issue Distribution
- Critical: 0
- High: 1
- Medium: 3
- Low: 2
Category Breakdown
- Architecture: 1 issues
- Security: 0 issues
- Performance: 2 issues
- Code Quality: 3 issues
Key Findings
Architecture & Design
The main architectural concern is the forced reactivity approach using nextTick and object reassignment. This indicates that the reactive data structure design needs improvement. The current solution is a workaround rather than a proper architectural solution to reactivity challenges.
Performance Impact
Two performance concerns were identified:
- Using Date.now() and object spread operations on every widget update could impact performance in graphs with many frequently-updating widgets
- Unnecessary array spread operations for non-batch upload scenarios create wasteful operations
Code Quality Issues
Several code quality improvements were identified:
- Type assertions without proper runtime validation could cause runtime errors
- Manual callback invocations without error handling could lead to uncaught exceptions
- Potential null reference errors in drag event handling where event.relatedTarget could be null
Positive Observations
- The drag-and-drop functionality implementation is well-structured with proper event handling
- The reactivity improvements address real user experience issues with Vue component updates
- Code includes helpful comments explaining the reasoning behind the changes
- The approach to maintaining LiteGraph/Vue synchronization shows good understanding of the dual-state management challenge
References
Next Steps
- Address the high-priority architectural issue around reactivity design before merge
- Consider implementing proper type guards for the medium-priority type safety issues
- Add error handling for callback invocations
- Optimize performance for frequently-updated widgets
This is a comprehensive automated review. For architectural decisions requiring human judgment, please request additional manual review.
github-actions
bot
removed
the
claude-review
dosubot
bot
removed
the
size:L
dosubot
bot
added
the
size:M
dosubot
bot
added
size:L
| const nodeContainerRef = ref<HTMLDivElement>() | ||
| // Track mouse position relative to node container for drag and drop | ||
| const { isOutside } = useMouseInElement(nodeContainerRef) |
Contributor
There was a problem hiding this comment.
How does useMouseInElement work? What are the performance tradeoffs?
Collaborator
Author
There was a problem hiding this comment.
No much perf gains,
The composable helps us know when the dragged file is within the node or not
@DrJKL suggests we should adopt the composable for checks like these
Contributor
There was a problem hiding this comment.
I was thinking of using Pragmatic Drag and Drop instead of re-implementing it in this one component.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces several improvements to Vue reactivity and user experience in the graph node and widget system. The main focus is on ensuring that changes to node and widget data reliably trigger updates in Vue components, improving drag-and-drop support for nodes, and enhancing widget value handling for better compatibility and reactivity.
Vue Reactivity Improvements:
useGraphNodeManager.ts, node data updates now create a completely new object and add a timestamp (_updateTs) to force Vue's reactivity system to detect changes. Additionally, node data is re-set on the next tick to guarantee component updates. [1] [2]useWidgetValueand related helpers) now accept either a direct value or a getter function formodelValue, and always normalize it to a getter. Watches are updated to use this getter for more reliable reactivity. [1] [2] [3] [4] [5] [6] [7]useImageUploadWidget.ts, widget value updates now use a new array/object to ensure Vue detects the change, especially for batch uploads.Drag-and-Drop Support for Nodes:
LGraphNode.vuecomponent adds drag-and-drop event handlers (dragover,dragleave,drop) and visual feedback (isDraggingOverstate and highlight ring) for improved user experience when dragging files onto nodes. Node callbacks (onDragOver,onDragDrop) are used for custom validation and handling. [1] [2] [3]Widget and Audio Upload Handling:
uploadAudio.ts, after uploading an audio file, the widget's callback is manually triggered to ensure Vue nodes update. There is also a commented-out call to mark the canvas as dirty for potential future refresh logic. [1] [2]These changes collectively improve the reliability and responsiveness of UI updates in the graph node system, especially in scenarios involving external updates, drag-and-drop interactions, and batch widget value changes.
Screen.Recording.2025-11-01.at.11.21.27.mov
┆Issue is synchronized with this Notion page by Unito