documentation icon indicating copy to clipboard operation
documentation copied to clipboard

Update diagrams on terms.md

Open samerfarida opened this issue 1 year ago • 8 comments

Updated the simple and advanced diagrams as a code using mermaid

samerfarida avatar Jul 29 '24 14:07 samerfarida

Sorry for the wait @samerfarida! This will get looked into.

rowansc1 avatar Sep 29 '24 18:09 rowansc1

Is there something missing from your PR? Seems this mermaid stuff isn't something it knows how to render. https://pr-595.ptero-doc-preview.pages.dev/project/terms.html

danny6167 avatar Sep 30 '24 06:09 danny6167

Is there something missing from your PR? Seems this mermaid stuff isn't something it knows how to render. https://pr-595.ptero-doc-preview.pages.dev/project/terms.html

There is no issue with the current PR itself. However, I wasn't aware that the .md file would be converted to .html, which introduces the rendering problem. After reviewing the Mermaid.js documentation at https://mermaid.js.org/config/usage.html#simple-full-example, it seems that in order to properly render Mermaid diagrams in HTML, the code needs to be wrapped as follows:

<!-- Other HTML code -->

<pre class="mermaid">
    <!-- Insert Mermaid code here -->
</pre>
<script type="module">
    import mermaid from 'https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.esm.min.mjs';
</script>

<!-- More HTML code -->

For example, I have attached an .html file that demonstrates this. Please download it (rename it by removing the .txt extension) and open it locally in a browser to see a Mermaid diagram rendered within HTML. Below is a sample of the Mermaid code inside the HTML structure:

mermaidTest.html.txt

<!doctype html>
<html lang="en">
  <body>
    <pre class="mermaid">
      graph LR
      A --- B
      B-->C[fa:fa-ban forbidden]
      B-->D(fa:fa-spinner);
    </pre>
    <script type="module">
      import mermaid from 'https://cdn.jsdelivr.net/npm/mermaid@11/dist/mermaid.esm.min.mjs';
    </script>
  </body>
</html>

At this point, we have three possible options moving forward:

  1. Save the Mermaid diagrams as .png files and replace the current diagrams. This way, they will render correctly on both GitHub and the website.
  2. Modify the build process to wrap the Mermaid code in the required HTML structure during the conversion from .md to .html, as recommended by Mermaid.js.
  3. I can update the current PR to include the necessary HTML structure. However, this will prevent the Mermaid code from rendering correctly in the .md file on GitHub, though it will work on the website.

Please let me know which option you and the Pterodactyl team would prefer.

Thanks, Sammy

samerfarida avatar Oct 03 '24 00:10 samerfarida

Hi Sammy,

Thank you very much for that detailed analysis.

Personally, I think option 2 is the most preferable. However, option 3 would also still be fine as the docs are meant to be viewed on the website anyway.

What do you think @danny6167 ?

rowansc1 avatar Oct 03 '24 00:10 rowansc1

I really don't want to see us adding little hacks to the build process for things like this. In-lining HTML, especially with external dependencies (something we don't do) really isn't a great option either.

pre-rendered images is really the only valid option of the options that's been listed. But the ideal solution would be if there was a native solution for vuepress to be able to render the mermaid syntax.

It really doesn't matter how it looks in github, what matters is how it looks when it's tested after the vuepress build.

danny6167 avatar Oct 03 '24 20:10 danny6167

I really don't want to see us adding little hacks to the build process for things like this. In-lining HTML, especially with external dependencies (something we don't do) really isn't a great option either.

pre-rendered images is really the only valid option of the options that's been listed. But the ideal solution would be if there was a native solution for vuepress to be able to render the mermaid syntax.

It really doesn't matter how it looks in github, what matters is how it looks when it's tested after the vuepress build.

Hey @danny6167, I have no problems with pre-rendered images. I did a quick search on https://vuepress.vuejs.org/ and it looks like they have Markdown plugins that can be enabled.

Apparently, they do have a Mermaid plugin for your VuePress site. Please review. It looks like it’s an easy install and enablement. This way, you guys are set for future usages of diagrams as code!

Please let me know if you still want me to correct this PR to include a pre-rendered images?

Thank you,

Sammy

samerfarida avatar Oct 04 '24 20:10 samerfarida

That's a good find @samerfarida - I'll take a look at that and update soon if it works out.

danny6167 avatar Oct 06 '24 08:10 danny6167

Hi!

Just letting you know I haven't forgotten about this.

We can't merge this right now. Looks like we are going to have to update some dependencies, possibly even move to the next version of vuepress to be able to support this properly. That's going to take some time.

Leave the PR open, and we will merge it when the rest of the code is ready for it.

Thank you for your contribution!

danny6167 avatar Oct 15 '24 10:10 danny6167