Skip to content

Commit e14e0c9

Browse files
Copilotjongalloway
andcommitted
fix: address review feedback - per-message Y stacking, deterministic ordering, clean test comment
Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
1 parent 24d641c commit e14e0c9

6 files changed

Lines changed: 92 additions & 10 deletions

File tree

src/DiagramForge/Layout/DefaultLayoutEngine.cs

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public void Layout(Diagram diagram, Theme theme)
5858

5959
if (string.Equals(diagram.DiagramType, "sequencediagram", StringComparison.OrdinalIgnoreCase))
6060
{
61-
LayoutSequenceDiagram(diagram, theme, minW, nodeH, hGap, pad);
61+
LayoutSequenceDiagram(diagram, theme, minW, nodeH, hGap, vGap, pad);
6262
return;
6363
}
6464

@@ -225,6 +225,7 @@ private static void LayoutSequenceDiagram(
225225
double minW,
226226
double nodeH,
227227
double hGap,
228+
double vGap,
228229
double pad)
229230
{
230231
// Size each participant node from its label.
@@ -237,10 +238,13 @@ private static void LayoutSequenceDiagram(
237238
}
238239

239240
// Order participants by their declared index (stored during parsing).
241+
// ThenBy ensures deterministic output when the index is missing or two
242+
// participants share the same value (e.g., programmatically-built diagrams).
240243
var ordered = diagram.Nodes.Values
241244
.OrderBy(n => n.Metadata.TryGetValue("sequence:participantIndex", out var v)
242245
? Convert.ToInt32(v, System.Globalization.CultureInfo.InvariantCulture)
243246
: int.MaxValue)
247+
.ThenBy(n => n.Id, StringComparer.Ordinal)
244248
.ToList();
245249

246250
// Place participants in a single row across the top of the diagram.
@@ -251,6 +255,25 @@ private static void LayoutSequenceDiagram(
251255
node.Y = pad;
252256
runX += node.Width + hGap;
253257
}
258+
259+
// Assign each message edge its own Y row below the participant strip.
260+
// Each row is vGap tall, giving space for the arrow line and any label above it.
261+
double firstMessageY = pad + nodeH + vGap / 2;
262+
double messageRowHeight = vGap;
263+
264+
foreach (var edge in diagram.Edges)
265+
{
266+
int msgIdx = edge.Metadata.TryGetValue("sequence:messageIndex", out var idxObj)
267+
? Convert.ToInt32(idxObj, System.Globalization.CultureInfo.InvariantCulture)
268+
: 0;
269+
edge.Metadata["sequence:messageY"] = firstMessageY + msgIdx * messageRowHeight;
270+
}
271+
272+
// Store the canvas height needed to fit all message rows so the renderer
273+
// can size the SVG correctly (node Y extents alone would clip the messages).
274+
int edgeCount = diagram.Edges.Count;
275+
double canvasHeight = firstMessageY + Math.Max(0, edgeCount) * messageRowHeight + pad;
276+
diagram.Metadata["sequence:canvasHeight"] = canvasHeight;
254277
}
255278

256279
private static void LayoutBlockDiagram(

src/DiagramForge/Models/Edge.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,9 @@ public Edge(string sourceId, string targetId)
3131

3232
/// <summary>Override stroke color (null = inherit from theme).</summary>
3333
public string? Color { get; set; }
34+
35+
/// <summary>Arbitrary parser-specific metadata shared with layout and rendering.</summary>
36+
public Dictionary<string, object> Metadata { get; } = new();
3437
}
3538

3639
public enum EdgeLineStyle

src/DiagramForge/Parsers/Mermaid/MermaidSequenceParser.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public Diagram Parse(MermaidDocument document)
2626

2727
var participants = new Dictionary<string, Node>(StringComparer.Ordinal);
2828
int participantIndex = 0;
29+
int messageIndex = 0;
2930

3031
Node GetOrCreateParticipant(string id)
3132
{
@@ -82,6 +83,7 @@ Node GetOrCreateParticipant(string id)
8283
LineStyle = lineStyle,
8384
ArrowHead = arrowHead,
8485
};
86+
edge.Metadata["sequence:messageIndex"] = messageIndex++;
8587

8688
if (!string.IsNullOrEmpty(msgLabel))
8789
edge.Label = new Label(msgLabel!);

src/DiagramForge/Rendering/SvgRenderer.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,21 @@ private static void AppendEdge(StringBuilder sb, Edge edge, Node source, Node ta
193193
cp2 = $"{F(x2)},{F(y2 - (dy >= 0 ? cpOffset : -cpOffset))}";
194194
}
195195

