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

Auto activation of mermaid diagrams (via use of hugo render hook) #990

Merged
merged 3 commits into from
Oct 27, 2022

Conversation

stephanlachnit
Copy link
Contributor

This switches Mermaid to use hugo's Markdown
render hooks
instead of manipulating the string at runtime, which
should improve page loading performance by a tiny bit.

Similar to #987, can probably also be done for PlantUML and MarkMap.

This could potentially improved even further by setting {{ .Page.Store.Set "hasXXX" true }} in render-codeblock-XXX.html and adjusting scripts.html so that these corresponding JavaScript functions are loaded only on that page instead of bundling them to a global script. This would also lift the requirements to set params.XXX.enable = true. However since this is a more complicated endeavor and I'm not really familiar with js, I did not attempt to do this.

@deining
Copy link
Collaborator

deining commented Oct 5, 2022

This could potentially improved even further by setting {{ .Page.Store.Set "hasXXX" true }} in render-codeblock-XXX.html and adjusting scripts.html so that these corresponding JavaScript functions are loaded only on that page instead of bundling them to a global script. This would also lift the requirements to set params.XXX.enable = true.

I like the idea. However render hooks for code blocks are availabe as of hugo version 0.93 only, so implementing this feature would increase the minimum hugo version needed when authoring mermaid or UML diagrams.

@stephanlachnit
Copy link
Contributor Author

stephanlachnit commented Oct 5, 2022

I like the idea. However render hooks for code blocks are availabe as of hugo version 0.93 only, so implementing this feature would increase the minimum hugo version needed when authoring mermaid or UML diagrams.

Yes, indeed. v0.93 was released in February, so it is not that new considering that hugo is a "just download a single binary to use" program. I don't know what the release plans are, but maybe it can be merged just after v0.5.0 so that there is a reasonable time span in between the release after that and hugo v0.93.

Copy link
Collaborator

@deining deining left a comment

Choose a reason for hiding this comment

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

As said, I like the idea to use render hooks here. However render hooks for code blocks are availabe as of hugo version 0.93 only. I reflected about your submitted code changes and found a way to implement this feature without the need to increase the required hugo version to 0.93 when authoring mermaid diagrams. See my code review for details.

This could potentially improved even further by setting {{ .Page.Store.Set "hasXXX" true }} in render-codeblock-XXX.html and adjusting scripts.html so that these corresponding JavaScript functions are loaded only on that page instead of bundling them to a global script. This would also lift the requirements to set params.XXX.enable = true.

This is very true. See my PR #1286 for details on how to implement this useful feature. As proposed, I too would prefer to load the scripts inside the render hook rather than activating them on a global basis as we do now. For the sake of compatibility I would keep the activation in the site config file, though.

However since this is a more complicated endeavor and I'm not really familiar with js, I did not attempt to do this.

That's fine. I can take over that part once the issues in my code review were addressed and this PR was merged.

assets/js/mermaid.js Show resolved Hide resolved
@deining
Copy link
Collaborator

deining commented Oct 19, 2022

I had a second thought on this and came up with an alternative solution that I would like to share:

  • we leave the code renderblock as proposed by this PR
  • inside mermaid.js, we use conditional output depending on the hugo version used:
