Conversation
|
It's still a buggy mess, but at least the code makes a lot more sense now. |
6e14725 to
e7b180e
Compare
|
Oops didnt mean to forcepush, @stijnpotters1 was that the only feedback you have? |
|
Waiting on #615 to be merged first |
…-dependencies-create-sub-modules-or-hooks-for-complex-standalone-logic # Conflicts: # src/main/frontend/app/routes/studio/canvas-flow/canvas-flow.tsx # src/main/frontend/app/routes/studio/canvas-flow/nodetypes/start-node.tsx
…-dependencies-create-sub-modules-or-hooks-for-complex-standalone-logic # Conflicts: # pnpm-lock.yaml # src/main/frontend/app/routes/configurations/component-row.tsx # src/main/frontend/app/routes/editor/editor.tsx # src/main/frontend/app/routes/projectlanding/project-landing.tsx # src/main/frontend/app/routes/studio/canvas-flow/canvas-flow.tsx # src/main/frontend/app/routes/studio/canvas-flow/nodetypes/exit-node.tsx # src/main/frontend/app/routes/studio/canvas-flow/nodetypes/group-node.tsx # src/main/frontend/app/routes/studio/canvas-flow/nodetypes/sticky-note.tsx # src/main/frontend/app/routes/studio/canvas-flow/nodetypes/zoomed-out-node.tsx # src/main/frontend/app/routes/studio/context/group-context.tsx # src/main/frontend/app/routes/studio/studio.tsx # src/main/frontend/app/routes/studio/xml-to-json-parser.ts # src/main/frontend/package.json
|
philipsens
left a comment
philipsens
left a comment
There was a problem hiding this comment.
Nice to see the right panel logic grouped in a new file.
I like the order of hooks, state, store, callback, memo etc. I wonder if there is any linting rule for this. I'd really like to enforce this kind of stuff, since the order in the frank-node could also be better. If not, it's just something to keep in mind, and maybe add to the front-end conventions.
It's a bit hard to review the logic itself since a lot has moved. but I don't see anything weird .
Adapters now open in the top left instead of in the middle and zoomed to fit. I think that is a bug.
Why is canvas renamed to canvas-flow? Was it so the component could be renamed to a more practical name?
There was a problem hiding this comment.
Pull request overview
This PR refactors the Studio “Flow” canvas by moving the implementation into a new canvas-flow/ area, extracting UI subcomponents (right panel, node UI pieces, handle menu, etc.), and updating related imports/usages across the frontend. It also updates frontend dependencies (React, react-router, xyflow, doc-library, pnpm) and tweaks the backend configurations endpoint status behavior.
Changes:
- Refactor Studio flow canvas into
routes/studio/canvas-flow/with new node/edge components and shared UI building blocks. - Extract right panel rendering into a dedicated
right-panel-content.tsxcomponent and migrate context menu plumbing. - Update backend/frontend handling of
/projects/configurations(client now tolerates204by returningundefined) and bump frontend dependency versions.
Reviewed changes
Copilot reviewed 26 out of 40 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/org/frankframework/flow/project/ConfigurationProjectController.java | Adjusts /projects/configurations behavior when no cluster member target is present. |
| src/main/frontend/package.json | Dependency/version bumps and pnpm packageManager update; removes dotenv dependency. |
| src/main/frontend/app/utils/xsd-utils.ts | Updates imports and refactors XSD element extraction to pass Document through recursion. |
| src/main/frontend/app/utils/flow-utils.ts | Updates FlowNode type import to new canvas-flow location. |
| src/main/frontend/app/stores/flow-store.ts | Updates imports to new canvas-flow node/edge/type locations. |
| src/main/frontend/app/stores/child-utilities.ts | Updates ChildNode type import to new canvas-flow location. |
| src/main/frontend/app/services/frank-framework-service.ts | Changes return type to allow undefined when API returns 204/no body. |
| src/main/frontend/app/routes/studio/xml-to-json-parser.ts | Updates Flow-related type imports to canvas-flow. |
| src/main/frontend/app/routes/studio/studio.tsx | Switches Studio to render FlowCanvas with ReactFlowProvider and extracted right panel content. |
| src/main/frontend/app/routes/studio/right-panel-content.tsx | New component containing the extracted right panel logic from studio.tsx. |
| src/main/frontend/app/routes/studio/flow-to-xml-parser.ts | Updates Flow-related type imports to canvas-flow. |
| src/main/frontend/app/routes/studio/context/sticky-note-context.tsx | Updates imports to canvas-flow node types/constants. |
| src/main/frontend/app/routes/studio/context/group-context.tsx | Updates imports to canvas-flow node types/constants. |
| src/main/frontend/app/routes/studio/context/context-input.tsx | Updates DeprecatedPopover import to new canvas-flow component location. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/zoomed-out-node.tsx | Updates FlowConfig import path to canvas-flow. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/sticky-note.tsx | Updates FlowConfig/ResizeIcon/context-menu imports to canvas-flow. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/group-node.tsx | Updates ResizeIcon import to canvas-flow. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/frank-node.tsx | Updates handle/config/context-menu imports to canvas-flow. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/exit-node.tsx | Updates ResizeIcon/FlowConfig imports to canvas-flow. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/components/node-header.tsx | New shared node header (colored title bar) component. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/components/node-children-container.tsx | New shared container styling for nested node children. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/components/missing-requirements.tsx | New UI for displaying missing mandatory children requirements. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/components/handle.tsx | New custom source handle implementation with menu support. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/components/handle-menu.tsx | New portal-based handle type selection menu. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/components/handle-menu-item.tsx | New menu item component for handle menu entries. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/components/deprecated-popover.tsx | New portal-based deprecated popover used by context input UI. |
| src/main/frontend/app/routes/studio/canvas-flow/nodetypes/child-node.tsx | New nested child node rendering + drag/drop child creation support. |
| src/main/frontend/app/routes/studio/canvas-flow/nodes.ts | New initial node data source for the canvas-flow implementation. |
| src/main/frontend/app/routes/studio/canvas-flow/node-context-menu-context.ts | New context provider/hook for toggling the node context menu from within canvas-flow components. |
| src/main/frontend/app/routes/studio/canvas-flow/flow.config.ts | New FlowConfig location plus compact-label scaling helper. |
| src/main/frontend/app/routes/studio/canvas-flow/edgetypes/frank-edge.tsx | Updates FlowConfig import path to canvas-flow. |
| src/main/frontend/app/routes/studio/canvas-flow/edgetypes/edge-label.tsx | New edge label renderer component (with delete affordance). |
| src/main/frontend/app/routes/studio/canvas-flow/edges.ts | New initial edges data source for the canvas-flow implementation. |
| src/main/frontend/app/routes/studio/canvas-flow/canvas-flow.tsx | Large refactor of the main canvas implementation (state mgmt, autosave/load, layout, grouping, edge drop, etc.). |
| src/main/frontend/app/routes/projectlanding/project-landing.tsx | Adds guard for possibly-undefined FFInstance due to 204/no body behavior. |
| src/main/frontend/app/main.tsx | Removes dotenv initialization from frontend entrypoint. |
| src/main/frontend/app/components/sidebars-layout/sidebar-layout-store.ts | Tightens function return types and fixes visibility toggle logic. |
| src/main/frontend/app/components/flow/create-node-modal.tsx | Updates FlowNode import to canvas-flow. |
| src/main/frontend/app/components/file-structure/use-file-tree-context-menu.ts | Fixes useCallback dependency list to include logApiError. |
| pnpm-lock.yaml | Lockfile updates corresponding to dependency bumps and removal of dotenv. |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (2)
src/main/frontend/app/routes/studio/canvas-flow/canvas-flow.tsx:620
- saveFlow() throws if the adapter can’t be found (or if XML parsing failed and allAdapters is empty) outside the try/catch that sets the save status back to idle. This can bubble up as an unhandled rejection and leave the UI stuck in the "saving" state.
src/main/frontend/app/routes/studio/canvas-flow/canvas-flow.tsx:1494 - There is a useCallback(...) hook invocation whose return value is discarded (it’s not assigned to a variable and not called). This adds a hook call with no effect and looks like leftover refactor code; it should either be removed or wired into the code path that needs it.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Would be very nice, though I think it doesnt exist in the form how we like it to be
There are more bugs, a new issue has been made to go over the loading and saving logic again since I feel like its done pretty badly still, I also am working on an issue to improve the flow-store so that should also hopefully improve a lot and maybe offload some things from the canvas-flow component
More descriptive and makes it so its not easy to be confused with the whole project when just saying its name. Mostly fits nicely with what already existed as well |
No description provided.