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
docs: add mermaid plugin and replace image #2606
Conversation
Jira: This PR is not related to a Jira item. (The PR title does not include a SC-#### reference) GitHub Issues: Launchpad Bugs: No Launchpad bugs are fixed by this PR. (No commits have LP: #### references) 👍 this comment to confirm that this is correct. |
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.
Nice, thanks for doing this! I just have two suggestions.
-
The diagram looks really nice when rendered on the light theme (which is the one I use by default). However, when you switch to dark mode it's really hard to see the arrows and where they point. I don't know if there's a way to specify "if light mode then this theme, if dark mode then that", but mermaid does allow different themes to be used https://mermaid.js.org/config/theming.html - probably worth doing, if it's possible.
-
It's probably a good idea for us to include at least a link to how to create these diagrams in the devdocs. It's not the easiest or most intuitive system to use, but the docs at least are really good. If we'd like people to use mermaid to reduce the maintenance burden of images in the future, let's make it real easy for them to find the info (to avoid explaining it ourselves). I'd suggest maybe this one about the basic syntax - it's easy to find more specific diagram info from here in their navigation https://mermaid.js.org/syntax/flowchart.html?id=flowcharts-basic-syntax
@s-makin excellent points! I just updated this PR to include dark mode support (by using the furo theme css variables) and to add a short dev-doc with some helpful links to getting started with mermaid. LMK what you think! |
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.
LGTM
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 good to me! Thanks for working on it :)
Why is this needed?
Fixes: #2559
Test Steps
Make sure the diagram is rendered correctly and is an appropriate replacement.
Make sure it works for each of
tox -e docs
,tox -e docs-dev
and on readthedocsChecklist
Does this PR require extra reviews?