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.

Sign up for GitHub

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

Add the new workflow not executed node type #10030

Merged
merged 9 commits into from
Feb 5, 2025
Merged

Add the new workflow not executed node type #10030

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0fdeb29
test: refactor the catalog stories to have a single catalog (that's a…
Devessier Feb 5, 2025
7b144e6
test: make the hover state work in the story
Devessier Feb 5, 2025
53d8a50
feat: add a hover state to the workflow nodes
Devessier Feb 5, 2025
6217eea
feat: add blue adaptive color
Devessier Feb 5, 2025
4ea2a43
feat: add not-executed variant
Devessier Feb 5, 2025
40c50cc
feat: fix text color for empty and not executed nodes when selected
Devessier Feb 5, 2025
855f3e3
feat: remove hovering effect when nodes are selected
Devessier Feb 5, 2025
16e781a
refactor: harmonize the CSS selector to get all focusable states
Devessier Feb 5, 2025
b513cb9
Merge branch 'main' into add-missing-workflow-node-states
Devessier Feb 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 39 additions & 17 deletions ...ty-front/src/modules/workflow/workflow-diagram/components/WorkflowDiagramBaseStepNode.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,16 @@ const StyledStepNodeType = styled.div<{
margin-left: ${({ theme }) => theme.spacing(2)};
padding: ${({ theme }) => theme.spacing(1)} ${({ theme }) => theme.spacing(2)};

.selectable.selected &,
.selectable:focus &,
.selectable:focus-visible & {
.selectable:is(.selected, :focus, :focus-visible) & {
${({ nodeVariant, theme }) => {
switch (nodeVariant) {
case 'empty':
case 'default': {
case 'default':
case 'not-executed':
return css`
background-color: ${theme.color.blue};
color: ${theme.font.color.inverted};
`;
}
}
}}
Comment on lines 54 to 64
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 return statement for success and failure cases in this switch. Could lead to inconsistent styling when those variants are selected.

}
Expand All @@ -70,7 +68,7 @@ const StyledStepNodeType = styled.div<{
const StyledStepNodeInnerContainer = styled.div<{
variant: WorkflowDiagramNodeVariant;
}>`
background-color: ${({ theme }) => theme.background.secondary};
background: ${({ theme }) => theme.background.secondary};
border-color: ${({ theme }) => theme.border.color.medium};

border-radius: ${({ theme }) => theme.border.radius.md};
Expand All @@ -84,26 +82,41 @@ const StyledStepNodeInnerContainer = styled.div<{

position: relative;

.selectable.selected &,
.selectable:focus &,
.selectable:focus-visible & {
transition: background ${({ theme }) => theme.animation.duration.fast} ease;

.workflow-node-container:hover & {
${({ theme }) => {
return css`
background: linear-gradient(
0deg,
${theme.background.transparent.lighter} 0%,
${theme.background.transparent.lighter} 100%
),
${theme.background.secondary};
`;
}}
}

.selectable:is(.selected, :focus, :focus-visible)
:is(.workflow-node-container, .workflow-node-container:hover)
& {
${({ theme, variant }) => {
switch (variant) {
case 'success': {
return css`
background-color: ${theme.adaptiveColors.turquoise1};
background: ${theme.adaptiveColors.turquoise1};
border-color: ${theme.adaptiveColors.turquoise4};
`;
}
case 'failure': {
return css`
background-color: ${theme.background.danger};
background: ${theme.background.danger};
border-color: ${theme.color.red};
`;
}
default: {
return css`
background-color: ${theme.accent.quaternary};
background: ${theme.adaptiveColors.blue1};
border-color: ${theme.color.blue};
`;
}
Expand All @@ -120,11 +133,20 @@ const StyledStepNodeLabel = styled.div<{
font-size: 13px;
font-weight: ${({ theme }) => theme.font.weight.medium};
column-gap: ${({ theme }) => theme.spacing(2)};
color: ${({ variant, theme }) =>
variant === 'empty'
? theme.font.color.extraLight
: theme.font.color.primary};
color: ${({ variant, theme }) => {
switch (variant) {
case 'empty':
case 'not-executed':
return theme.font.color.light;
default:
return theme.font.color.primary;
}
}};
max-width: 200px;

.selectable:is(.selected, :focus, :focus-visible) & {
color: ${({ theme }) => theme.font.color.primary};
}
`;

export const StyledHandle = styled(Handle)`
Expand Down Expand Up @@ -168,7 +190,7 @@ export const WorkflowDiagramBaseStepNode = ({
RightFloatingElement?: React.ReactNode;
}) => {
return (
<StyledStepNodeContainer>
<StyledStepNodeContainer className="workflow-node-container">
{nodeType !== 'trigger' ? (
<StyledTargetHandle type="target" position={Position.Top} />
) : null}
Expand Down
195 changes: 40 additions & 155 deletions ...orkflow-diagram/components/__stories__/WorkflowDiagramStepNodeEditableContent.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,33 @@
import { Meta, StoryObj } from '@storybook/react';

import { WorkflowDiagramStepNodeData } from '@/workflow/workflow-diagram/types/WorkflowDiagram';
import { WorkflowDiagramNodeVariant } from '@/workflow/workflow-diagram/types/WorkflowDiagramNodeVariant';
import { fn } from '@storybook/test';
import '@xyflow/react/dist/style.css';
import { ComponentProps } from 'react';
import { CatalogDecorator, CatalogStory } from 'twenty-ui';
import { ReactflowDecorator } from '~/testing/decorators/ReactflowDecorator';
import { graphqlMocks } from '~/testing/graphqlMocks';
import { WorkflowDiagramStepNodeEditableContent } from '../WorkflowDiagramStepNodeEditableContent';

const meta: Meta<typeof WorkflowDiagramStepNodeEditableContent> = {
type ComponentState = 'default' | 'hover' | 'selected';

type WrapperProps = ComponentProps<
typeof WorkflowDiagramStepNodeEditableContent
> & { state: ComponentState };

const Wrapper = (_props: WrapperProps) => {
return <div></div>;
};
Comment on lines +19 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Empty wrapper component doesn't render props. This could cause issues with storybook's prop table generation.

Suggested change
const Wrapper = (_props: WrapperProps) => {
return <div></div>;
};
const Wrapper = (props: WrapperProps) => {
return <WorkflowDiagramStepNodeEditableContent {...props} />;
};


const meta: Meta<WrapperProps> = {
title: 'Modules/Workflow/WorkflowDiagramStepNodeEditableContent',
component: WorkflowDiagramStepNodeEditableContent,
};

export default meta;

type Story = StoryObj<typeof WorkflowDiagramStepNodeEditableContent>;
type Story = StoryObj<typeof Wrapper>;

const ALL_STEPS = [
{
Expand Down Expand Up @@ -47,17 +59,13 @@ const ALL_STEPS = [
{ nodeType: 'action', actionType: 'CODE', name: 'Code' },
] satisfies WorkflowDiagramStepNodeData[];

export const All: CatalogStory<
Story,
typeof WorkflowDiagramStepNodeEditableContent
> = {
export const Catalog: CatalogStory<Story, typeof Wrapper> = {
args: {
onDelete: fn(),
variant: 'default',
selected: false,
},
parameters: {
msw: graphqlMocks,
pseudo: { hover: ['.hover'] },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had a hard time making the pseudo states plugin work because I didn't want my underlying components to expose a className property. I created a story decorator that creates a container with a hover class, which is used by the plugin to locate the element to hover. I also render a workflow-node-container class as the :hover pseudo-selector is linked to this class in the styles.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems a bit hacky but it works

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I couldn't find a better option 😅

catalog: {
options: {
elementContainer: {
Expand All @@ -71,159 +79,36 @@ export const All: CatalogStory<
values: ALL_STEPS,
props: (data: WorkflowDiagramStepNodeData) => ({ data }),
},
],
},
},
decorators: [CatalogDecorator, ReactflowDecorator],
};

export const AllSelected: CatalogStory<
Story,
typeof WorkflowDiagramStepNodeEditableContent
> = {
args: {
onDelete: fn(),
variant: 'default',
selected: true,
},
parameters: {
msw: graphqlMocks,
catalog: {
options: {
elementContainer: {
width: 250,
style: { position: 'relative' },
className: 'selectable selected',
},
},
dimensions: [
{
name: 'step type',
values: ALL_STEPS,
props: (data: WorkflowDiagramStepNodeData) => ({ data }),
name: 'variant',
values: [
'empty',
'default',
'success',
'failure',
'not-executed',
] satisfies WorkflowDiagramNodeVariant[],
props: (variant: WorkflowDiagramNodeVariant) => ({ variant }),
},
],
},
},
decorators: [CatalogDecorator, ReactflowDecorator],
};

export const AllSuccess: CatalogStory<
Story,
typeof WorkflowDiagramStepNodeEditableContent
> = {
args: {
onDelete: fn(),
variant: 'success',
},
parameters: {
msw: graphqlMocks,
catalog: {
options: {
elementContainer: {
width: 250,
style: { position: 'relative' },
},
},
dimensions: [
{
name: 'step type',
values: ALL_STEPS,
props: (data: WorkflowDiagramStepNodeData) => ({ data }),
name: 'state',
values: ['default', 'hover', 'selected'] satisfies ComponentState[],
props: (state: ComponentState) => ({ state }),
},
],
},
},
decorators: [CatalogDecorator, ReactflowDecorator],
};

export const AllSuccessSelected: CatalogStory<
Story,
typeof WorkflowDiagramStepNodeEditableContent
> = {
args: {
onDelete: fn(),
variant: 'success',
selected: true,
},
parameters: {
msw: graphqlMocks,
catalog: {
options: {
elementContainer: {
width: 250,
style: { position: 'relative' },
className: 'selectable selected',
},
},
dimensions: [
{
name: 'step type',
values: ALL_STEPS,
props: (data: WorkflowDiagramStepNodeData) => ({ data }),
},
],
},
},
decorators: [CatalogDecorator, ReactflowDecorator],
};

export const AllFailure: CatalogStory<
Story,
typeof WorkflowDiagramStepNodeEditableContent
> = {
args: {
onDelete: fn(),
variant: 'failure',
},
parameters: {
msw: graphqlMocks,
catalog: {
options: {
elementContainer: {
width: 250,
style: { position: 'relative' },
},
},
dimensions: [
{
name: 'step type',
values: ALL_STEPS,
props: (data: WorkflowDiagramStepNodeData) => ({ data }),
},
],
},
},
decorators: [CatalogDecorator, ReactflowDecorator],
};

export const AllFailureSelected: CatalogStory<
Story,
typeof WorkflowDiagramStepNodeEditableContent
> = {
args: {
onDelete: fn(),
variant: 'failure',
selected: true,
},
parameters: {
msw: graphqlMocks,
catalog: {
options: {
elementContainer: {
width: 250,
style: { position: 'relative' },
className: 'selectable selected',
},
},
dimensions: [
{
name: 'step type',
values: ALL_STEPS,
props: (data: WorkflowDiagramStepNodeData) => ({ data }),
},
],
decorators: [
(Story, { args }) => {
return (
<div
className={`selectable ${args.state === 'selected' ? 'selected' : args.state === 'hover' ? 'workflow-node-container hover' : ''}`}
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Ternary expression could be simplified using a template literal and conditional classes

>
<Story />
</div>
);
},
},
decorators: [CatalogDecorator, ReactflowDecorator],
CatalogDecorator,
ReactflowDecorator,
],
};
3 changes: 2 additions & 1 deletion ...es/twenty-front/src/modules/workflow/workflow-diagram/types/WorkflowDiagramNodeVariant.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@ export type WorkflowDiagramNodeVariant =
| 'default'
| 'success'
| 'failure'
| 'empty';
| 'empty'
| 'not-executed';
4 changes: 4 additions & 0 deletions packages/twenty-ui/src/theme/constants/AdaptiveColorsDark.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ export const ADAPTIVE_COLORS_DARK = {
turquoise2: THEME_COMMON.color.turquoise70,
turquoise3: THEME_COMMON.color.turquoise60,
turquoise4: THEME_COMMON.color.turquoise50,
blue1: THEME_COMMON.color.blue80,
blue2: THEME_COMMON.color.blue70,
blue3: THEME_COMMON.color.blue60,
blue4: THEME_COMMON.color.blue50,
};
4 changes: 4 additions & 0 deletions packages/twenty-ui/src/theme/constants/AdaptiveColorsLight.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ export const ADAPTIVE_COLORS_LIGHT = {
turquoise2: THEME_COMMON.color.turquoise20,
turquoise3: THEME_COMMON.color.turquoise30,
turquoise4: THEME_COMMON.color.turquoise40,
blue1: THEME_COMMON.color.blue10,
blue2: THEME_COMMON.color.blue20,
blue3: THEME_COMMON.color.blue30,
blue4: THEME_COMMON.color.blue40,
};