Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Node with text renders by default as Markdown #6087

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
+72 −28
Open

fix: Node with text renders by default as Markdown #6087

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f27ca07
Adding back create label for the cases where you do not want markdown…
knsv Nov 27, 2024
c7880d7
Adding changeset
knsv Nov 27, 2024
468b6a5
Update cypress/integration/rendering/flowchart.spec.js
jgreywolf Dec 2, 2024
4693aa2
Merge branch 'develop' into 5824-diff-between-markdown-and-strings
jgreywolf Dec 2, 2024
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
5 changes: 5 additions & 0 deletions .changeset/gold-ducks-sort.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'mermaid': patch
---

fix: Proper separation between strings and markdown strings
22 changes: 21 additions & 1 deletion cypress/integration/rendering/flowchart.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -895,7 +895,7 @@ graph TD
imgSnapshotTest(
`
graph TD
classDef default fill:#a34,stroke:#000,stroke-width:4px,color:#fff
classDef default fill:#a34,stroke:#000,stroke-width:4px,color:#fff
hello --> default
`,
{ htmlLabels: true, flowchart: { htmlLabels: true }, securityLevel: 'loose' }
Expand All @@ -917,4 +917,24 @@ graph TD
}
);
});
it('68: should be able to render string and markdown labels (#5824)', () => {
imgSnapshotTest(
`
flowchart TB
mermaid{"What is\nyourmermaid version?"} --> v10["<11"] --"\`<**1**1\`"--> fine["No bug"]
mermaid --> v11[">= v11"] -- ">= v11" --> broken["Affected by https://github.com/mermaid-js/mermaid/issues/5824"]
subgraph subgraph1["\`How to fix **fix**\`"]
broken --> B["B"]
end
githost["Github, Gitlab, BitBucket, etc."]
githost2["\`Github, Gitlab, BitBucket, etc.\`"]
a["1."]
b["- x"]
`,
{
flowchart: { htmlLabels: true },
securityLevel: 'loose',
}
);
});
});
26 changes: 13 additions & 13 deletions cypress/platform/knsv2.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@
D-->I
D-->I
</pre>
<pre id="diagram4" class="mermaid">
<pre id="diagram4" class="mermaid2">
---
config:
layout: elk
Expand Down Expand Up @@ -185,19 +185,19 @@
</pre>
<pre id="diagram4" class="mermaid">
---
config:
layout: elk
title: https://github.com/mermaid-js/mermaid/issues/5824
---
flowchart LR
subgraph s1["Untitled subgraph"]
n1["Evaluate"]
n2["Option 1"]
end
n1 -- One --> n2




%% 6048, 5824
flowchart TB
mermaid{"What is\nyourmermaid version?"} --> v10["<11"] --"`<**1**1`"--> fine["No bug"]
mermaid --> v11[">= v11"] -- ">= v11" --> broken["Affected by https://github.com/mermaid-js/mermaid/issues/5824"]
subgraph subgraph1["`How to fix **fix**`"]
broken --> B["B"]
end
githost["Github, Gitlab, BitBucket, etc."]
githost2["`Github, Gitlab, BitBucket, etc.`"]
a["1."]
b["- x"]
</pre>
<pre id="diagram4" class="mermaid">
---
Expand Down
2 changes: 2 additions & 0 deletions packages/mermaid/src/diagrams/flowchart/flowDb.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,7 @@ const addNodeFromVertex = (
const baseNode = {
id: vertex.id,
label: vertex.text,
labelType: vertex.labelType,
labelStyle: '',
parentId,
padding: config.flowchart?.padding || 8,
Expand Down Expand Up @@ -1002,6 +1003,7 @@ export const getData = () => {
end: rawEdge.end,
type: rawEdge.type ?? 'normal',
label: rawEdge.text,
labelType: rawEdge.labelType,
labelpos: 'c',
thickness: rawEdge.stroke,
minlen: rawEdge.length,
Expand Down
16 changes: 10 additions & 6 deletions packages/mermaid/src/rendering-util/rendering-elements/edges.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,16 @@ export const getLabelStyles = (styleArray) => {
export const insertEdgeLabel = async (elem, edge) => {
let useHtmlLabels = evaluate(getConfig().flowchart.htmlLabels);

const labelElement = await createText(elem, edge.label, {
style: getLabelStyles(edge.labelStyle),
useHtmlLabels,
addSvgBackground: true,
isNode: false,
});
const labelElement =
edge.labelType === 'markdown'
? await createText(elem, edge.label, {
style: getLabelStyles(edge.labelStyle),
useHtmlLabels,
addSvgBackground: true,
isNode: false,
})
: await createLabel(edge.label, getLabelStyles(edge.labelStyle), undefined, false);

log.info('abc82', edge, edge.labelType);

// Create outer g, edgeLabel, this will be positioned after graph layout
Expand Down
29 changes: 21 additions & 8 deletions packages/mermaid/src/rendering-util/rendering-elements/shapes/util.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import createLabel from '../createLabel.js';
import { createText } from '../../createText.js';
import type { Node } from '../../types.js';
import { getConfig } from '../../../diagram-api/diagramAPI.js';
Expand Down Expand Up @@ -40,14 +41,26 @@ export const labelHelper = async <T extends SVGGraphicsElement>(
label = typeof node.label === 'string' ? node.label : node.label[0];
}

const text = await createText(labelEl, sanitizeText(decodeEntities(label), getConfig()), {
useHtmlLabels,
width: node.width || getConfig().flowchart?.wrappingWidth,
// @ts-expect-error -- This is currently not used. Should this be `classes` instead?
cssClasses: 'markdown-node-label',
style: node.labelStyle,
addSvgBackground: !!node.icon || !!node.img,
});
let text;
if (node.labelType !== 'string') {
Copy link

Choose a reason for hiding this comment

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

Is there a reason why this is !== 'string' rather than === 'markdown' as was done in the edge?

As I understand it, in:

graph LR
  foo[foo] --> bar["bar"] --> baz["`baz`"]

| name | labelType |
|------+-----------|
| foo  | text      |
| bar  | string    |
| baz  | markdown  |

My position is that both text and string labelTypes should be rendered as plain text, and only labelType markdown should be rendered as markdown. Is there some case where that falls down for your needs?

text = await createText(labelEl, sanitizeText(decodeEntities(label), getConfig()), {
useHtmlLabels,
width: node.width || getConfig().flowchart?.wrappingWidth,
// @ts-expect-error -- This is currently not used. Should this be `classes` instead?
cssClasses: 'markdown-node-label',
style: node.labelStyle,
addSvgBackground: !!node.icon || !!node.img,
});
} else {
const labelElement = await createLabel(
sanitizeText(decodeEntities(label), getConfig()),
node.labelStyle,
false,
true
);
text = labelEl.node()?.appendChild(labelElement);
}

// Get the size of the label
let bbox = text.getBBox();
const halfPadding = (node?.padding ?? 0) / 2;
Expand Down
Loading