Improve the layout of the Workflow Visualizer #8372
Conversation
… zoom transform which made pixels computations hard
Devessier
force-pushed
the
improve-visualizer-layout
branch
from
06a5fb6
to
f9ff8d1
Compare
|
Great! Make sure to check dark mode |
|
I will fix the dark mode in another PR. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR improves the Workflow Visualizer's layout and interaction design with significant changes to node dimensions and viewport handling.
- Removed node scaling (from 1.3x to 1.0x) and increased base dimensions for better width calculations in
/packages/twenty-front/src/modules/workflow/components/WorkflowDiagramCanvasBase.tsx - Moved node type labels inside nodes and improved visual hierarchy in
/packages/twenty-front/src/modules/workflow/components/WorkflowDiagramBaseStepNode.tsx - Added viewport centering logic when flow mounts and right drawer state changes
- Increased icon sizes from 'sm' to 'lg' across all node types for better visibility
- Added
deletable: falseto workflow edges to prevent accidental connection removal
Note: There's a known issue with right drawer flickering that needs refactoring (Issue #8387).
9 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
| const viewportX = | ||
| (containerRef.current.offsetWidth + visibleRightDrawerWidth) / 2 - | ||
| flowBounds.width / 2; | ||
|
|
||
| reactflow.setViewport( | ||
| { | ||
| ...currentViewport, | ||
| x: viewportX - visibleRightDrawerWidth, | ||
| }, |
There was a problem hiding this comment.
logic: viewport calculation may cause layout shift when drawer width changes - consider caching previous drawer state to prevent unnecessary recalculations
| useEffect(() => { | ||
| if (!isDefined(containerRef.current) || !reactflow.viewportInitialized) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
logic: missing cleanup function in useEffect - could cause memory leaks if component unmounts during animation
| onInit={() => { | ||
| if (!isDefined(containerRef.current)) { | ||
| throw new Error('Expect the container ref to be defined'); | ||
| } | ||
|
|
||
| const flowBounds = getNodesBounds(reactflow.getNodes()); | ||
|
|
||
| reactflow.setViewport({ | ||
| x: containerRef.current.offsetWidth / 2 - flowBounds.width / 2, | ||
| y: 150, | ||
| zoom: defaultFitViewOptions.maxZoom, | ||
| }); | ||
| }} |
There was a problem hiding this comment.
logic: onInit runs before nodes are measured, which could cause incorrect initial positioning
| <ReactFlowProvider> | ||
| <WorkflowDiagramCanvasBase | ||
| key={workflowWithCurrentVersion.currentVersion.id} | ||
| diagram={diagram} | ||
| status={workflowWithCurrentVersion.currentVersion.status} | ||
| nodeTypes={{ | ||
| default: WorkflowDiagramStepNodeEditable, | ||
| 'create-step': WorkflowDiagramCreateStepNode, | ||
| 'empty-trigger': WorkflowDiagramEmptyTrigger, | ||
| }} | ||
| > | ||
| <WorkflowDiagramCanvasEditableEffect /> | ||
| </WorkflowDiagramCanvasBase> | ||
| </ReactFlowProvider> |
There was a problem hiding this comment.
style: ReactFlowProvider should be at a higher level in the component tree to avoid re-initializing the flow context on re-renders
|
Thanks @Devessier for your contribution! |
We'll have to make a refactor so the viewport can be animated properly: #8387.
CleanShot.2024-11-12.at.16.03.03.mp4