-
Notifications
You must be signed in to change notification settings - Fork 22.6k
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
feat: Add SVG diagrams for HTTP sequences #31812
Conversation
Preview URLs (comment last updated: 2024-02-12 11:03:08) |
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.
Can you embed the Mermaid source somewhere, either in the Markdown or in the SVG, as a comment? Makes it easier to make modifications in the future.
Yeah, it's collapsed in the PR description at the moment, but it would be nice to capture it in git. Another option is to put it in https://github.com/mdn/shared-assets/tree/main/images to keep it out of source comments here, what do you think? edit: Actually in the source is not that bad. We could do: See the diagram:
<!--
sequence.svg Mermaid syntax:
```mermaid
# ...
```
-->
![A sequence diagram](sequence.svg) |
Another would be to have it in a file next to xyz.svg (xyz.mermaid) that will not be deployed by yari. That way they would be next to each other. |
I also like this suggestion, that's a good one. I'm going to bring it up in content/eng sync so that Yari team are aware. |
Obviously mermaid support in Yari would be the preferred solution - is there an associated yari PR for that? There is another option - https://mermaid-js.github.io/ provides a mechanism to allow you to click a link to get some text that embeds the image and links to the place where it is edited. The flaw is that it relies on the service existing and running when needed. I.e.
Gives (click on image - the text for this image/link is exported in the actions section) |
Not yet, this might be something to consider for incoming Yari refactoring this year.
That's clever. I agree it's tricky to rely on a service like this. npm install -g @mermaid-js/mermaid-cli
mmdc -i input.mmd -o output.svg We could generate SVGs from Re: this PR I am happy to constrain it to replacing the .png files with SVG. I have checked everything in here so it's not lost: https://github.com/mdn/shared-assets/tree/main/images/diagrams/http |
FWIW this is what I already do for my mermaid diagrams. |
I would be happy to approve this, but I assume you're still going to add the mermaid text in comments as per #31812 (comment) or as a text file. Personally I think the right way to do this is to create an issue for the yari support for mermaid (can you/have you done this?). On the assumption that this will happen at some point having the text in the doc content is better. |
Done in 4236fe9
Sure, I've opened this one which might be a good tracker -> mdn/yari#10497 |
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.
Looks great to me. Thanks for this.
Thanks, all |
Description
While we do not yet have support for Mermaid inline in markdown, I've converted a few
.png
diagrams to SVG using https://www.mermaidchart.com.Motivation
Removing binaries from the repo, improving maintenance of diagrams, easing transition to mermaid syntax in future.
Additional details
Mermaid syntax
Related issues and pull requests