From 7a8cb63db809af061b7b7d000ed1f05a31ff3849 Mon Sep 17 00:00:00 2001 From: Camiel van Schoonhoven Date: Wed, 8 Oct 2025 10:58:27 -0700 Subject: [PATCH] Fix Node Replacement Issues on IO Rename --- .../FormFields/FormFields.tsx | 3 -- .../InputValueEditor.test.tsx | 13 +++--- .../InputValueEditor/InputValueEditor.tsx | 9 ++-- .../OutputNameEditor/OutputNameEditor.tsx | 8 ++-- .../ReactFlow/FlowCanvas/IONode/IONode.tsx | 17 +++---- src/hooks/useNodeSelectionTransfer.ts | 45 ------------------- 6 files changed, 19 insertions(+), 76 deletions(-) delete mode 100644 src/hooks/useNodeSelectionTransfer.ts diff --git a/src/components/Editor/IOEditor/InputValueEditor/FormFields/FormFields.tsx b/src/components/Editor/IOEditor/InputValueEditor/FormFields/FormFields.tsx index 81334f8b5..5c7de0d52 100644 --- a/src/components/Editor/IOEditor/InputValueEditor/FormFields/FormFields.tsx +++ b/src/components/Editor/IOEditor/InputValueEditor/FormFields/FormFields.tsx @@ -68,14 +68,12 @@ const NameField = ({ onBlur, error, disabled, - autoFocus = false, }: { inputName: string; onNameChange: (value: string) => void; onBlur?: () => void; error?: string | null; disabled?: boolean; - autoFocus?: boolean; }) => (
{error}
diff --git a/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.test.tsx b/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.test.tsx index 562762489..d4f37aa59 100644 --- a/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.test.tsx +++ b/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.test.tsx @@ -7,9 +7,9 @@ import { InputValueEditor } from "./InputValueEditor"; // Mock all the dependencies const mockSetComponentSpec = vi.fn(); -const mockTransferSelection = vi.fn(); const mockNotify = vi.fn(); const mockClearContent = vi.fn(); +const mockUpdateRefId = vi.fn(); vi.mock("@/providers/ComponentSpecProvider", () => ({ useComponentSpec: () => ({ @@ -48,9 +48,12 @@ vi.mock("@/providers/ContextPanelProvider", () => ({ }), })); -vi.mock("@/hooks/useNodeSelectionTransfer", () => ({ - useNodeSelectionTransfer: () => ({ - transferSelection: mockTransferSelection, +vi.mock("@/hooks/useNodeManager", () => ({ + useNodeManager: () => ({ + updateRefId: mockUpdateRefId, + getNodeId: vi.fn(), + getHandleNodeId: vi.fn(), + nodeManager: {}, }), })); @@ -122,7 +125,7 @@ describe("InputValueEditor", () => { // Verify that the component spec was updated and selection was transferred expect(mockSetComponentSpec).toHaveBeenCalled(); - expect(mockTransferSelection).toHaveBeenCalledWith("TestInput", "NewName"); + expect(mockUpdateRefId).toHaveBeenCalledWith("TestInput", "NewName"); }); it("shows validation error when renaming to existing input name", () => { diff --git a/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.tsx b/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.tsx index 1f89ab7b3..fadd43e17 100644 --- a/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.tsx +++ b/src/components/Editor/IOEditor/InputValueEditor/InputValueEditor.tsx @@ -10,7 +10,6 @@ import { BlockStack } from "@/components/ui/layout"; import { Heading, Paragraph } from "@/components/ui/typography"; import useConfirmationDialog from "@/hooks/useConfirmationDialog"; import { useNodeManager } from "@/hooks/useNodeManager"; -import { useNodeSelectionTransfer } from "@/hooks/useNodeSelectionTransfer"; import useToastNotification from "@/hooks/useToastNotification"; import { useComponentSpec } from "@/providers/ComponentSpecProvider"; import { useContextPanel } from "@/providers/ContextPanelProvider"; @@ -31,10 +30,9 @@ export const InputValueEditor = ({ input, disabled = false, }: InputValueEditorProps) => { - const { getInputNodeId } = useNodeManager(); + const { updateRefId } = useNodeManager(); const notify = useToastNotification(); - const { transferSelection } = useNodeSelectionTransfer(getInputNodeId); const { componentSpec, setComponentSpec, @@ -110,11 +108,11 @@ export const InputValueEditor = ({ newName, ); - transferSelection(oldName, newName); + updateRefId(oldName, newName); return updatedComponentSpec; }, - [currentSubgraphSpec, transferSelection], + [currentSubgraphSpec, updateRefId], ); const handleValueChange = useCallback((value: string) => { @@ -292,7 +290,6 @@ export const InputValueEditor = ({ onBlur={handleBlur} error={validationError} disabled={disabled} - autoFocus={!disabled} /> { - const { getOutputNodeId } = useNodeManager(); - const { transferSelection } = useNodeSelectionTransfer(getOutputNodeId); + const { updateRefId } = useNodeManager(); const { setComponentSpec, componentSpec, @@ -62,11 +60,11 @@ export const OutputNameEditor = ({ newName, ); - transferSelection(oldName, newName); + updateRefId(oldName, newName); return updatedComponentSpec; }, - [currentSubgraphSpec, transferSelection], + [currentSubgraphSpec, updateRefId], ); const saveChanges = useCallback(() => { diff --git a/src/components/shared/ReactFlow/FlowCanvas/IONode/IONode.tsx b/src/components/shared/ReactFlow/FlowCanvas/IONode/IONode.tsx index b62092fb2..aae6b9336 100644 --- a/src/components/shared/ReactFlow/FlowCanvas/IONode/IONode.tsx +++ b/src/components/shared/ReactFlow/FlowCanvas/IONode/IONode.tsx @@ -57,13 +57,7 @@ const IONode = ({ type, data, selected = false }: IONodeProps) => { useEffect(() => { if (selected) { if (input && isInput) { - setContent( - , - ); + setContent(); } if (output && !isInput) { @@ -75,19 +69,18 @@ const IONode = ({ type, data, selected = false }: IONodeProps) => { , ); } } + }, [selected, readOnly, input, output, isInput, currentGraphSpec]); + useEffect(() => { return () => { - if (selected) { - clearContent(); - } + clearContent(); }; - }, [input, output, selected, readOnly]); + }, []); const connectedOutput = getOutputConnectedDetails( currentGraphSpec, diff --git a/src/hooks/useNodeSelectionTransfer.ts b/src/hooks/useNodeSelectionTransfer.ts deleted file mode 100644 index 532cb0cde..000000000 --- a/src/hooks/useNodeSelectionTransfer.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { useReactFlow } from "@xyflow/react"; -import { useCallback } from "react"; - -/* - Transfer the node selection state from old node to new node when the node name changes. - This is a workaround for the fact that React Flow does not automatically transfer the selection state - when a node's id changes (which is derived from the input/output/task name). -*/ - -type NodeIdGenerator = (name: string) => string; - -export const useNodeSelectionTransfer = (nodeIdGenerator: NodeIdGenerator) => { - const { setNodes, getNodes } = useReactFlow(); - - const transferSelection = useCallback( - (oldName: string, newName: string) => { - if (oldName === newName) return; - - const oldNodeId = nodeIdGenerator(oldName); - const newNodeId = nodeIdGenerator(newName); - - const currentNodes = getNodes(); - const oldNode = currentNodes.find((node) => node.id === oldNodeId); - const wasSelected = oldNode?.selected ?? false; - - // Schedule the selection update after any component spec update (not an ideal solution -- we will revisit later) - setTimeout(() => { - setNodes((nodes) => { - return nodes.map((node) => { - if (node.id === oldNodeId) { - return { ...node, selected: false }; - } - if (node.id === newNodeId) { - return { ...node, selected: wasSelected }; - } - return node; - }); - }); - }, 0); - }, - [nodeIdGenerator, getNodes, setNodes], - ); - - return { transferSelection }; -};