From 202fce7c49df62d0b4d96ffdf6133fa958e252dc Mon Sep 17 00:00:00 2001 From: Shawn Chang Date: Sat, 20 Jan 2024 17:19:32 +0800 Subject: [PATCH] show warning when operation has been added before --- .../src/components/AddNodesPanel.test.tsx | 6 ++ .../src/components/AddNodesPanel.tsx | 3 + .../components/EditOperationDialog.test.tsx | 25 ++++++ .../src/components/EditOperationDialog.tsx | 18 +++- .../src/components/FeaturePanel.test.tsx | 5 +- .../src/components/FeaturePanel.tsx | 4 + .../src/components/GraphContainer.tsx | 87 ++++++------------- 7 files changed, 82 insertions(+), 66 deletions(-) diff --git a/interactive-computational-graph/src/components/AddNodesPanel.test.tsx b/interactive-computational-graph/src/components/AddNodesPanel.test.tsx index e87b0a5..c110767 100644 --- a/interactive-computational-graph/src/components/AddNodesPanel.test.tsx +++ b/interactive-computational-graph/src/components/AddNodesPanel.test.tsx @@ -24,6 +24,7 @@ test("should render operation types as headers", () => { render( { render( { render( { render( { render( { render( ; isDarkMode: boolean; onAddNode: (featureNodeType: FeatureNodeType) => void; onAddOperation: () => void; @@ -27,6 +28,7 @@ interface AddNodesPanelProps { const AddNodesPanel: FunctionComponent = ({ featureOperations, + operationIdsAddedAtLeastOnce, isDarkMode, onAddNode, onAddOperation, @@ -217,6 +219,7 @@ const AddNodesPanel: FunctionComponent = ({ { open={true} isDarkMode={false} readOperation={featureOperation} + operationIdsAddedAtLeastOnce={new Set()} onCancel={handleCancel} onSave={handleSave} onDelete={handleDelete} @@ -42,6 +43,28 @@ test("should render the tabs", () => { expect(screen.getByText("Test Data: X ID")).toBeInTheDocument(); }); +test("should render the operation added warning when the operation ID has been added at least once", () => { + const featureOperation = getFeatureOperation(); + const handleCancel = jest.fn(); + const handleSave = jest.fn(); + const handleDelete = jest.fn(); + render( + , + ); + + const message = `This operation has already been added to the graph once. \ +Editing it may cause inconsistent results.`; + expect(screen.getByText(message)).toBeInTheDocument(); +}); + test("should trigger event when clicking save button", () => { const featureOperation = getFeatureOperation(); const handleCancel = jest.fn(); @@ -52,6 +75,7 @@ test("should trigger event when clicking save button", () => { open={true} isDarkMode={false} readOperation={featureOperation} + operationIdsAddedAtLeastOnce={new Set()} onCancel={handleCancel} onSave={handleSave} onDelete={handleDelete} @@ -75,6 +99,7 @@ test("should trigger necessary event when clicking cancel button after change", open={true} isDarkMode={false} readOperation={featureOperation} + operationIdsAddedAtLeastOnce={new Set()} onCancel={handleCancel} onSave={handleSave} onDelete={handleDelete} diff --git a/interactive-computational-graph/src/components/EditOperationDialog.tsx b/interactive-computational-graph/src/components/EditOperationDialog.tsx index 7027f4b..c4c61f7 100644 --- a/interactive-computational-graph/src/components/EditOperationDialog.tsx +++ b/interactive-computational-graph/src/components/EditOperationDialog.tsx @@ -7,6 +7,7 @@ import { Dialog, DialogContent, IconButton, + Stack, Tab, Tabs, Toolbar, @@ -22,15 +23,16 @@ import Operation from "../core/Operation"; import type Port from "../core/Port"; import type FeatureOperation from "../features/FeatureOperation"; import EditOperationBasicTab from "./EditOperationBasicTab"; +import EditOperationDfdxCodeTab from "./EditOperationDfdxCodeTab"; import EditOperationDialogMenu from "./EditOperationDialogMenu"; import EditOperationFCodeTab from "./EditOperationFCodeTab"; import EditOperationInputPortsTab from "./EditOperationInputPortsTab"; import EditOperationTabPanel from "./EditOperationTabPanel"; -import EditOperationDfdxCodeTab from "./EditOperationDfdxCodeTab"; interface EditOperationDialogProps { open: boolean; readOperation: FeatureOperation; + operationIdsAddedAtLeastOnce: Set; isDarkMode: boolean; onCancel: () => void; onSave: (updatedOperation: FeatureOperation) => void; @@ -40,6 +42,7 @@ interface EditOperationDialogProps { const EditOperationDialog: FunctionComponent = ({ open, readOperation, + operationIdsAddedAtLeastOnce, isDarkMode, onCancel, onSave, @@ -231,11 +234,18 @@ const EditOperationDialog: FunctionComponent = ({ - + + {operationIdsAddedAtLeastOnce.has(editingOperation.id) && ( + + This operation has already been added to the graph once. Editing + it may cause inconsistent results. + + )} + - Changes will be lost after reloading the page + Changes will be lost after reloading the page. - + ); diff --git a/interactive-computational-graph/src/components/FeaturePanel.test.tsx b/interactive-computational-graph/src/components/FeaturePanel.test.tsx index b7a663f..3b929e8 100644 --- a/interactive-computational-graph/src/components/FeaturePanel.test.tsx +++ b/interactive-computational-graph/src/components/FeaturePanel.test.tsx @@ -2,8 +2,8 @@ import { fireEvent, render, screen } from "@testing-library/react"; import Operation from "../core/Operation"; import Port from "../core/Port"; import { ADD_DFDX_CODE, ADD_F_CODE } from "../features/BuiltInCode"; -import FeatureNodeType from "../features/FeatureNodeType"; -import FeatureOperation from "../features/FeatureOperation"; +import type FeatureNodeType from "../features/FeatureNodeType"; +import type FeatureOperation from "../features/FeatureOperation"; import FeaturePanel from "./FeaturePanel"; /** @@ -24,6 +24,7 @@ test("should trigger event when adding a node", () => { ; hasNodes: boolean; hasDerivativeTarget: boolean; explainDerivativeData: ExplainDerivativeData[]; @@ -27,6 +28,7 @@ interface FeaturePanelProps { const FeaturePanel: FunctionComponent = ({ feature, featureOperations, + operationIdsAddedAtLeastOnce, hasNodes, hasDerivativeTarget, explainDerivativeData, @@ -44,6 +46,7 @@ const FeaturePanel: FunctionComponent = ({ return ( = ({ return ( = ({ const [nextOperationId, setNextOperationId] = useState(0); // Feature panel states + const [operationIdsAddedAtLeastOnce, setOperationIdsAddedAtLeastOnce] = + useState(new Set()); const [explainDerivativeData, setExplainDerivativeData] = useState< ExplainDerivativeData[] >([]); @@ -350,8 +352,8 @@ const GraphContainer: FunctionComponent = ({ [onSelectFeature], ); - const handleAddNode = useCallback( - (featureNodeType: FeatureNodeType) => { + const addNode = useCallback( + (featureNodeType: FeatureNodeType, position: XYPosition | null) => { const featureOperation = findFeatureOperation( featureNodeType, featureOperations, @@ -363,6 +365,7 @@ const GraphContainer: FunctionComponent = ({ featureOperation, ); + // Add the core node coreGraphAdapter.addNode( featureNodeType, featureOperation, @@ -370,6 +373,7 @@ const GraphContainer: FunctionComponent = ({ nodeName, ); + // Add the ReactFlow node const initialOutputValue = coreGraphAdapter.getNodeValueById(nodeId); const derivativeTargetName = derivativeTarget === null @@ -391,10 +395,19 @@ const GraphContainer: FunctionComponent = ({ }; setReactFlowNodes((nodes) => { nodes = deselectAllNodes(nodes); - const position = getNewReactFlowNodePosition(nodes, lastSelectedNodeId); - return addReactFlowNode(addNodeData, position, nodes); + const finalPosition = + position ?? getNewReactFlowNodePosition(nodes, lastSelectedNodeId); + return addReactFlowNode(addNodeData, finalPosition, nodes); }); + // Add the operation ID to the set for operation nodes + if (featureOperation !== null) { + setOperationIdsAddedAtLeastOnce( + (oldOperationIds) => + new Set([...oldOperationIds, featureOperation.id]), + ); + } + setNextNodeId((nextNodeId) => nextNodeId + 1); }, [ @@ -413,6 +426,13 @@ const GraphContainer: FunctionComponent = ({ ], ); + const handleAddNode = useCallback( + (featureNodeType: FeatureNodeType) => { + addNode(featureNodeType, null); + }, + [addNode], + ); + const handleAddOperation = useCallback(() => { const id = `${nextOperationId}`; @@ -546,63 +566,9 @@ const GraphContainer: FunctionComponent = ({ const handleDropNode = useCallback( (featureNodeType: FeatureNodeType, position: XYPosition) => { - const featureOperation = findFeatureOperation( - featureNodeType, - featureOperations, - ); - - const nodeId = `${nextNodeId}`; - const nodeName = nodeNameBuilder.buildName( - featureNodeType, - featureOperation, - ); - - coreGraphAdapter.addNode( - featureNodeType, - featureOperation, - nodeId, - nodeName, - ); - - const initialOutputValue = coreGraphAdapter.getNodeValueById(nodeId); - const derivativeTargetName = - derivativeTarget === null - ? null - : coreGraphAdapter.getNodeNameById(derivativeTarget); - const addNodeData: AddNodeData = { - featureNodeType, - featureOperation, - nodeId, - nodeName, - initialOutputValue, - isReverseMode, - derivativeTargetName, - onNameChange: handleNameChange, - onInputChange: handleInputChange, - onBodyClick: handleBodyClick, - onDerivativeClick: handleDerivativeClick, - isDarkMode, - }; - setReactFlowNodes((nodes) => { - nodes = deselectAllNodes(nodes); - return addReactFlowNode(addNodeData, position, nodes); - }); - - setNextNodeId((nextNodeId) => nextNodeId + 1); + addNode(featureNodeType, position); }, - [ - coreGraphAdapter, - derivativeTarget, - featureOperations, - handleBodyClick, - handleDerivativeClick, - handleInputChange, - handleNameChange, - isDarkMode, - isReverseMode, - nextNodeId, - nodeNameBuilder, - ], + [addNode], ); const handleSnackbarClose = useCallback( @@ -757,6 +723,7 @@ const GraphContainer: FunctionComponent = ({ 0} hasDerivativeTarget={derivativeTarget !== null} explainDerivativeData={explainDerivativeData}