feat: render edges between minimized groups with floating edges

Implements proper edge rendering when both source and target actors are
in minimized groups. Edges are now correctly rerouted to connect the
minimized group nodes with dynamic floating edge calculations.

Key changes:
- Add edge rerouting logic to redirect edges from hidden actors to their
  parent minimized groups
- Implement edge deduplication to prevent multiple edges between same
  groups when multiple actors have connections
- Fix floating edge calculations with fallback dimensions to prevent NaN
- Use canonical parentId property instead of actorIds array for accuracy
- Create constants.ts for MINIMIZED_GROUP_WIDTH/HEIGHT (220x80)
- Replace magic numbers throughout codebase with named constants
- Remove sourceHandle/targetHandle properties when routing to groups to
  avoid React Flow validation errors

Technical details:
- GraphEditor: Build actor-to-group mapping using parentId, deduplicate
  edges with Map keyed by source_target, strip handle properties
- CustomEdge: Use floating edge params for minimized groups on both ends
- edgeUtils: Add fallback dimensions and division-by-zero guards in
  getNodeIntersection and getEdgePosition
- graphStore: Use constants for minimized group dimensions

Edges between minimized groups now render with proper Bezier curves and
dynamic connection points that adapt to group positions.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Jan-Henrik Bruhn 2025-10-21 11:28:48 +02:00
parent 3b7497ec99
commit 7c49ad0baa
5 changed files with 131 additions and 60 deletions

View file

