Skip to content

Commit c2fcf5e

Browse files
committed
Refine minimap variable highlighting logic
The updated logic now correctly handles four distinct cases: variables used by the selected cell, variables declared by the selected cell, variables flowing through from ancestors, and variables in downstream cells that will be re-executed. This provides more precise visual feedback about which variables are actually affected by cell selection in the minimap.
1 parent 5ace202 commit c2fcf5e

File tree

2 files changed

+38
-25
lines changed

2 files changed

+38
-25
lines changed

frontend/src/components/editor/chrome/wrapper/floating-minimap.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,11 @@ import { useLastFocusedCellId } from "@/core/cells/focus";
77
import type { CellId } from "@/core/cells/ids";
88
import { useVariables } from "@/core/variables/state";
99
import { cn } from "@/utils/cn";
10-
import { invariant } from "@/utils/invariant";
11-
import { cellGraphsAtom, isVariableInSelectedDataflow } from "./minimap-state";
10+
import {
11+
cellGraphsAtom,
12+
isVariableAffectedBySelectedCell,
13+
type CellGraphNode,
14+
} from "./minimap-state";
1215

1316
interface MinimapCellProps {
1417
cellId: CellId;
@@ -70,7 +73,7 @@ const MinimapCell: React.FC<MinimapCellProps> = (props) => {
7073
!isSelected &&
7174
selectedCellId &&
7275
selectedGraph &&
73-
isVariableInSelectedDataflow(variable, {
76+
isVariableAffectedBySelectedCell(variable, {
7477
cellId: selectedCellId,
7578
graph: selectedGraph,
7679
}),

frontend/src/components/editor/chrome/wrapper/minimap-state.ts

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -119,41 +119,51 @@ export const cellGraphsAtom = atom((get) => {
119119
return buildCellGraph(notebook.cellIds.inOrderIds, variables);
120120
});
121121

122-
export function isVariableInSelectedDataflow(
122+
/**
123+
* Determines if a variable should be highlighted when a cell is selected.
124+
*
125+
* For upstream dataflow: We can precisely track which variables flow INTO the selected cell
126+
* by checking what the selected cell actually uses.
127+
*
128+
* For downstream dataflow: We only have cell-level dependency information, not variable-level,
129+
* so we highlight ALL variables in descendant cells since they will all be re-executed
130+
* when the selected cell changes.
131+
*
132+
* @param variable - The variable to check for highlighting
133+
* @param selected - The currently selected cell and its dependency graph
134+
*/
135+
export function isVariableAffectedBySelectedCell(
123136
variable: Variable,
124-
selected: {
125-
cellId: CellId;
126-
graph: {
127-
ancestors: ReadonlySet<CellId>;
128-
descendants: ReadonlySet<CellId>;
129-
};
130-
},
137+
selected: { cellId: CellId; graph: CellGraphNode },
131138
): boolean {
132-
// Variable is used by the selected cell
139+
// Case 1: Variable is used by the selected cell (incoming dataflow)
133140
if (variable.usedBy.includes(selected.cellId)) {
134141
return true;
135142
}
136143

137-
// Variable is declared by the selected cell and used by someone
138-
if (
139-
variable.declaredBy.includes(selected.cellId) &&
140-
variable.usedBy.length > 0
141-
) {
144+
// Case 2: Variable is declared by the selected cell (outgoing dataflow)
145+
if (variable.declaredBy.includes(selected.cellId)) {
142146
return true;
143147
}
144148

145-
// Variable flows through the selected cell:
146-
// It must be declared by an ancestor AND used by the selected cell or a descendant
149+
// Case 3: Variable is declared by an ancestor AND used by selected cell or descendants
150+
// This captures the flow-through case
147151
const isDeclaredByAncestor = variable.declaredBy.some((declarer) =>
148-
selected.graph.ancestors.has(declarer),
152+
selected.graph.parents.has(declarer),
149153
);
150154

151155
if (isDeclaredByAncestor) {
152-
// Only highlight if this variable is actually used by selected cell or its descendants
153-
return (
154-
variable.usedBy.includes(selected.cellId) ||
155-
variable.usedBy.some((user) => selected.graph.descendants.has(user))
156-
);
156+
return variable.usedBy.some((user) => selected.graph.parents.has(user));
157+
}
158+
159+
// Case 4: Variable is declared by a descendant
160+
// These will be re-executed when the selected cell changes
161+
const isDeclaredByDescendant = variable.declaredBy.some((declarer) =>
162+
selected.graph.children.has(declarer),
163+
);
164+
165+
if (isDeclaredByDescendant) {
166+
return true;
157167
}
158168

159169
return false;

0 commit comments

Comments
 (0)