Skip to content

Commit f6476f2

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 261b51a commit f6476f2

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 CellGraph,
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
@@ -112,41 +112,51 @@ export const cellGraphsAtom = atom((get) => {
112112
return buildCellGraph(notebook.cellIds.inOrderIds, variables);
113113
});
114114

115-
export function isVariableInSelectedDataflow(
115+
/**
116+
* Determines if a variable should be highlighted when a cell is selected.
117+
*
118+
* For upstream dataflow: We can precisely track which variables flow INTO the selected cell
119+
* by checking what the selected cell actually uses.
120+
*
121+
* For downstream dataflow: We only have cell-level dependency information, not variable-level,
122+
* so we highlight ALL variables in descendant cells since they will all be re-executed
123+
* when the selected cell changes.
124+
*
125+
* @param variable - The variable to check for highlighting
126+
* @param selected - The currently selected cell and its dependency graph
127+
*/
128+
export function isVariableAffectedBySelectedCell(
116129
variable: Variable,
117-
selected: {
118-
cellId: CellId;
119-
graph: {
120-
ancestors: ReadonlySet<CellId>;
121-
descendants: ReadonlySet<CellId>;
122-
};
123-
},
130+
selected: { cellId: CellId; graph: CellGraph },
124131
): boolean {
125-
// Variable is used by the selected cell
132+
// Case 1: Variable is used by the selected cell (incoming dataflow)
126133
if (variable.usedBy.includes(selected.cellId)) {
127134
return true;
128135
}
129136

130-
// Variable is declared by the selected cell and used by someone
131-
if (
132-
variable.declaredBy.includes(selected.cellId) &&
133-
variable.usedBy.length > 0
134-
) {
137+
// Case 2: Variable is declared by the selected cell (outgoing dataflow)
138+
if (variable.declaredBy.includes(selected.cellId)) {
135139
return true;
136140
}
137141

138-
// Variable flows through the selected cell:
139-
// It must be declared by an ancestor AND used by the selected cell or a descendant
142+
// Case 3: Variable is declared by an ancestor AND used by selected cell or descendants
143+
// This captures the flow-through case
140144
const isDeclaredByAncestor = variable.declaredBy.some((declarer) =>
141-
selected.graph.ancestors.has(declarer),
145+
selected.graph.parents.has(declarer),
142146
);
143147

144148
if (isDeclaredByAncestor) {
145-
// Only highlight if this variable is actually used by selected cell or its descendants
146-
return (
147-
variable.usedBy.includes(selected.cellId) ||
148-
variable.usedBy.some((user) => selected.graph.descendants.has(user))
149-
);
149+
return variable.usedBy.some((user) => selected.graph.parents.has(user));
150+
}
151+
152+
// Case 4: Variable is declared by a descendant
153+
// These will be re-executed when the selected cell changes
154+
const isDeclaredByDescendant = variable.declaredBy.some((declarer) =>
155+
selected.graph.children.has(declarer),
156+
);
157+
158+
if (isDeclaredByDescendant) {
159+
return true;
150160
}
151161

152162
return false;

0 commit comments

Comments
 (0)