Skip to content

Commit 4ba4046

Browse files
Copilotjongalloway
andauthored
refactor: apply review feedback on sequence note layout and rendering
Agent-Logs-Url: https://github.com/jongalloway/DiagramForge/sessions/719e126c-3b9b-40be-b951-ae4d6347a477 Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
1 parent a8da432 commit 4ba4046

3 files changed

Lines changed: 74 additions & 22 deletions

File tree

src/DiagramForge/Layout/DefaultLayoutEngine.Sequence.cs

Lines changed: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,10 @@ private static void LayoutSequenceDiagram(
126126
else if (noteGroup != null)
127127
{
128128
noteGroup.Metadata["sequence:noteY"] = runY;
129-
runY += messageRowHeight;
129+
// Pre-compute note box height so multi-line notes don't overlap the
130+
// next row. Uses the same formula as LayoutSequenceNotes below.
131+
double preNoteHeight = ComputeNoteBoxHeight(noteGroup.Label.Text, theme);
132+
runY += Math.Max(messageRowHeight, preNoteHeight);
130133
}
131134
}
132135

@@ -161,7 +164,7 @@ private static void LayoutSequenceDiagram(
161164
double canvasWidth = maxNodeRight + extraRight + pad;
162165

163166
// Position note groups now that participant positions and sequence Y values are final.
164-
LayoutSequenceNotes(diagram, ordered, canvasWidth, messageRowHeight, theme, ref canvasWidth);
167+
LayoutSequenceNotes(diagram, messageRowHeight, theme, ref canvasWidth);
165168

166169
diagram.Metadata["sequence:canvasWidth"] = canvasWidth;
167170

@@ -212,22 +215,65 @@ private static void LayoutSequenceDiagram(
212215
}
213216
}
214217

