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

Render mermaid codeblocks #479

Closed
wants to merge 2 commits into from

Conversation

ben-krieger
Copy link

@ben-krieger ben-krieger commented Aug 18, 2022

This PR adds support for rendering mermaid in codeblocks / code fences.

Examples I was able to find for this feature, like https://anis.se/posts/add-mermaidjs-support-to-hugo/, had a few issues.

  1. Imported mermaid more than once
  2. Imported mermaid from different sources/versions
  3. Did not support dark mode

Sorry for not creating an issue first, like it recommends in CONTRIBUTING.md. However, I figured that this probably wasn't something that would be desired for the mainline release, since it changes existing behavior. If I'm wrong, maybe it can be put behind a config flag.

I also inserted a personal fix for mermaid-js/mermaid#2481, which may need to be omitted if this PR is to be considered for merge.

Maintainers: feel free to close this PR if you're not interested in the feature. I just hope that it helps some poor fool like past me who searches for "geekdoc mermaid codeblock / code fence support!"

EDIT: My branch is currently based off of v0.33.2, because Mermaid v9.1.4 is broken in such a way that having multiple sequence diagrams in one page cumulatively joins them! So sequence 1, sequence 1+2, sequence 1+3.

EDIT 2: Mermaid v9.1.5 and v9.1.6 seem to fix the error, so I will rebase this to merge into main, which already has updated to v9.1.5.

EDIT 3: The mermaid 9.1.4 bug being referenced was mermaid-js/mermaid#3305 and was fixed here: mermaid-js/mermaid#3310

@thegeeklab-bot
Copy link

thegeeklab-bot commented Aug 18, 2022

LHCI Report Overview

Report URL Performance Accessibility Best-Practices SEO PWA
Link http://localhost:37051/ 97 % 100 % 92 % 98 % 30 %
Link http://localhost:37051/404.html 87 % 100 % 100 % 100 % 30 %
Link http://localhost:37051/posts/ 91 % 100 % 100 % 100 % 30 %

@ben-krieger ben-krieger force-pushed the mermaid-code-fence branch 3 times, most recently from f72414f to cb08968 Compare August 19, 2022 18:22
@xoxys
Copy link
Member

xoxys commented Aug 19, 2022

Thanks for your effort. I would prefer an implementation using rendering hooks as this approach can be used for other extensions e.g. KaTeX as well. This way, it should be also possible to re-use the same code from the existing shortcode.

@ben-krieger
Copy link
Author

Thanks for your effort. I would prefer an implementation using rendering hooks as this approach can be used for other extensions e.g. KaTeX as well. This way, it should be also possible to re-use the same code from the existing shortcode.

I didn't realize you could do that for codeblocks now in Hugo! I'll take a look.

@ben-krieger
Copy link
Author

And the solution is literally given in the docs... https://gohugo.io/content-management/diagrams/#mermaid-diagrams

But there is a small amount of work needed to integrate with the existing mermaid shortcode so that using a codeblock and a shortcode in the same page doesn't result in importing and initializing mermaid twice.

@xoxys
Copy link
Member

xoxys commented Aug 20, 2022

The shortcode has already a check using the exact same code in the render template should avoid double imoorts.

@ben-krieger
Copy link
Author

The shortcode has already a check using the exact same code in the render template should avoid double imoorts.

I just mean change .Page.Store.Get("hasMermaid") from the doc example to match .Page.Scratch.Get("mermaid") from the theme source (and matching Set commands). :)

@ben-krieger
Copy link
Author

I removed mermaid.init call which specified the two possible mermaid class names (mermaid and language-mermaid) and added rendering hook for a mermaid codeblock instead. I also rebased onto main.

// Fix height of mermaid SVG elements (see https://github.com/mermaid-js/mermaid/issues/2481)
callback: (id) => {
document.getElementById(id).setAttribute("height", "100%")
}
Copy link
Author

Choose a reason for hiding this comment

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

Should I move this change to a separate PR?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, please create a dedicated PR, make it easier in case we need to revert it. Wondering why this was not fixed upstream yet if a fix would be that simple 🤔

@ben-krieger ben-krieger changed the title Render mermaid from code fences Render mermaid codeblocks Aug 21, 2022
Comment on lines +1 to +12
<!-- prettier-ignore-start -->
{{ if not (.Page.Scratch.Get "mermaid") }}
<!-- Include mermaid only first time -->
<script defer src="{{ index (index .Site.Data.assets "mermaid.js") "src" | relURL }}"></script>
{{ .Page.Scratch.Set "mermaid" true }}
{{ end }}
<!-- prettier-ignore-end -->

{{ .Page.Scratch.Set "mermaid" true }}
<pre class="gdoc-mermaid mermaid">
{{- .Inner -}}
</pre>
Copy link
Author

Choose a reason for hiding this comment

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

Is there somewhere in the docs to specify a minimum Hugo version? This requires v0.93.0 or later, which was only released at the end of February 2022.

Copy link
Member

Choose a reason for hiding this comment

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

@@ -22,6 +22,12 @@ npm install

# run the build script to build required assets
npm run build

Copy link
Member

Choose a reason for hiding this comment

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

I would like to move this to a dedicated PR as well and implement it as a reusable NPM script. It also needs to be documented in the docs as well.

@xoxys
Copy link
Member

xoxys commented Aug 29, 2022

Superseded by: #485 #486 #488

@xoxys xoxys closed this Aug 29, 2022
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.

None yet

3 participants