@ -53,7 +53,8 @@ const CustomEdge = ({
const targetIsMinimizedGroup = targetNode?.type === 'group' && (targetNode.data as Group['data']).minimized; const targetIsMinimizedGroup = targetNode?.type === 'group' && (targetNode.data as Group['data']).minimized;
// Calculate floating edge parameters if needed // Calculate floating edge parameters if needed
// Only float the side(s) that connect to minimized groups // When connecting to groups (especially minimized ones), we need to use floating edges
// because groups don't have specific handles
let finalSourceX = sourceX; let finalSourceX = sourceX;
let finalSourceY = sourceY; let finalSourceY = sourceY;
let finalTargetX = targetX; let finalTargetX = targetX;
@ -61,10 +62,14 @@ const CustomEdge = ({
let finalSourcePosition = sourcePosition; let finalSourcePosition = sourcePosition;
let finalTargetPosition = targetPosition; let finalTargetPosition = targetPosition;
if ((sourceIsMinimizedGroup || targetIsMinimizedGroup) && sourceNode && targetNode) { // Check if we need to use floating edge calculations
const needsFloatingEdge = (sourceIsMinimizedGroup || targetIsMinimizedGroup) && sourceNode && targetNode;
if (needsFloatingEdge) {
const floatingParams = getFloatingEdgeParams(sourceNode, targetNode); const floatingParams = getFloatingEdgeParams(sourceNode, targetNode);
// Only use floating position for the minimized group side(s) // When either endpoint is a minimized group, use floating positions for that side
// IMPORTANT: When BOTH are groups, we must use floating for BOTH sides
if (sourceIsMinimizedGroup) { if (sourceIsMinimizedGroup) {
finalSourceX = floatingParams.sx; finalSourceX = floatingParams.sx;
finalSourceY = floatingParams.sy; finalSourceY = floatingParams.sy;

View file

@ -170,20 +170,29 @@ const GraphEditor = ({ onNodeSelect, onEdgeSelect, onGroupSelect, onMultiSelect,
// Reroute edges to minimized groups and filter internal edges // Reroute edges to minimized groups and filter internal edges
const visibleEdges = useMemo(() => { const visibleEdges = useMemo(() => {
// Build a map of actor -> group for actors in minimized groups // Build a map of actor -> group by examining each actor's parentId
// This is the canonical source of truth in React Flow
const actorToMinimizedGroup = new Map<string, string>(); const actorToMinimizedGroup = new Map<string, string>();
storeGroups.forEach((group) => {
if (group.data.minimized) { // Get set of minimized group IDs
group.data.actorIds.forEach((actorId) => { const minimizedGroupIds = new Set(
actorToMinimizedGroup.set(actorId, group.id); storeGroups.filter((group) => group.data.minimized).map((group) => group.id)
}); );
// Map each actor to its parent group (if the group is minimized)
storeNodes.forEach((node) => {
const nodeWithParent = node as Actor & { parentId?: string };
if (nodeWithParent.parentId && minimizedGroupIds.has(nodeWithParent.parentId)) {
actorToMinimizedGroup.set(node.id, nodeWithParent.parentId);
} }
}); });
// Map to deduplicate edges between groups: "source_target" -> edge
const edgeMap = new Map<string, Edge>();
// Reroute edges: if source or target is in a minimized group, redirect to the group // Reroute edges: if source or target is in a minimized group, redirect to the group
// Filter out edges that are internal to a minimized group (both source and target in same group) // Filter out edges that are internal to a minimized group (both source and target in same group)
return (storeEdges as Edge[]) (storeEdges as Edge[]).forEach((edge) => {
.map((edge) => {
const newSource = actorToMinimizedGroup.get(edge.source) || edge.source; const newSource = actorToMinimizedGroup.get(edge.source) || edge.source;
const newTarget = actorToMinimizedGroup.get(edge.target) || edge.target; const newTarget = actorToMinimizedGroup.get(edge.target) || edge.target;
@ -193,35 +202,53 @@ const GraphEditor = ({ onNodeSelect, onEdgeSelect, onGroupSelect, onMultiSelect,
// Filter: if both source and target are rerouted to the SAME group, hide this edge // Filter: if both source and target are rerouted to the SAME group, hide this edge
// (it's an internal edge within a minimized group) // (it's an internal edge within a minimized group)
if (sourceChanged && targetChanged && newSource === newTarget) { if (sourceChanged && targetChanged && newSource === newTarget) {
return null; // Mark for filtering return; // Skip this edge
} }
// Create edge key for deduplication
const edgeKey = `${newSource}_${newTarget}`;
// Only update if source or target changed // Only update if source or target changed
if (sourceChanged || targetChanged) { if (sourceChanged || targetChanged) {
// Destructure to separate handle properties from the rest // Destructure to separate handle properties from the rest
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const { sourceHandle, targetHandle, ...edgeWithoutHandles } = edge; const { sourceHandle, targetHandle, ...edgeWithoutHandles } = edge;
// Create new edge object, omitting handle properties when rerouting to groups // Create new edge object with updated source/target
const newEdge: Edge = { const newEdge: Edge = {
...edgeWithoutHandles, ...edgeWithoutHandles,
source: newSource, source: newSource,
target: newTarget, target: newTarget,
}; };
// Only include handle IDs if not rerouted to a group // Explicitly delete handle properties to ensure they don't exist
if (!sourceChanged && sourceHandle) { // This is critical - React Flow will error if handles are present but invalid
delete (newEdge as Record<string, unknown>).sourceHandle;
delete (newEdge as Record<string, unknown>).targetHandle;
// Only re-add handle IDs if the endpoint was NOT rerouted to a group
if (!sourceChanged && sourceHandle !== undefined && sourceHandle !== null) {
newEdge.sourceHandle = sourceHandle; newEdge.sourceHandle = sourceHandle;
} }
if (!targetChanged && targetHandle) { if (!targetChanged && targetHandle !== undefined && targetHandle !== null) {
newEdge.targetHandle = targetHandle; newEdge.targetHandle = targetHandle;
} }
return newEdge; // If we already have an edge between these nodes, keep the first one
// (you could also merge labels or aggregate data here if needed)
if (!edgeMap.has(edgeKey)) {
edgeMap.set(edgeKey, newEdge);
} }
return edge; } else {
}) // No rerouting needed, just add the edge
.filter((edge): edge is Edge => edge !== null); // Remove null entries if (!edgeMap.has(edgeKey)) {
}, [storeEdges, storeGroups]); edgeMap.set(edgeKey, edge);
}
}
});
return Array.from(edgeMap.values());
}, [storeEdges, storeGroups, storeNodes]);
const [edges, setEdgesState, onEdgesChange] = useEdgesState( const [edges, setEdgesState, onEdgesChange] = useEdgesState(
visibleEdges, visibleEdges,
@ -284,10 +311,17 @@ const GraphEditor = ({ onNodeSelect, onEdgeSelect, onGroupSelect, onMultiSelect,
if (hasPendingSelection) { if (hasPendingSelection) {
const pendingEdgeId = pendingType === 'edge' ? pendingId : null; const pendingEdgeId = pendingType === 'edge' ? pendingId : null;
const newEdges = visibleEdges.map((edge) => ({ const newEdges = visibleEdges.map((edge) => {
...edge, // eslint-disable-next-line @typescript-eslint/no-unused-vars
const { sourceHandle, targetHandle, ...edgeWithoutHandles } = edge;
return {
...edgeWithoutHandles,
selected: edge.id === pendingEdgeId, selected: edge.id === pendingEdgeId,
})); // Only include handles if they exist and are not null
...(sourceHandle !== undefined && sourceHandle !== null ? { sourceHandle } : {}),
...(targetHandle !== undefined && targetHandle !== null ? { targetHandle } : {}),
};
});
// Clear pending selection after applying it to both nodes and edges // Clear pending selection after applying it to both nodes and edges
pendingSelectionRef.current = null; pendingSelectionRef.current = null;
@ -300,10 +334,17 @@ const GraphEditor = ({ onNodeSelect, onEdgeSelect, onGroupSelect, onMultiSelect,
currentEdges.map((edge) => [edge.id, edge.selected]) currentEdges.map((edge) => [edge.id, edge.selected])
); );
return visibleEdges.map((edge) => ({ return visibleEdges.map((edge) => {
...edge, // eslint-disable-next-line @typescript-eslint/no-unused-vars
const { sourceHandle, targetHandle, ...edgeWithoutHandles } = edge;
return {
...edgeWithoutHandles,
selected: selectionMap.get(edge.id) || false, selected: selectionMap.get(edge.id) || false,
})); // Only include handles if they exist and are not null
...(sourceHandle !== undefined && sourceHandle !== null ? { sourceHandle } : {}),
...(targetHandle !== undefined && targetHandle !== null ? { targetHandle } : {}),
};
});
}); });
}, [allNodes, visibleEdges, setNodesState, setEdgesState]); }, [allNodes, visibleEdges, setNodesState, setEdgesState]);

15
src/constants.ts Normal file
View file

@ -0,0 +1,15 @@
/**
* Application-wide constants
*/
/**
* Minimized group dimensions
*/
export const MINIMIZED_GROUP_WIDTH = 220;
export const MINIMIZED_GROUP_HEIGHT = 80;
/**
* Default actor/node dimensions (approximate)
*/
export const DEFAULT_ACTOR_WIDTH = 150;
export const DEFAULT_ACTOR_HEIGHT = 80;

View file

@ -11,6 +11,7 @@ import type {
GroupData, GroupData,
GraphActions GraphActions
} from '../types'; } from '../types';
import { MINIMIZED_GROUP_WIDTH, MINIMIZED_GROUP_HEIGHT } from '../constants';
/** /**
* IMPORTANT: DO NOT USE THIS STORE DIRECTLY IN COMPONENTS * IMPORTANT: DO NOT USE THIS STORE DIRECTLY IN COMPONENTS
@ -443,8 +444,8 @@ export const useGraphStore = create<GraphStore & GraphActions>((set) => ({
}, },
style: { style: {
...g.style, ...g.style,
width: 220, width: MINIMIZED_GROUP_WIDTH,
height: 80, height: MINIMIZED_GROUP_HEIGHT,
}, },
}; };
} else { } else {

View file

@ -1,6 +1,7 @@
import type { Relation, RelationData } from '../types'; import type { Relation, RelationData } from '../types';
import type { Node } from '@xyflow/react'; import type { Node } from '@xyflow/react';
import { Position } from '@xyflow/react'; import { Position } from '@xyflow/react';
import { MINIMIZED_GROUP_WIDTH, MINIMIZED_GROUP_HEIGHT } from '../constants';
/** /**
* Generates a unique ID for edges * Generates a unique ID for edges
@ -21,13 +22,20 @@ function getNodeIntersection(intersectionNode: Node, targetNode: Node) {
} = intersectionNode; } = intersectionNode;
const targetPosition = targetNode.position; const targetPosition = targetNode.position;
const w = (intersectionNodeWidth ?? 0) / 2; // Use fallback dimensions if width/height are not set (e.g., for groups without measured dimensions)
const h = (intersectionNodeHeight ?? 0) / 2; const w = (intersectionNodeWidth ?? MINIMIZED_GROUP_WIDTH) / 2;
const h = (intersectionNodeHeight ?? MINIMIZED_GROUP_HEIGHT) / 2;
const x2 = intersectionNodePosition.x + w; const x2 = intersectionNodePosition.x + w;
const y2 = intersectionNodePosition.y + h; const y2 = intersectionNodePosition.y + h;
const x1 = targetPosition.x + (targetNode.width ?? 0) / 2; const x1 = targetPosition.x + (targetNode.width ?? MINIMIZED_GROUP_WIDTH) / 2;
const y1 = targetPosition.y + (targetNode.height ?? 0) / 2; const y1 = targetPosition.y + (targetNode.height ?? MINIMIZED_GROUP_HEIGHT) / 2;
// Guard against division by zero
if (w === 0 || h === 0) {
// If node has no dimensions, return center point
return { x: x2, y: y2 };
}
const xx1 = (x1 - x2) / (2 * w) - (y1 - y2) / (2 * h); const xx1 = (x1 - x2) / (2 * w) - (y1 - y2) / (2 * h);
const yy1 = (x1 - x2) / (2 * w) + (y1 - y2) / (2 * h); const yy1 = (x1 - x2) / (2 * w) + (y1 - y2) / (2 * h);
@ -50,8 +58,9 @@ function getEdgePosition(node: Node, intersectionPoint: { x: number; y: number }
const px = Math.round(intersectionPoint.x); const px = Math.round(intersectionPoint.x);
const py = Math.round(intersectionPoint.y); const py = Math.round(intersectionPoint.y);
const width = node.width ?? 0; // Use fallback dimensions if not set (same as getNodeIntersection)
const height = node.height ?? 0; const width = node.width ?? MINIMIZED_GROUP_WIDTH;
const height = node.height ?? MINIMIZED_GROUP_HEIGHT;
if (px <= nx + 1) { if (px <= nx + 1) {
return Position.Left; return Position.Left;