Add ReactFlow to Workflow Details page #42
Conversation
Signed-off-by: Tyler Ohlsen <[email protected]>
Signed-off-by: Tyler Ohlsen <[email protected]>
ohltyler
requested review from
dbwiddis,
owaiskazi19,
joshpalis,
amitgalitz and
jackiehanyang
as code owners
Signed-off-by: Tyler Ohlsen <[email protected]>
|
nit: should we use the newest reactflow version available right now? 11.9.2 |
Oh I just realized 11.8.3 -> 11.9.x happened 12 hours ago, just looked at the release date. Anyways we can probably wait as I see they had fast iteration with 11.9.1/2 fixing things in 11.9.0 |
Also, it will be changing to |
| @@ -3,24 +3,161 @@ | |||
| * SPDX-License-Identifier: Apache-2.0 | |||
| */ | |||
|
|
|||
| import React from 'react'; | |||
| import React, { useRef, useContext, useCallback, useEffect } from 'react'; | |||
There was a problem hiding this comment.
nit: unrelated to this file but we should use prettier like we do in AD https://github.com/opensearch-project/anomaly-detection-dashboards-plugin/blob/main/DEVELOPER_GUIDE.md#formatting.
We should probably try and stick to some specific formatter from day one. I can open an issue if we don't want to do it in this PR
There was a problem hiding this comment.
I've integrated with prettier on the initial commit #2. I've not run into any auto-formatting issues, are you having issues on your local maybe?
| }, | ||
| }; | ||
|
|
||
| // TODO: add logic to add node into the redux datastore |
There was a problem hiding this comment.
I thought we aren't storing the nodes in the redux while updating the workflow and just the hook here and reactflowContext? Is that in-correct. We will update a redux datastore as we drop in nodes?
There was a problem hiding this comment.
Thanks for the catch, yes you're correct. We will utilize reactFlowContext and useNodesState() / useEdgesState() to maintain all of this for us. I'll remove the comment
| event.dataTransfer.dropEffect = 'move'; | ||
| }, []); | ||
|
|
||
| const onDrop = useCallback( |
There was a problem hiding this comment.
Might require its own different conversation but for things like credentials that users might not want to type in here on the UI we might need to pre-populate some variable during "on drop" here if we allow some pre-configuration
There was a problem hiding this comment.
Yeah, we can handle this logic outside of the overall workspace component and treat it at the node/component-level. Flowise for example, has a CredentialInputHandler which is a special handler rendered when a credential input is included in the component's interface.
This will require some more digging to understand and implement a secure way for this part, though. Let me add an issue to track this, thanks for the callout.
|
|
||
| // Initialization hook | ||
| useEffect(() => { | ||
| // TODO: fetch the nodes/edges dynamically (loading existing flow, |
There was a problem hiding this comment.
Will all these options live here? So basically we will have some check if we have some current flowID in our state? or from our in house fixed templates and etc.
There was a problem hiding this comment.
We will need to have some initialization logic to determine whether the page is loaded from an existing workflow, a new workflow, or (potentially) redirected from a sample template as a starting point, similar to Flowise.
For loading an existing workflow, we can parse the url for a workflow ID, fetch workflow details that contain the set of nodes/edges in the UI metadata, and load them into the reactflow instance via setNodes() / setEdges().
For starting with a blank canvas, we can simply set nodes/edges to empty arrays.
For redirecting from a sample template, we can focus on that as the UX design becomes more clear on that flow and if it is something we plan to do.
| // @ts-ignore | ||
| const reactFlowBounds = reactFlowWrapper.current.getBoundingClientRect(); | ||
| // @ts-ignore | ||
| const position = reactFlowInstance.project({ |
There was a problem hiding this comment.
just wondering, where does the height and width info come from for these nodes to be rendered?
There was a problem hiding this comment.
The reactFlowWrapper contains the element positioning in the webpage, and is used for calculating the bounds on line 69. The position based on these hardcoded bounds is saying that for every element that is dragged into the workspace, place it 80 pixels from the left bounds, and 90 pixels from the top bounds. We will likely want this to be dynamic based on the user mouse, for example, but just placing those hardcoded values for ease of use for now.
| export function ReactFlowContextProvider({ children }: any) { | ||
| const [reactFlowInstance, setReactFlowInstance] = useState(null); | ||
|
|
||
| const deleteNode = (nodeId: string) => { |
There was a problem hiding this comment.
Why isn't this where the onConnect onDrop and other workspace functions are. I understand why it can live here also but surprised its not all in the same place as other functions
There was a problem hiding this comment.
It is part of the context so the functions will be easily accessible by other React components later on that are not directly stored in the main Workspace component (and may not even be children of Workspace, such as helper menus to delete the component, or handlers for removing connections of edges of a UI component, etc.
The onConnect/onDrop/etc. fns are included just in the Workspace for now, but may potentially be refactored or added into the context later on if that pattern will make sense. Right now, the configuration I have matches with the Flowise pattern which is beneficial for us to start with for now and change as needed down the line.
Signed-off-by: Tyler Ohlsen <[email protected]>
Signed-off-by: Tyler Ohlsen <[email protected]> (cherry picked from commit 8980cb2)
Signed-off-by: Tyler Ohlsen <[email protected]> (cherry picked from commit 8980cb2) Co-authored-by: Tyler Ohlsen <[email protected]>
Description
This PR adds a new ReactFlow instance into the workflow details page.
More details:
useNodesState()/useEdgesState(), respectively. It also includes defining other flow lifecycle functions, likeonDrop(),onDragOver(), andonConnect(). This is where we will define how we want to manipulate the ReactFlow state, and gives us the opportunity for any other data we may need to fetch from redux store (or trigger any API calls ourselves)useContext(). By default, we add a few stub functions to delete nodes and edges, which are not something that comes by default via<ReactFlow>component. We don't add this in redux store since it serves a different purpose (sharing mostly-static data across components vs. having a centralized datastore that is updated often).Next steps:
Demo video w/ hardcoded workspace state:
screen-capture.18.webm
Issues Resolved
Makes progress on #10, #12
Check List
--signoffBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.