-
Notifications
You must be signed in to change notification settings - Fork 101
Technical exploration of dark mode #356
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
Conversation
This optimizes our darkmode JS by directly injecting it just prior to the opening body tag. It also converts some jQuery to vanilla JS to execute reading localStorage and adding the class to the body as quickly as possible.
TedGoas
left a comment
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 the walk-through, very helpful. This looks good to go for Stacks, though you're much closer to this than I.
…ng render (precaution for dark mode render / fouc)
b-kelly
left a comment
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.
lgtm! Awesome work on this. Just added some notes where there will be non-dark mode related breaking changes. I don't think there needs to be any action on these, it's just for keeping track for future documentation / release notes.
|
|
||
| // $ BADGES | ||
| // ---------------------------------------------------------------------------- | ||
| @badge-gold: #ffcc01; |
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.
This is a breaking change. Double check instances of this before updating any clients
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.
I'll make sure this rename goes into the production PR.
| &::-ms-clear { display: none; } | ||
|
|
||
| // -- STYLE SCROLLBARS | ||
| @scrollbar-styles(); |
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.
This is potentially a breaking change too. Should see how this looks on scrollable inputs before merge.
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.
Checked ✌️
| --bronze-bg: @bronze-bg; | ||
| --bronze-border: @bronze-border; | ||
| // Gold | ||
| --gold: @gold; |
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.
Potential breaking change, but I doubt anyone is using our css variables in any capacity, much less these badge variables.
| @brc-orange: @orange-400; | ||
| // -- Dark mode | ||
| .dark-colors() { | ||
| --white: #2d2d2d; |
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.
Looks like we're not really caring to expose these colors as variables (eg @dark-black-025)? I think overall this is the right choice, since we want our users to ultimately consume these colors as var(--black-025) anyways and exposing as less variables would be time consuming and make migration harder in the future.
| - variable: "@bs-lg" | ||
| default: "0 @su4 @su12 fade(@black-800, 20%)" | ||
| description: | ||
| - variable: "@bs-i-sm" |
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.
Removing these is also a breaking change, but these classes aren't used anywhere in production.
This PR explores some of the ideas discussed in #297.
A note for Stack Overflow users: This does not mean we're shipping dark mode. This PR is an exploration only.
@variablecolor variables