Use data-theme attribute for themes on root element #1706
Conversation
* Add custom CSS property --color-scheme * Remove theme check in setTheme function
* Also declare --color-scheme property under each theme selector
|
This seems reasonable, but I'm not sure it's sufficient. If you add a new dark theme option, but the user's OS theme is set to light, code blocks in the rendered page will be rendered with the light theme when selecting your dark theme... Is there any major benefit to using a data-theme attribute rather than keeping with classes? Doing this switch is a breaking change that doesn't seem necessary. |
|
Based on my testing, the code blocks will match the selected theme. https://typedoc-theme-test-shane4368.surge.sh/classes/Bird.html#beginFlight Using classes for themes would require additional logic to replace the current theme with the selected theme. <html class="default no-js not-mobile toggle-visibilityprivate toggle-inherited dark">Using the above as reference, if you wanted to change theme, you'd have to do
If it's a breaking change, you may add this whenever you decide to release 0.23. |
|
Tracking the current theme doesn't seem too bad to me - at least until 0.23, where the data attribute is probably better going forward. I didn't know about the colored scrollbars possibility - that's neat, and a really compelling reason to move to Just a note: The hardcoded hl-1 through hl-7 might work today, but there's no guarantee that it works in the future. Those classes are completely dependent on the themes the user chooses. If someone writes a theme that has 50 different colors, it'll behave badly. I'd really rather give plugins like yours a way to handle this nicely. Maybe themes in the dropdown could specify two values? |
|
The hl colours being out of my control was an oversight.
Like setting a custom hl theme before the html gets written to file, if it's supported? I'm okay with that. |
|
I was thinking of making values contain two items, e.g. Unfortunately supporting custom highlighter themes for each scheme is expensive, adding ~400ms per 1000 lines of code highlighted for each theme (shikijs/shiki#234) |
|
Oh. That should be fine. |
Use CSS color-scheme property
This sets light/dark theme on scrollbars and form elements.
Remove switch statement and directly apply theme in setTheme function
This change allows additional themes added by a plugin to work without attaching a new listener to #theme element; also prevents rechecking of localStorage to add the custom theme to body.classList.