remcohaszing / rehype-mermaid

A rehype plugin to render mermaid diagrams
MIT License
68 stars 8 forks source link

Idea: dark mode support #6

Closed stereobooster closed 8 months ago

stereobooster commented 8 months ago

I was wondering if this is a good idea, for img-png, img-svg generate

<picture>
  <source media="(prefers-color-scheme: dark)" src="data:image/png;base64,iVBORw0KGgoA…" />
  <img alt="" height="215" id="mermaid-0" src="data:image/png;base64,iVBORw0KGgoA…" width="118" />
</picture>

instead of

<img alt="" height="215" id="mermaid-0" src="data:image/png;base64,iVBORw0KGgoA…" width="118" />

for this it needs to prernder diagram 2 times:

remcohaszing commented 8 months ago

This is a great idea! Dark mode is really important for accessibility.

Default behaviour should stay the same. I imagine a new option dark of type boolean | MermaidConfig. If this is defined, a dark version is included.

A couple of notes:

  1. The <source> element should specify both width and height attributes.
  2. I believe the alt attributr should stay on <img>, and <source> should not get one, right?
  3. What should happen to the title attribute?
  4. What should happen to the id attribute? Should it move to the <picture> element? Should the <picture>, <source>, and <img> elements all get id attributes?
  5. Can we support the inline-svg strategy? What would it look like?
  6. Could mermaid support responsive dark mode upstream instead? Definitely not for the PNG output, but maybe it can for SVG output?
stereobooster commented 8 months ago

I believe the alt attribute should stay on <img>, and <source> should not get one, right?

Yes I think all a11y related attributes should go to img, because

Implicit ARIA role No corresponding role Permitted ARIA roles No role permitted -- https://developer.mozilla.org/en-US/docs/Web/HTML/Element/picture#attributes

What should happen to the title attribute?

Goes to img (for the same reason as above)

What should happen to the id attribute? Should it move to the element? Should the , , and elements all get id attributes?

Easiest option, which would be backward compatible is to leave it on <img> (I think)

Can we support the inline-svg strategy? What would it look like?

Ideally (but not possible with mermaid, I think) if all colors would be specified using CSS-variables (or CSS classes), then Inline SVG could be styled with CSS, just as any other element. Then it would be enough to render one inline SVG and dark theme would be implemented witth the help of CSS.

stereobooster commented 8 months ago

A bit of context: I'm looking for "ideal" diagram solution for Astro. See my post

remcohaszing commented 8 months ago

A bit of context: I'm looking for "ideal" diagram solution for Astro. See my post

I have some insights for your research:

What should happen to the id attribute? Should it move to the Easiest option, which would be backward compatible is to leave it on (I think)

Backwards compatibility isn’t an issue if we add a new option. The HTML output is going to be different anyway.

I’m leaning towards either:

<picture>
  <source id="mermaid-dark-0" … />
  <img id="mermaid-0" … />
</picture>

Or:

<picture id="mermaid-0">
  <source … />
  <img … />
</picture>

Can we support the inline-svg strategy? What would it look like?

Ideally (but not possible with mermaid, I think) if all colors would be specified using CSS-variables (or CSS classes), then Inline SVG could be styled with CSS, just as any other element. Then it would be enough to render one inline SVG and dark theme would be implemented witth the help of CSS.

Hence the idea that maybe support could be added by Mermaid instead.

Anyway, we could support dark mode for the img-png and img-svg strategies first, and investigate the inline-svg strategy later. Note that you need the inline-svg strategy if you want people to be able to search the text.

stereobooster commented 8 months ago

Thank you for adding details. I will update article.

RE id attribute: to be fair I'm not sure what is the intended use for it. Is it for JS or for a11y or for...?

remcohaszing commented 8 months ago

The mermaid CSS targets the id, which is relevant only for the inline-svg strategy.

I’m not entirely sure why I added it for the other strategies. I suppose because the information is available. :shrug: It allows you to link the diagram using a URL hash, or target it using CSS. I’m not sure if that’s useful. Maybe I should remove it in the next major version.

stereobooster commented 8 months ago

It seems they (Astro team) had the similar problem with Shiki highlighter, which doesn't support css variables, so they are putting some predefined colors and replace with variables before outputting

const ASTRO_COLOR_REPLACEMENTS: Record<string, string> = {
    '#000001': 'var(--astro-code-color-text)',
    '#000002': 'var(--astro-code-color-background)',
    '#000004': 'var(--astro-code-token-constant)',
    '#000005': 'var(--astro-code-token-string)',
    '#000006': 'var(--astro-code-token-comment)',
    '#000007': 'var(--astro-code-token-keyword)',
    '#000008': 'var(--astro-code-token-parameter)',
    '#000009': 'var(--astro-code-token-function)',
    '#000010': 'var(--astro-code-token-string-expression)',
    '#000011': 'var(--astro-code-token-punctuation)',
    '#000012': 'var(--astro-code-token-link)',
};

https://github.com/withastro/astro/blob/3011f15d0012eb095273a505695eb25e643ab98b/packages/markdown/remark/src/shiki.ts#L9-L25 🤔