Skip to content

Conversation

harshit078
Copy link
Contributor

@harshit078 harshit078 commented Sep 23, 2025

Description

visual

Screen.Recording.2025-09-25.at.1.00.47.AM.mov
Screen.Recording.2025-09-25.at.1.02.05.AM.mov

@harshit078 harshit078 marked this pull request as ready for review September 24, 2025 18:05
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Summary

This PR adds drag functionality to arrow tips in workflow diagrams, allowing users to reconnect edges by dragging the arrow tip to different nodes. The implementation includes a new draggable component, state management for drag operations, and visual feedback with preview paths.

Key changes include:

  • New WorkflowDiagramArrowTipDraggable component that handles mouse interactions for dragging arrow tips
  • Enhanced state management with new Recoil states for tracking drag operations and hovered arrow tips
  • Utility functions for safe coordinate handling, path computation, and drag position calculation
  • Visual improvements with hover effects, drag previews, and proper z-index management
  • Integration with existing workflow through edge creation/deletion hooks

The implementation follows React best practices with proper event handling, state management via Recoil, and integration with React Flow. However, there is a potential memory leak issue with event listeners that needs addressing.

Confidence Score: 3/5

  • This PR introduces significant functionality but has one critical memory leak issue that should be resolved before merging
  • Score reflects well-structured code with proper patterns and defensive programming, but the memory leak from unremoved event listeners in component unmount scenarios is a critical issue that needs fixing
  • The WorkflowDiagramArrowTipDraggable.tsx file needs attention to fix the potential memory leak from event listeners

Important Files Changed

File Analysis

Filename        Score        Overview
packages/twenty-front/src/modules/workflow/workflow-diagram/workflow-edges/components/WorkflowDiagramArrowTipDraggable.tsx 3/5 New component implementing arrow tip drag functionality with proper event handling but potential memory leak from event listeners
packages/twenty-front/src/modules/workflow/workflow-diagram/workflow-edges/hooks/useArrowTipInteractions.ts 4/5 Well-structured custom hook managing arrow tip drag state with proper Recoil integration and React Flow manipulation
packages/twenty-front/src/modules/workflow/workflow-diagram/workflow-edges/hooks/useEdgeState.ts 4/5 Enhanced edge state management hook with new isDragging functionality and comprehensive edge state tracking
packages/twenty-front/src/modules/workflow/workflow-diagram/workflow-edges/utils/computePath.ts 5/5 Safe path computation utility with proper error handling and coordinate validation
packages/twenty-front/src/modules/workflow/workflow-diagram/workflow-edges/components/WorkflowDiagramBaseEdge.tsx 4/5 Base edge component updated to include dragging state logic, improving visual feedback during drag operations

Sequence Diagram

sequenceDiagram
    participant User
    participant ArrowTip as WorkflowDiagramArrowTipDraggable
    participant Hook as useArrowTipInteractions
    participant State as ArrowTipDragState
    participant ReactFlow
    participant Edge as useCreateEdge/useDeleteEdge
    
    User->>ArrowTip: mouseEnter (hover arrow tip)
    ArrowTip->>Hook: handleArrowTipHover(edgeDescriptor)
    Hook->>State: setHoveredEdge(edgeDescriptor)
    Hook->>ReactFlow: setEdges (update marker to Dragging)
    
    User->>ArrowTip: mouseDown (start drag)
    ArrowTip->>Hook: handleDragStart(edgeDescriptor, mouse, target)
    Hook->>ReactFlow: getNode(source)
    Hook->>State: setDragState({isDragging: true, draggedEdge})
    Hook->>State: setHoveredEdge(undefined)
    ArrowTip->>ArrowTip: addEventListener('mousemove', handleMouseMove)
    ArrowTip->>ArrowTip: addEventListener('mouseup', handleMouseUp)
    
    loop During Drag
        User->>ArrowTip: mousemove
        ArrowTip->>Hook: handleDragMove(mouseStart, currentMouse, targetPos, sourcePos)
        Hook->>Hook: getDragPosition(targetPos, mouseStart, currentMouse)
        Hook->>Hook: computePath(sourcePos, newDragPos)
        Hook->>State: setDragState({dragPosition, previewPath})
        ArrowTip->>ArrowTip: render drag preview with dashed line
    end
    
    User->>ArrowTip: mouseUp (end drag)
    ArrowTip->>Hook: handleDragEnd(targetPosition)
    Hook->>State: setDragState({isDragging: false})
    ArrowTip->>ReactFlow: screenToFlowPosition(mousePosition)
    ArrowTip->>ReactFlow: getNodes()
    ArrowTip->>ArrowTip: find target node under mouse
    
    alt Target node found and different
        ArrowTip->>Edge: deleteEdge(oldEdge)
        ArrowTip->>Edge: createEdge(newEdge)
    else No valid target
        ArrowTip->>ArrowTip: no action taken
    end
    
    ArrowTip->>ArrowTip: removeEventListener('mousemove')
    ArrowTip->>ArrowTip: removeEventListener('mouseup')
Loading

15 files reviewed, 1 comment

Edit Code Review Bot Settings | Greptile

Copy link
Contributor

github-actions bot commented Sep 24, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:33700

This environment will automatically shut down when the PR is closed or after 5 hours.

@Devessier
Copy link
Contributor

Hi @harshit078, thank you a lot for working on this issue.
Did you see that Reactflow provides tools to reconnect edges? I think it would make the implementation more concise. The documentation: https://reactflow.dev/examples/edges/reconnect-edge.

By providing an onReconnect event handler to the <ReactFlow /> component, you get this behavior for free:

CleanShot.2025-09-25.at.11.01.37.mp4

Once we said that, some work remains to be done:

  • Let's only authorize reconnections from the target, not the source. You must update generateWorkflowDiagram to set the reconnectable property to "target" when the function is called for a workflow. Keep edges unreconnectable for workflow versions and workflow runs.
  • Keep scaling the markerEnd arrow on hover.
  • You can use the onReconnectStart and onReconnectEnd event listeners to know when a reconnection starts and ends. onReconnectEnd will also be called if a reconnection is canceled, so you should be good using it to set a state storing whether a reconnection is occurring, and keep the arrow scaled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants