Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,25 @@ describe('renderFlowOverlay', () => {
expect(svg).toContain('keyPoints="0;1"');
});

it('renders badges from the schema description field when summary is absent', () => {
const schemaFlow: CalmFlow = {
'unique-id': 'schema-flow',
name: 'Schema Flow',
description: 'Flow using only schema-defined fields',
transitions: [
{
'relationship-unique-id': 'rel-1',
'sequence-number': 1,
description: 'Client sends order request',
direction: 'source-to-destination',
},
],
};
const svg = renderFlowOverlay(schemaFlow, twoEdgeLayouts);
expect(svg).toContain('Client sends order request');
expect(svg).not.toContain('undefined');
});
Comment thread
rocketstack-matt marked this conversation as resolved.

it('Test 4: renders sequence badge with correct number for each transition', () => {
const svg = renderFlowOverlay(sampleFlow, twoEdgeLayouts);
// Sequence numbers 1 and 2 should appear in badge text elements
Expand Down Expand Up @@ -207,6 +226,69 @@ describe('renderFlowOverlay', () => {
expect(badgeCount).toBe(1);
});

it('spreads badges when multiple transitions share a relationship, and all remain visible', () => {
const sharedFlow: CalmFlow = {
'unique-id': 'shared-flow',
name: 'Shared',
description: 'Two transitions over one relationship',
transitions: [
{
'relationship-unique-id': 'rel-1',
'sequence-number': 1,
description: 'Request',
direction: 'source-to-destination',
},
{
'relationship-unique-id': 'rel-1',
'sequence-number': 2,
description: 'Response',
direction: 'destination-to-source',
},
],
};
const svg = renderFlowOverlay(sharedFlow, twoEdgeLayouts);
const badges = (svg.match(/class="flow-badge/g) ?? []).length;
expect(badges).toBe(2);
expect(svg).toContain('>1<');
expect(svg).toContain('>2<');
// Badge centers must not coincide: collect cx values of badge circles
const cxs = [...svg.matchAll(/<circle cx="([\d.-]+)" cy="([\d.-]+)" r="10"/g)].map((m) => m[1] + ',' + m[2]);
expect(new Set(cxs).size).toBe(cxs.length);
});

it('separates request/response badges even on a short edge (return-lane offset)', () => {
const shortLayouts = new Map([
['rel-s', [{ id: 'rel-s', points: [ { x: 100, y: 100 }, { x: 100, y: 140 } ] }]],
]);
const rrFlow: CalmFlow = {
'unique-id': 'rr',
name: 'RR',
description: 'request-response on a 40px edge',
transitions: [
{ 'relationship-unique-id': 'rel-s', 'sequence-number': 1, description: 'req', direction: 'source-to-destination' },
{ 'relationship-unique-id': 'rel-s', 'sequence-number': 2, description: 'res', direction: 'destination-to-source' },
],
};
const svg = renderFlowOverlay(rrFlow, shortLayouts);
const centers = [...svg.matchAll(/<circle cx="([\d.-]+)" cy="([\d.-]+)" r="10"/g)].map((m) => [Number(m[1]), Number(m[2])]);
expect(centers.length).toBe(2);
const [a, b] = centers as [[number, number], [number, number]];
const dist = Math.hypot(a[0] - b[0], a[1] - b[1]);
expect(dist).toBeGreaterThanOrEqual(18);
});

it('renders destination-to-source badges hollow (response convention)', () => {
const svg = renderFlowOverlay(reverseFlow, twoEdgeLayouts);
// reverseFlow's single transition is destination-to-source
expect(svg).toContain('class="flow-badge flow-badge-reverse"');
expect(svg).toContain('fill="#ffffff"');
});

it('forward badges stay solid', () => {
const svg = renderFlowOverlay(sampleFlow, twoEdgeLayouts);
expect(svg).not.toContain('flow-badge-reverse');
});

it('Test 7: handles flow with 3+ transitions (multi-edge flow)', () => {
const svg = renderFlowOverlay(multiEdgeFlow, twoEdgeLayouts);
// Three transitions, rel-3 exists in layout
Expand Down
116 changes: 106 additions & 10 deletions calm-studio/packages/web-component/src/render/flowOverlay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,15 @@ export function renderFlowOverlay(
): string {
const parts: string[] = ['<g class="flow-overlay">'];

// Count transitions per relationship so badges on a shared edge can be
// spread along the path instead of stacking at the midpoint.
const perRelationshipCount = new Map<string, number>();
for (const t of flow.transitions) {
const id = t['relationship-unique-id'];
perRelationshipCount.set(id, (perRelationshipCount.get(id) ?? 0) + 1);
}
const perRelationshipSeen = new Map<string, number>();

for (const transition of flow.transitions) {
const layouts = (edgeLayoutsByRelationship.get(transition['relationship-unique-id']) ?? []).filter(
(l) => l.points.length >= 2
Expand Down Expand Up @@ -70,18 +79,41 @@ export function renderFlowOverlay(
);
}

// One sequence badge per transition, placed at the first edge's midpoint
const midIdx = Math.floor(badgeEdge.points.length / 2);
const midPoint = badgeEdge.points[midIdx] ?? badgeEdge.points[0];
if (midPoint === undefined) continue;
const midX = midPoint.x;
const midY = midPoint.y;
// One sequence badge per transition. Transitions sharing a relationship
// spread along the first edge's path (k-th of n sits at (k+1)/(n+1)) so
// request/response pairs never stack invisibly on the midpoint.
const relId = transition['relationship-unique-id'];
const seen = perRelationshipSeen.get(relId) ?? 0;
perRelationshipSeen.set(relId, seen + 1);
const count = perRelationshipCount.get(relId) ?? 1;
const fraction = (seen + 1) / (count + 1);
Comment thread
rocketstack-matt marked this conversation as resolved.
Outdated
const badgePoint = pointAtFraction(badgeEdge.points, fraction);
if (badgePoint === undefined) continue;
// Reverse (response) badges shift perpendicular to the path — a "return
// lane" beside the edge — so request/response pairs stay separated even
// on edges shorter than a badge diameter.
const isReverse = direction === 'destination-to-source';
const normal = pathNormalAt(badgeEdge.points, fraction);
const laneOffset = isReverse && normal !== undefined ? 22 : 0;
Comment thread
rocketstack-matt marked this conversation as resolved.
const midX = badgePoint.x + (normal?.x ?? 0) * laneOffset;
const midY = badgePoint.y + (normal?.y ?? 0) * laneOffset;
Comment thread
rocketstack-matt marked this conversation as resolved.
Outdated

// The flow schema defines `description` on transitions; `summary` was a
// legacy CalmStudio field. Prefer the schema field, fall back for older files.
const transitionLabel =
transition.description ?? (transition as { summary?: string }).summary ?? '';
// Static-render convention: forward (request) badges are solid; reverse
// destination-to-source (response) badges render hollow, so direction is
// legible without the animation.
const badgeClass = isReverse ? 'flow-badge flow-badge-reverse' : 'flow-badge';
const circleFill = isReverse ? '#ffffff' : '#3b82f6';
const circleExtra = isReverse ? ' stroke="#3b82f6" stroke-width="2"' : '';
const numberFill = isReverse ? '#3b82f6' : 'white';
parts.push(
`<g class="flow-badge" data-summary="${escapeAttr(transition.summary)}">`,
` <circle cx="${midX}" cy="${midY}" r="10" fill="#3b82f6"/>`,
` <text x="${midX}" y="${midY}" fill="white" font-size="9" font-weight="bold" text-anchor="middle" dominant-baseline="central">${transition['sequence-number']}</text>`,
` <title>${escapeAttr(transition.summary)}</title>`,
`<g class="${badgeClass}" data-summary="${escapeAttr(transitionLabel)}">`,
Comment thread
rocketstack-matt marked this conversation as resolved.
Outdated
` <circle cx="${midX}" cy="${midY}" r="10" fill="${circleFill}"${circleExtra}/>`,
` <text x="${midX}" y="${midY}" fill="${numberFill}" font-size="9" font-weight="bold" text-anchor="middle" dominant-baseline="central">${transition['sequence-number']}</text>`,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

transition['sequence-number'] goes into the badge text unescaped, while every other dynamic value here (and in nodeRenderer.ts) is routed through an escape helper. The type says number, but this is parsed JSON reaching {@html svgContent} in CalmDiagram.svelte:166 — a document with a string in that field injects markup. Raised in the original review body as a while-you're-here item; still open.

Suggested change
` <text x="${midX}" y="${midY}" fill="${numberFill}" font-size="9" font-weight="bold" text-anchor="middle" dominant-baseline="central">${transition['sequence-number']}</text>`,
` <text x="${midX}" y="${midY}" fill="${numberFill}" font-size="9" font-weight="bold" text-anchor="middle" dominant-baseline="central">${escapeAttr(String(transition['sequence-number']))}</text>`,

` <title>${escapeAttr(transitionLabel)}</title>`,
`</g>`
);
}
Expand Down Expand Up @@ -158,6 +190,70 @@ function getReferencedNodeIdsWithFlatFallback(rel: CalmRelationship): string[] {
// Helpers
// ---------------------------------------------------------------------------

/** Point at a given fraction (0..1) of a polyline's total length. */
function pointAtFraction(
points: Array<{ x: number; y: number }>,
fraction: number
): { x: number; y: number } | undefined {
if (points.length === 0) return undefined;
const first = points[0];
if (points.length === 1 || first === undefined) return first;
let total = 0;
const segments: Array<{ a: { x: number; y: number }; b: { x: number; y: number }; len: number }> = [];
for (let i = 1; i < points.length; i++) {
const a = points[i - 1];
const b = points[i];
if (a === undefined || b === undefined) continue;
const len = Math.hypot(b.x - a.x, b.y - a.y);
segments.push({ a, b, len });
total += len;
}
if (total === 0) return first;
let target = Math.min(Math.max(fraction, 0), 1) * total;
for (const seg of segments) {
if (target <= seg.len) {
const t = seg.len === 0 ? 0 : target / seg.len;
return { x: seg.a.x + (seg.b.x - seg.a.x) * t, y: seg.a.y + (seg.b.y - seg.a.y) * t };
}
target -= seg.len;
}
const last = points[points.length - 1];
return last;
}

/** Unit normal (perpendicular) of the polyline segment containing the fraction point. */
function pathNormalAt(
points: Array<{ x: number; y: number }>,
fraction: number
): { x: number; y: number } | undefined {
if (points.length < 2) return undefined;
// Locate the segment the fraction falls in (same walk as pointAtFraction).
let total = 0;
const lens: number[] = [];
for (let i = 1; i < points.length; i++) {
const a = points[i - 1];
const b = points[i];
const len = a !== undefined && b !== undefined ? Math.hypot(b.x - a.x, b.y - a.y) : 0;
lens.push(len);
total += len;
}
if (total === 0) return undefined;
let target = Math.min(Math.max(fraction, 0), 1) * total;
for (let i = 0; i < lens.length; i++) {
const len = lens[i] ?? 0;
if (target <= len) {
const a = points[i];
const b = points[i + 1];
if (a === undefined || b === undefined || len === 0) return undefined;
const tx = (b.x - a.x) / len;
const ty = (b.y - a.y) / len;
return { x: -ty, y: tx };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The normal is always (-ty, tx), so which side of the edge a reverse badge lands on depends purely on the layout engine's point ordering — nothing stops it landing on top of a node, another edge, or off-canvas. A comment acknowledging that is probably enough for now; biasing away from the diagram centroid would be the fuller fix.

Separately, this function re-walks the segment lengths exactly as pointAtFraction does, and the two copies handle degenerate input differently (pointAtFraction returns first when total === 0, this returns undefined). One function returning { point, normal } would remove the duplication and the divergence.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on the duplication point from the earlier review: walkPolyline now shares the segment/length precompute, but pointOnWalk and normalOnWalk still each re-walk the segment list to find the target fraction. A single function returning { point, normal } (as originally suggested) would still collapse that. Non-blocking.

}
target -= len;
}
return undefined;
}

function escapeAttr(str: string): string {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-existing, not introduced here: this duplicates nodeRenderer.ts's escapeXml but doesn't escape '. Since this file is already being touched, worth exporting escapeXml from nodeRenderer.ts and importing it here instead of maintaining two slightly different escaping helpers in the same package. Non-blocking.

return str
.replace(/&/g, '&amp;')
Expand Down
Loading