218+
/// <summary>
219+
/// Returns the computed box height for a note with the given text using note font metrics.
220+
/// Shared by both the Y-assignment pre-pass and the <see cref="LayoutSequenceNotes"/> main pass
221+
/// to guarantee consistent values.
222+
/// </summary>
223+
private static double ComputeNoteBoxHeight(string noteText, Theme theme)
224+
{
225+
double noteFontSize = theme.FontSize * 0.85;
226+
double lineH = noteFontSize * 1.3;
227+
int lineCount = noteText.Split('\n').Length;
228+
return Math.Max(NoteMinHeight, lineCount * lineH + 2 * NoteVPad);
229+
}
230+
215231
/// <summary>
216232
/// Computes the position (X, Y, Width, Height) of each sequence note group and
217-
/// extends <paramref name="canvasWidth"/> when right-of or left-of notes fall
218-
/// outside the existing canvas bounds.
233+
/// adjusts participant positions / canvas width as needed when notes protrude
234+
/// outside the diagram bounds.
219235
/// </summary>
220236
private static void LayoutSequenceNotes(
221237
Diagram diagram,
222-
IReadOnlyList<Node> orderedParticipants,
223-
double currentCanvasWidth,
224238
double messageRowHeight,
225239
Theme theme,
226240
ref double canvasWidth)
227241
{
228242
double noteFontSize = theme.FontSize * 0.85;
229-
double lineHeight = noteFontSize * 1.3;
230243

244+
// --- Pre-pass: compute how much extra space leftOf notes need on the left -------
245+
double extraLeft = 0;
246+
foreach (var group in diagram.Groups)
247+
{
248+
if (!group.Metadata.TryGetValue("sequence:noteGroup", out var isNoteObj) || isNoteObj is not true)
249+
continue;
250+
if (!group.Metadata.TryGetValue("sequence:notePosition", out var posObj) || posObj is not string pos || pos != "leftOf")
251+
continue;
252+
if (!group.Metadata.TryGetValue("sequence:noteParticipant", out var p1Obj) || p1Obj is not string p1)
253+
continue;
254+
if (!diagram.Nodes.TryGetValue(p1, out var p1Node))
255+
continue;
256+
257+
string noteText = group.Label.Text;
258+
string[] noteLines = noteText.Split('\n');
259+
double textWidth = noteLines.Max(l => EstimateTextWidth(l, noteFontSize));
260+
double noteWidth = Math.Max(NoteMinWidth, textWidth + 2 * NoteHPad);
261+
262+
double lifelineX = p1Node.X + p1Node.Width / 2;
263+
double noteX = lifelineX - noteWidth - NoteLifelineGap;
264+
if (noteX < theme.DiagramPadding)
265+
extraLeft = Math.Max(extraLeft, theme.DiagramPadding - noteX);
266+
}
267+
268+
// Shift all participants right when any leftOf note would be clipped at the left edge.
269+
if (extraLeft > 0)
270+
{
271+
foreach (var node in diagram.Nodes.Values)
272+
node.X += extraLeft;
273+
canvasWidth += extraLeft;
274+
}
275+
276+
// --- Main pass: compute note box geometry ----------------------------------------
231277
foreach (var group in diagram.Groups)
232278
{
233279
if (!group.Metadata.TryGetValue("sequence:noteGroup", out var isNoteObj) || isNoteObj is not true)
@@ -244,17 +290,18 @@ private static void LayoutSequenceNotes(
244290
continue;
245291

246292
double noteY = Convert.ToDouble(noteYObj, System.Globalization.CultureInfo.InvariantCulture);
247-
string notePos = group.Metadata.TryGetValue("sequence:notePosition", out var posObj)
248-
&& posObj is string ps ? ps : "over";
293+
string notePos = group.Metadata.TryGetValue("sequence:notePosition", out var notePosObj)
294+
&& notePosObj is string ps ? ps : "over";
249295

250296
string noteText = group.Label.Text;
251297
string[] noteLines = noteText.Split('\n');
252298
double textWidth = noteLines.Max(l => EstimateTextWidth(l, noteFontSize));
253299
double noteWidth = Math.Max(NoteMinWidth, textWidth + 2 * NoteHPad);
254-
double noteHeight = Math.Max(NoteMinHeight, noteLines.Length * lineHeight + 2 * NoteVPad);
300+
double noteHeight = ComputeNoteBoxHeight(noteText, theme);
255301

256302
// Vertically center the note box within its sequence row.
257-
double vOffset = Math.Max(0, (messageRowHeight - noteHeight) / 2);
303+
double rowH = Math.Max(messageRowHeight, noteHeight);
304+
double vOffset = Math.Max(0, (rowH - noteHeight) / 2);
258305
double boxY = noteY + vOffset;
259306

260307
double noteX;
@@ -279,18 +326,18 @@ private static void LayoutSequenceNotes(
279326
case "over":
280327
default:
281328
{
329+
Node? p2Node = null;
282330
bool hasTwoParticipants =
283331
group.Metadata.TryGetValue("sequence:noteParticipant2", out var p2Obj)
284332
&& p2Obj is string p2
285333
&& !string.IsNullOrEmpty(p2)
286-
&& diagram.Nodes.TryGetValue(p2, out var p2Node)
334+
&& diagram.Nodes.TryGetValue(p2, out p2Node)
287335
&& p2Node is not null;
288336

289337
if (hasTwoParticipants)
290338
{
291-
diagram.Nodes.TryGetValue((string)group.Metadata["sequence:noteParticipant2"]!, out var p2NodeRef);
292-
double left = Math.Min(p1Node.X, p2NodeRef!.X);
293-
double right = Math.Max(p1Node.X + p1Node.Width, p2NodeRef.X + p2NodeRef.Width);
339+
double left = Math.Min(p1Node.X, p2Node!.X);
340+
double right = Math.Max(p1Node.X + p1Node.Width, p2Node.X + p2Node.Width);
294341
noteX = left;
295342
noteWidth = Math.Max(noteWidth, right - left);
296343
}

src/DiagramForge/Parsers/Mermaid/MermaidSequenceParser.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ Node GetOrCreateParticipant(string id)
6565

6666
if (isIndented && !IsSequenceKeyword(line))
6767
{
68-
pendingNoteGroup.Label = new Label(pendingNoteGroup.Label.Text + "\n" + line);
68+
pendingNoteGroup.Label.Text += "\n" + line;
6969
continue;
7070
}
7171

src/DiagramForge/Rendering/SvgStructureWriter.cs

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -698,13 +698,18 @@ private static double NormalizeAngle(double angle)
698698
/// </summary>
699699
internal static void AppendSequenceNotes(StringBuilder sb, Diagram diagram, Theme theme)
700700
{
701-
foreach (var group in diagram.Groups)
701+
var noteGroups = diagram.Groups
702+
.Where(g => g.Metadata.TryGetValue("sequence:noteGroup", out var v) && v is true)
703+
.OrderBy(g => g.Metadata.TryGetValue("sequence:noteSequenceIndex", out var idx)
704+
? Convert.ToInt32(idx, System.Globalization.CultureInfo.InvariantCulture) : 0)
705+
.ThenBy(g => g.Id, StringComparer.Ordinal)
706+
.ToList();
707+
708+
foreach (var group in noteGroups)
702709
{
703-
if (!group.Metadata.TryGetValue("sequence:noteGroup", out var isNoteObj) || isNoteObj is not true)
704-
continue;
705-
706-
// Skip groups that the layout engine did not position (e.g., missing participant).
707-
if (!group.Metadata.ContainsKey("sequence:noteY"))
710+
// Skip groups that the layout engine did not fully position/size
711+
// (e.g., missing participant).
712+
if (!group.Metadata.ContainsKey("sequence:noteY") || group.Width <= 0 || group.Height <= 0)
708713
continue;
709714

710715
AppendSequenceNote(sb, group, theme);

0 commit comments

Comments
 (0)