Skip to content
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

refactor: make some dark mode colors less contrast #237

Merged
merged 4 commits into from
Apr 21, 2022

Conversation

lex111
Copy link
Contributor

@lex111 lex111 commented Apr 15, 2022

Related to #199

After doing a little studying of dark mode sites, I've come to the conclusion that it's really worth using less contrast for base content color if dark mode is on:

Site Contrast ratio (dark mode)
docs.microsoft.com 14.36
developers.cloudflare.com 11.45
Mk Docs Material 11.03
appwrite 11.96
stitches.dev/ 15.34
developers.cloudflare.com 11.45
angular.io 12.64

Current dark mode contrast ratio - 16.27
New one - 13.71 13.4

This is quite subjective, but the current contrast ratio is a bit of an inconvenience, so darkening the current content color would not be superfluous, given that in fact many sites use less contrast color in general.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 15, 2022
@github-actions
Copy link

github-actions bot commented Apr 15, 2022

Size Change: -9.76 kB (-2%)

Total Size: 569 kB

Filename Size Change
./packages/core/dist/css/default-dark/default-dark-rtl.css 82.7 kB -1.42 kB (-2%)
./packages/core/dist/css/default-dark/default-dark-rtl.min.css 58.1 kB -1.02 kB (-2%)
./packages/core/dist/css/default-dark/default-dark.css 82.6 kB -1.42 kB (-2%)
./packages/core/dist/css/default-dark/default-dark.min.css 58 kB -1.02 kB (-2%)
./packages/core/dist/css/default/default-rtl.css 80.6 kB -1.42 kB (-2%)
./packages/core/dist/css/default/default-rtl.min.css 56.9 kB -1.02 kB (-2%)
./packages/core/dist/css/default/default.css 80.6 kB -1.42 kB (-2%)
./packages/core/dist/css/default/default.min.css 56.9 kB -1.02 kB (-2%)
ℹ️ View Unchanged
Filename Size
./packages/core/dist/js/alerts.js 409 B
./packages/core/dist/js/alerts.min.js 409 B
./packages/core/dist/js/button-groups.js 267 B
./packages/core/dist/js/button-groups.min.js 267 B
./packages/core/dist/js/dropdowns.js 1.01 kB
./packages/core/dist/js/dropdowns.min.js 1.01 kB
./packages/core/dist/js/menu.js 2.4 kB
./packages/core/dist/js/menu.min.js 2.4 kB
./packages/core/dist/js/navbar.js 1.08 kB
./packages/core/dist/js/navbar.min.js 1.08 kB
./packages/core/dist/js/pills.js 270 B
./packages/core/dist/js/pills.min.js 270 B
./packages/core/dist/js/radio-behavior.js 705 B
./packages/core/dist/js/radio-behavior.min.js 705 B
./packages/core/dist/js/tabs.js 267 B
./packages/core/dist/js/tabs.min.js 267 B

compressed-size-action

@github-actions
Copy link

github-actions bot commented Apr 15, 2022

Dist CSS Diff

diff --git a/packages/core/dist/css/default/default.css b/packages/core/dist-branch/css/default/default.css
index d4458d2..4266566 100644
--- a/packages/core/dist/css/default/default.css
+++ b/packages/core/dist-branch/css/default/default.css
@@ -185,7 +185,7 @@
   --ifm-code-padding-horizontal: 0.1rem;
   --ifm-code-padding-vertical: 0.1rem;
 
-  --ifm-pre-background: var(--ifm-color-emphasis-100);
+  --ifm-pre-background: var(--ifm-code-background);
   --ifm-pre-border-radius: var(--ifm-code-border-radius);
   --ifm-pre-color: inherit;
   --ifm-pre-line-height: 1.45;
