From 498f75eece669ecb73bbec7dc5b5af29e761011f Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Sun, 3 Sep 2023 02:32:28 +0530 Subject: [PATCH 1/5] fix: #4691 Align arrowheads properly in sequenceDiagram --- .../sequence/parser/sequenceDiagram.jison | 2 +- .../src/diagrams/sequence/sequenceDb.js | 6 +- .../src/diagrams/sequence/sequenceRenderer.ts | 58 ++++++++++++++----- .../mermaid/src/diagrams/sequence/svgDraw.js | 6 +- 4 files changed, 50 insertions(+), 22 deletions(-) diff --git a/packages/mermaid/src/diagrams/sequence/parser/sequenceDiagram.jison b/packages/mermaid/src/diagrams/sequence/parser/sequenceDiagram.jison index 04f0de20e..4e971d989 100644 --- a/packages/mermaid/src/diagrams/sequence/parser/sequenceDiagram.jison +++ b/packages/mermaid/src/diagrams/sequence/parser/sequenceDiagram.jison @@ -301,7 +301,7 @@ placement signal : actor signaltype '+' actor text2 - { $$ = [$1,$4,{type: 'addMessage', from:$1.actor, to:$4.actor, signalType:$2, msg:$5}, + { $$ = [$1,$4,{type: 'addMessage', from:$1.actor, to:$4.actor, signalType:$2, msg:$5, activate: true}, {type: 'activeStart', signalType: yy.LINETYPE.ACTIVE_START, actor: $4} ]} | actor signaltype '-' actor text2 diff --git a/packages/mermaid/src/diagrams/sequence/sequenceDb.js b/packages/mermaid/src/diagrams/sequence/sequenceDb.js index b5d68fb9f..69a764a98 100644 --- a/packages/mermaid/src/diagrams/sequence/sequenceDb.js +++ b/packages/mermaid/src/diagrams/sequence/sequenceDb.js @@ -124,7 +124,8 @@ export const addSignal = function ( idFrom, idTo, message = { text: undefined, wrap: undefined }, - messageType + messageType, + activate = false ) { if (messageType === LINETYPE.ACTIVE_END) { const cnt = activationCount(idFrom.actor); @@ -147,6 +148,7 @@ export const addSignal = function ( message: message.text, wrap: (message.wrap === undefined && autoWrap()) || !!message.wrap, type: messageType, + activate, }); return true; }; @@ -530,7 +532,7 @@ export const apply = function (param) { lastDestroyed = undefined; } } - addSignal(param.from, param.to, param.msg, param.signalType); + addSignal(param.from, param.to, param.msg, param.signalType, param.activate); break; case 'boxStart': addBox(param.boxData); diff --git a/packages/mermaid/src/diagrams/sequence/sequenceRenderer.ts b/packages/mermaid/src/diagrams/sequence/sequenceRenderer.ts index e84d2254c..4bc8f7487 100644 --- a/packages/mermaid/src/diagrams/sequence/sequenceRenderer.ts +++ b/packages/mermaid/src/diagrams/sequence/sequenceRenderer.ts @@ -1,5 +1,5 @@ // @ts-nocheck TODO: fix file -import { select, selectAll } from 'd3'; +import { select } from 'd3'; import svgDraw, { ACTOR_TYPE_WIDTH, drawText, fixLifeLineHeights } from './svgDraw.js'; import { log } from '../../logger.js'; import common from '../common/common.js'; @@ -622,10 +622,10 @@ const activationBounds = function (actor, actors) { const left = activations.reduce(function (acc, activation) { return common.getMin(acc, activation.startx); - }, actorObj.x + actorObj.width / 2); + }, actorObj.x + actorObj.width / 2 - 1); const right = activations.reduce(function (acc, activation) { return common.getMax(acc, activation.stopx); - }, actorObj.x + actorObj.width / 2); + }, actorObj.x + actorObj.width / 2 + 1); return [left, right]; }; @@ -1389,9 +1389,8 @@ const buildNoteModel = function (msg, actors, diagObj) { }; const buildMessageModel = function (msg, actors, diagObj) { - let process = false; if ( - [ + ![ diagObj.db.LINETYPE.SOLID_OPEN, diagObj.db.LINETYPE.DOTTED_OPEN, diagObj.db.LINETYPE.SOLID, @@ -1402,17 +1401,44 @@ const buildMessageModel = function (msg, actors, diagObj) { diagObj.db.LINETYPE.DOTTED_POINT, ].includes(msg.type) ) { - process = true; - } - if (!process) { return {}; } - const fromBounds = activationBounds(msg.from, actors); - const toBounds = activationBounds(msg.to, actors); - const fromIdx = fromBounds[0] <= toBounds[0] ? 1 : 0; - const toIdx = fromBounds[0] < toBounds[0] ? 0 : 1; - const allBounds = [...fromBounds, ...toBounds]; - const boundedWidth = Math.abs(toBounds[toIdx] - fromBounds[fromIdx]); + const [fromLeft, fromRight] = activationBounds(msg.from, actors); + const [toLeft, toRight] = activationBounds(msg.to, actors); + const arrowDirection = fromLeft <= toLeft ? 'right' : 'left'; + const startx = arrowDirection === 'right' ? fromRight : fromLeft; + let stopx = arrowDirection === 'right' ? toLeft : toRight; + const isToActivation = Math.abs(toLeft - toRight) > 2; + + /** + * This is an edge case for the first activation. + * Proper fix would require significant changes. + * So, we set an activate flag in the message, and cross check that with isToActivation + * In cases where the message is to an activation that was properly detected, we don't want to move the arrow head + * The activation will not be detected on the first message, so we need to move the arrow head + */ + if (msg.activate && !isToActivation) { + if (arrowDirection === 'right') { + stopx -= conf.activationWidth / 2 - 1; + } else { + stopx += conf.activationWidth / 2 - 1; + } + } + + /** + * Shorten the length of arrow at the end and move the marker forward (using refX) to have a clean arrowhead + * This is not required for open arrows that don't have arrowheads + */ + if (![diagObj.db.LINETYPE.SOLID_OPEN, diagObj.db.LINETYPE.DOTTED_OPEN].includes(msg.type)) { + if (arrowDirection === 'right') { + stopx -= 3; + } else { + stopx += 3; + } + } + + const allBounds = [fromLeft, fromRight, toLeft, toRight]; + const boundedWidth = Math.abs(startx - stopx); if (msg.wrap && msg.message) { msg.message = utils.wrapLabel( msg.message, @@ -1429,8 +1455,8 @@ const buildMessageModel = function (msg, actors, diagObj) { conf.width ), height: 0, - startx: fromBounds[fromIdx], - stopx: toBounds[toIdx], + startx, + stopx, starty: 0, stopy: 0, message: msg.message, diff --git a/packages/mermaid/src/diagrams/sequence/svgDraw.js b/packages/mermaid/src/diagrams/sequence/svgDraw.js index e0aaa1eb9..f81147c10 100644 --- a/packages/mermaid/src/diagrams/sequence/svgDraw.js +++ b/packages/mermaid/src/diagrams/sequence/svgDraw.js @@ -703,7 +703,7 @@ export const insertArrowHead = function (elem) { .append('defs') .append('marker') .attr('id', 'arrowhead') - .attr('refX', 9) + .attr('refX', 7.9) .attr('refY', 5) .attr('markerUnits', 'userSpaceOnUse') .attr('markerWidth', 12) @@ -723,7 +723,7 @@ export const insertArrowFilledHead = function (elem) { .append('defs') .append('marker') .attr('id', 'filled-head') - .attr('refX', 18) + .attr('refX', 15.5) .attr('refY', 7) .attr('markerWidth', 20) .attr('markerHeight', 28) @@ -768,7 +768,7 @@ export const insertArrowCrossHead = function (elem) { .attr('markerHeight', 8) .attr('orient', 'auto') .attr('refX', 4) - .attr('refY', 5); + .attr('refY', 4.5); // The cross marker .append('path') From 02a0596e3ca152fa32535612a251d1cc4369f0f1 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Sun, 3 Sep 2023 02:33:06 +0530 Subject: [PATCH 2/5] chore: Update tests snapshot --- .../mermaid/src/diagrams/sequence/sequenceDiagram.spec.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js b/packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js index 0b84fbe35..cff890869 100644 --- a/packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js +++ b/packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js @@ -104,6 +104,7 @@ describe('more than one sequence diagram', () => { expect(diagram1.db.getMessages()).toMatchInlineSnapshot(` [ { + "activate": false, "from": "Alice", "message": "Hello Bob, how are you?", "to": "Bob", @@ -111,6 +112,7 @@ describe('more than one sequence diagram', () => { "wrap": false, }, { + "activate": false, "from": "Bob", "message": "I am good thanks!", "to": "Alice", @@ -127,6 +129,7 @@ describe('more than one sequence diagram', () => { expect(diagram2.db.getMessages()).toMatchInlineSnapshot(` [ { + "activate": false, "from": "Alice", "message": "Hello Bob, how are you?", "to": "Bob", @@ -134,6 +137,7 @@ describe('more than one sequence diagram', () => { "wrap": false, }, { + "activate": false, "from": "Bob", "message": "I am good thanks!", "to": "Alice", @@ -152,6 +156,7 @@ describe('more than one sequence diagram', () => { expect(diagram3.db.getMessages()).toMatchInlineSnapshot(` [ { + "activate": false, "from": "Alice", "message": "Hello John, how are you?", "to": "John", @@ -159,6 +164,7 @@ describe('more than one sequence diagram', () => { "wrap": false, }, { + "activate": false, "from": "John", "message": "I am good thanks!", "to": "Alice", From 784e235ff90f6e37f5b316d1ea1992a02e354b9b Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Sun, 3 Sep 2023 02:34:46 +0530 Subject: [PATCH 3/5] chore: Add test to verify activate --- packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js b/packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js index cff890869..ed6f07300 100644 --- a/packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js +++ b/packages/mermaid/src/diagrams/sequence/sequenceDiagram.spec.js @@ -554,6 +554,7 @@ deactivate Bob`; expect(messages.length).toBe(4); expect(messages[0].type).toBe(diagram.db.LINETYPE.DOTTED); + expect(messages[0].activate).toBeTruthy(); expect(messages[1].type).toBe(diagram.db.LINETYPE.ACTIVE_START); expect(messages[1].from.actor).toBe('Bob'); expect(messages[2].type).toBe(diagram.db.LINETYPE.DOTTED); From 20fd6d35f070909a8d0cef836c0feac892bd14e8 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Sun, 3 Sep 2023 10:46:26 +0530 Subject: [PATCH 4/5] refactor: Tidy up direction handling --- .../src/diagrams/sequence/sequenceRenderer.ts | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/mermaid/src/diagrams/sequence/sequenceRenderer.ts b/packages/mermaid/src/diagrams/sequence/sequenceRenderer.ts index 4bc8f7487..a596a3a02 100644 --- a/packages/mermaid/src/diagrams/sequence/sequenceRenderer.ts +++ b/packages/mermaid/src/diagrams/sequence/sequenceRenderer.ts @@ -1405,10 +1405,21 @@ const buildMessageModel = function (msg, actors, diagObj) { } const [fromLeft, fromRight] = activationBounds(msg.from, actors); const [toLeft, toRight] = activationBounds(msg.to, actors); - const arrowDirection = fromLeft <= toLeft ? 'right' : 'left'; - const startx = arrowDirection === 'right' ? fromRight : fromLeft; - let stopx = arrowDirection === 'right' ? toLeft : toRight; - const isToActivation = Math.abs(toLeft - toRight) > 2; + const isArrowToRight = fromLeft <= toLeft; + const startx = isArrowToRight ? fromRight : fromLeft; + let stopx = isArrowToRight ? toLeft : toRight; + + // As the line width is considered, the left and right values will be off by 2. + const isArrowToActivation = Math.abs(toLeft - toRight) > 2; + + /** + * Adjust the value based on the arrow direction + * @param value - The value to adjust + * @returns The adjustment with correct sign to be added to the actual value. + */ + const adjustValue = (value: number) => { + return isArrowToRight ? -value : value; + }; /** * This is an edge case for the first activation. @@ -1417,12 +1428,8 @@ const buildMessageModel = function (msg, actors, diagObj) { * In cases where the message is to an activation that was properly detected, we don't want to move the arrow head * The activation will not be detected on the first message, so we need to move the arrow head */ - if (msg.activate && !isToActivation) { - if (arrowDirection === 'right') { - stopx -= conf.activationWidth / 2 - 1; - } else { - stopx += conf.activationWidth / 2 - 1; - } + if (msg.activate && !isArrowToActivation) { + stopx += adjustValue(conf.activationWidth / 2 - 1); } /** @@ -1430,11 +1437,7 @@ const buildMessageModel = function (msg, actors, diagObj) { * This is not required for open arrows that don't have arrowheads */ if (![diagObj.db.LINETYPE.SOLID_OPEN, diagObj.db.LINETYPE.DOTTED_OPEN].includes(msg.type)) { - if (arrowDirection === 'right') { - stopx -= 3; - } else { - stopx += 3; - } + stopx += adjustValue(3); } const allBounds = [fromLeft, fromRight, toLeft, toRight]; From be3829232c061864767df8716dcbf61623c97951 Mon Sep 17 00:00:00 2001 From: Sidharth Vinod Date: Sun, 3 Sep 2023 10:54:18 +0530 Subject: [PATCH 5/5] chore: Add JSDoc to apply in sequenceDB Co-authored-by: Alois Klink --- .../mermaid/src/diagrams/sequence/sequenceDb.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/mermaid/src/diagrams/sequence/sequenceDb.js b/packages/mermaid/src/diagrams/sequence/sequenceDb.js index 69a764a98..5e593e8a5 100644 --- a/packages/mermaid/src/diagrams/sequence/sequenceDb.js +++ b/packages/mermaid/src/diagrams/sequence/sequenceDb.js @@ -452,6 +452,19 @@ export const getActorProperty = function (actor, key) { return undefined; }; +/** + * @typedef {object} AddMessageParams A message from one actor to another. + * @property {string} from - The id of the actor sending the message. + * @property {string} to - The id of the actor receiving the message. + * @property {string} msg - The message text. + * @property {number} signalType - The type of signal. + * @property {"addMessage"} type - Set to `"addMessage"` if this is an `AddMessageParams`. + * @property {boolean} [activate] - If `true`, this signal starts an activation. + */ + +/** + * @param {object | object[] | AddMessageParams} param - Object of parameters. + */ export const apply = function (param) { if (Array.isArray(param)) { param.forEach(function (item) {