diff --git a/src/components/Edges/CustomEdge.tsx b/src/components/Edges/CustomEdge.tsx index 191c694..ddd884f 100644 --- a/src/components/Edges/CustomEdge.tsx +++ b/src/components/Edges/CustomEdge.tsx @@ -53,7 +53,8 @@ const CustomEdge = ({ const targetIsMinimizedGroup = targetNode?.type === 'group' && (targetNode.data as Group['data']).minimized; // 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 finalSourceY = sourceY; let finalTargetX = targetX; @@ -61,10 +62,14 @@ const CustomEdge = ({ let finalSourcePosition = sourcePosition; 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); - // 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) { finalSourceX = floatingParams.sx; finalSourceY = floatingParams.sy; diff --git a/src/components/Editor/GraphEditor.tsx b/src/components/Editor/GraphEditor.tsx index b5d7acc..1f2753f 100644 --- a/src/components/Editor/GraphEditor.tsx +++ b/src/components/Editor/GraphEditor.tsx @@ -170,58 +170,85 @@ const GraphEditor = ({ onNodeSelect, onEdgeSelect, onGroupSelect, onMultiSelect, // Reroute edges to minimized groups and filter internal edges 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(); - storeGroups.forEach((group) => { - if (group.data.minimized) { - group.data.actorIds.forEach((actorId) => { - actorToMinimizedGroup.set(actorId, group.id); - }); + + // Get set of minimized group IDs + const minimizedGroupIds = new Set( + 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(); + // 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) - return (storeEdges as Edge[]) - .map((edge) => { - const newSource = actorToMinimizedGroup.get(edge.source) || edge.source; - const newTarget = actorToMinimizedGroup.get(edge.target) || edge.target; + (storeEdges as Edge[]).forEach((edge) => { + const newSource = actorToMinimizedGroup.get(edge.source) || edge.source; + const newTarget = actorToMinimizedGroup.get(edge.target) || edge.target; - const sourceChanged = newSource !== edge.source; - const targetChanged = newTarget !== edge.target; + const sourceChanged = newSource !== edge.source; + const targetChanged = newTarget !== edge.target; - // Filter: if both source and target are rerouted to the SAME group, hide this edge - // (it's an internal edge within a minimized group) - if (sourceChanged && targetChanged && newSource === newTarget) { - return null; // Mark for filtering + // Filter: if both source and target are rerouted to the SAME group, hide this edge + // (it's an internal edge within a minimized group) + if (sourceChanged && targetChanged && newSource === newTarget) { + return; // Skip this edge + } + + // Create edge key for deduplication + const edgeKey = `${newSource}_${newTarget}`; + + // Only update if source or target changed + if (sourceChanged || targetChanged) { + // Destructure to separate handle properties from the rest + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { sourceHandle, targetHandle, ...edgeWithoutHandles } = edge; + + // Create new edge object with updated source/target + const newEdge: Edge = { + ...edgeWithoutHandles, + source: newSource, + target: newTarget, + }; + + // Explicitly delete handle properties to ensure they don't exist + // This is critical - React Flow will error if handles are present but invalid + delete (newEdge as Record).sourceHandle; + delete (newEdge as Record).targetHandle; + + // Only re-add handle IDs if the endpoint was NOT rerouted to a group + if (!sourceChanged && sourceHandle !== undefined && sourceHandle !== null) { + newEdge.sourceHandle = sourceHandle; + } + if (!targetChanged && targetHandle !== undefined && targetHandle !== null) { + newEdge.targetHandle = targetHandle; } - // Only update if source or target changed - if (sourceChanged || targetChanged) { - // Destructure to separate handle properties from the rest - const { sourceHandle, targetHandle, ...edgeWithoutHandles } = edge; - - // Create new edge object, omitting handle properties when rerouting to groups - const newEdge: Edge = { - ...edgeWithoutHandles, - source: newSource, - target: newTarget, - }; - - // Only include handle IDs if not rerouted to a group - if (!sourceChanged && sourceHandle) { - newEdge.sourceHandle = sourceHandle; - } - if (!targetChanged && 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; - }) - .filter((edge): edge is Edge => edge !== null); // Remove null entries - }, [storeEdges, storeGroups]); + } else { + // No rerouting needed, just add the edge + if (!edgeMap.has(edgeKey)) { + edgeMap.set(edgeKey, edge); + } + } + }); + + return Array.from(edgeMap.values()); + }, [storeEdges, storeGroups, storeNodes]); const [edges, setEdgesState, onEdgesChange] = useEdgesState( visibleEdges, @@ -284,10 +311,17 @@ const GraphEditor = ({ onNodeSelect, onEdgeSelect, onGroupSelect, onMultiSelect, if (hasPendingSelection) { const pendingEdgeId = pendingType === 'edge' ? pendingId : null; - const newEdges = visibleEdges.map((edge) => ({ - ...edge, - selected: edge.id === pendingEdgeId, - })); + const newEdges = visibleEdges.map((edge) => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { sourceHandle, targetHandle, ...edgeWithoutHandles } = edge; + return { + ...edgeWithoutHandles, + 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 pendingSelectionRef.current = null; @@ -300,10 +334,17 @@ const GraphEditor = ({ onNodeSelect, onEdgeSelect, onGroupSelect, onMultiSelect, currentEdges.map((edge) => [edge.id, edge.selected]) ); - return visibleEdges.map((edge) => ({ - ...edge, - selected: selectionMap.get(edge.id) || false, - })); + return visibleEdges.map((edge) => { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { sourceHandle, targetHandle, ...edgeWithoutHandles } = edge; + return { + ...edgeWithoutHandles, + 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]); diff --git a/src/constants.ts b/src/constants.ts new file mode 100644 index 0000000..a2b186b --- /dev/null +++ b/src/constants.ts @@ -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; diff --git a/src/stores/graphStore.ts b/src/stores/graphStore.ts index 9e7653f..e028bdd 100644 --- a/src/stores/graphStore.ts +++ b/src/stores/graphStore.ts @@ -11,6 +11,7 @@ import type { GroupData, GraphActions } from '../types'; +import { MINIMIZED_GROUP_WIDTH, MINIMIZED_GROUP_HEIGHT } from '../constants'; /** * ⚠️ IMPORTANT: DO NOT USE THIS STORE DIRECTLY IN COMPONENTS ⚠️ @@ -443,8 +444,8 @@ export const useGraphStore = create((set) => ({ }, style: { ...g.style, - width: 220, - height: 80, + width: MINIMIZED_GROUP_WIDTH, + height: MINIMIZED_GROUP_HEIGHT, }, }; } else { diff --git a/src/utils/edgeUtils.ts b/src/utils/edgeUtils.ts index c3fedba..1b19ca9 100644 --- a/src/utils/edgeUtils.ts +++ b/src/utils/edgeUtils.ts @@ -1,6 +1,7 @@ import type { Relation, RelationData } from '../types'; import type { Node } from '@xyflow/react'; import { Position } from '@xyflow/react'; +import { MINIMIZED_GROUP_WIDTH, MINIMIZED_GROUP_HEIGHT } from '../constants'; /** * Generates a unique ID for edges @@ -21,13 +22,20 @@ function getNodeIntersection(intersectionNode: Node, targetNode: Node) { } = intersectionNode; const targetPosition = targetNode.position; - const w = (intersectionNodeWidth ?? 0) / 2; - const h = (intersectionNodeHeight ?? 0) / 2; + // Use fallback dimensions if width/height are not set (e.g., for groups without measured dimensions) + const w = (intersectionNodeWidth ?? MINIMIZED_GROUP_WIDTH) / 2; + const h = (intersectionNodeHeight ?? MINIMIZED_GROUP_HEIGHT) / 2; const x2 = intersectionNodePosition.x + w; const y2 = intersectionNodePosition.y + h; - const x1 = targetPosition.x + (targetNode.width ?? 0) / 2; - const y1 = targetPosition.y + (targetNode.height ?? 0) / 2; + const x1 = targetPosition.x + (targetNode.width ?? MINIMIZED_GROUP_WIDTH) / 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 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 py = Math.round(intersectionPoint.y); - const width = node.width ?? 0; - const height = node.height ?? 0; + // Use fallback dimensions if not set (same as getNodeIntersection) + const width = node.width ?? MINIMIZED_GROUP_WIDTH; + const height = node.height ?? MINIMIZED_GROUP_HEIGHT; if (px <= nx + 1) { return Position.Left;