@@ -2961,23 +2961,26 @@ html[data-theme='dark'] {
   --ifm-color-emphasis-900: var(--ifm-color-gray-100);
   --ifm-color-emphasis-1000: var(--ifm-color-gray-0);
 
-  --ifm-background-color: #18191a;
+  --ifm-background-color: #1b1b1d;
   --ifm-background-surface-color: #242526;
 
   --ifm-hover-overlay: rgba(255, 255, 255, 0.05);
 
+  --ifm-color-content: #e3e3e3;
   --ifm-color-content-secondary: rgba(255, 255, 255, 1);
 
   --ifm-breadcrumb-separator-filter: invert(64%) sepia(11%) saturate(0%)
     hue-rotate(149deg) brightness(99%) contrast(95%);
 
-  --ifm-code-background: rgb(51, 52, 55);
+  --ifm-code-background: rgba(255, 255, 255, 0.1);
 
   --ifm-scrollbar-track-background-color: #444444;
   --ifm-scrollbar-thumb-background-color: #686868;
   --ifm-scrollbar-thumb-hover-background-color: #7a7a7a;
 
   --ifm-table-stripe-background: rgba(255, 255, 255, 0.07);
+
+  --ifm-toc-border-color: var(--ifm-color-emphasis-200);
     --ifm-color-primary-contrast-background: rgb(16, 36, 69);
     --ifm-color-primary-contrast-foreground: rgb(235, 242, 252);
     --ifm-color-secondary-contrast-background: rgb(71, 71, 72);

@netlify
Copy link

netlify bot commented Apr 15, 2022

Deploy Preview for infima ready!

Name Link
🔨 Latest commit 39918b5
🔍 Latest deploy log https://app.netlify.com/sites/infima/deploys/62614196fabf7a0008598202
😎 Deploy Preview https://deploy-preview-237--infima.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@Josh-Cena
Copy link
Contributor

Can we also brighten the background a bit so it approaches ~12.5?

@lex111
Copy link
Contributor Author

lex111 commented Apr 15, 2022

I tried to do something with the background too, but it is quite unusually bright then, maybe decrease its brightness just a little bit so that the contrast ratio ends up being 13.4.

@Josh-Cena
Copy link
Contributor

I'm looking at the CF docs which IMO is the closest to ours (the others are a bit too... colorful). Their background seems to be grayer than us. But looking at the deploy demo, this change already looks much, much better than before.

@lex111
Copy link
Contributor Author

lex111 commented Apr 15, 2022

Yes, since I don't have a clear understanding of what dark mode should be at all, I just want avoid to make abrupt changes, so the new contrast ratio seems quite good.

@Josh-Cena
Copy link
Contributor

Same, I don't even know what dark mode should look like 😄 But everyone seems to implement it differently anyways

Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can give this a try 👍

Wondering if it's not worth it to make category/toc/navbar/title lighter, and only darken main text and walls of text?

This looks a little bit sad to me

CleanShot 2022-04-20 at 17 32 33@2x

Compared to other docs like Cloudflare where some elements stand out a bit more:

CleanShot 2022-04-20 at 17 34 41@2x

but Microsoft docs also looks a bit sad so 🤷‍♂️


Note: Cloudflare uses a different color for markdown titles, and IMHO that feels more "alive"

--ifm-code-background: color-mod(
var(--ifm-color-gray-900) tint(var(--ifm-dark-value))
);
--ifm-code-background: rgba(255, 255, 255, 0.08);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find the code background does not contrast much compared to regular background, it could be lighter?

CleanShot 2022-04-20 at 17 30 12@2x

Maybe it's not relevant due to using a Prism theme?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe we should make background for code blocks (pre) the same as for inline code background?

Copy link
Contributor

@Josh-Cena Josh-Cena Apr 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is always overridden in practice by Prism themes anyways, not critical.

@lex111
Copy link
Contributor Author

lex111 commented Apr 20, 2022

Wondering if it's not worth it to make category/toc/navbar/title lighter, and only darken main text and walls of text?

I wouldn't do that yet, the current result is quite good.

Copy link
Contributor

@Josh-Cena Josh-Cena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is all good, no changes needed. @lex111 Ready for merge?

@slorber
Copy link
Collaborator

slorber commented Apr 21, 2022

IMHO code block is still not very visible (less than before)

And the pre-background too (not sure we have this case on Docusaurus but the Infima demo it's less visible than before)

CleanShot 2022-04-21 at 11 42 40@2x

Otherwise that seems nice

@Josh-Cena
Copy link
Contributor

@slorber I'm not aware of a place where the pre background is actually used in Docusaurus, unless the user uses <pre> in JSX (unlikely). Everywhere else, we have Prism themes.

@lex111
Copy link
Contributor Author

lex111 commented Apr 21, 2022

Should be better this time -- the background for inline blocks is darker, and it is also used for multi-line code blocks.

@slorber
Copy link
Collaborator

slorber commented Apr 21, 2022

LGTM 👍

@slorber slorber merged commit b9e8c5f into main Apr 21, 2022
@slorber slorber deleted the lex111/dark-mode-adjust branch April 21, 2022 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants