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

Mermaid: allow customizing sequence diagram notes node style #5824

Closed
4 tasks done
sisp opened this issue Aug 9, 2023 · 9 comments · Fixed by #5940
Closed
4 tasks done

Mermaid: allow customizing sequence diagram notes node style #5824

sisp opened this issue Aug 9, 2023 · 9 comments · Fixed by #5940
Labels
change request Issue requests a new feature or improvement resolved Issue is resolved, yet unreleased if open

Comments

@sisp
Copy link
Contributor

sisp commented Aug 9, 2023

Context

The color of Mermaid sequence diagram notes looks a bit off IMO (see, e.g., https://squidfunk.github.io/mkdocs-material/reference/diagrams/#using-sequence-diagrams), at least not as nicely integrated with the rest of the theme colors.

Description

mkdocs-material overrides some Mermaid colors with own CSS variables but not the "note" node. How about introducing new CSS variables, e.g. --md-mermaid-note-bg-color as the fill color and --md-mermaid-note-fg-color as the stroke color (consistent with existing naming), and add an override for Mermaid's note CSS class? The default colors could be based on the info admonition color.

There exists some customization for note text, so perhaps not customizing the note rect itself was an oversight.

Related links

Use Cases

Having a nicely integrated Mermaid diagram makes documentation more pleasant to read. It's mostly good already, but sequence diagram notes look a bit off.

Visuals

No response

Before submitting

@squidfunk
Copy link
Owner

Thanks for suggesting. Yes, we can certainly make Mermaid diagrams more configurable in the future 😊

@squidfunk squidfunk added the change request Issue requests a new feature or improvement label Aug 9, 2023
@nejch
Copy link
Contributor

nejch commented Aug 9, 2023

@squidfunk awesome! Would you accept a little PR for this? I think even the default schemes could benefit from this, see the current demo in slate:

image

@squidfunk
Copy link
Owner

squidfunk commented Aug 10, 2023

Yes, but we should actually add an additional layer of abstraction for each component in each diagram type, so that you can comfortable overwrite it with CSS. If you would like to work on this, a PR would be amazing. We could start with the sequence diagram and after then translate the solution over to the other types 😊

@squidfunk squidfunk changed the title Support customizing Mermaid sequence diagram notes node style Mermaid: allow customizing sequence diagram notes node style Aug 10, 2023
@nejch
Copy link
Contributor

nejch commented Aug 11, 2023

Yes, but we should actually add an additional layer of abstraction for each component in each diagram type, so that you can comfortable overwrite it with CSS. If you would like to work on this, a PR would be amazing. We could start with the sequence diagram and after then translate the solution over to the other types blush

Thanks @squidfunk! Just to clarify, do you mean simply in the color variables, e.g --md-mermaid-label-bg-color -> --md-mermaid-sequence-label-bg-color and so on, or narrowing down the selectors themselves based on diagram types?

@squidfunk
Copy link
Owner

Yes. I'd say it might make sense to introduce a little hierarchy to comfortably style Mermaid.js components, and specific components inside diagrams. I haven't made up my mind how a hierarchy of CSS variables could look like, but I'd be happy if somebody was up to do the legwork and submit a PR for it. We need a first draft which we can discuss ☺️ As with all of Material for MkDocs, if the approach proposed could have sane defaults, and make it easy to deviate where desired, plus be extensible to account for new components that Mermaid.js adds, we're all for it!

@squidfunk
Copy link
Owner

@nejch @sisp did you manage to start working on this topic? I just want to make sure we're not duplicating efforts.

@sisp
Copy link
Contributor Author

sisp commented Aug 26, 2023

@squidfunk Not yet, but it's on my list for next week. Thanks for asking and checking! 👍

@sisp
Copy link
Contributor Author

sisp commented Sep 1, 2023

I've submitted a PR: #5940

@squidfunk squidfunk added the resolved Issue is resolved, yet unreleased if open label Sep 2, 2023
@squidfunk
Copy link
Owner

Released as part of 9.2.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
change request Issue requests a new feature or improvement resolved Issue is resolved, yet unreleased if open
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants