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

Add dark-mode support and improve readability #257

Closed
wants to merge 2 commits into from
Closed

Add dark-mode support and improve readability #257

wants to merge 2 commits into from

Conversation

Alhadis
Copy link

@Alhadis Alhadis commented Sep 19, 2023

This commit adds dark-mode support to no-color.org and makes several tweaks to the existing styling to improve readability:

Before and after refactoringBefore and after refactoring
Dark mode previews
Desktop
Desktop view, dark-modeDesktop view, light-mode
Mobile
Mobile view, dark-modeMobile view, light-mode

}

/* Dark mode */
@media (prefers-color-scheme: dark) {
Copy link

@SethFalco SethFalco Sep 25, 2023

Choose a reason for hiding this comment

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

I think it'd be better to define CSS variables in :root and override those variables in @media (prefers-color-scheme: dark)?

I've seen some projects maintain themes like without them before, but it tends to lead to inconsistent use of colors, and broken themes on updates. ^-^'

Maintaining color pallets is a lot easier than maintaining individual overrides.

For example:

:root {
  --pri-bg-color: #fff;
  --link-color: #0969da;
}

@media (prefers-color-scheme: dark) {
  :root {
    --pri-bg-color: #161b22;
    --link-color: #2f81f7;
  }
}

body {
  background-color: var(--pri-bg-color);
}

a:link, a:visited {
  color: var(--link-color);
}

Copy link
Author

Choose a reason for hiding this comment

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

Using variables for values that're only referenced once is wasteful, IMHO. Especially for such a decidedly simple design.

It's your call, though.

@DeflateAwning
Copy link
Contributor

Would be awesome to merge this! Is there anything missing?

@jcs
Copy link
Owner

jcs commented Mar 6, 2024

I don't agree that these changes improve readability. Dark mode can be supported with a meta tag that doesn't require overriding the user's colors, which I can do in a separate commit.

@jcs jcs closed this Mar 6, 2024
@Alhadis
Copy link
Author

Alhadis commented Mar 12, 2024

Dark mode can be supported with a meta tag that doesn't require overriding the user's colours

🤦 I didn't even know you could do that…

(Christ, I really have fallen behind with staying atop recent developments to the Web platform… 😥)

@Alhadis Alhadis deleted the dark-mode branch March 12, 2024 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants