Setup dark mode based on work in rdf-primer and elsewhere. #88
Conversation
gkellogg
added
the
spec:editorial
spec/index.html
Outdated
| --hue-5-2: #be5046; | ||
| --hue-6: #d19a66; | ||
| --hue-6-2: #e6c07b; | ||
| } |
There was a problem hiding this comment.
This might lead to bugs in the future if respec/hljs is updated. I don't think we pin ReSpec version. I am going to open a ticket in ReSpec about it.
There was a problem hiding this comment.
@Tpt That would be great! I failed to act on w3c/rdf-primer#24 and haven't moved this CSS to rdf-common yet. But fixing it in ReSpec would be much better for all! (I probably should have gone that route from the beginning.)
(I might follow with what I've come up with re. highlighting, since ReSpec seems to handle that in a somewhat deprecated manner (CommonJS vs. ESM).)
There was a problem hiding this comment.
I removed the hljs styling. Until it is applied to ReSpec, example blocks will continue to have a white background in dark mode, but better not tie that style into the HTML on a one-off basis.
There was a problem hiding this comment.
I commented on speced/respec#4871 (comment) to mention the tentative solution for the Primer. Unless someone picks it up there, I might have some time for a PR on ReSpec, which as @Tpt says would be for the best (and the only robust solution).
There was a problem hiding this comment.
I also commented on speced/respec#4871 (comment) Sadly, fixing properly does not sound trivial.
There was a problem hiding this comment.
@Tpt Important comment! (I agree it's not trivial, alas; but I think you're on to something there!)
There was a problem hiding this comment.
Aiming to solve this via speced/respec#4925.
| @@ -52,13 +53,16 @@ | |||
| </script> | |||
|
|
|||
| <style> | |||
| body { color-scheme: light; } | |||
There was a problem hiding this comment.
This prevents darkmode to work when JS is not enabled but not sure if it's a big deal.
There was a problem hiding this comment.
That should be handled in speced/respec#4925 (to avoid the JS dependency both for hljs and SVGs). If that PR lands, the two places where color-scheme is used this PR is obsolete (since they depend on JS)). Do we want to hold off on merging dark mode on RDF specs until we get a signal on if respec will accept the fix?
There was a problem hiding this comment.
Imho we should merge now. JS only dark mode is better than no dark mode.
There was a problem hiding this comment.
Agreed; with the added suggestion to remove those two lines to avoid required further changes (the use of color-scheme here should only affect SVGs).
| body { color-scheme: light; } | ||
| body.darkmode { color-scheme: dark; } |
There was a problem hiding this comment.
| body { color-scheme: light; } | |
| body.darkmode { color-scheme: dark; } |
These should only affect linked SVGs (which this spec does not currently use). Since general support for that will be handled by respec if speced/respec#4925 is merged, removing these lines should happen anyway when that lands, to avoid hard-to-spot regressions (since the upstream fix is not JS-dependent).
Some simplifications to table styling were done for simplicity. Not sure that the subtle use of color for tables in light-mode is as effective in dark-mode.
Preview | Diff