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

fix mermaid dark mode #1115

Closed
wants to merge 1 commit into from
Closed

fix mermaid dark mode #1115

wants to merge 1 commit into from

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Mar 20, 2024

This PR adopts dark to define the color of mermaid charts.

This fixes two issues:

  • when the theme is not reactive to dark mode (when it has bee defined to be always light or always dark), the function could be wrong as it was based on user preference; it is now consistent with the actual colors of the page.
  • when the user preference changed, the charts did not update; now they are regenerated automatically (when generated by the template literal; if you're using the function, you still have to make your expression depend explicitly on dark).

@Fil Fil requested a review from mbostock March 20, 2024 09:06
Copy link
Member

@mbostock mbostock left a comment

Choose a reason for hiding this comment

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

Hmm, I don’t like the idea that the mermaid tagged template literal behaves differently (non-responsively) than the Mermaid code blocks. That means if you use the tagged template literal directly, either in Markdown or by importing it into a JavaScript component, it won’t be responsive.

I’m not sure this is really worth the trouble to fix. But I guess it would mean that the contents of the element returned by mermaid could change later (be rewritten) if the theme changes?

Maybe the better solution is to define our own theme that uses CSS variables instead of hard-coded colors, and then mermaid doesn’t need to be responsive at all. I think it’s possible to define custom themes that could reference our --theme-* variables.

Update: Mermaid requires colors to be specified as hex, so CSS custom properties presumably aren’t supported. So, I think we probably do nothing here except perhaps fix the static check to look at the computed color-scheme instead of the preferred color scheme.

@mbostock mbostock mentioned this pull request Mar 21, 2024
@Fil
Copy link
Contributor Author

Fil commented Mar 21, 2024

superseded by #1122

@Fil Fil closed this Mar 21, 2024
@Fil Fil deleted the fil/mermaid-dark-mode branch March 21, 2024 23:59
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

2 participants