Skip to content

Commit

Permalink
Merge pull request #46 from sc420/34-disable-editing-operation-when-i…
Browse files Browse the repository at this point in the history 
…t-has-been-added-to-the-canvas

show warning when operation has been added before
  • Loading branch information
@sc420
sc420 authored Jan 20, 2024
2 parents 5c7de08 + 202fce7 commit 340489b
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 66 deletions.
6 changes: 6 additions & 0 deletions interactive-computational-graph/src/components/AddNodesPanel.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ test("should render operation types as headers", () => {
render(
<AddNodesPanel
featureOperations={featureOperations}
operationIdsAddedAtLeastOnce={new Set()}
isDarkMode={false}
onAddNode={handleAddNode}
onAddOperation={handleAddOperation}
Expand All @@ -49,6 +50,7 @@ test("should trigger event when clicking item", () => {
render(
<AddNodesPanel
featureOperations={featureOperations}
operationIdsAddedAtLeastOnce={new Set()}
isDarkMode={false}
onAddNode={handleAddNode}
onAddOperation={handleAddOperation}
Expand All @@ -75,6 +77,7 @@ test("should trigger event when clicking add operation button", () => {
render(
<AddNodesPanel
featureOperations={featureOperations}
operationIdsAddedAtLeastOnce={new Set()}
isDarkMode={false}
onAddNode={handleAddNode}
onAddOperation={handleAddOperation}
Expand All @@ -97,6 +100,7 @@ test("should not trigger event when canceling editing operation", () => {
render(
<AddNodesPanel
featureOperations={featureOperations}
operationIdsAddedAtLeastOnce={new Set()}
isDarkMode={false}
onAddNode={handleAddNode}
onAddOperation={handleAddOperation}
Expand Down Expand Up @@ -126,6 +130,7 @@ test("should trigger event when clicking edit icon button", () => {
render(
<AddNodesPanel
featureOperations={featureOperations}
operationIdsAddedAtLeastOnce={new Set()}
isDarkMode={false}
onAddNode={handleAddNode}
onAddOperation={handleAddOperation}
Expand Down Expand Up @@ -153,6 +158,7 @@ test("should trigger event when deleting operation", () => {
render(
<AddNodesPanel
featureOperations={featureOperations}
operationIdsAddedAtLeastOnce={new Set()}
isDarkMode={false}
onAddNode={handleAddNode}
onAddOperation={handleAddOperation}
Expand Down
3 changes: 3 additions & 0 deletions interactive-computational-graph/src/components/AddNodesPanel.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import EditOperationDialog from "./EditOperationDialog";

interface AddNodesPanelProps {
featureOperations: FeatureOperation[];
operationIdsAddedAtLeastOnce: Set<string>;
isDarkMode: boolean;
onAddNode: (featureNodeType: FeatureNodeType) => void;
onAddOperation: () => void;
Expand All @@ -27,6 +28,7 @@ interface AddNodesPanelProps {

const AddNodesPanel: FunctionComponent<AddNodesPanelProps> = ({
featureOperations,
operationIdsAddedAtLeastOnce,
isDarkMode,
onAddNode,
onAddOperation,
Expand Down Expand Up @@ -217,6 +219,7 @@ const AddNodesPanel: FunctionComponent<AddNodesPanelProps> = ({
<EditOperationDialog
open={isEditDialogOpen}
readOperation={editingOperation}
operationIdsAddedAtLeastOnce={operationIdsAddedAtLeastOnce}
isDarkMode={isDarkMode}
onCancel={handleCloseEditDialog}
onSave={handleSaveOperation}
Expand Down
25 changes: 25 additions & 0 deletions interactive-computational-graph/src/components/EditOperationDialog.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ test("should render the tabs", () => {
open={true}
isDarkMode={false}
readOperation={featureOperation}
operationIdsAddedAtLeastOnce={new Set()}
onCancel={handleCancel}
onSave={handleSave}
onDelete={handleDelete}
Expand All @@ -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(
<EditOperationDialog
open={true}
isDarkMode={false}
readOperation={featureOperation}
operationIdsAddedAtLeastOnce={new Set(["add", "multiply"])}
onCancel={handleCancel}
onSave={handleSave}
onDelete={handleDelete}
/>,
);

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();
Expand All @@ -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}
Expand All @@ -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}
Expand Down
18 changes: 14 additions & 4 deletions interactive-computational-graph/src/components/EditOperationDialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
Dialog,
DialogContent,
IconButton,
Stack,
Tab,
Tabs,
Toolbar,
Expand All @@ -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<string>;
isDarkMode: boolean;
onCancel: () => void;
onSave: (updatedOperation: FeatureOperation) => void;
Expand All @@ -40,6 +42,7 @@ interface EditOperationDialogProps {
const EditOperationDialog: FunctionComponent<EditOperationDialogProps> = ({
open,
readOperation,
operationIdsAddedAtLeastOnce,
isDarkMode,
onCancel,
onSave,
Expand Down Expand Up @@ -231,11 +234,18 @@ const EditOperationDialog: FunctionComponent<EditOperationDialogProps> = ({
</Box>
</EditOperationTabPanel>

<Box sx={{ p: 3 }}>
<Stack p={3} spacing={3}>
{operationIdsAddedAtLeastOnce.has(editingOperation.id) && (
<Alert variant="outlined" severity="warning">
This operation has already been added to the graph once. Editing
it may cause inconsistent results.
</Alert>
)}

<Alert variant="outlined" severity="info">
Changes will be lost after reloading the page
Changes will be lost after reloading the page.
</Alert>
</Box>
</Stack>
</DialogContent>
</Dialog>
);
Expand Down
5 changes: 3 additions & 2 deletions interactive-computational-graph/src/components/FeaturePanel.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";

/**
Expand All @@ -24,6 +24,7 @@ test("should trigger event when adding a node", () => {
<FeaturePanel
feature="add-nodes"
featureOperations={featureOperations}
operationIdsAddedAtLeastOnce={new Set()}
hasNodes={false}
hasDerivativeTarget={false}
explainDerivativeData={[]}
Expand Down
4 changes: 4 additions & 0 deletions interactive-computational-graph/src/components/FeaturePanel.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import ExplainDerivativesPanel from "./ExplainDerivativesPanel";
interface FeaturePanelProps {
feature: SelectedFeature;
featureOperations: FeatureOperation[];
operationIdsAddedAtLeastOnce: Set<string>;
hasNodes: boolean;
hasDerivativeTarget: boolean;
explainDerivativeData: ExplainDerivativeData[];
Expand All @@ -27,6 +28,7 @@ interface FeaturePanelProps {
const FeaturePanel: FunctionComponent<FeaturePanelProps> = ({
feature,
featureOperations,
operationIdsAddedAtLeastOnce,
hasNodes,
hasDerivativeTarget,
explainDerivativeData,
Expand All @@ -44,6 +46,7 @@ const FeaturePanel: FunctionComponent<FeaturePanelProps> = ({
return (
<AddNodesPanel
featureOperations={featureOperations}
operationIdsAddedAtLeastOnce={operationIdsAddedAtLeastOnce}
isDarkMode={isDarkMode}
onAddNode={onAddNode}
onAddOperation={onAddOperation}
Expand All @@ -67,6 +70,7 @@ const FeaturePanel: FunctionComponent<FeaturePanelProps> = ({
return (
<AddNodesPanel
featureOperations={featureOperations}
operationIdsAddedAtLeastOnce={operationIdsAddedAtLeastOnce}
isDarkMode={isDarkMode}
onAddNode={onAddNode}
onAddOperation={onAddOperation}
Expand Down
87 changes: 27 additions & 60 deletions interactive-computational-graph/src/components/GraphContainer.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,8 @@ const GraphContainer: FunctionComponent<GraphContainerProps> = ({
const [nextOperationId, setNextOperationId] = useState<number>(0);

// Feature panel states
const [operationIdsAddedAtLeastOnce, setOperationIdsAddedAtLeastOnce] =
useState(new Set<string>());
const [explainDerivativeData, setExplainDerivativeData] = useState<
ExplainDerivativeData[]
>([]);
Expand Down Expand Up @@ -350,8 +352,8 @@ const GraphContainer: FunctionComponent<GraphContainerProps> = ({
[onSelectFeature],
);

const handleAddNode = useCallback(
(featureNodeType: FeatureNodeType) => {
const addNode = useCallback(
(featureNodeType: FeatureNodeType, position: XYPosition | null) => {
const featureOperation = findFeatureOperation(
featureNodeType,
featureOperations,
Expand All @@ -363,13 +365,15 @@ const GraphContainer: FunctionComponent<GraphContainerProps> = ({
featureOperation,
);

// Add the core node
coreGraphAdapter.addNode(
featureNodeType,
featureOperation,
nodeId,
nodeName,
);

// Add the ReactFlow node
const initialOutputValue = coreGraphAdapter.getNodeValueById(nodeId);
const derivativeTargetName =
derivativeTarget === null
Expand All @@ -391,10 +395,19 @@ const GraphContainer: FunctionComponent<GraphContainerProps> = ({
};
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);
},
[
Expand All @@ -413,6 +426,13 @@ const GraphContainer: FunctionComponent<GraphContainerProps> = ({
],
);

const handleAddNode = useCallback(
(featureNodeType: FeatureNodeType) => {
addNode(featureNodeType, null);
},
[addNode],
);

const handleAddOperation = useCallback(() => {
const id = `${nextOperationId}`;

Expand Down Expand Up @@ -546,63 +566,9 @@ const GraphContainer: FunctionComponent<GraphContainerProps> = ({

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(
Expand Down Expand Up @@ -757,6 +723,7 @@ const GraphContainer: FunctionComponent<GraphContainerProps> = ({
<FeaturePanel
feature={selectedFeature}
featureOperations={featureOperations}
operationIdsAddedAtLeastOnce={operationIdsAddedAtLeastOnce}
hasNodes={reactFlowNodes.length > 0}
hasDerivativeTarget={derivativeTarget !== null}
explainDerivativeData={explainDerivativeData}
Expand Down

0 comments on commit 340489b

Please sign in to comment.