-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat: support dark mode #612
base: gh-pages
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for clreq ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Thanks for your PR, @kirklin! Unfortunately, simply adding Lines 147 to 160 in 60649c5
Line 59 in 60649c5
|
We also need to make dark mode versions for at least some of the images in the document. |
dark.css
Outdated
#langSwitch button:lang(zh-hans) { | ||
background-color: #005a9c; | ||
} | ||
|
||
#langSwitch button:lang(zh-hant) { | ||
background-color: #0171c4; | ||
} |
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.
These two colours are too close to each other. The different colours here are to allow people who do not understand Chinese to better distinguish Traditional and Simplified Chinese.
In addition, we need to update
Lines 183 to 188 in 7d51569
.is-multilingual [its-locale-filter-list="zh-hant"] { | |
border-color: rgba(64,196,64,.5); | |
} | |
.is-multilingual [its-locale-filter-list="zh-hans"] { | |
border-color: rgba(248,138,5,.5); | |
} |
dark.css
Outdated
} | ||
|
||
.checkme { | ||
background-color: #627262; |
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.
Personally, I would prefer a darker background, but not strong enough to object this.
@@ -366,7 +367,7 @@ <h5> | |||
</h5> | |||
|
|||
<figure id="fig-loose-setting"> | |||
<span style="text-align:center;"><img src="images/en/increased-inter-character-spacing.svg" alt="Examples of loose setting in horizontal writing mode." width="600" height="209"></span> | |||
<span style="text-align:center;"><img class="invert-on-dark-mode" src="images/en/increased-inter-character-spacing.svg" alt="Examples of loose setting in horizontal writing mode." width="600" height="209"></span> |
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.
Our theme colour, green, will be missing if we invert the images. We'll discuss if we need to fix this in the teleconference.
Thanks for updating the PR! I made some comments. IMHO we can consider using CSS custom properties (like https://github.com/w3c/bp-i18n-specdev/pull/130/files ), so that when updating the colour, it is easier to remember that we also have a dark theme that needs to be updated. |
And we also need to update Line 327 in 7d51569
|
Per https://github.com/w3c/respec/pull/4700 , please remove <meta name="color-scheme" content="light dark"> |
There are still some problems with manually switching the colour theme. Let’s wait until https://github.com/w3c/respec/issues/4687 is resolved. |
Fixes #611
Preview | Diff