196+
// Per-message Y override: sequence diagrams store an explicit Y position for each
197+
// message arrow so that multiple messages between the same participants are stacked
198+
// vertically rather than all drawn on top of each other at the node center.
199+
if (edge.Metadata.TryGetValue("sequence:messageY", out var msgYObj))
200+
{
201+
double msgY = Convert.ToDouble(msgYObj, System.Globalization.CultureInfo.InvariantCulture);
202+
x1 = sourceCenterX;
203+
y1 = msgY;
204+
x2 = targetCenterX;
205+
y2 = msgY;
206+
cpOffset = Math.Abs(x2 - x1) * 0.4;
207+
cp1 = $"{F(x1 + (x2 >= x1 ? cpOffset : -cpOffset))},{F(y1)}";
208+
cp2 = $"{F(x2 - (x2 >= x1 ? cpOffset : -cpOffset))},{F(y2)}";
209+
}
210+
196211
string strokeColor = Escape(edge.Color ?? theme.EdgeColor);
197212
string strokeDash = edge.LineStyle == EdgeLineStyle.Dashed ? """ stroke-dasharray="6,3" """ :
198213
edge.LineStyle == EdgeLineStyle.Dotted ? """ stroke-dasharray="2,3" """ : " ";
@@ -257,6 +272,11 @@ private static double ComputeHeight(Diagram diagram, Theme theme)
257272
if (diagram.Nodes.Count == 0)
258273
return 100;
259274

275+
// Sequence diagrams extend below the participant nodes with per-message rows;
276+
// the layout engine stores the required height so node extents don't clip messages.
277+
if (diagram.Metadata.TryGetValue("sequence:canvasHeight", out var seqH))
278+
return Convert.ToDouble(seqH, System.Globalization.CultureInfo.InvariantCulture);
279+
260280
double maxY = diagram.Nodes.Values.Max(n => n.Y + n.Height);
261281
if (diagram.Groups.Count > 0)
262282
maxY = Math.Max(maxY, diagram.Groups.Max(g => g.Y + g.Height));

tests/DiagramForge.E2ETests/Fixtures/mermaid-sequence.expected.svg

Lines changed: 8 additions & 8 deletions
Loading

tests/DiagramForge.Tests/Parsers/Mermaid/MermaidSequenceParserTests.cs

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ public void Parse_ArrowOperator_MapsToCorrectStyles(
113113
[Fact]
114114
public void Parse_LongerOperatorPreferred_DashedArrow()
115115
{
116-
// "-->>" must win over "-->" on the same position
116+
// "-->>" must win over "-->" on the same position
117117
var diagram = _parser.Parse("sequenceDiagram\n A-->>B: hi");
118118

119119
var edge = Assert.Single(diagram.Edges);
@@ -188,4 +188,38 @@ public void Parse_LayoutDirection_IsLeftToRight()
188188

189189
Assert.Equal(LayoutDirection.LeftToRight, diagram.LayoutHints.Direction);
190190
}
191+
192+
// ── Message index metadata ────────────────────────────────────────────────
193+
194+
[Fact]
195+
public void Parse_MessageIndex_StoredOnEdge()
196+
{
197+
var diagram = _parser.Parse("sequenceDiagram\n A->>B: First");
198+
199+
var edge = Assert.Single(diagram.Edges);
200+
Assert.True(edge.Metadata.ContainsKey("sequence:messageIndex"));
201+
Assert.Equal(0, Convert.ToInt32(edge.Metadata["sequence:messageIndex"],
202+
System.Globalization.CultureInfo.InvariantCulture));
203+
}
204+
205+
[Fact]
206+
public void Parse_MultipleMessages_IndexesAreSequential()
207+
{
208+
const string text = """
209+
sequenceDiagram
210+
A->>B: First
211+
B-->>A: Second
212+
A->>B: Third
213+
""";
214+
215+
var diagram = _parser.Parse(text);
216+
217+
Assert.Equal(3, diagram.Edges.Count);
218+
for (int i = 0; i < diagram.Edges.Count; i++)
219+
{
220+
int idx = Convert.ToInt32(diagram.Edges[i].Metadata["sequence:messageIndex"],
221+
System.Globalization.CultureInfo.InvariantCulture);
222+
Assert.Equal(i, idx);
223+
}
224+
}
191225
}

0 commit comments

Comments
 (0)