Merge pull request #226 from vitruv-tools/UML-issue

Refactor UML relationship rendering and merge point logic

Author: TsotneMikadze

src/components/flow/FlowCanvas.tsx

@@ -1841,56 +1841,77 @@ execute actions in ${targetPackageName}
     const calculateMergePoint = useCallback((avgSource: { x: number; y: number }, targetNode: Node) => {
       const targetCenterX = targetNode.position.x + NODE_DIMENSIONS.width / 2;
       const targetCenterY = targetNode.position.y + NODE_DIMENSIONS.height / 2;
+
+      // Place merge point vertically under/above the target center so
+      // the last segment into the class box is a straight line into
+      // the middle (no diagonal hit on the left/right side).
       return {
-        x: avgSource.x + (targetCenterX - avgSource.x) * 0.4,
-        y: avgSource.y + (targetCenterY - avgSource.y) * 0.4
+        x: targetCenterX,
+        y: avgSource.y + (targetCenterY - avgSource.y) * 0.4,
       };
     }, []);
 
-    // Calculate merge points for UML edges with same target
+    // Calculate merge points for UML inheritance edges with same target.
+    // We keep merge visualization ONLY for inheritance (like multiple
+    // subclasses pointing to the same superclass), not for other UML
+    // relationships. This gives a clean "fan-in" into the superclass
+    // while keeping compositions and associations as simple lines.
     const umlMergeData = useMemo(() => {
       const mergePointsMap = new Map<string, { x: number; y: number; mergeGroupId: string }>();
       const firstInGroupMap = new Map<string, string>();
       const mergeGroupSourceNodesMap = new Map<string, string[]>();
 
-      // Count UML edges per source node
+      // Consider only UML inheritance edges for merging
+      const umlInheritanceEdges = uniqueEdges.filter(
+        (e) => e.type === 'uml' && (e.data as any)?.relationshipType === 'inheritance'
+      );
+
+      if (umlInheritanceEdges.length === 0) {
+        return { mergePointsMap, firstInGroupMap, mergeGroupSourceNodesMap };
+      }
+
+      // Count inheritance edges per source node
       const edgesPerSource = new Map<string, number>();
-      uniqueEdges.filter(e => e.type === 'uml').forEach(edge => {
+      umlInheritanceEdges.forEach((edge) => {
         edgesPerSource.set(edge.source, (edgesPerSource.get(edge.source) || 0) + 1);
       });
 
-      // Group UML edges by target
+      // Group inheritance edges by target (superclass)
       const edgesByTarget = new Map<string, Edge[]>();
-      uniqueEdges.filter(e => e.type === 'uml').forEach(edge => {
+      umlInheritanceEdges.forEach((edge) => {
         const existing = edgesByTarget.get(edge.target) || [];
         existing.push(edge);
         edgesByTarget.set(edge.target, existing);
       });
 
-      // Process each target group
+      // For each superclass, create one merge point for eligible subclasses
       edgesByTarget.forEach((edgesGroup, targetId) => {
         if (edgesGroup.length < 2) return;
 
-        const eligibleEdges = edgesGroup.filter(edge => (edgesPerSource.get(edge.source) || 0) === 1);
+        // Only merge subclasses that connect to this superclass once
+        const eligibleEdges = edgesGroup.filter(
+          (edge) => (edgesPerSource.get(edge.source) || 0) === 1
+        );
         if (eligibleEdges.length < 2) return;
 
         eligibleEdges.sort((a, b) => a.source.localeCompare(b.source));
 
-        const targetNode = nodes.find(n => n.id === targetId);
+        const targetNode = nodes.find((n) => n.id === targetId);
         if (!targetNode) return;
 
         const avgSourcePos = calculateAverageSourcePosition(eligibleEdges);
         const mergePoint = calculateMergePoint(avgSourcePos, targetNode);
         const mergeGroupId = `merge-${targetId}`;
 
-        mergeGroupSourceNodesMap.set(mergeGroupId, eligibleEdges.map(e => e.source));
-        eligibleEdges.forEach((edge, index) => {
+        mergeGroupSourceNodesMap.set(
+          mergeGroupId,
+          eligibleEdges.map((e) => e.source)
+        );
+        eligibleEdges.forEach((edge) => {
           mergePointsMap.set(edge.id, { ...mergePoint, mergeGroupId });
-          console.log(`   Edge ${index}: ${edge.id.slice(-8)} (source: ${edge.source.slice(-6)})`);
         });
 
         firstInGroupMap.set(mergeGroupId, eligibleEdges[0].id);
-        console.log(`✨ Merge group ${mergeGroupId}: First edge = ${eligibleEdges[0].id.slice(-8)}`);
       });
 
       return { mergePointsMap, firstInGroupMap, mergeGroupSourceNodesMap };

src/components/flow/UMLRelationship.tsx

@@ -314,22 +314,21 @@ export function UMLRelationship({
   const isMergeGroupHovered = !!(mergeGroupId && data?.hoveredMergeGroup === mergeGroupId);
 
   // Logic for highlighting:
-  // 1. Individual edge (source to merge point) is red if: 
-  //    - edge selected OR source node selected OR target node selected OR merge group hovered
-  const isIndividualEdgeHighlighted: boolean = !!(selected || isSourceSelected || isTargetSelected || isMergeGroupHovered);
+  // 1. Individual edge (source to merge point) is red ONLY if:
+  //    - this edge is selected OR this edge is hovered.
+  //    Node selection alone no longer turns every incident edge red.
+  const isIndividualEdgeHighlighted: boolean = !!(selected || isHovered);
 
-  // 2. MERGE DOT LOGIC: Merge dot is red if ANY node that has a connection through it is selected
-  //    OR if the merge group is being hovered
-  //    This includes: current source, target, any other source node in the merge group, OR hover
-  const isMergeDotRed: boolean = data?.hasMerge 
-    ? !!(isSourceSelected || isTargetSelected || isMergeGroupNodeSelected || isMergeGroupHovered)
+  // 2. MERGE DOT LOGIC: Merge dot is red when any edge in the merge group
+  //    is highlighted (selected/hovered) OR when the merge group itself is hovered.
+  const isMergeDotRed: boolean = data?.hasMerge
+    ? !!(isIndividualEdgeHighlighted || isMergeGroupHovered)
     : isIndividualEdgeHighlighted;
 
-  // 3. SHARED SEGMENT LOGIC: Shared segment is red if merge dot is red
-  //    When merge dot is red, everything after it (to target) must be red
+  // 3. SHARED SEGMENT LOGIC: Shared segment is red only when merge dot is red.
   const isSharedSegmentHighlighted: boolean = isMergeDotRed;
 
-  // For the main edge path (source to merge), use individual edge highlighting
+  // For the main edge path (source to merge), use individual edge highlighting.
   const isHighlighted: boolean = isIndividualEdgeHighlighted;
 
   // Debug logging for merged edges
@@ -551,8 +550,16 @@ export function UMLRelationship({
           ...getRelationshipStyle(),
           strokeLinecap: 'butt',
           strokeLinejoin: 'miter',
-          // Remove markers from individual branches when merged
-          ...(hasMerge && { markerEnd: 'none', markerStart: 'none' })
+          // For merged edges we suppress markers only for relationships
+          // that use arrowheads at the target side (inheritance/realization/dependency).
+          // For compositions/aggregations we KEEP the diamond on the owning
+          // side (source class) to preserve correct UML semantics.
+          ...(hasMerge &&
+            data?.relationshipType !== 'composition' &&
+            data?.relationshipType !== 'aggregation' && {
+              markerEnd: 'none',
+              markerStart: 'none',
+            })
         }}
         d={edgePath}
         onClick={handleClick}
@@ -600,7 +607,19 @@ export function UMLRelationship({
             id={`${id}-shared`}
             d={sharedSegmentPath}
             style={{
-              ...getRelationshipStyle(isSharedSegmentHighlighted),
+              // On the shared segment we never want the composition/
+              // aggregation diamond to appear at the merge point – it
+              // must stay attached to the owning class at the start of
+              // each branch. Therefore we explicitly clear markerStart
+              // for these relationship types while still allowing
+              // arrowheads for inheritance/realization/dependency.
+              ...(data?.relationshipType === 'composition' ||
+              data?.relationshipType === 'aggregation'
+                ? {
+                    ...getRelationshipStyle(isSharedSegmentHighlighted),
+                    markerStart: 'none',
+                  }
+                : getRelationshipStyle(isSharedSegmentHighlighted)),
               strokeLinecap: 'butt',
               strokeLinejoin: 'miter',
             }}