fix: flowchart rendering issues (#61)

* feat(utils): enhance computeEdgePositions to maintain direction points This commit modifies the computeEdgePositions function in utils.ts to keep points that are on a straight line in the same direction. The filter condition is updated to only remove points that have both the same x and y coordinates as the previous point. This ensures that points on a straight line in the same direction are maintained. * fix(flowchart): handle cases when not all edges are retrieved in flowchart.ts * chore: add #45 issue testcase * add function closure for processed edges instead of passing as param * working in solution * possible limit * test: add testcases for remove second last point * fix: add a map to count repeated edges * tweaks and improvements * fix tests --------- Co-authored-by: Aakansha Doshi <[email protected]>
excalidraw · May 16, 2024 · d541e54 · d541e54
1 parent 602a6f2
commit d541e54
Show file tree

Hide file tree

Showing 4 changed files with 143 additions and 13 deletions.
diff --git a/playground/testcases/flowchart.ts b/playground/testcases/flowchart.ts
@@ -465,6 +465,61 @@ C -->|Two| E[Result two]
   AA[Loop] --> AG[Subprocess]
   AG[Subprocess] --> J[Task 1]
   AG[Subprocess] --> B[Initialize]
+`,
+    type: "flowchart",
+  },
+  {
+    name: "Multiple Edges, Relations to a Single Entity",
+    definition: `flowchart LR
+    style Entity1 fill: gold, stroke:#333, stroke-width:4px
+    
+    Entity1[Entity 1]
+    Entity2[Entity 2 fa:fa-suitcase]
+    Entity3[Entity 3 fa:fa-suitcase]
+    Entity4[Entity 4]
+    Entity5[Entity 5]
+    Entity6[Entity 6<br><sub>Entity6</sub>]
+    Entity7[Entity 7]
+    
+    Entity2 -..->|Relation1| Entity1
+    Entity3 -..->|Relation2| Entity1
+    Entity4 -..->|Relation3| Entity1
+    Entity3 -..->|Relation4| Entity1
+    Entity5 -..->|Relation5| Entity1
+    Entity5 -..->|Relation6| Entity1
+    Entity6 -..->|Relation7| Entity1
+    Entity7 -..->|Relation8| Entity1
+    
+    Entity8[Entity 8<br><sub>Entity8</sub>]
+    Entity1[Entity 1]
+    Entity9[Entity 9<br><sub>Entity9</sub>]
+    Entity10[Entity 10]
+    Entity4[Entity 4]
+    Entity11[Entity 11]
+    Entity12[Entity 12]
+    Entity13[Entity 13<br><sub>Entity13</sub>]
+    Entity14[Entity 14]
+    Entity15[Entity 15]
+    Entity16[Entity 16 fa:fa-suitcase]
+    Entity17[Entity 17 fa:fa-suitcase]
+    Entity18[Entity 18 fa:fa-suitcase]
+    Entity19[Entity 19 fa:fa-suitcase]
+    
+    Entity1 -..->|Relation9| Entity8
+    Entity1 -..->|Relation10| Entity9
+    Entity1 -..->|Relation11| Entity10
+    Entity1 -..->|Relation12| Entity4
+    Entity1 ===>|fa:fa-link Relation13| Entity11
+    Entity1 -..->|Relation14| Entity12
+    Entity1 -..->|Relation15| Entity13
+    Entity1 -..->|Relation16| Entity14
+    Entity1 ===>|fa:fa-link Relation17| Entity15
+    Entity1 -..->|Relation18| Entity16
+    Entity1 -..->|Relation19| Entity17
+    Entity1 -..->|Relation20| Entity17
+    Entity1 -..->|Relation21| Entity17
+    Entity1 -..->|Relation22| Entity18
+    Entity1 -..->|Relation23| Entity19
 `,
     type: "flowchart",
   },

diff --git a/src/parser/flowchart.ts b/src/parser/flowchart.ts
@@ -141,18 +141,23 @@ const parseVertex = (data: any, containerEl: Element): Vertex | undefined => {
   };
 };
 
-const parseEdge = (data: any, containerEl: Element): Edge => {
+const parseEdge = (
+  data: any,
+  edgeIndex: number,
+  containerEl: Element
+): Edge => {
   // Find edge element
-  const el: SVGPathElement | null = containerEl.querySelector(
-    `[id*="L-${data.start}-${data.end}"]`
+  const edge = containerEl.querySelector<SVGPathElement>(
+    `[id*="L-${data.start}-${data.end}-${edgeIndex}"]`
   );
-  if (!el) {
+
+  if (!edge) {
     throw new Error("Edge element not found");
   }
 
   // Compute edge position data
-  const position = computeElementPosition(el, containerEl);
-  const edgePositionData = computeEdgePositions(el, position);
+  const position = computeElementPosition(edge, containerEl);
+  const edgePositionData = computeEdgePositions(edge, position);
 
   // Remove irrelevant properties
   data.length = undefined;
@@ -228,13 +233,23 @@ export const parseMermaidFlowChartDiagram = (
   Object.keys(vertices).forEach((id) => {
     vertices[id] = parseVertex(vertices[id], containerEl);
   });
+
+  // Track the count of edges based on the edge id
+  const edgeCountMap = new Map<string, number>();
   const edges = mermaidParser
     .getEdges()
     .filter((edge: any) => {
       // Sometimes mermaid parser returns edges which are not present in the DOM hence this is a safety check to only consider edges present in the DOM, issue - https://github.com/mermaid-js/mermaid/issues/5516
       return containerEl.querySelector(`[id*="L-${edge.start}-${edge.end}"]`);
     })
-    .map((edge: any) => parseEdge(edge, containerEl));
+    .map((data: any) => {
+      const edgeId = `${data.start}-${data.end}`;
+
+      const count = edgeCountMap.get(edgeId) || 0;
+      edgeCountMap.set(edgeId, count + 1);
+
+      return parseEdge(data, count, containerEl);
+    });
 
   const subGraphs = mermaidParser
     .getSubGraphs()

diff --git a/src/utils.ts b/src/utils.ts
@@ -113,14 +113,36 @@ export const computeEdgePositions = (
     })
     .filter((point, index, array) => {
       // Always include the last point
-      if (index === array.length - 1) {
+      if (index === 0 || index === array.length - 1) {
         return true;
       }
-      // Include the start point or if the current point if it's not the same as the previous point
-      const prevPoint = array[index - 1];
-      return (
-        index === 0 || (point.x !== prevPoint.x && point.y !== prevPoint.y)
-      );
+
+      // Exclude the points which are the same as the previous point
+      if (point.x === array[index - 1].x && point.y === array[index - 1].y) {
+        return false;
+      }
+
+      // The below check is exclusively for second last point
+      if (
+        index === array.length - 2 &&
+        (array[index - 1].x === point.x || array[index - 1].y === point.y)
+      ) {
+        const lastPoint = array[array.length - 1];
+
+        // Get the distance between the last point and second last point using Euclidean distance formula
+        const distance = Math.hypot(
+          lastPoint.x - point.x,
+          lastPoint.y - point.y
+        );
+        // Include the second last point if the distance between the
+        // last point and second last point is > 20.
+        // This is to ensure we have a distance for render the edge.
+        // 20 seems to be a good enough distance to render the edge
+        return distance > 20;
+      }
+
+      // Always include if the current point is not the same as the previous point
+      return point.x !== array[index - 1].x || point.y !== array[index - 1].y;
     })
     .map((p) => {
       // Offset the point by the provided offset
@@ -129,6 +151,7 @@ export const computeEdgePositions = (
         y: p.y + offset.y,
       };
     });
+
   // Return the edge positions
   return {
     startX: startPosition[0] + offset.x,

diff --git a/tests/utils.test.ts b/tests/utils.test.ts
@@ -70,5 +70,42 @@ describe("Test Utils", () => {
         { x: 29.383, y: 83.2 },
       ]);
     });
+
+    it("should include the second last point if the distance to the last point is greater than 20 and second last point is straight line", () => {
+      const commands = ["M29.383,38.5", "L29.383,83.2", "L90.383,83.2"];
+      pathElement.setAttribute("d", commands.join(""));
+
+      const result = computeEdgePositions(pathElement);
+
+      expect(result.reflectionPoints).toEqual([
+        { x: 29.383, y: 38.5 },
+        { x: 29.383, y: 83.2 },
+        { x: 90.383, y: 83.2 },
+      ]);
+    });
+
+    it("should exclude the second last point if the distance to the last point is less than 20 and second last point is straight line", () => {
+      const commands = ["M29.383,38.5", "L29.383,83.2", "L33.383,83.2"];
+      pathElement.setAttribute("d", commands.join(""));
+
+      const result = computeEdgePositions(pathElement);
+
+      expect(result.reflectionPoints).toEqual([
+        { x: 29.383, y: 38.5 },
+        { x: 33.383, y: 83.2 },
+      ]);
+    });
+
+    it("should filter out points that are the same as the previous point", () => {
+      const commands = ["M29.383,38.5", "L29.383,38.5", "L29.383,83.2"];
+      pathElement.setAttribute("d", commands.join(""));
+
+      const result = computeEdgePositions(pathElement);
+
+      expect(result.reflectionPoints).toEqual([
+        { x: 29.383, y: 38.5 },
+        { x: 29.383, y: 83.2 },
+      ]);
+    });
   });
 });