Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions canvas/src/EntityNode.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { describe, expect, it } from "vitest";
import { type NodeProps } from "reactflow";
import { entityNodePropsEqual, type EntityNodeData } from "./EntityNode";
import type { Entity } from "./types";

const ENTITY: Entity = {
id: "01ENT00000000000000000000",
name: "interventions",
pattern: null,
conceptual: null,
attributes: [],
};

function data(over: Partial<EntityNodeData> = {}): EntityNodeData {
return {
entity: ENTITY,
color: "#8a7d6a", // NO_SA grey
dimmed: false,
highlighted: false,
showTypes: true,
collapseDetail: false,
expanded: false,
...over,
};
}

// Only `data` and `selected` are read by the comparator; the rest of NodeProps is
// irrelevant to the memo decision, so a bare cast keeps the fixtures minimal.
function props(d: EntityNodeData, selected = false): NodeProps<EntityNodeData> {
return { data: d, selected } as NodeProps<EntityNodeData>;
}

describe("entityNodePropsEqual (memo comparator)", () => {
it("re-renders when the subject-area colour changes (the stale-recolor bug)", () => {
const grey = props(data({ color: "#8a7d6a" }));
const purple = props(data({ color: "#c4a3e0" }));
// false === NOT equal === React re-renders -> the accent repaints in place
expect(entityNodePropsEqual(grey, purple)).toBe(false);
});

it("skips re-render when nothing render-affecting changed (new data ref, same fields)", () => {
// the derivation hands a fresh data object every refresh; identical fields must bail
expect(entityNodePropsEqual(props(data()), props(data()))).toBe(true);
});

it("re-renders on dimmed / highlighted / expand / selected changes", () => {
const base = data();
expect(entityNodePropsEqual(props(base), props(data({ dimmed: true })))).toBe(false);
expect(entityNodePropsEqual(props(base), props(data({ highlighted: true })))).toBe(false);
expect(entityNodePropsEqual(props(base), props(data({ expanded: true })))).toBe(false);
expect(entityNodePropsEqual(props(base, false), props(base, true))).toBe(false);
});

it("re-renders when the entity object identity changes (content edit)", () => {
const edited: Entity = { ...ENTITY, name: "renamed" };
expect(entityNodePropsEqual(props(data()), props(data({ entity: edited })))).toBe(false);
});

it("re-renders when a hover/highlight Set identity changes", () => {
const base = data();
const hovered = data({ highlightAttrs: new Set(["a"]) });
expect(entityNodePropsEqual(props(base), props(hovered))).toBe(false);
});
});
34 changes: 33 additions & 1 deletion canvas/src/EntityNode.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,4 +175,36 @@ export const EntityNode = memo(function EntityNode({ data, selected }: NodeProps
</div>
</div>
);
});
}, entityNodePropsEqual);

/** Re-render the card exactly when a field it actually renders changes. A bare `memo`
* default-compares the whole `data` object by reference, which is fragile here: the
* derivation hands React Flow a fresh `data` object on every model refresh, yet the
* subject-area accent could still paint stale (a membership change only moves `color`).
* Comparing the render-affecting fields explicitly guarantees the accent — and the
* hover/search/expand state — repaint when, and only when, they change. `color` is the
* one this fixes: a node moved into or out of a subject area recolours in place without
* any reflow. Sets are compared by identity (the derivation makes a new Set on a real
* change), which is what the lightweight hover effect already relies on. */
export function entityNodePropsEqual(
a: NodeProps<EntityNodeData>,
b: NodeProps<EntityNodeData>,
): boolean {
if (a.selected !== b.selected) return false;
const x = a.data;
const y = b.data;
return (
x.entity === y.entity &&
x.color === y.color &&
x.dimmed === y.dimmed &&
x.highlighted === y.highlighted &&
x.showTypes === y.showTypes &&
x.collapseDetail === y.collapseDetail &&
x.expanded === y.expanded &&
x.endpointAttrs === y.endpointAttrs &&
x.highlightAttrs === y.highlightAttrs &&
x.dimAttrs === y.dimAttrs &&
x.onToggleExpand === y.onToggleExpand &&
x.onHoverAttr === y.onHoverAttr
);
}
20 changes: 17 additions & 3 deletions canvas/src/ModelCanvas.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -484,13 +484,27 @@ export function ModelCanvas({
const known = newNodes.every((n) => posById.has(n.id));
// When a subject-area filter is active, lay out ONLY the visible nodes so the focused
// domain fills the canvas (a stale full-model layout would leave it in a far corner).
// Otherwise keep known positions so a re-derivation (search, selection) doesn't reflow.
const filtering = (activeAreas?.size ?? 0) > 0;
if (layoutedRef.current && known && prev.length >= newNodes.length && !filtering) {
// That re-layout belongs to a VIEW CHANGE (ticking a filter) — which resets
// layoutedRef to false via the view-load effect — NOT to every re-derivation. Once a
// view is laid out (layoutedRef true), an incidental re-run (search, SELECTING or
// closing the Inspector, a recolour) must keep positions and must not re-fit, filter
// active or not. Keying only on layoutedRef (not `filtering`) fixes the bug where
// closing the Inspector while filtered zoomed out and reset in-filter manual drags:
// every entity is always present in newNodes (hidden ones flagged, never removed), so
// `known` and the length check hold across a filter switch too — layoutedRef is the
// only thing that distinguishes "view changed" from "selection changed".
if (layoutedRef.current && known && prev.length >= newNodes.length) {
// The entity SET is unchanged (same ids, no growth), so keep every saved/manual
// position — a recolour/selection must never reflow a laid-out diagram (and must not
// fight the saved-layout restore). newNodes already carries the fresh subject-area
// colour and hidden flags; we only swap in the kept positions. React Flow then
// re-renders each node whose `data` reference changed, and EntityNode's memo
// comparator (which includes `color`) makes the accent repaint.
const kept = newNodes.map((n) => ({ ...n, position: posById.get(n.id) ?? n.position }));
emitEdges(kept);
return kept;
}
const filtering = (activeAreas?.size ?? 0) > 0;
const toLay = filtering ? newNodes.filter((n) => !n.hidden) : newNodes;
const laid = layoutGraph(toLay, newEdges, entityIndex, "auto", areaByEntity);
const laidPos = new Map(laid.map((n) => [n.id, n.position]));
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading