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.

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

Add markdown support to sequence diagram notes #5705

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

schadenn
Copy link

@schadenn schadenn commented Aug 9, 2024

📑 Summary

This adds markdown support using Marked (including highlighting).
Took some inspiration from the other seemingly abandoned PRs.

Please review & let me know if the approach I took is fine 🙏

Resolves #1844 #4381 #4725 #5460

📏 Design Decisions

Instead of adding markdown syntax to the mermaid jison syntax definition (like the other PRs tried to do), I just added the option to use a [ to open and ] to close a note. Inbetween the brackets, newlines are supported and the whole string will be fed into marked.

🎨 Demo

Screenshot 2024-08-09 at 11 13 01

📋 Tasks

Make sure you

Copy link

netlify bot commented Aug 9, 2024

Deploy Preview for mermaid-js ready!

Name Link
🔨 Latest commit 04732b9
🔍 Latest deploy log https://app.netlify.com/sites/mermaid-js/deploys/66c3330a173f4c000812eee5
😎 Deploy Preview https://deploy-preview-5705--mermaid-js.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

codecov bot commented Aug 9, 2024

Codecov Report

Attention: Patch coverage is 12.73885% with 274 lines in your changes missing coverage. Please review.

Project coverage is 5.26%. Comparing base (c5eb07c) to head (04732b9).

Files Patch % Lines
packages/mermaid/src/diagrams/sequence/styles.js 0.00% 133 Missing ⚠️
packages/mermaid/src/diagrams/common/common.ts 37.37% 62 Missing ⚠️
packages/mermaid/src/diagrams/sequence/svgDraw.js 0.00% 51 Missing ⚠️
.../mermaid/src/diagrams/sequence/sequenceRenderer.ts 0.00% 24 Missing ⚠️
packages/mermaid/src/utils.ts 50.00% 3 Missing ⚠️
...kages/mermaid/src/diagrams/common/svgDrawCommon.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           develop   #5705      +/-   ##
==========================================
+ Coverage     5.22%   5.26%   +0.04%     
==========================================
  Files          322     323       +1     
  Lines        46201   46510     +309     
  Branches       561     537      -24     
==========================================
+ Hits          2415    2451      +36     
- Misses       43786   44059     +273     
Flag Coverage Δ
unit 5.26% <12.73%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...kages/mermaid/src/diagrams/common/svgDrawCommon.ts 0.00% <0.00%> (ø)
packages/mermaid/src/utils.ts 41.84% <50.00%> (+0.12%) ⬆️
.../mermaid/src/diagrams/sequence/sequenceRenderer.ts 0.00% <0.00%> (ø)
packages/mermaid/src/diagrams/sequence/svgDraw.js 0.00% <0.00%> (ø)
packages/mermaid/src/diagrams/common/common.ts 49.37% <37.37%> (-3.51%) ⬇️
packages/mermaid/src/diagrams/sequence/styles.js 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

Copy link

argos-ci bot commented Aug 9, 2024

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) ⚠️ Changes detected (Review) 46 changed, 1 added Aug 19, 2024, 12:08 PM

@schadenn
Copy link
Author

schadenn commented Sep 9, 2024

@sidharthv96 Hi there 👋
Not really sure how to follow up on this (and with whom). I think the only check failing is due to changed visuals, which I can not approve. Otherwise I think everything should be ready. Wdyt about the approach? I would really like to get markdown support in, so let me know if I should explore other ways instead.

@sidharthv96
Copy link
Member

Hi @schadenn,
There is some ongoing work to standardise rendering, which would cover markdown (basic stuff, not code highlights and all). This might also bring markdown support to all strings, which means we won't have to do special handling. The details are not yet confirmed, that's why we haven't reviewed this PR.

Also, HighlightJS is a big dependency, which increases the bundle size significantly.

@John-Paul-R
Copy link

@sidharthv96 Is there a tracking Issue or PR we can follow for this "bring markdown support to all strings"? I am interested in this feature!

@sidharthv96
Copy link
Member

These two are related. Other diagrams needs to be migrated to the new approach.

#5825
#5604

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using Sequence Diagram is there any way to make some parts of the text in bold?
3 participants