Enable dragging actions from execute_actions nodes to canvas to create new nodes #732
+416
−32
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
Co-authored-by: ericnewcomer <[email protected]>
Co-authored-by: ericnewcomer <[email protected]>
Co-authored-by: ericnewcomer <[email protected]>
…plication Co-authored-by: ericnewcomer <[email protected]>
Copilot
AI
changed the title
[WIP] Enhance dragging actions within execute_actions node
Enable dragging actions from execute_actions nodes to canvas to create new nodes
Oct 31, 2025
Copilot finished work on behalf of
ericnewcomer
October 31, 2025 23:43
|
@copilot When dragging out onto the canvas I've noticed two things:
|
- Add requestUpdate() call to force re-render when preview position changes - Add z-index to ensure preview is visible on top of other elements - Stop propagation of original SortableList events to prevent duplicate handling Co-authored-by: ericnewcomer <[email protected]>
Fixed both issues in commit 76934a9:
The preview now shows continuously as you drag over the canvas and the node is created at the exact preview location when dropped. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot finished work on behalf of
ericnewcomer
October 31, 2025 23:58
|
@copilot Closer but not quite there. I'd like the drop placement to be closer to the dragged item, so only offset a small amount. Also it looks like the vertical placement is still not correct. I think it being affected by the vertical scroll location for the entire canvas making it place it too low. |
|
@copilot Actually, I think preview drop looks good if we hide the dragging element.. so only show the preview in that state. When the user drags back over the old list, then show the dragged item again. ie, only one should be visible at a time, the dragged element for sorting in the node or the the preview action for when it is dropped on the canvas. |
- Reduce DROP_PREVIEW_OFFSET from 100x50 to 20x20 for closer placement - Remove incorrect scroll offset addition - getBoundingClientRect already accounts for scroll - Drop location now matches preview position accurately Co-authored-by: ericnewcomer <[email protected]>
Fixed both issues in commit 83ebd66:
Preview and drop location now match accurately regardless of scroll position. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot finished work on behalf of
ericnewcomer
November 1, 2025 00:08
- Hide ghost element when dragging externally (display: none) - Show ghost element when dragging back to node (display: block) - Only one visual indicator shown at a time: ghost OR canvas preview Co-authored-by: ericnewcomer <[email protected]>
Implemented in commit 3786f2c:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Copilot finished work on behalf of
ericnewcomer
November 1, 2025 00:15
|
@copilot When dragging around and showing the canvas preview, lets just move the preview smoothly with the cursor (instead of showing the exact grid placement), then let the grid placement occur on drop. |
- Preview now moves smoothly with cursor without grid snapping - Grid snapping applied only when action is actually dropped - Added applyGridSnapping parameter to calculateCanvasDropPosition Co-authored-by: ericnewcomer <[email protected]>
Implemented in commit 214976c:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements a drag-to-canvas feature that allows actions to be dragged from nodes onto the canvas to create new action nodes. When an action is dragged outside its container, it triggers external drag events with a preview, and dropping it on the canvas creates a new standalone action node.
Key changes:
- Added external drag detection to
SortableListwith boundary padding logic - Implemented canvas drop preview and node creation in
Editor - Added event propagation handlers in
CanvasNodeto relay drag events to the editor
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/interfaces.ts | Added new event types DragExternal and DragInternal for tracking drag state transitions |
| src/list/SortableList.ts | Implemented external drag detection with boundary checking, ghost element hiding, and event firing for external/internal drag transitions |
| src/flow/Editor.ts | Added canvas drop preview rendering, position calculation with grid snapping, and new node creation when actions are dropped externally |
| src/flow/CanvasNode.ts | Added event listeners and handlers to propagate drag events from SortableList to Editor with action metadata |
| test/temba-sortable-list.test.ts | Added tests for external drag detection and DragStop event with isExternal flag |
Comment on lines
+815
to
+817
| ${actionConfig.render | ||
| ? actionConfig.render({ actions: [action] } as any, action) | ||
| : html`<pre>${action.type}</pre>`} |
There was a problem hiding this comment.
The use of as any bypasses type safety. Consider defining a proper interface or type for the render function's first parameter to maintain type safety, or at minimum document why the type cast is necessary here.
Comment on lines
1367
to
1382
| private handleActionDragExternal(event: CustomEvent): void { | ||
| const { action, nodeUuid, actionIndex, mouseX, mouseY } = event.detail; | ||
|
|
||
| // Don't snap to grid for preview - let it follow cursor smoothly | ||
| const position = this.calculateCanvasDropPosition(mouseX, mouseY, false); | ||
|
|
||
| this.canvasDropPreview = { | ||
| action, | ||
| nodeUuid, | ||
| actionIndex, | ||
| position | ||
| }; | ||
|
|
||
| // Force re-render to update preview position | ||
| this.requestUpdate(); | ||
| } |
There was a problem hiding this comment.
The new methods handleActionDragExternal, handleActionDragInternal, handleActionDropExternal, calculateCanvasDropPosition, and renderCanvasDropPreview in Editor.ts lack test coverage. According to the project's coding guidelines requiring 100% code coverage, comprehensive tests should be added for this new drag-to-canvas functionality.
Comment on lines
622
to
639
| private handleActionDragExternal(event: CustomEvent) { | ||
| // stop propagation of the original event from SortableList | ||
| event.stopPropagation(); | ||
|
|
||
| // get the action being dragged | ||
| const actionId = event.detail.id; | ||
| const actionIndex = parseInt(actionId.split('-')[1]); | ||
| const action = this.node.actions[actionIndex]; | ||
|
|
||
| // fire event to editor to show canvas drop preview | ||
| this.fireCustomEvent(CustomEventType.DragExternal, { | ||
| action, | ||
| nodeUuid: this.node.uuid, | ||
| actionIndex, | ||
| mouseX: event.detail.mouseX, | ||
| mouseY: event.detail.mouseY | ||
| }); | ||
| } |
There was a problem hiding this comment.
The new event handlers handleActionDragExternal, handleActionDragInternal, and handleActionDragStop in CanvasNode.ts lack test coverage. According to the project's coding guidelines requiring 100% code coverage, tests should be added to verify these handlers correctly parse action IDs, extract action data, and propagate events to the editor.
📊 Coverage ReportSummary
|
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.
Summary
Successfully implemented the ability to drag actions from execute_actions nodes onto the canvas to create new execute_actions nodes.
Recent Fixes
Smooth Preview Movement (commit: [latest])
Improved preview UX:
Visual Feedback Improvement (commit: 3786f2c)
Improved drag visual feedback:
Drop Placement Improvements (commit: 83ebd66)
Fixed drop placement issues:
getBoundingClientRect()already accounts for scroll positionCanvas Drop Preview Issues (commit: 76934a9)
Fixed preview visibility:
requestUpdate()to force re-render andz-index: 10000Changes:
DROP_PREVIEW_OFFSET_X/Yfrom 100/50 to 20/20requestUpdate()inhandleActionDragExternalto force preview updatesz-index: 10000to canvas drop preview for visibilityevent.stopPropagation()to prevent duplicate event handlingImplementation Details
Core Features
SortableListcomponent to detect external drags (outside container bounds)DragExternalandDragInternalCanvasNodeto relay drag events to the EditorTechnical Details
requestUpdate()forcing re-renders)getBoundingClientRect()which accounts for scrollTesting
Usage
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.