From 1118c3399ca2f53cb93c4757a363d4ad91f6b98e Mon Sep 17 00:00:00 2001 From: Chris Moran Date: Fri, 19 Jun 2020 08:19:03 -0400 Subject: [PATCH] Fixed incorrect positioning and bounds for actors and notes with wrapping enabled and disabled --- src/diagrams/sequence/sequenceDiagram.spec.js | 33 +++++ src/diagrams/sequence/sequenceRenderer.js | 131 +++++++++--------- src/themes/dark/index.scss | 2 +- src/utils.js | 7 +- 4 files changed, 102 insertions(+), 71 deletions(-) diff --git a/src/diagrams/sequence/sequenceDiagram.spec.js b/src/diagrams/sequence/sequenceDiagram.spec.js index 736bf04b8..1963457fc 100644 --- a/src/diagrams/sequence/sequenceDiagram.spec.js +++ b/src/diagrams/sequence/sequenceDiagram.spec.js @@ -1274,6 +1274,39 @@ end`; }); }); +describe('issue fixes', function() { + beforeEach(() => { + parser.yy = sequenceDb; + parser.yy.clear(); + mermaidAPI.reset(); + mermaidAPI.initialize({ logLevel: 0, theme: 'dark' }); + renderer.bounds.init(); + }); + it('should fix issue 1480', function() { + const str = ` + sequenceDiagram +participant Event as Very long name goes here +participant API as Some API +participant API2 as A different API +participant Client as Another very long name goes here + +activate Event +Event ->> Event: Something important happens +Note over Event,Client: Some explanation goes here +activate Client +Event --x Client: Publish event +deactivate Event + +deactivate Client`; + parser.parse(str); + renderer.draw(str, 'tst'); + const bounds = renderer.bounds.getBounds(); + const actors = parser.yy.getActors(); + const messages = parser.yy.getMessages(); + expect(bounds).toBeTruthy(); + }) +}); + describe('when rendering a sequenceDiagram with actor mirror activated', function() { let conf; beforeEach(function() { diff --git a/src/diagrams/sequence/sequenceRenderer.js b/src/diagrams/sequence/sequenceRenderer.js index fa882e1f5..6be5456db 100644 --- a/src/diagrams/sequence/sequenceRenderer.js +++ b/src/diagrams/sequence/sequenceRenderer.js @@ -128,10 +128,10 @@ export const bounds = { this.updateBounds(_startx, _starty, _stopx, _stopy); }, - newActivation: function(message, diagram) { - const actorRect = parser.yy.getActors()[message.from.actor]; + newActivation: function(message, diagram, actors) { + const actorRect = actors[message.from.actor]; const stackedSize = actorActivations(message.from.actor).length; - const x = actorRect.x + conf.width / 2 + ((stackedSize - 1) * conf.activationWidth) / 2; + const x = actorRect.x + actorRect.width / 2 + ((stackedSize - 1) * conf.activationWidth) / 2; this.activations.push({ startx: x, starty: this.verticalPos + 2, @@ -436,7 +436,7 @@ export const drawActors = function(diagram, actors, actorKeys, verticalPos) { const actor = actors[actorKeys[i]]; // Add some rendering data to the object - actor.width = typeof actor.width === 'undefined' ? calculateActorWidth(actor) : actor.width; + actor.width = actor.width ? actor.width : conf.width; actor.height = conf.height; actor.margin = conf.actorMargin; @@ -489,38 +489,10 @@ const actorFlowVerticaBounds = function(actor) { return [left, right]; }; -/** - * This calculates the actor's width, taking into account both the statically configured width, - * and the actor's description. - * - * If the description text has greater length, we extend the width of the actor, so it's description - * won't overflow. - * - * @param actor - An actor object - * @return - The width for the given actor - */ -const calculateActorWidth = function(actor) { - if (!actor.description) { - return conf.width; - } - - return actor.wrap - ? conf.width - : Math.max( - conf.width, - utils.calculateTextWidth(actor.description, { - fontSize: conf.actorFontSize, - fontFamily: conf.actorFontFamily, - fontWeight: conf.actorFontWeight, - margin: conf.wrapPadding - }) - ); -}; - function adjustLoopHeightForWrap(loopWidths, msg, preMargin, postMargin, addLoopFn) { let heightAdjust = 0; bounds.bumpVerticalPos(preMargin); - if (msg.message && msg.wrap && loopWidths[msg.message]) { + if (msg.message && loopWidths[msg.message]) { let loopWidth = loopWidths[msg.message].width; let minSize = Math.round((3 * conf.fontSize) / 4) < 10 @@ -532,9 +504,10 @@ function adjustLoopHeightForWrap(loopWidths, msg, preMargin, postMargin, addLoop fontWeight: conf.messageFontWeight, margin: conf.wrapPadding }; - msg.message = msg.message + msg.message = msg.wrap ? utils.wrapLabel(`[${msg.message}]`, loopWidth, textConf) - : msg.message; + : `[${msg.message}]`; + heightAdjust = Math.max( 0, utils.calculateTextHeight(msg.message, textConf) - (preMargin + postMargin) @@ -559,7 +532,6 @@ export const draw = function(text, id) { let startx; let stopx; - let forceWidth; // Fetch data from the parsing const actors = parser.yy.getActors(); @@ -573,6 +545,7 @@ export const draw = function(text, id) { drawActors(diagram, actors, actorKeys, 0); const loopWidths = calculateLoopMargins(messages, actors); + logger.debug('actors:', actors); // The arrow head definition is attached to the svg once svgDraw.insertArrowHead(diagram); svgDraw.insertArrowCrossHead(diagram); @@ -616,12 +589,18 @@ export const draw = function(text, id) { fontWeight: conf.noteFontWeight, margin: conf.wrapPadding }; - textWidth = utils.calculateTextWidth(msg.message, textConf); - noteWidth = shouldWrap ? conf.width : Math.max(conf.width, textWidth); + textWidth = utils.calculateTextWidth( + shouldWrap ? utils.wrapLabel(msg.message, conf.width, textConf) : msg.message, + textConf + ); + noteWidth = shouldWrap ? conf.width : Math.max(conf.width, textWidth + 2 * conf.noteMargin); + logger.debug( + `msg:${msg.message} start:${startx} stop:${stopx} tw:${textWidth} nw:${noteWidth}` + ); if (msg.placement === parser.yy.PLACEMENT.RIGHTOF) { if (shouldWrap) { - msg.message = utils.wrapLabel(msg.message, noteWidth, textConf); + msg.message = utils.wrapLabel(msg.message, noteWidth - 2 * conf.wrapPadding, textConf); } drawNote( diagram, @@ -632,7 +611,7 @@ export const draw = function(text, id) { ); } else if (msg.placement === parser.yy.PLACEMENT.LEFTOF) { if (shouldWrap) { - msg.message = utils.wrapLabel(msg.message, noteWidth, textConf); + msg.message = utils.wrapLabel(msg.message, noteWidth - 2 * conf.wrapPadding, textConf); } drawNote( diagram, @@ -643,8 +622,17 @@ export const draw = function(text, id) { ); } else if (msg.to === msg.from) { // Single-actor over + textWidth = utils.calculateTextWidth( + shouldWrap + ? utils.wrapLabel(msg.message, Math.max(conf.width, actors[msg.to].width), textConf) + : msg.message, + textConf + ); + noteWidth = shouldWrap + ? Math.max(conf.width, actors[msg.to].width) + : Math.max(actors[msg.to].width, conf.width, textWidth + 2 * conf.noteMargin); if (shouldWrap) { - msg.message = utils.wrapLabel(msg.message, noteWidth, textConf); + msg.message = utils.wrapLabel(msg.message, noteWidth - 2 * conf.wrapPadding, textConf); } drawNote( diagram, @@ -655,23 +643,25 @@ export const draw = function(text, id) { ); } else { // Multi-actor over - forceWidth = Math.abs(startx - stopx) + conf.actorMargin / 2; + let noteStart = startx + actors[msg.from].width / 2; + let noteEnd = stopx + actors[msg.to].width / 2; + noteWidth = Math.abs(noteStart - noteEnd) + conf.actorMargin; if (shouldWrap) { - noteWidth = forceWidth; msg.message = utils.wrapLabel(msg.message, noteWidth, textConf); - } else { - noteWidth = Math.max(forceWidth, textWidth - 2 * conf.noteMargin); } let x = startx < stopx - ? startx + (actors[msg.from].width - conf.actorMargin / 2) / 2 - : stopx + (actors[msg.to].width - conf.actorMargin / 2) / 2; + ? startx + actors[msg.from].width / 2 - conf.actorMargin / 2 + : stopx + actors[msg.to].width / 2 - conf.actorMargin / 2; - drawNote(diagram, x, bounds.getVerticalPos(), msg, forceWidth); + logger.debug( + `msg:${msg.message} start:${startx} stop:${stopx} tw:${textWidth} nw:${noteWidth}` + ); + drawNote(diagram, x, bounds.getVerticalPos(), msg, noteWidth); } break; case parser.yy.LINETYPE.ACTIVE_START: - bounds.newActivation(msg, diagram); + bounds.newActivation(msg, diagram, actors); break; case parser.yy.LINETYPE.ACTIVE_END: activeEnd(msg, bounds.getVerticalPos()); @@ -967,7 +957,31 @@ const getMaxMessageWidthPerActor = function(actors, messages) { * @param actorToMessageWidth - A map of actor key -> max message width it holds */ const calculateActorMargins = function(actors, actorToMessageWidth) { + const textConf = { + fontSize: conf.actorFontSize, + fontFamily: conf.actorFontFamily, + fontWeight: conf.actorFontWeight, + margin: conf.wrapPadding + }; let maxHeight = 0; + Object.keys(actors).forEach(prop => { + const actor = actors[prop]; + if (actor.wrap) { + actor.description = utils.wrapLabel( + actor.description, + conf.width - 2 * conf.wrapPadding, + textConf + ); + } + const actDims = utils.calculateTextDimensions(actor.description, textConf); + actor.width = actor.wrap + ? conf.width + : Math.max(conf.width, actDims.width + 2 * conf.wrapPadding); + + actor.height = actor.wrap ? Math.max(actDims.height, conf.height) : conf.height; + maxHeight = Math.max(maxHeight, actor.height); + }); + for (let actorKey in actorToMessageWidth) { const actor = actors[actorKey]; @@ -975,13 +989,6 @@ const calculateActorMargins = function(actors, actorToMessageWidth) { continue; } - const textConf = { - fontSize: conf.actorFontSize, - fontFamily: conf.actorFontFamily, - fontWeight: conf.actorFontWeight, - margin: conf.wrapPadding - }; - const nextActor = actors[actor.nextActor]; // No need to space out an actor that doesn't have a next link @@ -989,18 +996,6 @@ const calculateActorMargins = function(actors, actorToMessageWidth) { continue; } - [actor, nextActor].forEach(function(act) { - if (act.wrap) { - act.description = utils.wrapLabel(act.description, conf.width, textConf); - } - const actDims = utils.calculateTextDimensions(act.description, textConf); - act.width = act.wrap ? conf.width : Math.max(conf.width, actDims.width); - - act.height = act.wrap ? Math.max(actDims.height, conf.height) : conf.height; - logger.debug(`Actor h:${act.height} ${actDims.height} d:${act.description}`); - maxHeight = Math.max(maxHeight, act.height); - }); - const messageWidth = actorToMessageWidth[actorKey]; const actorWidth = messageWidth + conf.actorMargin - actor.width / 2 - nextActor.width / 2; diff --git a/src/themes/dark/index.scss b/src/themes/dark/index.scss index d05159177..46b2a621e 100644 --- a/src/themes/dark/index.scss +++ b/src/themes/dark/index.scss @@ -10,7 +10,7 @@ $arrowheadColor: $mainContrastColor; /* Flowchart variables */ $nodeBkg: $mainBkg; -$nodeBorder: purple; +$nodeBorder: $border1; $clusterBkg: $secondBkg; $clusterBorder: $border2; $defaultLinkColor: $lineColor; diff --git a/src/utils.js b/src/utils.js index dae028c01..3cffc3f46 100644 --- a/src/utils.js +++ b/src/utils.js @@ -494,6 +494,9 @@ export const drawSimpleText = function(elem, textData) { }; export const wrapLabel = (label, maxWidth, config) => { + if (!label) { + return label; + } config = Object.assign( { fontSize: 12, fontWeight: 400, fontFamily: 'Arial', margin: 15, joinWith: '
' }, config @@ -596,7 +599,7 @@ export const calculateTextDimensions = function(text, config) { { fontSize: 12, fontWeight: 400, fontFamily: 'Arial', margin: 15 }, config ); - const { fontSize, fontFamily, fontWeight, margin } = config; + const { fontSize, fontFamily, fontWeight } = config; if (!text) { return 0; } @@ -638,7 +641,7 @@ export const calculateTextDimensions = function(text, config) { g.remove(); // Adds some padding, so the text won't sit exactly within the actor's borders - return { width: maxWidth + 2 * margin, height: height + 2 * margin }; + return { width: Math.round(maxWidth), height: Math.round(height) }; }; export default {