Skip to content
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.

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

Improve the layout of the Workflow Visualizer #8372

Merged
merged 12 commits into from
Nov 12, 2024
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@ const StyledStepNodeType = styled.div`
${({ theme }) => theme.border.radius.sm} 0 0;

color: ${({ theme }) => theme.color.gray50};
font-size: ${({ theme }) => theme.font.size.xs};
font-size: ${({ theme }) => theme.font.size.md};
font-weight: ${({ theme }) => theme.font.weight.semiBold};

margin-left: ${({ theme }) => theme.spacing(2)};
padding: ${({ theme }) => theme.spacing(1)} ${({ theme }) => theme.spacing(2)};
position: absolute;
top: 0;
transform: translateY(-100%);
align-self: flex-start;

.selectable.selected &,
.selectable:focus &,
Expand Down Expand Up @@ -62,9 +61,9 @@ const StyledStepNodeInnerContainer = styled.div<{ variant?: Variant }>`
const StyledStepNodeLabel = styled.div<{ variant?: Variant }>`
align-items: center;
display: flex;
font-size: ${({ theme }) => theme.font.size.md};
font-size: ${({ theme }) => theme.font.size.lg};
font-weight: ${({ theme }) => theme.font.weight.medium};
column-gap: ${({ theme }) => theme.spacing(2)};
column-gap: ${({ theme }) => theme.spacing(3)};
color: ${({ variant, theme }) =>
variant === 'placeholder'
? theme.font.color.extraLight
Expand All @@ -80,9 +79,13 @@ export const StyledTargetHandle = styled(Handle)`
`;

const StyledRightFloatingElementContainer = styled.div`
display: flex;
align-items: center;
position: absolute;
right: ${({ theme }) => theme.spacing(-3)};
bottom: 0;
top: 0;
transform: translateX(100%);
right: ${({ theme }) => theme.spacing(-2)};
`;

export const WorkflowDiagramBaseStepNode = ({
Expand All @@ -104,9 +107,9 @@ export const WorkflowDiagramBaseStepNode = ({
<StyledTargetHandle type="target" position={Position.Top} />
) : null}

<StyledStepNodeInnerContainer variant={variant}>
<StyledStepNodeType>{capitalize(nodeType)}</StyledStepNodeType>
<StyledStepNodeType>{capitalize(nodeType)}</StyledStepNodeType>

<StyledStepNodeInnerContainer variant={variant}>
<StyledStepNodeLabel variant={variant}>
{Icon}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { isRightDrawerMinimizedState } from '@/ui/layout/right-drawer/states/isRightDrawerMinimizedState';
import { isRightDrawerOpenState } from '@/ui/layout/right-drawer/states/isRightDrawerOpenState';
import { useIsMobile } from '@/ui/utilities/responsive/hooks/useIsMobile';
import { WorkflowVersionStatusTag } from '@/workflow/components/WorkflowVersionStatusTag';
import { workflowDiagramState } from '@/workflow/states/workflowDiagramState';
import { WorkflowVersionStatus } from '@/workflow/types/Workflow';
Expand All @@ -15,14 +18,16 @@ import {
Background,
EdgeChange,
FitViewOptions,
getNodesBounds,
NodeChange,
NodeProps,
ReactFlow,
useReactFlow,
} from '@xyflow/react';
import '@xyflow/react/dist/style.css';
import React, { useMemo } from 'react';
import { useSetRecoilState } from 'recoil';
import { GRAY_SCALE, isDefined } from 'twenty-ui';
import React, { useEffect, useMemo, useRef } from 'react';
import { useRecoilValue, useSetRecoilState } from 'recoil';
import { GRAY_SCALE, isDefined, THEME_COMMON } from 'twenty-ui';

const StyledResetReactflowStyles = styled.div`
height: 100%;
Expand All @@ -35,6 +40,9 @@ const StyledResetReactflowStyles = styled.div`
.react-flow__node-output,
.react-flow__node-group {
padding: 0;
width: auto;
text-align: start;
white-space: nowrap;
}

--xy-node-border-radius: none;
Expand All @@ -51,10 +59,10 @@ const StyledStatusTagContainer = styled.div`
padding: ${({ theme }) => theme.spacing(2)};
`;

const defaultFitViewOptions: FitViewOptions = {
minZoom: 1.3,
maxZoom: 1.3,
};
const defaultFitViewOptions = {
minZoom: 1,
maxZoom: 1,
} satisfies FitViewOptions;

export const WorkflowDiagramCanvasBase = ({
diagram,
Expand All @@ -77,11 +85,29 @@ export const WorkflowDiagramCanvasBase = ({
>;
children?: React.ReactNode;
}) => {
const reactflow = useReactFlow();

const { nodes, edges } = useMemo(
() => getOrganizedDiagram(diagram),
[diagram],
);

const isRightDrawerOpen = useRecoilValue(isRightDrawerOpenState);
const isRightDrawerMinimized = useRecoilValue(isRightDrawerMinimizedState);
const isMobile = useIsMobile();

const rightDrawerState = !isRightDrawerOpen
? 'closed'
: isRightDrawerMinimized
? 'minimized'
: isMobile
? 'fullScreen'
: 'normal';

const rightDrawerWidth = Number(
THEME_COMMON.rightDrawerWidth.replace('px', ''),
);

const setWorkflowDiagram = useSetRecoilState(workflowDiagramState);

const handleNodesChange = (
Expand Down Expand Up @@ -118,27 +144,68 @@ export const WorkflowDiagramCanvasBase = ({
});
};

const containerRef = useRef<HTMLDivElement>(null);

useEffect(() => {
if (!isDefined(containerRef.current) || !reactflow.viewportInitialized) {
return;
}
Comment on lines +149 to +152
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: missing cleanup function in useEffect - could cause memory leaks if component unmounts during animation


const currentViewport = reactflow.getViewport();

const flowBounds = getNodesBounds(reactflow.getNodes());

let visibleRightDrawerWidth = 0;
if (rightDrawerState === 'normal') {
visibleRightDrawerWidth = rightDrawerWidth;
}

const viewportX =
(containerRef.current.offsetWidth + visibleRightDrawerWidth) / 2 -
flowBounds.width / 2;

reactflow.setViewport(
{
...currentViewport,
x: viewportX - visibleRightDrawerWidth,
},
Comment on lines +163 to +171
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: viewport calculation may cause layout shift when drawer width changes - consider caching previous drawer state to prevent unnecessary recalculations

{ duration: 300 },
);
}, [reactflow, rightDrawerState, rightDrawerWidth]);

return (
<StyledResetReactflowStyles>
<StyledResetReactflowStyles ref={containerRef}>
<ReactFlow
onInit={({ fitView }) => {
fitView(defaultFitViewOptions);
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,
});
}}
Comment on lines +179 to 191
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: onInit runs before nodes are measured, which could cause incorrect initial positioning

minZoom={defaultFitViewOptions.minZoom}
maxZoom={defaultFitViewOptions.maxZoom}
nodeTypes={nodeTypes}
fitView
nodes={nodes.map((node) => ({ ...node, draggable: false }))}
edges={edges}
onNodesChange={handleNodesChange}
onEdgesChange={handleEdgesChange}
proOptions={{ hideAttribution: true }}
>
<Background color={GRAY_SCALE.gray25} size={2} />

{children}

<StyledStatusTagContainer>
<WorkflowVersionStatusTag versionStatus={status} />
</StyledStatusTagContainer>
</ReactFlow>

<StyledStatusTagContainer>
<WorkflowVersionStatusTag versionStatus={status} />
</StyledStatusTagContainer>
</StyledResetReactflowStyles>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { WorkflowDiagramEmptyTrigger } from '@/workflow/components/WorkflowDiagr
import { WorkflowDiagramStepNodeEditable } from '@/workflow/components/WorkflowDiagramStepNodeEditable';
import { WorkflowWithCurrentVersion } from '@/workflow/types/Workflow';
import { WorkflowDiagram } from '@/workflow/types/WorkflowDiagram';
import { ReactFlowProvider } from '@xyflow/react';

export const WorkflowDiagramCanvasEditable = ({
diagram,
Expand All @@ -14,17 +15,19 @@ export const WorkflowDiagramCanvasEditable = ({
workflowWithCurrentVersion: WorkflowWithCurrentVersion;
}) => {
return (
<WorkflowDiagramCanvasBase
key={workflowWithCurrentVersion.currentVersion.id}
diagram={diagram}
status={workflowWithCurrentVersion.currentVersion.status}
nodeTypes={{
default: WorkflowDiagramStepNodeEditable,
'create-step': WorkflowDiagramCreateStepNode,
'empty-trigger': WorkflowDiagramEmptyTrigger,
}}
>
<WorkflowDiagramCanvasEditableEffect />
</WorkflowDiagramCanvasBase>
<ReactFlowProvider>
<WorkflowDiagramCanvasBase
key={workflowWithCurrentVersion.currentVersion.id}
diagram={diagram}
status={workflowWithCurrentVersion.currentVersion.status}
nodeTypes={{
default: WorkflowDiagramStepNodeEditable,
'create-step': WorkflowDiagramCreateStepNode,
'empty-trigger': WorkflowDiagramEmptyTrigger,
}}
>
<WorkflowDiagramCanvasEditableEffect />
</WorkflowDiagramCanvasBase>
</ReactFlowProvider>
Comment on lines +18 to +31
Copy link
Contributor

Choose a reason for hiding this comment

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

style: ReactFlowProvider should be at a higher level in the component tree to avoid re-initializing the flow context on re-renders

);
};
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { WorkflowDiagramEmptyTrigger } from '@/workflow/components/WorkflowDiagr
import { WorkflowDiagramStepNodeReadonly } from '@/workflow/components/WorkflowDiagramStepNodeReadonly';
import { WorkflowVersion } from '@/workflow/types/Workflow';
import { WorkflowDiagram } from '@/workflow/types/WorkflowDiagram';
import { ReactFlowProvider } from '@xyflow/react';

export const WorkflowDiagramCanvasReadonly = ({
diagram,
Expand All @@ -13,16 +14,18 @@ export const WorkflowDiagramCanvasReadonly = ({
workflowVersion: WorkflowVersion;
}) => {
return (
<WorkflowDiagramCanvasBase
key={workflowVersion.id}
diagram={diagram}
status={workflowVersion.status}
nodeTypes={{
default: WorkflowDiagramStepNodeReadonly,
'empty-trigger': WorkflowDiagramEmptyTrigger,
}}
>
<WorkflowDiagramCanvasReadonlyEffect />
</WorkflowDiagramCanvasBase>
<ReactFlowProvider>
<WorkflowDiagramCanvasBase
key={workflowVersion.id}
diagram={diagram}
status={workflowVersion.status}
nodeTypes={{
default: WorkflowDiagramStepNodeReadonly,
'empty-trigger': WorkflowDiagramEmptyTrigger,
}}
>
<WorkflowDiagramCanvasReadonlyEffect />
</WorkflowDiagramCanvasBase>
</ReactFlowProvider>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export const WorkflowDiagramCreateStepNode = () => {
<>
<StyledTargetHandle type="target" position={Position.Top} />

<IconButton Icon={IconPlus} size="small" />
<IconButton Icon={IconPlus} size="medium" />
</>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export const WorkflowDiagramStepNodeBase = ({
return (
<StyledStepNodeLabelIconContainer>
<IconPlaylistAdd
size={theme.icon.size.sm}
size={theme.icon.size.lg}
color={theme.font.color.tertiary}
/>
</StyledStepNodeLabelIconContainer>
Expand All @@ -41,7 +41,7 @@ export const WorkflowDiagramStepNodeBase = ({
return (
<StyledStepNodeLabelIconContainer>
<IconHandMove
size={theme.icon.size.sm}
size={theme.icon.size.lg}
color={theme.font.color.tertiary}
/>
</StyledStepNodeLabelIconContainer>
Expand All @@ -57,7 +57,7 @@ export const WorkflowDiagramStepNodeBase = ({
return (
<StyledStepNodeLabelIconContainer>
<IconCode
size={theme.icon.size.sm}
size={theme.icon.size.lg}
color={theme.color.orange}
/>
</StyledStepNodeLabelIconContainer>
Expand All @@ -66,7 +66,7 @@ export const WorkflowDiagramStepNodeBase = ({
case 'SEND_EMAIL': {
return (
<StyledStepNodeLabelIconContainer>
<IconMail size={theme.icon.size.sm} color={theme.color.blue} />
<IconMail size={theme.icon.size.lg} color={theme.color.blue} />
</StyledStepNodeLabelIconContainer>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export const WorkflowDiagramStepNodeEditable = ({
RightFloatingElement={
selected ? (
<FloatingIconButton
size="medium"
Icon={IconTrash}
onClick={() => {
return deleteOneStep();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const addCreateStepNodes = ({ nodes, edges }: WorkflowDiagram) => {
markerEnd: {
type: MarkerType.ArrowClosed,
},
deletable: false,
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export const generateWorkflowDiagram = ({
markerEnd: {
type: MarkerType.ArrowClosed,
},
deletable: false,
});

return nodeId;
Expand Down
Loading