feat: Define node types and create React Flow node components #51#108
feat: Define node types and create React Flow node components #51#108handreyrc wants to merge 1 commit intoserverlessworkflow:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces React Flow custom node types/components for the 12 Serverless Workflow task types, wires them into the diagram renderer, and adds styling + tests to validate that the custom nodes are renderable.
Changes:
- Added a
NodeTypesmapping and placeholder React Flow node components for the 12 task types. - Updated the React Flow
Diagramto use these node types and a shared default node size. - Added CSS to restore node border/hover/selection behaviors and added/updated unit tests around Diagram + node rendering.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
pnpm-lock.yaml |
Bumps vitest-related packages and oxlint versions in the lockfile. |
packages/serverless-workflow-diagram-editor/src/react-flow/nodes/Nodes.tsx |
Defines NodeTypes and placeholder custom node components for all 12 task types. |
packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx |
Uses NodeTypes, sets node type to GraphNodeType.*, and applies default sizing/test ids. |
packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.css |
Adds styling for node border/hover/selection behavior. |
packages/serverless-workflow-diagram-editor/src/core/autoLayout.ts |
Introduces DEFAULT_NODE_SIZE for reuse across diagram/tests. |
packages/serverless-workflow-diagram-editor/tests/react-flow/nodes/Nodes.test.tsx |
Adds a test that renders all 12 custom node types and asserts they exist. |
packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx |
Updates Diagram test to assert container/canvas presence via test ids. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
handreyrc
force-pushed
the
add-reactflow-nodes
branch
from
7366ab6 to
ff45b5a
Compare
handreyrc
force-pushed
the
add-reactflow-nodes
branch
from
ff45b5a to
c7e5eeb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
handreyrc
force-pushed
the
add-reactflow-nodes
branch
from
c7e5eeb to
d795832
Compare
| }: RF.NodeProps<RaiseNodeType>) { | ||
| // TODO: This component is just a placeholder | ||
| return ( | ||
| <div |
There was a problem hiding this comment.
I know this is a placeholder but since they are all the same placeholder, maybe better to just add a PlaceholderNode a the top and reference that rather than the repeated divs? Then it becomes very obvious when we are adding the real node
| > | ||
| <RF.Handle type="target" position={RF.Position.Top} /> | ||
| <div | ||
| style={{ height: height, width: width, whiteSpace: "pre-line", padding: 7 }} |
There was a problem hiding this comment.
also maybe move inline styles to nodes css file? Again I know its a placeholder but will be good to set up that for future as we will have to do it anyway and also dont want to establish inline style pattern in case its followed when replacing the placeholder node
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| test("render react flow custom node types", () => { |
There was a problem hiding this comment.
this will need to change ti it though as its a new file
| '@types/react-dom': | ||
| specifier: ^19.2.3 | ||
| version: 19.2.3 | ||
| '@vitest/coverage-v8': |
There was a problem hiding this comment.
wondering why the lock file is updated on its own? it doesnt look like packages were updated or maybe a file wasnt checked in or else you might need to pull latest and run an install?
There was a problem hiding this comment.
@lornakelly,
I investigated a bit, deleting pnpm_lock.yaml and node_modules and then reinstalling the dependencies cause those versions to update.
Anyway I rebased again and reverted it to main versions.
|
Thanks for PR @handreyrc, just added a few comments |
Signed-off-by: handreyrc <handrey.cunha@gmail.com> # Conflicts: # pnpm-lock.yaml
handreyrc
force-pushed
the
add-reactflow-nodes
branch
from
d795832 to
30358cb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| height: 100%; | ||
| width: 100%; | ||
| white-space: pre; | ||
| padding: 7; |
There was a problem hiding this comment.
.node-label-container sets padding: 7; without a unit, which is invalid CSS and will be ignored by the browser (so the label ends up flush with the border). Use an explicit unit (e.g., px/rem) for consistent rendering.
| padding: 7; | |
| padding: 7px; |
| border-radius: 5px; | ||
| background: white; | ||
| border: 1px solid #000; | ||
| transition: border ease, box-shadow ease; |
There was a problem hiding this comment.
transition: border ease, box-shadow ease; omits a duration, so it effectively defaults to 0s and won't animate. Specify a duration (and ideally the specific sub-properties you want to transition) to make this rule meaningful.
| transition: border ease, box-shadow ease; | |
| transition: border-color 0.2s ease, box-shadow 0.2s ease; |
Closes #51
Summary
This PR adds react flow node types and customizable node components for each task type also reenabling base behaviours such as borders, selection / deselection and mouse hover.
Changes