-
-
Notifications
You must be signed in to change notification settings - Fork 8.4k
Add support for markdown-style links #6158
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
base: develop
Are you sure you want to change the base?
Add support for markdown-style links #6158
Conversation
🦋 Changeset detectedLatest commit: ba259a0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for mermaid-js ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
commit: |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #6158 +/- ##
==========================================
- Coverage 4.48% 3.88% -0.60%
==========================================
Files 383 399 +16
Lines 54038 42121 -11917
Branches 596 638 +42
==========================================
- Hits 2425 1638 -787
+ Misses 51613 40483 -11130
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Works great in the preview!!! @ryanwjackson, the link in
appears to be slightly below the "Look at" at the preview link above for me. |
|
@bernhardkaindl do you have an image of it? I see it as vertically aligned. The underline of the linked text is underneath, but the top and bottom of the text lines up for me. Let me know if I'm missing something. |
|
Hi @ryanwjackson thanks a bunch for implementing! I've tested in the preview (thanks @bernhardkaindl for pointing this out). And links on arrow messages are working, the alignment also seems as expected (using latest Edge).
The note link also basically works when it's a "simple" URL:
However, I've noticed two things:
Thanks and regards! EDIT: |
Hi @DanielSmithGK - Encoding the
|
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
|
I made some updates to address those issues. Let me know if that resolves them. |
|
Hi @ryanwjackson thanks for the updates, I've checked the stuff I reported and these look resolved. Thank you! |
|
@DanielSmithGK any clue who can review this or how to get it merged? |
|
@DanielSmithGK Any progress on this? |
|
@ryanwjackson Cool addition! Do you think it's possible to apply the same to the participant names in the diagram? I'd imagine it to be very helpful to be able to link documentation pages for each participant. |
|
@rngcntr You can adding link to participant in current version. It is under Actor Menus in the sequence diagram page. I really don't understand why this feature didn't get into release after it had been done for 10 months. Mermaid diagram is adopted by AI mostly. Its slow progress of evolution makes me disappointed. |
|
@awangatsanjel I thought more of a direct link without the need of a drop down, ideally with the capability to reference relative document locations instead of full URLs. |
|
@rngcntr I am with you on the thought of direct link. Mermaid was started as a diagram drawing tool, however it occasionally used markdown as the descripting language. Since AI adopted it markdown because it is pure text can be processed by the LLM, Mermaid became the default diagram language unless you request other specific diagram format. So, Mermaid is turning to a modeling tool. That the way I have used Mermaid for use. GitHub makes it as extension, it is used a lot in the issues and wiki. So it is evolved from offline diagraming tool to online tool. I don't know if the developers had realized this trends. I hope they can move this feature to the production faster. |
|
Hi @sidharthv96, I've no idea if you could help with this or point us in any direction with this issue here (or maybe tag the correct person, if you know them): The argos check fails for this PR, and it seems like a false-positive. There are no changes noticeable for us, however, the AI thinks there's something off. Do you perhaps know if this can be approved manually (or skip it) somehow? Apologies if you're the wrong person, I've just (desperately) checked the last couple repo activities, perhaps you have some insights how we could advance with this. Thanks and kind regards! |
sidharthv96
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code, implementation, and documentation is great for this particular feature. The text below is not a reflection of the work you've done, but rather due to a tricky situation mermaid is in regarding markdown support.
We have an ongoing issue with consistent (and universal) markdown support, mainly around syntax for markdown.
The main issue I see is that we have a proposed syntax for flowcharts, and some diagrams support markdown without that syntax. So it creates confusion.
Currently sequence diagrams do not support markdown strings. So adding just URL support, without the other formatting options, and without the markdown syntax, creates more edge cases while we try to unify markdown syntax across the diagrams.
I will bring this PR for discussion in our next team meeting and give you an update. Thanks for your patience.
| it('should render links', () => { | ||
| imgSnapshotTest( | ||
| ` | ||
| sequenceDiagram | ||
| Alice->John: Look at [MermaidJS](https://mermaid.js.org/) | ||
| John->>Alice: Thanks for the link | ||
| ` | ||
| ); | ||
| }); | ||
| it('should handle different line breaks', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need more test cases, with the edge cases that have been discussed in previous comments.
| 'mermaid': minor | ||
| --- | ||
|
|
||
| feat:Added support for markdown-style links in line messages and notes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| feat:Added support for markdown-style links in line messages and notes | |
| feat(sequence): Add support for markdown-style links in line messages and notes |
|
Standardising markdown support was discussed as the exclusive topic in today's weekly call. |








📑 Summary
Add support for markdown-style links on line messages and notes
Resolves #1279
📏 Design Decisions
This currently only affects line messages and notes on sequence diagrams
📋 Tasks
Make sure you
MERMAID_RELEASE_VERSIONis used for all new features.pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.