feat(ui): Added subflow creation on multi select. #756
Conversation
WalkthroughThis pull request introduces a new functionality for adding workflows from selected nodes through a keyboard shortcut. It adds a method Changes
Sequence DiagramsequenceDiagram
participant User
participant Editor
participant WorkflowManager
User->>Editor: Press "s" key
Editor->>WorkflowManager: handleWorkflowAddFromSelection(nodes, edges)
WorkflowManager-->>WorkflowManager: Process selected nodes
WorkflowManager-->>Editor: Create new sub-workflow
Editor-->>User: Display updated workflow
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
✅ Deploy Preview for reearth-flow ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
billcookie
changed the title
…arth/reearth-flow into ui/create-subworkflow-from-selection
…-subworkflow-from-selection
…-subworkflow-from-selection
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
ui/src/types/shortcuts.ts (1)
51-52: Consider using a more descriptive action name.The action name
groupToSubWorkFlowcould be more descriptive. ConsidercreateSubWorkflowFromSelectionto better reflect its purpose.- | "groupToSubWorkFlow"; + | "createSubWorkflowFromSelection";ui/src/lib/yjs/useYjsStore.ts (1)
196-197: Consider adding TypeScript types for the new properties.The new properties
setOpenWorkflowIdsandsetWorkflowsare added without explicit type annotations. Consider adding types to improve code maintainability and prevent potential type-related bugs.return { // ... other properties - setOpenWorkflowIds, - setWorkflows, + setOpenWorkflowIds: setOpenWorkflowIds as (ids: string[]) => void, + setWorkflows: setWorkflows as (workflows: Workflow[]) => void, };ui/src/features/Editor/hooks.ts (1)
14-14: Remove or uncomment the import statement.The commented import
useSubworkflowCreationsuggests incomplete implementation. Either remove it if unused or uncomment if needed.-// import { useSubworkflowCreation } from "./useSubworkflowCreation";ui/src/lib/yjs/useYWorkflow.ts (3)
58-68: Consider adding documentation for the node inclusion logic.The implementation is solid, but complex logic like this would benefit from documentation explaining the node hierarchy handling.
Add JSDoc comments to explain the node inclusion logic:
+/** + * Collects all nodes that should be included in the sub-workflow: + * - Selected standalone nodes + * - Selected batch nodes and their nested children + */ const allIncludedNodes = selectedNodes?.flatMap((node) => { if (node.type === "batch") { return [node, ...getBatchNodes(node.id)]; } return [node]; }) ?? [];
69-75: Extract default position values into constants.The position calculation logic is correct, but the hardcoded values should be constants for better maintainability.
Consider this refactor:
+const DEFAULT_SUBWORKFLOW_POSITION = { + x: 600, + y: 200 +} as const; const calculatedPosition = hasSelectedNodes ? { x: Math.min(...selectedNodes.map((n) => n.position.x)), y: Math.min(...selectedNodes.map((n) => n.position.y)), } - : (position ?? { x: 600, y: 200 }); + : (position ?? DEFAULT_SUBWORKFLOW_POSITION);
111-141: Improve readability of node position adjustment logic.The position adjustment logic is correct but could be more maintainable.
Consider these improvements:
+const NODE_OFFSET = { + x: 400, + y: 200 +} as const; +const adjustNodePosition = (node: Node, basePosition: XYPosition) => ({ + ...node, + position: { + x: node.position.x - basePosition.x + NODE_OFFSET.x, + y: node.position.y - basePosition.y + NODE_OFFSET.y, + }, + selected: false, +}); if (hasSelectedNodes && edges) { const internalEdges = edges.filter( (e) => allIncludedNodeIds.includes(e.source) && allIncludedNodeIds.includes(e.target), ); const adjustedNodes = allIncludedNodes.map((node) => - if (node.parentId) { - return { - ...node, - selected: false, - }; - } - return { - ...node, - position: { - x: node.position.x - calculatedPosition.x + 400, - y: node.position.y - calculatedPosition.y + 200, - }, - selected: false, - }; + node.parentId + ? { ...node, selected: false } + : adjustNodePosition(node, calculatedPosition) ); workflowNodes = [newInputNode, ...adjustedNodes, newOutputNode]; workflowEdges = internalEdges; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
ui/src/features/Editor/hooks.ts(2 hunks)ui/src/features/KeyboardShortcutDialog/useHooks.ts(1 hunks)ui/src/lib/i18n/locales/en.json(1 hunks)ui/src/lib/i18n/locales/es.json(1 hunks)ui/src/lib/i18n/locales/fr.json(1 hunks)ui/src/lib/i18n/locales/ja.json(1 hunks)ui/src/lib/i18n/locales/zh.json(1 hunks)ui/src/lib/yjs/useYWorkflow.ts(3 hunks)ui/src/lib/yjs/useYjsStore.ts(1 hunks)ui/src/types/shortcuts.ts(3 hunks)
🔇 Additional comments (9)
ui/src/types/shortcuts.ts (2)
20-21: LGTM! Clear and consistent comment formatting.The comment formatting is consistent with other entries, and the description clearly indicates the purpose of the 's' key shortcut.
89-89: Verify shortcut conflicts.The 's' key is being added without any modifier keys. Please ensure this doesn't conflict with other text input scenarios, such as when editing node names or using the search functionality.
✅ Verification successful
No conflicts found with the 's' shortcut.
The shortcut system has built-in protection that automatically disables shortcuts when any input field is focused, including search fields and node name editing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other uses of the 's' key in shortcuts or keyboard handlers rg -i "key: ['\"]s['\"]" --type tsLength of output: 185
Script:
#!/bin/bash # Check the context of the 's' key usage in hooks.ts rg -B 5 -A 5 'key: "s"' ui/src/features/Editor/hooks.ts # Look for search or input-related components rg -i "search|input" --type tsLength of output: 99488
ui/src/features/KeyboardShortcutDialog/useHooks.ts (1)
55-58: LGTM! Well-structured shortcut entry with i18n support.The shortcut entry is properly structured and follows the pattern of other shortcuts. Good use of translation for internationalization.
ui/src/lib/yjs/useYWorkflow.ts (2)
45-57: LGTM! Well-structured function signature update and helper implementation.The changes effectively support the multi-select functionality:
- Updated signature properly handles optional parameters
- Helper function is well-scoped for batch node processing
- Selected nodes handling includes proper null checks
174-185: LGTM! Clean implementation of parent workflow node handling.The logic correctly handles both multi-select and single node cases while properly preserving the workflow state.
ui/src/lib/i18n/locales/zh.json (1)
88-88: LGTM! Accurate Chinese translation.The translation "从选定的节点创建新的子工作流" accurately conveys the meaning of creating a new sub-workflow from selected nodes.
ui/src/lib/i18n/locales/ja.json (1)
88-88: LGTM! Accurate Japanese translation.The translation "選択したノードから新しいサブワークフローを作成" accurately conveys the meaning of creating a new sub-workflow from selected nodes.
ui/src/lib/i18n/locales/es.json (1)
88-88: LGTM! Spanish translation looks accurate.The Spanish translation "Crear un nuevo subflujo de trabajo a partir de los nodos seleccionados" is grammatically correct and effectively conveys the meaning of creating a new sub-workflow from selected nodes.
Let's verify the consistency of this translation across all language files:
✅ Verification successful
Translation verification successful
The translation key exists consistently across all language files with appropriate translations in each supported language (es, fr, ja, zh), and the empty value in en.json is expected as it serves as the base language file.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the translation key exists in all language files # Expected: The key should exist in en.json, es.json, fr.json, ja.json, and zh.json for lang in en es fr ja zh; do echo "Checking ${lang}.json:" rg -A 1 '"Create new sub workflow from selected nodes"' "ui/src/lib/i18n/locales/${lang}.json" doneLength of output: 1493
ui/src/lib/i18n/locales/fr.json (1)
88-88: LGTM! French translation looks accurate.The French translation "Créer un nouveau sous-flux de travail à partir des nœuds sélectionnés" is grammatically correct and effectively conveys the meaning of creating a new sub-workflow from selected nodes.
ui/src/features/Editor/hooks.ts
Outdated
| { | ||
| keyBinding: { key: "s", commandKey: false }, | ||
| callback: () => handleWorkflowAdd(undefined, nodes, edges), | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Verify selected nodes before creating sub-workflow.
The shortcut callback should verify that there are selected nodes before attempting to create a sub-workflow. Consider adding a guard clause.
{
keyBinding: { key: "s", commandKey: false },
- callback: () => handleWorkflowAdd(undefined, nodes, edges),
+ callback: () => {
+ const selectedNodes = nodes.filter(n => n.selected);
+ if (selectedNodes.length === 0) return;
+ handleWorkflowAdd(undefined, selectedNodes, edges);
+ },
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| { | |
| keyBinding: { key: "s", commandKey: false }, | |
| callback: () => handleWorkflowAdd(undefined, nodes, edges), | |
| }, | |
| { | |
| keyBinding: { key: "s", commandKey: false }, | |
| callback: () => { | |
| const selectedNodes = nodes.filter(n => n.selected); | |
| if (selectedNodes.length === 0) return; | |
| handleWorkflowAdd(undefined, selectedNodes, edges); | |
| }, | |
| }, |
| @@ -85,6 +85,7 @@ | |||
| "Open the reader dialog": "", | |||
| "Open the transformer dialog": "", | |||
| "Open the writer dialog": "", | |||
| "Create new sub workflow from selected nodes": "", | |||
There was a problem hiding this comment.
Add English translation for the new feature.
The new entry is missing its English translation.
Add the translation:
- "Create new sub workflow from selected nodes": "",
+ "Create new sub workflow from selected nodes": "Create new sub workflow from selected nodes",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Create new sub workflow from selected nodes": "", | |
| "Create new sub workflow from selected nodes": "Create new sub workflow from selected nodes", |
…-subworkflow-from-selection
…arth/reearth-flow into ui/create-subworkflow-from-selection
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
ui/src/lib/yjs/useYWorkflow.ts (4)
45-48: Consider improving the workflow naming strategy.The current naming strategy using an incremental counter (
Sub Workflow-${yWorkflows.length}) could lead to naming conflicts if workflows are deleted. Consider using a more robust naming scheme that includes a timestamp or random suffix.-const workflowName = `Sub Workflow-${yWorkflows.length}`; +const workflowName = `Sub Workflow-${new Date().getTime().toString(36)}`;
50-79: Consider extracting node processing logic for better maintainability.The node processing logic is complex and handles multiple responsibilities. Consider extracting it into separate utility functions for better maintainability and reusability.
+const getNodesByParentId = (nodes: Node[]): Map<string, Node[]> => { + const nodesByParentId = new Map<string, Node[]>(); + nodes?.forEach((node) => { + if (node.parentId) { + if (!nodesByParentId.has(node.parentId)) { + nodesByParentId.set(node.parentId, []); + } + nodesByParentId.get(node.parentId)?.push(node); + } + }); + return nodesByParentId; +}; +const getAllIncludedNodeIds = ( + selectedNodes: Node[], + getBatchNodes: (batchId: string) => Node[] +): Set<string> => { + const allIncludedNodeIds = new Set<string>(); + selectedNodes?.forEach((node) => { + allIncludedNodeIds.add(node.id); + if (node.type === "batch") { + getBatchNodes(node.id).forEach((batchNode) => + allIncludedNodeIds.add(batchNode.id) + ); + } + }); + return allIncludedNodeIds; +};
80-87: Consider enhancing position calculation for better UX.The current position calculation uses the minimum x and y coordinates of selected nodes. For widely spread selections, this might place the subworkflow node far from the visual center of the selection.
-const calculatedPosition = hasSelectedNodes - ? { - x: Math.min(...selectedNodes.map((n) => n.position.x)), - y: Math.min(...selectedNodes.map((n) => n.position.y)), - } - : (position ?? { x: 600, y: 200 }); +const calculatedPosition = hasSelectedNodes + ? { + x: (Math.min(...selectedNodes.map((n) => n.position.x)) + + Math.max(...selectedNodes.map((n) => n.position.x))) / 2, + y: (Math.min(...selectedNodes.map((n) => n.position.y)) + + Math.max(...selectedNodes.map((n) => n.position.y))) / 2, + } + : (position ?? { x: 600, y: 200 });
186-197: Consider implementing atomic operations.The workflow update process involves multiple steps (deleting nodes, adding new nodes) that could leave the workflow in an inconsistent state if an error occurs midway. Consider implementing a transaction-like mechanism.
+const updateParentWorkflow = ( + parentWorkflowNodes: YNodesArray, + remainingNodes: Node[], + newSubworkflowNode: Node +) => { + try { if (hasSelectedNodes) { - const remainingNodes = nodes?.filter( - (n) => !allIncludedNodeIds.has(n.id), - ); parentWorkflowNodes?.delete(0, parentWorkflowNodes.length); parentWorkflowNodes?.push([ ...(remainingNodes ?? []), newSubworkflowNode, ]); } else { parentWorkflowNodes?.push([newSubworkflowNode]); } + } catch (error) { + console.error('Failed to update parent workflow:', error); + // Implement rollback mechanism here + throw new Error('Failed to update parent workflow'); + } +};
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ui/src/lib/yjs/useYWorkflow.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Redirect rules - reearth-flow
- GitHub Check: Header rules - reearth-flow
- GitHub Check: Pages changed - reearth-flow
🔇 Additional comments (1)
ui/src/lib/yjs/useYWorkflow.ts (1)
126-153: Add validation for edge connections.When processing internal edges, there's no validation to ensure that both source and target nodes still exist after node filtering. This could lead to invalid edge references.
const internalEdges = edges.filter( (e) => allIncludedNodeIds.has(e.source) && - allIncludedNodeIds.has(e.target), + allIncludedNodeIds.has(e.target) && + workflowNodes.some(n => n.id === e.source) && + workflowNodes.some(n => n.id === e.target), );
ui/src/lib/yjs/useYWorkflow.ts
Outdated
| const [inputRouter, outputRouter] = await Promise.all([ | ||
| fetcher<Action>(`${api}/actions/InputRouter`), | ||
| fetcher<Action>(`${api}/actions/OutputRouter`), | ||
| ]); |
There was a problem hiding this comment.
Add error handling for API calls.
The API calls to fetch router actions lack error handling. Consider adding try-catch blocks and proper error handling to improve reliability.
-const [inputRouter, outputRouter] = await Promise.all([
- fetcher<Action>(`${api}/actions/InputRouter`),
- fetcher<Action>(`${api}/actions/OutputRouter`),
-]);
+try {
+ const [inputRouter, outputRouter] = await Promise.all([
+ fetcher<Action>(`${api}/actions/InputRouter`),
+ fetcher<Action>(`${api}/actions/OutputRouter`),
+ ]);
+} catch (error) {
+ console.error('Failed to fetch router actions:', error);
+ throw new Error('Failed to create subworkflow: Could not fetch router actions');
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [inputRouter, outputRouter] = await Promise.all([ | |
| fetcher<Action>(`${api}/actions/InputRouter`), | |
| fetcher<Action>(`${api}/actions/OutputRouter`), | |
| ]); | |
| try { | |
| const [inputRouter, outputRouter] = await Promise.all([ | |
| fetcher<Action>(`${api}/actions/InputRouter`), | |
| fetcher<Action>(`${api}/actions/OutputRouter`), | |
| ]); | |
| } catch (error) { | |
| console.error('Failed to fetch router actions:', error); | |
| throw new Error('Failed to create subworkflow: Could not fetch router actions'); | |
| } |
ui/src/features/Editor/hooks.ts
Outdated
| @@ -11,6 +11,7 @@ import { cancellableDebounce } from "@flow/utils"; | |||
|
|
|||
| import useCanvasCopyPaste from "./useCanvasCopyPaste"; | |||
| import useNodeLocker from "./useNodeLocker"; | |||
| // import { useSubworkflowCreation } from "./useSubworkflowCreation"; | |||
|
|
||
| const inputRouter = await fetcher<Action>(`${api}/actions/InputRouter`); | ||
| const nodesByParentId = new Map<string, Node[]>(); |
There was a problem hiding this comment.
Isn't a lot of this logic in useBatch (or elsewhere)?
There was a problem hiding this comment.
We have something similar to this in the batchnode component but not necessarily in the useBatch. The one in the batchnode component largely handles resize logic and direct parent-child relationships and the subworkflow one has some additional complexities and edge handling. I would be a little bit worried that if we tried to make it so abstracted that it could make it harder to debug in the future as at the moment they are self contained in the places they are used. We could possibly have a shared function that could be more confusing to see what it is doing if the logic is integrated together
ui/src/lib/yjs/useYWorkflow.ts
Outdated
| @@ -42,12 +42,53 @@ export default ({ | |||
| ); | |||
|
|
|||
| const handleWorkflowAdd = useCallback( | |||
| (position?: XYPosition) => | |||
| (position?: XYPosition, nodes?: Node[], edges?: Edge[]) => | |||
There was a problem hiding this comment.
Adding nodes and edges to here is a little confusing since most places handleWorkflowAdd is used only wants position.
Consider a separate function like handleWorkflowAddFromSelection or something, while keeping in mind keeping duplicated logic to a minimum.
…-subworkflow-from-selection
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
ui/src/lib/yjs/useYWorkflow.ts (1)
52-55:⚠️ Potential issueAdd error handling for API calls.
The API calls to fetch router actions lack error handling. Consider adding try-catch blocks to improve reliability.
- const [inputRouter, outputRouter] = await Promise.all([ - fetcher<Action>(`${api}/actions/InputRouter`), - fetcher<Action>(`${api}/actions/OutputRouter`), - ]); + try { + const [inputRouter, outputRouter] = await Promise.all([ + fetcher<Action>(`${api}/actions/InputRouter`), + fetcher<Action>(`${api}/actions/OutputRouter`), + ]); + } catch (error) { + console.error('Failed to fetch router actions:', error); + throw new Error('Failed to create subworkflow: Could not fetch router actions'); + }
🧹 Nitpick comments (4)
ui/src/lib/yjs/useYWorkflow.ts (4)
61-61: Calculate input/output node positions relative to the subworkflow position.The positions for input and output nodes are hardcoded, which might not work well with different viewport sizes or zoom levels.
- position: { x: 200, y: 200 }, + position: { x: position.x - 400, y: position.y }, - position: { x: 1000, y: 200 }, + position: { x: position.x + 400, y: position.y },Also applies to: 76-76
Line range hint
156-246: Consider breaking down the complex selection logic into smaller functions.The function handles multiple responsibilities: grouping nodes by parent, collecting batch nodes, adjusting positions, and managing edges. Consider extracting these into separate helper functions for better maintainability.
+ const groupNodesByParent = (nodes: Node[]): Map<string, Node[]> => { + const nodesByParentId = new Map<string, Node[]>(); + nodes.forEach((node) => { + if (node.parentId) { + if (!nodesByParentId.has(node.parentId)) { + nodesByParentId.set(node.parentId, []); + } + nodesByParentId.get(node.parentId)?.push(node); + } + }); + return nodesByParentId; + }; + const collectIncludedNodeIds = ( + selectedNodes: Node[], + getBatchNodes: (batchId: string) => Node[], + ): Set<string> => { + const allIncludedNodeIds = new Set<string>(); + selectedNodes.forEach((node) => { + allIncludedNodeIds.add(node.id); + if (node.type === "batch") { + getBatchNodes(node.id).forEach((batchNode) => + allIncludedNodeIds.add(batchNode.id), + ); + } + }); + return allIncludedNodeIds; + }; const handleWorkflowAddFromSelection = useCallback( (nodes: Node[], edges: Edge[]) => undoTrackerActionWrapper(async () => { - const nodesByParentId = new Map<string, Node[]>(); - nodes.forEach((node) => { - if (node.parentId) { - if (!nodesByParentId.has(node.parentId)) { - nodesByParentId.set(node.parentId, []); - } - nodesByParentId.get(node.parentId)?.push(node); - } - }); + const nodesByParentId = groupNodesByParent(nodes); // ... rest of the function
193-202: Document position adjustment constants and improve readability.The position adjustment logic uses magic numbers (400, 200) without explanation. Consider extracting these as named constants and documenting their purpose.
+ // Constants for node position adjustments + const NODE_X_OFFSET = 400; // Horizontal offset from subworkflow position + const NODE_Y_OFFSET = 200; // Vertical offset from subworkflow position const adjustedNodes = allIncludedNodes.map((node) => ({ ...node, position: node.parentId ? node.position : { - x: node.position.x - position.x + 400, - y: node.position.y - position.y + 200, + x: node.position.x - position.x + NODE_X_OFFSET, + y: node.position.y - position.y + NODE_Y_OFFSET, }, selected: false, }));
227-232: Consider optimizing parent workflow node updates.The current approach of deleting all nodes and re-adding them might be inefficient and could cause UI flickering. Consider updating only the affected nodes.
- parentWorkflowNodes?.delete(0, parentWorkflowNodes.length); - parentWorkflowNodes?.push([...remainingNodes, newSubworkflowNode]); + // Find indices of nodes to remove + const indicesToRemove = []; + for (let i = 0; i < parentWorkflowNodes?.length || 0; i++) { + if (allIncludedNodeIds.has(parentWorkflowNodes?.get(i).id)) { + indicesToRemove.push(i); + } + } + // Remove nodes in reverse order to maintain correct indices + for (let i = indicesToRemove.length - 1; i >= 0; i--) { + parentWorkflowNodes?.delete(indicesToRemove[i], 1); + } + // Add the new subworkflow node + parentWorkflowNodes?.push([newSubworkflowNode]);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
ui/src/features/Editor/hooks.ts(2 hunks)ui/src/lib/i18n/locales/en.json(1 hunks)ui/src/lib/i18n/locales/es.json(1 hunks)ui/src/lib/i18n/locales/fr.json(1 hunks)ui/src/lib/i18n/locales/ja.json(1 hunks)ui/src/lib/i18n/locales/zh.json(1 hunks)ui/src/lib/yjs/useYWorkflow.ts(3 hunks)ui/src/lib/yjs/useYjsStore.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- ui/src/lib/i18n/locales/en.json
- ui/src/lib/i18n/locales/zh.json
- ui/src/lib/i18n/locales/fr.json
- ui/src/lib/i18n/locales/es.json
- ui/src/lib/i18n/locales/ja.json
- ui/src/features/Editor/hooks.ts
🔇 Additional comments (2)
ui/src/lib/yjs/useYjsStore.ts (1)
81-81: LGTM! Clean interface extension.The additions to the hook's return object are well-structured and align with the PR's objective of enabling sub-workflow creation from selections.
Also applies to: 190-190, 198-199
ui/src/lib/yjs/useYWorkflow.ts (1)
120-154: LGTM! Clean refactor using the new createWorkflow helper.The function is well-structured and properly integrated with the new helper function.
Overview
Sub workflows could not be created via multi selecting.
What I've done
What I haven't done
How I tested
manually
Screenshot
Which point I want you to review particularly
Memo
Summary by CodeRabbit
Release Notes
New Features
Localization
Improvements