-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Create new steps in workflow editor #6764
Changes from 28 commits
28bc974
52b8681
6f1beca
58275a3
e4b93ac
1c60193
7b9e960
ad0aa21
136bd54
52da10b
5ade683
23b98fd
57fda7e
f38f3d4
2764d45
1fbd7a4
95f505c
42c61a1
d17470b
77f2221
337b5b6
caf95a8
42ca561
e280b0f
c97cd9b
ab696d5
0a46f9b
194986b
4cc20b1
42fa8f5
c3ad01b
bb6e62f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,3 +3,14 @@ | |
// expect(element).toHaveTextContent(/react/i) | ||
// learn more: https://github.com/testing-library/jest-dom | ||
import '@testing-library/jest-dom'; | ||
|
||
/** | ||
* The structuredClone global function is not available in jsdom, it needs to be mocked for now. | ||
* | ||
* The most naive way to mock structuredClone is to use JSON.stringify and JSON.parse. This works | ||
* for arguments with simple types like primitives, arrays and objects, but doesn't work with functions, | ||
* Map, Set, etc. | ||
*/ | ||
global.structuredClone = (val) => { | ||
return JSON.parse(JSON.stringify(val)); | ||
}; | ||
Comment on lines
+14
to
+16
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: This mock implementation of structuredClone will not work correctly for complex types like functions, Map, or Set. Consider using a more robust solution for accurate testing. |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
import { showPageWorkflowSelectedNodeState } from '@/workflow/states/showPageWorkflowSelectedNodeState'; | ||
import { useRecoilValue } from 'recoil'; | ||
|
||
export const RightDrawerWorkflowEditStep = () => { | ||
const showPageWorkflowSelectedNode = useRecoilValue( | ||
showPageWorkflowSelectedNodeState, | ||
); | ||
|
||
return <p>{showPageWorkflowSelectedNode}</p>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do you need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This component is a placeholder for now. It was used to prove that selecting a node worked. It's styled in my next PR and the I understood that |
||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular'; | ||
import { useFindOneRecord } from '@/object-record/hooks/useFindOneRecord'; | ||
import { RightDrawerWorkflowSelectActionContent } from '@/workflow/components/RightDrawerWorkflowSelectActionContent'; | ||
import { showPageWorkflowIdState } from '@/workflow/states/showPageWorkflowIdState'; | ||
import { Workflow } from '@/workflow/types/Workflow'; | ||
import { useRecoilValue } from 'recoil'; | ||
import { isDefined } from 'twenty-ui'; | ||
|
||
export const RightDrawerWorkflowSelectAction = () => { | ||
const showPageWorkflowId = useRecoilValue(showPageWorkflowIdState); | ||
|
||
const { record: workflow } = useFindOneRecord<Workflow>({ | ||
objectNameSingular: CoreObjectNameSingular.Workflow, | ||
objectRecordId: showPageWorkflowId, | ||
recordGqlFields: { | ||
id: true, | ||
name: true, | ||
versions: true, | ||
publishedVersionId: true, | ||
}, | ||
}); | ||
|
||
if (!isDefined(workflow)) { | ||
return null; | ||
} | ||
|
||
return <RightDrawerWorkflowSelectActionContent workflow={workflow} />; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
import { TabList } from '@/ui/layout/tab/components/TabList'; | ||
import { MenuItem } from '@/ui/navigation/menu-item/components/MenuItem'; | ||
import { useRightDrawerWorkflowSelectAction } from '@/workflow/hooks/useRightDrawerWorkflowSelectAction'; | ||
import { Workflow } from '@/workflow/types/Workflow'; | ||
import styled from '@emotion/styled'; | ||
|
||
// FIXME: copy-pasted | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so this style could not be put in commun with others? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This component styles the tab I used to show in the SelectAction drawer. As we stated, we don't want a tab, so this duplicated component will be removed in my next PR. |
||
const StyledTabListContainer = styled.div` | ||
align-items: center; | ||
border-bottom: ${({ theme }) => `1px solid ${theme.border.color.light}`}; | ||
box-sizing: border-box; | ||
display: flex; | ||
gap: ${({ theme }) => theme.spacing(2)}; | ||
height: 40px; | ||
`; | ||
Comment on lines
+7
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Remove FIXME comment and consider extracting this styled component to a shared file if it's used in multiple places |
||
|
||
const StyledActionListContainer = styled.div` | ||
display: flex; | ||
flex-direction: column; | ||
height: 100%; | ||
overflow-y: auto; | ||
|
||
padding-block: ${({ theme }) => theme.spacing(1)}; | ||
padding-inline: ${({ theme }) => theme.spacing(2)}; | ||
`; | ||
|
||
export const TAB_LIST_COMPONENT_ID = | ||
'workflow-select-action-page-right-tab-list'; | ||
|
||
export const RightDrawerWorkflowSelectActionContent = ({ | ||
workflow, | ||
}: { | ||
workflow: Workflow; | ||
}) => { | ||
const tabListId = `${TAB_LIST_COMPONENT_ID}`; | ||
|
||
const { tabs, options, handleActionClick } = | ||
useRightDrawerWorkflowSelectAction({ tabListId, workflow }); | ||
|
||
return ( | ||
<> | ||
<StyledTabListContainer> | ||
<TabList loading={false} tabListId={tabListId} tabs={tabs} /> | ||
</StyledTabListContainer> | ||
|
||
<StyledActionListContainer> | ||
{options.map((option) => ( | ||
<MenuItem | ||
key={option.id} | ||
LeftIcon={option.icon} | ||
text={option.name} | ||
onClick={() => { | ||
handleActionClick(option.id); | ||
}} | ||
/> | ||
))} | ||
</StyledActionListContainer> | ||
</> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
import { useRightDrawer } from '@/ui/layout/right-drawer/hooks/useRightDrawer'; | ||
import { RightDrawerPages } from '@/ui/layout/right-drawer/types/RightDrawerPages'; | ||
import { useStartNodeCreation } from '@/workflow/hooks/useStartNodeCreation'; | ||
import { showPageWorkflowDiagramTriggerNodeSelectionState } from '@/workflow/states/showPageWorkflowDiagramTriggerNodeSelectionState'; | ||
import { showPageWorkflowSelectedNodeState } from '@/workflow/states/showPageWorkflowSelectedNodeState'; | ||
import { | ||
WorkflowDiagramEdge, | ||
WorkflowDiagramNode, | ||
} from '@/workflow/types/WorkflowDiagram'; | ||
import { | ||
OnSelectionChangeParams, | ||
useOnSelectionChange, | ||
useReactFlow, | ||
} from '@xyflow/react'; | ||
import { useCallback, useEffect } from 'react'; | ||
import { useRecoilValue, useSetRecoilState } from 'recoil'; | ||
import { isDefined } from 'twenty-ui'; | ||
|
||
export const WorkflowShowPageDiagramEffect = () => { | ||
const reactflow = useReactFlow<WorkflowDiagramNode, WorkflowDiagramEdge>(); | ||
|
||
const { startNodeCreation } = useStartNodeCreation(); | ||
|
||
const { openRightDrawer, closeRightDrawer } = useRightDrawer(); | ||
const setShowPageWorkflowSelectedNode = useSetRecoilState( | ||
showPageWorkflowSelectedNodeState, | ||
); | ||
|
||
const showPageWorkflowDiagramTriggerNodeSelection = useRecoilValue( | ||
showPageWorkflowDiagramTriggerNodeSelectionState, | ||
); | ||
|
||
/** | ||
* The callback executed when the selection state of nodes or edges changes. | ||
* It's called when a node or an edge is selected or unselected. | ||
* | ||
* Relying on this callback is safer than listing to click events as nodes and edges | ||
* can be selected in many ways, either via mouse click, tab key or even programatically. | ||
* | ||
* The callback is currently used to open right drawers for step creation or step editing. | ||
*/ | ||
const handleSelectionChange = useCallback( | ||
({ nodes }: OnSelectionChangeParams) => { | ||
const selectedNode = nodes[0] as WorkflowDiagramNode; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Consider handling the case where nodes array is empty to avoid potential runtime errors |
||
const isClosingStep = isDefined(selectedNode) === false; | ||
|
||
if (isClosingStep) { | ||
closeRightDrawer(); | ||
|
||
return; | ||
} | ||
|
||
const isCreateStepNode = selectedNode.type === 'create-step'; | ||
if (isCreateStepNode) { | ||
if (selectedNode.data.nodeType !== 'create-step') { | ||
throw new Error('Expected selected node to be a create step node.'); | ||
} | ||
Comment on lines
+46
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: This check seems redundant. If selectedNode.type === 'create-step', then selectedNode.data.nodeType should always be 'create-step'. Consider removing this check or clarifying why it's necessary |
||
|
||
startNodeCreation(selectedNode.data.parentNodeId); | ||
|
||
return; | ||
} | ||
|
||
setShowPageWorkflowSelectedNode(selectedNode.id); | ||
openRightDrawer(RightDrawerPages.WorkflowStepEdit); | ||
}, | ||
[ | ||
closeRightDrawer, | ||
openRightDrawer, | ||
setShowPageWorkflowSelectedNode, | ||
startNodeCreation, | ||
], | ||
); | ||
|
||
useOnSelectionChange({ | ||
onChange: handleSelectionChange, | ||
}); | ||
|
||
/** | ||
* We can't access the reactflow instance everywhere, only in children components of the <Reactflow /> component, | ||
* so we use a useEffect and a Recoil state to trigger actions on the diagram, like programatically selecting a node. | ||
*/ | ||
useEffect(() => { | ||
if (!isDefined(showPageWorkflowDiagramTriggerNodeSelection)) { | ||
return; | ||
} | ||
|
||
reactflow.updateNode(showPageWorkflowDiagramTriggerNodeSelection, { | ||
selected: true, | ||
}); | ||
}, [reactflow, showPageWorkflowDiagramTriggerNodeSelection]); | ||
|
||
return null; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, this introduce a new way of cloning object, this should be the new project convention.
Let's share about it in next daily