Skip to content

Commit be68117

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 8b55dc3 commit be68117

File tree

2 files changed

+37
-24
lines changed

2 files changed

+37
-24
lines changed

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

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

1316
interface MinimapCellProps {
1417
cellId: CellId;
@@ -191,7 +194,7 @@ const MinimapCell: React.FC<MinimapCellProps> = (props) => {
191194
!isSelected &&
192195
selectedCellId &&
193196
selectedGraph &&
194-
isVariableInSelectedDataflow(variable, {
197+
isVariableAffectedBySelectedCell(variable, {
195198
cellId: selectedCellId,
196199
graph: selectedGraph,
197200
}),

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)