-
Notifications
You must be signed in to change notification settings - Fork 209
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
Theme detection #1100
Theme detection #1100
Conversation
1e8e969
to
e7382a2
Compare
cc @Cldfire if you have any thoughts |
From my perspective it looks great! The simple variable definitions will make it really easy to add themes. Thank you :) |
Only other windows get notified when we change local storage, so we have an | ||
invisible iframe that sends us a message when local storage changes so we | ||
can detect rustdoc changing the theme |
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.
Only other windows get notified when we change local storage
Who came up with that idea??? 🤦
Well, if that's the case this looks fine.
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.
Yeah, and I was very surprised to find out that neither dedicated or shared workers get the events either, even though they should be treated as a separate context, requiring going all the way to loading another document in an iframe
.
templates/style/_themes.scss
Outdated
html { | ||
--color-background-code: #f5f5f5; | ||
--color-background: #fff; | ||
--color-border-light: lighten(#ddd, 5%); |
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.
Should probably be
--color-border-light: lighten(#ddd, 5%); | |
--color-border-light: lighten(var(--color-border), 5%); |
although not sure if that syntax works.
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.
Nope, lighten
is an SCSS function which needs the value at compile time, but lucky you mentioned it cause it turns out that function doesn't run in this context for some reason and instead lighten
is put into the output CSS, I think we need to just precalculate the values here.
lighten/darken only appear to apply when called from color properties, not for arbitrary variables.
Thanks for working on this :) |
First step of #918
Moves all the colours out of the style files and uses CSS variables instead, applying a default set on
html
, then adds a couple of snippets that detect the current rustdoc theme and copies it to adata-theme
variable onhtml
, allowing overriding the CSS variables to change the theming.Next step after this is to add overrides for all the
pure
specified colours, using CSS variables, so that we can change them; then adding colour sets matching the currentdark
andayu
rustdoc themes.