-
Notifications
You must be signed in to change notification settings - Fork 448
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
dynamic dark theme based on CSS class selector #1803
base: master
Are you sure you want to change the base?
Conversation
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.
Thanks for this PR. Let me understand the context a bit more.
- Certain websites add a CSS class to all images when dark mode is turned on.
- This flag makes it so that this CSS class flows down into D2's CSS to control dark mode.
The downside is obviously that the image is no longer portable -- it's generated to be on a certain website.
It seems that apps/sites that do this type of dark mode configuration know that this is a limitation on most images and just have you specify two images, e.g.:
- https://docusaurus.io/docs/markdown-features/assets#github-style-themed-images
- https://github.blog/changelog/2021-11-24-specify-theme-context-for-images-in-markdown/
The benefit of using D2 is that you can have all your diagrams adapt with just 1 image.
I guess rendering an SVG for a particular website and thus having it not be portable for others is fine. I can't think of anything better and recognize this is a benefit for these use cases.
I think this flag should be either class or ID. If it starts with .something
, it's a class. If it's #something
, it's ID.
edit: nvm, IDs don't make sense here.
The "dark" class is technically applied to the
Writerside also supports this, enabling us to generate or provide two different static images, but I wouldn't say it is a downside. When rendering SVGs into bitmap format, the user loses the ability to copy text from the diagrams. To me, this is a valuable feature. Additionally, we should also consider scaling — Retina screens, for example, require 2x resolution images to provide crisp, detailed visuals. |
Cool and lastly, can you sign your PR please @develar ? We have it turned on as an organization-wide setting. |
* introduce `--dark-theme-class`: the CSS class to enable dark mode. When left unset prefers-color-scheme media query is used; * add test for combined light and dark themes (original test uses light theme in dark theme test, not clear why); When D2-produced SVG file is embedded into some documentation/site framework where dark mode is controller by CSS class, using `prefers-color-scheme` media query leads to inability to switch dark theme if needed. Media Query Level 5, where it is possible to use some class as condition, is not supported by any modern browser.
52d4880
to
c2e881e
Compare
Sign Git commits? Done. |
--dark-theme-class
: the CSS class to enable dark mode. When left unset prefers-color-scheme media query is used;When D2-produced SVG file is embedded into some documentation/site framework where dark mode is controlled by CSS class, using
prefers-color-scheme
media query leads to inability to switch dark theme if needed. Writerside issue. Same for mkdocs and so on. IntelliJ D2 plugin also will use a new flag.Media Query Level 5, where it is possible to use some class as a condition, is not supported by any modern browser.
dark-dynamic.mov