(function($) {
    var needMermaid = false;
{{ if ge hugo.Version "0.93.0" -}}
    if ($('.mermaid').length > 0) {
        needMermaid = true;
    };
{{ else -}}
    $('.language-mermaid').parent().replaceWith(function() {
        needMermaid = true;
        return $('<pre class="mermaid">').text($(this).text());
    });
{{ end -}}

This way, we can keep the performance gain introduced with this PR while maintaining compatibility with hugo versions <0.93.3.

WDYT?

@stephanlachnit
Copy link
Contributor Author

WDYT?

Sounds like a good idea, feel free to push to this branch

@deining deining force-pushed the p/mermaid-via-render-hook branch 2 times, most recently from 008c27e to 8107dab Compare October 21, 2022 11:35
Copy link
Collaborator

@deining deining left a comment

Choose a reason for hiding this comment

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

`mermaid´ code blocks now work all hugo versions. For hugo versions <=0.92, you still need to manually activate mermaid.

@deining deining changed the title performance: use hugo render hook for Mermaid Auto activation of mermaid diagrams (via use of hugo render hook) Oct 21, 2022
@deining
Copy link
Collaborator

deining commented Oct 21, 2022

This PR is ready for review and subsequent merging now:

  • with hugo versions >=0.93.0, the use of a mermaid code block inside your page automatically enabled mermaid diagrams.
  • with hugo versions <0.93.0, mermaid diagrams still work. You still must activate mermaid support in your config file, though.

The documentation was adapted in order to reflect the changes introduced with this patch.

Advantages introduced with this PR:

  • more user friendly use of mermaid diagrams, no manual activation needed (with hugo version >= 0.93.0).
  • mermaid scripts are only loaded for pages that actually make use of mermaid diagrams.
  • mermaid was updated to latest version 9.1.7.

@geriom @chalin: Can you please review and report back if you like these changes? If so, we could take them over for other diagram types (plantuml, markmap) as well.

@deining deining requested a review from geriom October 21, 2022 13:00
stephanlachnit and others added 2 commits October 26, 2022 21:09
This switches Mermaid to use hugo's Markdown
render hooks [1] instead of manipulating the string at runtime, which
should improve performance by a bit.

[1]: https://gohugo.io/templates/render-hooks/#render-hooks-for-code-blocks

Signed-off-by: Stephan Lachnit <stephanlachnit@debian.org>
Copy link
Collaborator

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Overall, LGTM. I haven't thoroughly reviewed the code, but I trust y'all.

Consider adding a note to the changelog so that folks using Mermaid will know that they can simplify their config if they're using Hugo 0.93+.

@LisaFC - did you want to review the UG changes?

assets/js/mermaid.js Outdated Show resolved Hide resolved
layouts/partials/scripts.html Outdated Show resolved Hide resolved
@deining
Copy link
Collaborator

deining commented Oct 27, 2022

Consider adding a note to the changelog so that folks using Mermaid will know that they can simplify their config if they're using Hugo 0.93+.

Done.

@LisaFC - can you review the UG changes, please? Thanks in advance.

@deining deining requested review from LisaFC and removed request for geriom October 27, 2022 07:15
Copy link
Collaborator

@LisaFC LisaFC left a comment

Choose a reason for hiding this comment

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

User guide changes LGTM!

Copy link
Collaborator

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Thanks all!

@chalin chalin merged commit 90843e3 into google:main Oct 27, 2022
@stephanlachnit stephanlachnit deleted the p/mermaid-via-render-hook branch October 28, 2022 08:42
marshalc added a commit to ouhft/docsy that referenced this pull request Dec 23, 2022
commit 324942a
Author: Patrice Chalin <chalin@users.noreply.github.com>
Date:   Mon Dec 19 12:15:42 2022 -0500

    Docsy examples: drop private-repo (google#1338)

commit 799fc54
Author: Andreas Deininger <andreas@deininger.net>
Date:   Thu Dec 8 14:36:44 2022 +0100

    Bump Font-Awesome to latest version 6.2.1 (google#1325)

commit 8924f28
Author: Geri Ochoa <gerino@google.com>
Date:   Mon Dec 5 13:45:54 2022 -0500

    Update post-release changelog links

commit 5597d43
Author: Geri Ochoa <gerino@google.com>
Date:   Mon Dec 5 11:18:13 2022 -0500

    Release v0.6.0 preparation

commit 2c11f53
Author: Andreas Deininger <adeininger@urbanonline.de>
Date:   Fri Oct 14 17:23:14 2022 +0200

    Add render hook for 'chem' code blocks
    Auto activation of `math' and 'chem' blocks
    Add ability to enable KateX and/or mhchem for individual pages only
    Upgrade to KaTeX 0.16.3

commit 1fb0e23
Author: Victor "multun" Collod <victor.collod@epita.fr>
Date:   Fri Nov 25 22:36:26 2022 +0100

    prevent navbar text wrapping

    When a navbar item has multiple words, and there are enough navbar item to
    cause the flexbox to be full at reduced window widths without transitioning
    through the lower media breakpoint, the navbar link name can wrap, which causes
    the alignment of all navbar items to break.

    This change forbids navbar text wrapping even at bigger screen sizes.

commit 33f3dfe
Author: Andreas Deininger <andreas@deininger.net>
Date:   Wed Nov 23 08:48:12 2022 +0100

    Fix: mermaid diagrams with parameters in site config

commit b4d66f7
Merge: f0a06ef 9cc0cd0
Author: Erin McKean <emckean@google.com>
Date:   Mon Nov 21 09:07:08 2022 -0800

    Merge pull request google#1314 from google/LisaFC-fontawesome

    Add FontAwesome info to logos and images page

commit 9cc0cd0
Merge: f18a4e8 f0a06ef
Author: LisaFC <lcarey@google.com>
Date:   Mon Nov 21 17:04:42 2022 +0000

    Merge branch 'main' into LisaFC-fontawesome

commit f0a06ef
Author: copy rogers <40619032+rogerogers@users.noreply.github.com>
Date:   Fri Nov 18 20:17:34 2022 +0800

    Add etcd, CloudWeGo and Dapr website to examples (google#1315)

    Signed-off-by: rogerogers <rogers@rogerogers.com>

    Signed-off-by: rogerogers <rogers@rogerogers.com>

commit 1c9e125
Author: LisaFC <lcarey@google.com>
Date:   Thu Nov 17 16:17:25 2022 +0000

    Updated to better "mostly docs" example

commit c6f138a
Author: Robert Pająk <pellared@hotmail.com>
Date:   Thu Nov 17 17:09:41 2022 +0100

    Add OpenTelemetry webpage to examples (google#1312)

commit f18a4e8
Author: LisaFC <lcarey@google.com>
Date:   Thu Nov 17 16:05:11 2022 +0000

    Update iconsimages.md

commit 0cc3704
Author: LisaFC <lcarey@google.com>
Date:   Thu Nov 17 16:01:23 2022 +0000

    Add FontAwesome info to logos and images page

    Currently only mention FontAwesome in the context of menus, which has confused some users

commit 246a2e1
Author: Geri Ochoa <gerino@google.com>
Date:   Tue Nov 8 11:34:43 2022 -0500

    Fix readfile not rendering html code

commit 90ee481
Author: Patrice Chalin <chalin@users.noreply.github.com>
Date:   Wed Nov 9 11:14:17 2022 -0500

    Update CHANGELOG.md

commit e6a44df
Author: Takayama Fumihiko <tekezo@pqrs.org>
Date:   Thu Nov 3 21:48:20 2022 +0900

    Fix offline search popover (google#1305)

commit 42f6ef8
Author: LisaFC <lcarey@google.com>
Date:   Thu Nov 3 11:56:22 2022 +0000

    Algolia updates (google#1270)

    Updates for google#1250

commit aa7c0ae
Author: erwin-faceit <43995928+erwin-faceit@users.noreply.github.com>
Date:   Wed Nov 2 21:15:29 2022 +0100

    Update nl (google#1301)

    I18N: update nl.toml

commit 33fd6ea
Author: mboukhalfa <mohammed.boukhalfa@est.tech>
Date:   Sun Oct 23 23:53:13 2022 +0300

    Add more translation to ar.toml

commit d9bf98b
Author: Andreas Deininger <andreas@deininger.net>
Date:   Fri Oct 28 17:22:30 2022 +0200

    CHANGELOG: add breaking change (tabbed panes, text display)

commit 90843e3
Author: Stephan Lachnit <stephanlachnit@debian.org>
Date:   Thu Oct 27 17:00:08 2022 +0200

    Auto activation of mermaid diagrams (via use of hugo render hook) (google#990)

commit f27daed
Author: Geri Ochoa <gerino@google.com>
Date:   Fri Oct 21 13:46:19 2022 -0400

    Release v0.5.1 preparation

commit dd2e2b9
Author: Andreas Deininger <adeininger@urbanonline.de>
Date:   Wed Oct 19 16:27:39 2022 +0200

    Tabbed pane, fix: avoid duplicate html id if content is pulled in via readfile shortcode (google#1289)

commit a2f7ecd
Author: Andreas Deininger <adeininger@urbanonline.de>
Date:   Fri Oct 14 08:41:24 2022 +0200

    User guide: fix invalid html by adding alt attribute to image

    Remove trailing whitespace in a few .md files

commit cfa70e3
Author: Andreas Deininger <adeininger@urbanonline.de>
Date:   Fri Oct 14 08:35:51 2022 +0200

    Fix: Tabpane shows multiple tabs at the same time google#1271

commit abb7307
Author: Andreas Deininger <adeininger@urbanonline.de>
Date:   Thu Oct 13 12:30:22 2022 +0200

    Shortcodes for tabbed panes: improved whitespace handling

commit 99eacb0
Author: Andreas Deininger <adeininger@urbanonline.de>
Date:   Sat Oct 15 12:49:29 2022 +0200

    Module setup: update docsy/dependencies to tip of main

commit 123800f
Author: Stephan Lachnit <stephanlachnit@debian.org>
Date:   Sun Oct 16 17:32:02 2022 +0200

    feature: add support for GLFM math blocks (google#987)

    * feature: add support for GLFM math blocks

    This add supports for GLFM's math blocks [1] using hugo's Markdown
    render hooks [2].

    [1]: https://docs.gitlab.com/ee/user/markdown.html#math
    [2]: https://gohugo.io/templates/render-hooks/#render-hooks-for-code-blocks

    Signed-off-by: Stephan Lachnit <stephanlachnit@debian.org>

commit be5da07
Author: Andreas Deininger <andreas@deininger.net>
Date:   Fri Oct 14 18:16:17 2022 +0200

    Fix: hugo serve does not serve Font Awesome anymore (google#1282)

commit 1e7e329
Author: the-kraljica <25987891+the-kraljica@users.noreply.github.com>
Date:   Wed Oct 5 17:14:26 2022 +0200

    Remove > selector from elements

commit 19ec7be
Author: Patrice Chalin <chalin@users.noreply.github.com>
Date:   Thu Oct 13 16:15:01 2022 -0400

    Search styling refactoring, and fix for offline search (google#1279)

commit ba4ed72
Author: Mike Kruskal <62662355+mkruskal-google@users.noreply.github.com>
Date:   Thu Oct 13 00:37:57 2022 -0700

    Fixing FontAwesome icon handling in search placeholder (google#1247)

commit 7489c35
Author: Patrice Chalin <pchalin@gmail.com>
Date:   Mon Oct 10 13:08:44 2022 -0400

    CHANGELOG adjustments

commit ded6a0d
Author: Patrice Chalin <chalin@users.noreply.github.com>
Date:   Mon Oct 10 12:22:25 2022 -0400

    Revert google#1200 to recover desc blog post order (google#1269)

commit 14cee5a
Author: Patrice Chalin <pchalin@gmail.com>
Date:   Sat Oct 8 11:25:31 2022 -0400

    Scripts: one more whitespace tweak

commit 0e79123
Author: Patrice Chalin <chalin@users.noreply.github.com>
Date:   Sat Oct 8 11:18:56 2022 -0400

    Scripts: get Popper from Bootstrap bundle (google#1268)

commit 0204dd9
Author: Patrice Chalin <chalin@users.noreply.github.com>
Date:   Sat Oct 8 11:02:03 2022 -0400

    Revert to Popper v1 so tooltips work (google#1266)

commit c4216a8
Author: Patrice Chalin <pchalin@gmail.com>
Date:   Sat Oct 8 10:32:08 2022 -0400

    More whitespace cleanup - partials/scripts.html

commit 20b0c70
Author: Patrice Chalin <chalin@users.noreply.github.com>
Date:   Sat Oct 8 09:45:52 2022 -0400

    Scripts tweak to cleanup whitespace etc (google#1262)

commit 25b0205
Author: Patrice Chalin <chalin@users.noreply.github.com>
Date:   Sat Oct 8 09:03:57 2022 -0400

    CHANGELOG: adjust prose since FA was further upgraded (google#1261)

commit 264566c
Author: Andreas Deininger <andreas@deininger.net>
Date:   Sat Oct 8 12:57:30 2022 +0200

    Upgrade to Bootstrap 4.6.2 (module install) (google#1218)

commit 3147661
Author: Andreas Deininger <andreas@deininger.net>
Date:   Fri Oct 7 22:35:35 2022 +0200

    User guide, conversion to modules: fixing config.yaml (google#1209)

commit 326b0f1
Author: Patrice Chalin <chalin@users.noreply.github.com>
Date:   Fri Oct 7 12:13:39 2022 -0400

    Navbar-brand: define classes for the logo and name (google#1190)

commit 646ef44
Author: Geri Ochoa <gerino@google.com>
Date:   Thu Oct 6 18:24:29 2022 -0400

    Release v0.5.0 prep, fix module dependency

commit 0c183ef
Author: Geri Ochoa <gerino@google.com>
Date:   Fri Sep 30 10:12:08 2022 -0400

    Release v0.5.0 preparation

commit de7048f
Author: Geri Ochoa <gerino@google.com>
Date:   Wed Sep 28 15:45:24 2022 -0400

    Add click to copy button

    Adds a click to copy button to code samples that don't use the Prism
    highlighter.

commit 5c4662c
Author: Geri Ochoa <gerino@google.com>
Date:   Wed Sep 28 17:47:10 2022 -0400

    Fix importing wrong drawio script

commit bd991a5
Author: Patrice Chalin <pchalin@gmail.com>
Date:   Tue Oct 4 19:20:55 2022 -0400

    Update CHANGELOG.md

commit 7f9dc28
Author: Patrice Chalin <pchalin@gmail.com>
Date:   Wed Aug 17 15:57:38 2022 -0400

    Use gtag.js analytics library for all site tags

commit c937bcd
Author: Andreas Deininger <andreas@deininger.net>
Date:   Wed Oct 5 01:47:21 2022 +0200

    Userguide, installation via npm: correct themes directory (google#1213)

commit 07ff9a5
Author: Patrice Chalin <chalin@users.noreply.github.com>
Date:   Tue Oct 4 19:18:04 2022 -0400

    CHANGELOG: mention FA font-glyph width changes (google#1251)

commit 01bf2a2
Author: Patrice Chalin <chalin@users.noreply.github.com>
Date:   Tue Oct 4 18:40:27 2022 -0400

    Mention inadvertent style leakage in CHANGELOG (google#1167)
dseynaev pushed a commit to openanalytics/docsy that referenced this pull request Jan 30, 2023
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

4 participants