Skip to content

Commit c424d9a

Browse files
authored
fix context panel on node dragging (#709)
## Description Closes Shopify/oasis-frontend#202 This PR improves performance and code quality through several optimizations: 1. Removed unnecessary `isDragging` check in TaskNodeCard that was causing performance issues 2. Optimized the `useToastNotification` hook with `useCallback` to prevent unnecessary re-renders 3. Fixed a memory leak in ContextPanelProvider by using a ref for defaultContent 4. Refactored TaskNodeProvider to extract callbacks from data with a fallback to default no-op functions ## Type of Change - [x] Improvement - [x] Cleanup/Refactor - [x] Bug fix ## Checklist - [x] I have tested this does not break current pipelines / runs functionality - [ ] I have tested the changes on staging ## Test Instructions 1. Verify task nodes can still be selected and manipulated without issues 2. Check that toast notifications work properly 3. Confirm context panel content updates correctly when selecting nodes 4. Verify task node callbacks (delete, duplicate, upgrade) function as expected [Screen Recording 2025-12-04 at 3.33.30 PM.mov <span class="graphite__hidden">(uploaded via Graphite)</span> <img class="graphite__hidden" src="https://app.graphite.com/user-attachments/thumbnails/eeb09801-784c-409f-9774-60b1a9615b5e.mov" />](https://app.graphite.com/user-attachments/video/eeb09801-784c-409f-9774-60b1a9615b5e.mov)
1 parent 096efbc commit c424d9a

File tree

5 files changed

+65
-41
lines changed

5 files changed

+65
-41
lines changed

src/components/shared/ReactFlow/FlowCanvas/TaskNode/TaskNodeCard/TaskNodeCard.tsx

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import { useNavigate } from "@tanstack/react-router";
2-
import { useStore } from "@xyflow/react";
32
import { CircleFadingArrowUp, CopyIcon } from "lucide-react";
43
import { useCallback, useEffect, useMemo, useRef, useState } from "react";
54

@@ -51,11 +50,6 @@ const TaskNodeCard = () => {
5150
const rootExecutionId = executionData?.rootExecutionId;
5251
const details = executionData?.details;
5352

54-
const isDragging = useStore((state) => {
55-
const thisNode = state.nodes.find((node) => node.id === taskNode.nodeId);
56-
return thisNode?.dragging || false;
57-
});
58-
5953
const nodeRef = useRef<HTMLDivElement | null>(null);
6054
const contentRef = useRef<HTMLDivElement>(null);
6155

@@ -241,7 +235,7 @@ const TaskNodeCard = () => {
241235
}, [scrollHeight, dimensions.h]);
242236

243237
useEffect(() => {
244-
if (selected && !isDragging) {
238+
if (selected) {
245239
setContent(taskConfigMarkup);
246240
setContextPanelOpen(true);
247241
}

src/hooks/useToastNotification.ts

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,29 @@
1-
import { useRef } from "react";
1+
import { useCallback, useRef } from "react";
22
import { Bounce, type Id, toast } from "react-toastify";
33

44
const useToastNotification = () => {
55
const toastId = useRef<Id | null>(null);
66

7-
const notify = (
8-
message: string | React.ReactNode,
9-
type: "success" | "warning" | "error" | "info" = "info",
10-
) => {
11-
toastId.current = toast(message, {
12-
position: "bottom-right",
13-
autoClose: 3000,
14-
hideProgressBar: false,
15-
closeOnClick: false,
16-
pauseOnHover: true,
17-
draggable: true,
18-
progress: undefined,
19-
theme: "colored",
20-
transition: Bounce,
21-
type,
22-
});
23-
};
7+
const notify = useCallback(
8+
(
9+
message: string | React.ReactNode,
10+
type: "success" | "warning" | "error" | "info" = "info",
11+
) => {
12+
toastId.current = toast(message, {
13+
position: "bottom-right",
14+
autoClose: 3000,
15+
hideProgressBar: false,
16+
closeOnClick: false,
17+
pauseOnHover: true,
18+
draggable: true,
19+
progress: undefined,
20+
theme: "colored",
21+
transition: Bounce,
22+
type,
23+
});
24+
},
25+
[],
26+
);
2427

2528
return notify;
2629
};

src/providers/ContextPanelProvider.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
useCallback,
55
useEffect,
66
useMemo,
7+
useRef,
78
useState,
89
} from "react";
910

@@ -40,7 +41,7 @@ export const ContextPanelProvider = ({
4041
children: ReactNode;
4142
}) => {
4243
const { setNodes } = useReactFlow();
43-
44+
const defaultContentRef = useRef<ReactNode>(defaultContent);
4445
const [content, setContentState] = useState<ReactNode>(defaultContent);
4546
const [open, setOpen] = useState(true);
4647

@@ -49,10 +50,12 @@ export const ContextPanelProvider = ({
4950
}, []);
5051

5152
const clearContent = useCallback(() => {
52-
setContentState(defaultContent);
53-
}, [defaultContent]);
53+
setContentState(defaultContentRef.current);
54+
}, []);
5455

5556
useEffect(() => {
57+
defaultContentRef.current = defaultContent;
58+
5659
const handleKeyDown = (event: KeyboardEvent) => {
5760
if (event.key === "Escape") {
5861
clearContent();

src/providers/TaskNodeProvider.tsx

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ import { useTaskNodeDimensions } from "@/hooks/useTaskNodeDimensions";
77
import useToastNotification from "@/hooks/useToastNotification";
88
import type { Annotations } from "@/types/annotations";
99
import type { RunStatus } from "@/types/pipelineRun";
10-
import type { TaskNodeData, TaskNodeDimensions } from "@/types/taskNode";
10+
import {
11+
DEFAULT_TASK_NODE_CALLBACKS,
12+
type TaskNodeData,
13+
type TaskNodeDimensions,
14+
} from "@/types/taskNode";
1115
import type {
1216
ArgumentType,
1317
ComponentReference,
@@ -82,6 +86,15 @@ export const TaskNodeProvider = ({
8286
const taskId = data.taskId;
8387
const nodeId = taskId ? taskIdToNodeId(taskId) : "";
8488

89+
const {
90+
onDelete,
91+
onDuplicate,
92+
onUpgrade,
93+
setArguments,
94+
setAnnotations,
95+
setCacheStaleness,
96+
} = data.callbacks ?? DEFAULT_TASK_NODE_CALLBACKS;
97+
8598
const componentRef = taskSpec?.componentRef ?? EMPTY_COMPONENT_REF;
8699
const inputs = componentRef.spec?.inputs ?? EMPTY_INPUTS;
87100
const outputs = componentRef.spec?.outputs ?? EMPTY_OUTPUTS;
@@ -100,41 +113,41 @@ export const TaskNodeProvider = ({
100113

101114
const handleSetArguments = useCallback(
102115
(args: Record<string, ArgumentType>) => {
103-
data.callbacks?.setArguments(args);
116+
setArguments(args);
104117
},
105-
[data.callbacks],
118+
[setArguments],
106119
);
107120

108121
const handleSetAnnotations = useCallback(
109122
(annotations: Annotations) => {
110-
data.callbacks?.setAnnotations(annotations);
123+
setAnnotations(annotations);
111124
},
112-
[data.callbacks],
125+
[setAnnotations],
113126
);
114127

115128
const handleSetCacheStaleness = useCallback(
116129
(cacheStaleness: string | undefined) => {
117-
data.callbacks?.setCacheStaleness(cacheStaleness);
130+
setCacheStaleness(cacheStaleness);
118131
},
119-
[data.callbacks],
132+
[setCacheStaleness, notify],
120133
);
121134

122135
const handleDeleteTaskNode = useCallback(() => {
123-
data.callbacks?.onDelete();
124-
}, [data.callbacks]);
136+
onDelete();
137+
}, [onDelete]);
125138

126139
const handleDuplicateTaskNode = useCallback(() => {
127-
data.callbacks?.onDuplicate();
128-
}, [data.callbacks]);
140+
onDuplicate();
141+
}, [onDuplicate]);
129142

130143
const handleUpgradeTaskNode = useCallback(() => {
131144
if (!isOutdated) {
132145
notify("Component version already matches source URL", "info");
133146
return;
134147
}
135148

136-
data.callbacks?.onUpgrade(mostRecentComponentRef);
137-
}, [data.callbacks, isOutdated, mostRecentComponentRef, notify]);
149+
onUpgrade(mostRecentComponentRef);
150+
}, [onUpgrade, isOutdated, mostRecentComponentRef, notify]);
138151

139152
const select = useCallback(() => {
140153
reactFlowInstance.setNodes((nodes) =>

src/types/taskNode.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,17 @@ interface TaskNodeCallbacks {
3434
onUpgrade: (newComponentRef: ComponentReference) => void;
3535
}
3636

37+
function noop() {}
38+
39+
export const DEFAULT_TASK_NODE_CALLBACKS: TaskNodeCallbacks = {
40+
setArguments: noop,
41+
setAnnotations: noop,
42+
onDelete: noop,
43+
onDuplicate: noop,
44+
onUpgrade: noop,
45+
setCacheStaleness: noop,
46+
};
47+
3748
// Dynamic Node Callback types - every callback has a version with the node & task id added to it as an input parameter
3849
export type CallbackWithIds<K extends keyof TaskNodeCallbacks> =
3950
TaskNodeCallbacks[K] extends (...args: infer A) => infer R

0 commit comments

Comments
 (0)