Merge pull request #4804 from mermaid-js/fix/4691_sequenceArrowHeads

Align arrows on sequence diagram

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

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;
 };
@@ -450,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) {
@@ -530,7 +545,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);

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",
@@ -548,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);

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,47 @@ 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 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.
+   * 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 && !isArrowToActivation) {
+    stopx += adjustValue(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)) {
+    stopx += adjustValue(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 +1458,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,

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')