diff --git a/.changeset/angry-bags-brake.md b/.changeset/angry-bags-brake.md new file mode 100644 index 000000000..472e486ec --- /dev/null +++ b/.changeset/angry-bags-brake.md @@ -0,0 +1,5 @@ +--- +'mermaid': patch +--- + +fix: architecture diagrams no longer grow to extreme heights due to conflicting alignments diff --git a/cypress/integration/rendering/architecture.spec.ts b/cypress/integration/rendering/architecture.spec.ts index bc2276352..25326ff80 100644 --- a/cypress/integration/rendering/architecture.spec.ts +++ b/cypress/integration/rendering/architecture.spec.ts @@ -171,6 +171,58 @@ describe.skip('architecture diagram', () => { ` ); }); + + it('should render an architecture diagram with a resonable height', () => { + imgSnapshotTest( + `architecture-beta + group federated(cloud)[Federated Environment] + service server1(server)[System] in federated + service edge(server)[Edge Device] in federated + server1:R -- L:edge + + group on_prem(cloud)[Hub] + service firewall(server)[Firewall Device] in on_prem + service server(server)[Server] in on_prem + firewall:R -- L:server + + service db1(database)[db1] in on_prem + service db2(database)[db2] in on_prem + service db3(database)[db3] in on_prem + service db4(database)[db4] in on_prem + service db5(database)[db5] in on_prem + service db6(database)[db6] in on_prem + + junction mid in on_prem + server:B -- T:mid + + junction 1Leftofmid in on_prem + 1Leftofmid:R -- L:mid + 1Leftofmid:B -- T:db1 + + junction 2Leftofmid in on_prem + 2Leftofmid:R -- L:1Leftofmid + 2Leftofmid:B -- T:db2 + + junction 3Leftofmid in on_prem + 3Leftofmid:R -- L:2Leftofmid + 3Leftofmid:B -- T:db3 + + junction 1RightOfMid in on_prem + mid:R -- L:1RightOfMid + 1RightOfMid:B -- T:db4 + + junction 2RightOfMid in on_prem + 1RightOfMid:R -- L:2RightOfMid + 2RightOfMid:B -- T:db5 + + junction 3RightOfMid in on_prem + 2RightOfMid:R -- L:3RightOfMid + 3RightOfMid:B -- T:db6 + + edge:R -- L:firewall + ` + ); + }); }); // Skipped as the layout is not deterministic, and causes issues in E2E tests. diff --git a/cypress/integration/rendering/flowchart-elk.spec.js b/cypress/integration/rendering/flowchart-elk.spec.js index c3aba53ea..38bfe6440 100644 --- a/cypress/integration/rendering/flowchart-elk.spec.js +++ b/cypress/integration/rendering/flowchart-elk.spec.js @@ -900,6 +900,153 @@ flowchart LR n7@{ shape: rect} n8@{ shape: rect} +`, + { flowchart: { titleTopMargin: 0 } } + ); + }); + + it('6088-1: should handle diamond shape intersections', () => { + imgSnapshotTest( + `--- +config: + layout: elk +--- + flowchart LR + subgraph S2 + subgraph s1["APA"] + D{"Use the editor"} + end + + + D -- Mermaid js --> I{"fa:fa-code Text"} + D --> I + D --> I + + end +`, + { flowchart: { titleTopMargin: 0 } } + ); + }); + + it('6088-2: should handle diamond shape intersections', () => { + imgSnapshotTest( + `--- +config: + layout: elk +--- + flowchart LR + a + subgraph s0["APA"] + subgraph s8["APA"] + subgraph s1["APA"] + D{"X"} + E[Q] + end + subgraph s3["BAPA"] + F[Q] + I + end + D --> I + D --> I + D --> I + + I{"X"} + end + end + +`, + { flowchart: { titleTopMargin: 0 } } + ); + }); + + it('6088-3: should handle diamond shape intersections', () => { + imgSnapshotTest( + `--- +config: + layout: elk +--- + flowchart LR + a + D{"Use the editor"} + + D -- Mermaid js --> I{"fa:fa-code Text"} + D-->I + D-->I + +`, + { flowchart: { titleTopMargin: 0 } } + ); + }); + + it('6088-4: should handle diamond shape intersections', () => { + imgSnapshotTest( + `--- +config: + layout: elk +--- +flowchart LR + subgraph s1["Untitled subgraph"] + n1["Evaluate"] + n2["Option 1"] + n3["Option 2"] + n4["fa:fa-car Option 3"] + end + subgraph s2["Untitled subgraph"] + n5["Evaluate"] + n6["Option 1"] + n7["Option 2"] + n8["fa:fa-car Option 3"] + end + A["Start"] -- Some text --> B("Continue") + B --> C{"Evaluate"} + C -- One --> D["Option 1"] + C -- Two --> E["Option 2"] + C -- Three --> F["fa:fa-car Option 3"] + n1 -- One --> n2 + n1 -- Two --> n3 + n1 -- Three --> n4 + n5 -- One --> n6 + n5 -- Two --> n7 + n5 -- Three --> n8 + n1@{ shape: diam} + n2@{ shape: rect} + n3@{ shape: rect} + n4@{ shape: rect} + n5@{ shape: diam} + n6@{ shape: rect} + n7@{ shape: rect} + n8@{ shape: rect} + +`, + { flowchart: { titleTopMargin: 0 } } + ); + }); + + it('6088-5: should handle diamond shape intersections', () => { + imgSnapshotTest( + `--- +config: + layout: elk +--- +flowchart LR + A{A} --> B & C + +`, + { flowchart: { titleTopMargin: 0 } } + ); + }); + it('6088-6: should handle diamond shape intersections', () => { + imgSnapshotTest( + `--- +config: + layout: elk +--- +flowchart LR + A{A} --> B & C + subgraph "subbe" + A + end + `, { flowchart: { titleTopMargin: 0 } } ); diff --git a/cypress/platform/knsv2.html b/cypress/platform/knsv2.html index 713e76b89..92d7e8c31 100644 --- a/cypress/platform/knsv2.html +++ b/cypress/platform/knsv2.html @@ -88,33 +88,61 @@
-+--- config: layout: elk --- -flowchart LR - subgraph s1["Untitled subgraph"] - n1["Evaluate"] - n2["Option 1"] - n3["Option 2"] - n4["fa:fa-car Option 3"] - end - n1 -- One --> n2 - n1 -- Two --> n3 - n1 -- Three --> n4 - n5 - n1@{ shape: diam} - n2@{ shape: rect} - n3@{ shape: rect} - n4@{ shape: rect} - A["Start"] -- Some text --> B("Continue") - B --> C{"Evaluate"} - C -- One --> D["Option 1"] - C -- Two --> E["Option 2"] - C -- Three --> F["fa:fa-car Option 3"] + flowchart LR + subgraph S2 + subgraph s1["APA"] + D{"Use the editor"} + end + D -- Mermaid js --> I{"fa:fa-code Text"} + D --> I + D --> I + + end +++--- +config: + layout: elk +--- + flowchart LR + a + subgraph s0["APA"] + subgraph s8["APA"] + subgraph s1["APA"] + D{"X"} + E[Q] + end + subgraph s3["BAPA"] + F[Q] + I + end + D --> I + D --> I + D --> I + + I{"X"} + end + end +++--- +config: + layout: elk +--- + flowchart LR + a + D{"Use the editor"} + + D -- Mermaid js --> I{"fa:fa-code Text"} + D-->I + D-->I--- @@ -171,7 +199,7 @@ flowchart TB a["1."] b["- x"]-+--- config: layout: elk @@ -180,7 +208,19 @@ flowchart LR A{A} --> B & C-++--- +config: + layout: elk +--- +flowchart LR + A{A} --> B & C + subgraph "subbe" + A + end ++--- config: layout: elk diff --git a/docs/syntax/gantt.md b/docs/syntax/gantt.md index cdaf0c2ac..ff6be97aa 100644 --- a/docs/syntax/gantt.md +++ b/docs/syntax/gantt.md @@ -500,7 +500,7 @@ mermaid.ganttConfig = { sectionFontSize: 24, // Font size for sections numberSectionStyles: 1, // The number of alternating section styles axisFormat: '%d/%m', // Date/time format of the axis - tickInterval: '1 week', // Axis ticks + tickInterval: '1week', // Axis ticks topAxis: true, // When this flag is set, date labels will be added to the top of the chart displayMode: 'compact', // Turns compact mode on weekday: 'sunday', // On which day a week-based interval should start diff --git a/packages/mermaid-layout-elk/CHANGELOG.md b/packages/mermaid-layout-elk/CHANGELOG.md index dd5115cde..319d6b828 100644 --- a/packages/mermaid-layout-elk/CHANGELOG.md +++ b/packages/mermaid-layout-elk/CHANGELOG.md @@ -1,5 +1,11 @@ # @mermaid-js/layout-elk +## 0.1.7 + +### Patch Changes + +- [#6090](https://github.com/mermaid-js/mermaid/pull/6090) [`654097c`](https://github.com/mermaid-js/mermaid/commit/654097c43801b2d606bc3d2bef8c6fbc3301e9e4) Thanks [@knsv](https://github.com/knsv)! - fix: Updated offset calculations for diamond shape when handling intersections + ## 0.1.6 ### Patch Changes diff --git a/packages/mermaid-layout-elk/package.json b/packages/mermaid-layout-elk/package.json index dad981fb3..a043d4c38 100644 --- a/packages/mermaid-layout-elk/package.json +++ b/packages/mermaid-layout-elk/package.json @@ -1,6 +1,6 @@ { "name": "@mermaid-js/layout-elk", - "version": "0.1.6", + "version": "0.1.7", "description": "ELK layout engine for mermaid", "module": "dist/mermaid-layout-elk.core.mjs", "types": "dist/layouts.d.ts", diff --git a/packages/mermaid-layout-elk/src/render.ts b/packages/mermaid-layout-elk/src/render.ts index 1216b5dc8..59b97c557 100644 --- a/packages/mermaid-layout-elk/src/render.ts +++ b/packages/mermaid-layout-elk/src/render.ts @@ -705,14 +705,11 @@ export const render = async ( bounds: { x: any; y: any; width: any; height: any; padding: any }, isDiamond: boolean ) => { - log.debug('UIO cutPathAtIntersect Points:', _points, 'node:', bounds, 'isDiamond', isDiamond); + log.debug('APA18 cutPathAtIntersect Points:', _points, 'node:', bounds, 'isDiamond', isDiamond); const points: any[] = []; let lastPointOutside = _points[0]; let isInside = false; _points.forEach((point: any) => { - // const node = clusterDb[edge.toCluster].node; - log.debug(' checking point', point, bounds); - // check if point is inside the boundary rect if (!outsideNode(bounds, point) && !isInside) { // First point inside the rect found @@ -906,7 +903,7 @@ export const render = async ( const offset = calcOffset(sourceId, targetId, parentLookupDb); log.debug( - 'offset', + 'APA18 offset', offset, sourceId, ' ==> ', @@ -971,29 +968,22 @@ export const render = async ( } if (startNode.shape === 'diamond' || startNode.shape === 'diam') { edge.points.unshift({ - x: startNode.x + startNode.width / 2 + offset.x, - y: startNode.y + startNode.height / 2 + offset.y, + x: startNode.offset.posX + startNode.width / 2, + y: startNode.offset.posY + startNode.height / 2, }); } if (endNode.shape === 'diamond' || endNode.shape === 'diam') { - const x = endNode.x + endNode.width / 2 + offset.x; - // Add a point at the center of the diamond - if ( - Math.abs(edge.points[edge.points.length - 1].y - endNode.y - offset.y) > 0.01 || - Math.abs(edge.points[edge.points.length - 1].x - x) > 0.001 - ) { - edge.points.push({ - x: endNode.x + endNode.width / 2 + offset.x, - y: endNode.y + endNode.height / 2 + offset.y, - }); - } + edge.points.push({ + x: endNode.offset.posX + endNode.width / 2, + y: endNode.offset.posY + endNode.height / 2, + }); } edge.points = cutPathAtIntersect( edge.points.reverse(), { - x: startNode.x + startNode.width / 2 + offset.x, - y: startNode.y + startNode.height / 2 + offset.y, + x: startNode.offset.posX + startNode.width / 2, + y: startNode.offset.posY + startNode.height / 2, width: sw, height: startNode.height, padding: startNode.padding, @@ -1004,8 +994,8 @@ export const render = async ( edge.points = cutPathAtIntersect( edge.points, { - x: endNode.x + ew / 2 + endNode.offset.x, - y: endNode.y + endNode.height / 2 + endNode.offset.y, + x: endNode.offset.posX + endNode.width / 2, + y: endNode.offset.posY + endNode.height / 2, width: ew, height: endNode.height, padding: endNode.padding, diff --git a/packages/mermaid/src/diagrams/architecture/architectureDb.ts b/packages/mermaid/src/diagrams/architecture/architectureDb.ts index 93fa71ca3..2174ebe19 100644 --- a/packages/mermaid/src/diagrams/architecture/architectureDb.ts +++ b/packages/mermaid/src/diagrams/architecture/architectureDb.ts @@ -13,6 +13,7 @@ import { setDiagramTitle, } from '../common/commonDb.js'; import type { + ArchitectureAlignment, ArchitectureDB, ArchitectureDirectionPair, ArchitectureDirectionPairMap, @@ -25,6 +26,7 @@ import type { ArchitectureState, } from './architectureTypes.js'; import { + getArchitectureDirectionAlignment, getArchitectureDirectionPair, isArchitectureDirection, isArchitectureJunction, @@ -211,12 +213,18 @@ const addEdge = function ({ const getEdges = (): ArchitectureEdge[] => state.records.edges; /** - * Returns the current diagram's adjacency list & spatial map. + * Returns the current diagram's adjacency list, spatial map, & group alignments. * If they have not been created, run the algorithms to generate them. * @returns */ const getDataStructures = () => { if (state.records.dataStructures === undefined) { + // Tracks how groups are aligned with one another. Generated while creating the adj list + const groupAlignments: Record< + string, + Record> + > = {}; + // Create an adjacency list of the diagram to perform BFS on // Outer reduce applied on all services // Inner reduce applied on the edges for a service @@ -224,6 +232,19 @@ const getDataStructures = () => { Record >((prevOuter, [id, service]) => { prevOuter[id] = service.edges.reduce ((prevInner, edge) => { + // track the direction groups connect to one another + const lhsGroupId = getNode(edge.lhsId)?.in; + const rhsGroupId = getNode(edge.rhsId)?.in; + if (lhsGroupId && rhsGroupId && lhsGroupId !== rhsGroupId) { + const alignment = getArchitectureDirectionAlignment(edge.lhsDir, edge.rhsDir); + if (alignment !== 'bend') { + groupAlignments[lhsGroupId] ??= {}; + groupAlignments[lhsGroupId][rhsGroupId] = alignment; + groupAlignments[rhsGroupId] ??= {}; + groupAlignments[rhsGroupId][lhsGroupId] = alignment; + } + } + if (edge.lhsId === id) { // source is LHS const pair = getArchitectureDirectionPair(edge.lhsDir, edge.rhsDir); @@ -245,6 +266,7 @@ const getDataStructures = () => { // Configuration for the initial pass of BFS const firstId = Object.keys(adjList)[0]; const visited = { [firstId]: 1 }; + // If a key is present in this object, it has not been visited const notVisited = Object.keys(adjList).reduce( (prev, id) => (id === firstId ? prev : { ...prev, [id]: 1 }), {} as Record @@ -283,6 +305,7 @@ const getDataStructures = () => { state.records.dataStructures = { adjList, spatialMaps, + groupAlignments, }; } return state.records.dataStructures; diff --git a/packages/mermaid/src/diagrams/architecture/architectureRenderer.ts b/packages/mermaid/src/diagrams/architecture/architectureRenderer.ts index af9429539..768c174b0 100644 --- a/packages/mermaid/src/diagrams/architecture/architectureRenderer.ts +++ b/packages/mermaid/src/diagrams/architecture/architectureRenderer.ts @@ -12,7 +12,9 @@ import { setupGraphViewbox } from '../../setupGraphViewbox.js'; import { getConfigField } from './architectureDb.js'; import { architectureIcons } from './architectureIcons.js'; import type { + ArchitectureAlignment, ArchitectureDataStructures, + ArchitectureGroupAlignments, ArchitectureJunction, ArchitectureSpatialMap, EdgeSingular, @@ -149,25 +151,91 @@ function addEdges(edges: ArchitectureEdge[], cy: cytoscape.Core) { }); } -function getAlignments(spatialMaps: ArchitectureSpatialMap[]): fcose.FcoseAlignmentConstraint { +function getAlignments( + db: ArchitectureDB, + spatialMaps: ArchitectureSpatialMap[], + groupAlignments: ArchitectureGroupAlignments +): fcose.FcoseAlignmentConstraint { + /** + * Flattens the alignment object so nodes in different groups will be in the same alignment array IFF their groups don't connect in a conflicting alignment + * + * i.e., two groups which connect horizontally should not have nodes with vertical alignments to one another + * + * See: #5952 + * + * @param alignmentObj - alignment object with the outer key being the row/col # and the inner key being the group name mapped to the nodes on that axis in the group + * @param alignmentDir - alignment direction + * @returns flattened alignment object with an arbitrary key mapping to nodes in the same row/col + */ + const flattenAlignments = ( + alignmentObj: Record >, + alignmentDir: ArchitectureAlignment + ): Record => { + return Object.entries(alignmentObj).reduce( + (prev, [dir, alignments]) => { + // prev is the mapping of x/y coordinate to an array of the nodes in that row/column + let cnt = 0; + const arr = Object.entries(alignments); // [group name, array of nodes within the group on axis dir] + if (arr.length === 1) { + // If only one group exists in the row/column, we don't need to do anything else + prev[dir] = arr[0][1]; + return prev; + } + for (let i = 0; i < arr.length - 1; i++) { + for (let j = i + 1; j < arr.length; j++) { + const [aGroupId, aNodeIds] = arr[i]; + const [bGroupId, bNodeIds] = arr[j]; + const alignment = groupAlignments[aGroupId]?.[bGroupId]; // Get how the two groups are intended to align (undefined if they aren't) + + if (alignment === alignmentDir) { + // If the intended alignment between the two groups is the same as the alignment we are parsing + prev[dir] ??= []; + prev[dir] = [...prev[dir], ...aNodeIds, ...bNodeIds]; // add the node ids of both groups to the axis array in prev + } else if (aGroupId === 'default' || bGroupId === 'default') { + // If either of the groups are in the default space (not in a group), use the same behavior as above + prev[dir] ??= []; + prev[dir] = [...prev[dir], ...aNodeIds, ...bNodeIds]; + } else { + // Otherwise, the nodes in the two groups are not intended to align + const keyA = `${dir}-${cnt++}`; + prev[keyA] = aNodeIds; + const keyB = `${dir}-${cnt++}`; + prev[keyB] = bNodeIds; + } + } + } + + return prev; + }, + {} as Record + ); + }; + const alignments = spatialMaps.map((spatialMap) => { - const horizontalAlignments: Record = {}; - const verticalAlignments: Record = {}; + const horizontalAlignments: Record > = {}; + const verticalAlignments: Record > = {}; + // Group service ids in an object with their x and y coordinate as the key Object.entries(spatialMap).forEach(([id, [x, y]]) => { - if (!horizontalAlignments[y]) { - horizontalAlignments[y] = []; - } - if (!verticalAlignments[x]) { - verticalAlignments[x] = []; - } - horizontalAlignments[y].push(id); - verticalAlignments[x].push(id); + const nodeGroup = db.getNode(id)?.in ?? 'default'; + + horizontalAlignments[y] ??= {}; + horizontalAlignments[y][nodeGroup] ??= []; + horizontalAlignments[y][nodeGroup].push(id); + + verticalAlignments[x] ??= {}; + verticalAlignments[x][nodeGroup] ??= []; + verticalAlignments[x][nodeGroup].push(id); }); + // Merge the values of each object into a list if the inner list has at least 2 elements return { - horiz: Object.values(horizontalAlignments).filter((arr) => arr.length > 1), - vert: Object.values(verticalAlignments).filter((arr) => arr.length > 1), + horiz: Object.values(flattenAlignments(horizontalAlignments, 'horizontal')).filter( + (arr) => arr.length > 1 + ), + vert: Object.values(flattenAlignments(verticalAlignments, 'vertical')).filter( + (arr) => arr.length > 1 + ), }; }); @@ -244,7 +312,8 @@ function layoutArchitecture( junctions: ArchitectureJunction[], groups: ArchitectureGroup[], edges: ArchitectureEdge[], - { spatialMaps }: ArchitectureDataStructures + db: ArchitectureDB, + { spatialMaps, groupAlignments }: ArchitectureDataStructures ): Promise { return new Promise((resolve) => { const renderEl = select('body').append('div').attr('id', 'cy').attr('style', 'display:none'); @@ -318,9 +387,8 @@ function layoutArchitecture( addServices(services, cy); addJunctions(junctions, cy); addEdges(edges, cy); - // Use the spatial map to create alignment arrays for fcose - const alignmentConstraint = getAlignments(spatialMaps); + const alignmentConstraint = getAlignments(db, spatialMaps, groupAlignments); // Create the relative constraints for fcose by using an inverse of the spatial map and performing BFS on it const relativePlacementConstraint = getRelativeConstraints(spatialMaps); @@ -454,7 +522,7 @@ export const draw: DrawDefinition = async (text, id, _version, diagObj: Diagram) await drawServices(db, servicesElem, services); drawJunctions(db, servicesElem, junctions); - const cy = await layoutArchitecture(services, junctions, groups, edges, ds); + const cy = await layoutArchitecture(services, junctions, groups, edges, db, ds); await drawEdges(edgesElem, cy); await drawGroups(groupElem, cy); diff --git a/packages/mermaid/src/diagrams/architecture/architectureTypes.ts b/packages/mermaid/src/diagrams/architecture/architectureTypes.ts index b3ef55ec6..cad2c5c36 100644 --- a/packages/mermaid/src/diagrams/architecture/architectureTypes.ts +++ b/packages/mermaid/src/diagrams/architecture/architectureTypes.ts @@ -7,6 +7,8 @@ import type cytoscape from 'cytoscape'; | Architecture Diagram Types | \*=======================================*/ +export type ArchitectureAlignment = 'vertical' | 'horizontal' | 'bend'; + export type ArchitectureDirection = 'L' | 'R' | 'T' | 'B'; export type ArchitectureDirectionX = Extract ; export type ArchitectureDirectionY = Extract ; @@ -170,6 +172,18 @@ export const getArchitectureDirectionXYFactors = function ( } }; +export const getArchitectureDirectionAlignment = function ( + a: ArchitectureDirection, + b: ArchitectureDirection +): ArchitectureAlignment { + if (isArchitectureDirectionXY(a, b)) { + return 'bend'; + } else if (isArchitectureDirectionX(a)) { + return 'horizontal'; + } + return 'vertical'; +}; + export interface ArchitectureStyleOptions { archEdgeColor: string; archEdgeArrowColor: string; @@ -249,9 +263,27 @@ export interface ArchitectureDB extends DiagramDB { export type ArchitectureAdjacencyList = Record ; export type ArchitectureSpatialMap = Record ; + +/** + * Maps the direction that groups connect from. + * + * **Outer key**: ID of group A + * + * **Inner key**: ID of group B + * + * **Value**: 'vertical' or 'horizontal' + * + * Note: tmp[groupA][groupB] == tmp[groupB][groupA] + */ +export type ArchitectureGroupAlignments = Record< + string, + Record > +>; + export interface ArchitectureDataStructures { adjList: ArchitectureAdjacencyList; spatialMaps: ArchitectureSpatialMap[]; + groupAlignments: ArchitectureGroupAlignments; } export interface ArchitectureState extends Record { diff --git a/packages/mermaid/src/docs/syntax/gantt.md b/packages/mermaid/src/docs/syntax/gantt.md index 01a9f041d..eab35d09f 100644 --- a/packages/mermaid/src/docs/syntax/gantt.md +++ b/packages/mermaid/src/docs/syntax/gantt.md @@ -390,7 +390,7 @@ mermaid.ganttConfig = { sectionFontSize: 24, // Font size for sections numberSectionStyles: 1, // The number of alternating section styles axisFormat: '%d/%m', // Date/time format of the axis - tickInterval: '1 week', // Axis ticks + tickInterval: '1week', // Axis ticks topAxis: true, // When this flag is set, date labels will be added to the top of the chart displayMode: 'compact', // Turns compact mode on weekday: 'sunday', // On which day a week-based interval should start