Make workflow read only by disabling drag-and-drop and edge handles#165
Make workflow read only by disabling drag-and-drop and edge handles#165handreyrc wants to merge 5 commits into
Conversation
Signed-off-by: handreyrc <handrey.cunha@gmail.com>
✅ Deploy Preview for swf-editor ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a “read-only” mode in the React Flow diagram editor and verifies it via unit tests.
Changes:
- Add
isReadOnly-driven styling and interaction disabling inDiagram(CSS class + ReactFlow props). - Add CSS to hide React Flow handles in read-only mode.
- Add tests asserting the read-only CSS class behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.tsx | Applies a read-only class and disables dragging/connecting when isReadOnly is true. |
| packages/serverless-workflow-diagram-editor/src/react-flow/diagram/Diagram.css | Hides .react-flow__handle elements under .read-only to prevent handle UI in read-only mode. |
| packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx | Adds tests for read-only class presence/absence and a (currently weak) handle-hiding check. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: handreyrc <handrey.cunha@gmail.com>
fantonangeli
left a comment
fantonangeli
left a comment
There was a problem hiding this comment.
Just a small improvement
|
I would also suggest you to add a changeset for this functionality: |
| * @param options.content - The workflow content to render | ||
| * @param options.locale - The locale to use for i18n | ||
| */ | ||
| function renderDiagram({ |
There was a problem hiding this comment.
Wondering is it worth moving this to common render-helpers file, there is some crossover with providers there taht could be used or you think thats overkill and should just stay here, wdyt?
There was a problem hiding this comment.
IMO it may be a bit overkill. I would be more inclined to move it to render-helpers if something more complex was happening there. At the end it is just a render call.
There was a problem hiding this comment.
Sounds good, yes, we can always move it later if we need to use it anywhere else, thanks
|
Thanks for PR @handreyrc, added one small comment as well |
Signed-off-by: handreyrc <handrey.cunha@gmail.com>
Signed-off-by: handreyrc <handrey.cunha@gmail.com>
|
|
||
| // Verify that ReactFlow was called with nodesDraggable={false} and nodesConnectable={false} | ||
| const mockReactFlow = vi.mocked(ReactFlow); | ||
| const reactFlowProps = mockReactFlow.mock.calls[mockReactFlow.mock.calls.length - 1][0]; |
There was a problem hiding this comment.
We can use .at() to access the last element and I would test that there have been a call to the mock, in order to have a clear message if it is not called.
| const reactFlowProps = mockReactFlow.mock.calls[mockReactFlow.mock.calls.length - 1][0]; | |
| const lastCall = mockReactFlow.mock.calls.at(-1); | |
| expect(lastCall).toBeDefined(); | |
| const reactFlowProps = lastCall[0]; |
| return { | ||
| ...actual, | ||
| ReactFlow: vi.fn((props) => { | ||
| return <div data-testid="react-flow-canvas" {...props} />; |
There was a problem hiding this comment.
pnpm --filter @serverlessworkflow/diagram-editor test tests/react-flow/diagram/Diagram.test.ts
Show some error like this:
React does not recognize the `nodeTypes` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `nodetypes` instead. If you accidentally passed it from a parent component, remove it from the DOM element.
This can be fixed by avoiding passing props, if you agree:
| return <div data-testid="react-flow-canvas" {...props} />; | |
| return <div data-testid="react-flow-canvas" />; |
lornakelly
left a comment
lornakelly
left a comment
There was a problem hiding this comment.
Apart from Fabrizio's comments LGTM
Signed-off-by: handreyrc <handrey.cunha@gmail.com>
Closes #163
Summary
This PR enables the read only mode by locking the workflow positioning in the canvas, however, it is still possible to select the nodes and edges and interact with the nodes.
Changes