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

Support dark mode #1888

Open
wants to merge 19 commits into
base: gh-pages
Choose a base branch
from
Open

Support dark mode #1888

wants to merge 19 commits into from

Conversation

dscho
Copy link
Member

@dscho dscho commented Sep 27, 2024

Changes

This is a very early draft to support toggling between light and dark mode

Context

@dscho dscho mentioned this pull request Sep 27, 2024
@dscho
Copy link
Member Author

dscho commented Sep 27, 2024

This will need a ton of work.

There are way too many hard-coded colors in the current version of our CSS. They will need to be expressed in terms of CSS custom properties, and I have a hunch that there lurks also a really good opportunity to use not quite as many different colors, but to reduce by reusing fewer colors, which should also make the overall appearance nicer.

However, I think I saw a couple of color definitions that were not even used. So before trying to encapsulate the color palette, we should probably use something like https://github.com/purifycss/purifycss to identify parts that are no longer needed (I manually identified three lines already that can easily go and stop bothering us). No need to adapt unused CSS.

And finally, as I said elsewhere, I have no illusions about being a designer, and hope that someone with a proven record of a better design taste than myself (@To1ne maybe?) will give this some TLC.

@dscho
Copy link
Member Author

dscho commented Sep 27, 2024

For shiggles, I branch-deployed this to https://dscho.github.io/git-scm.com/.

@rimrul
Copy link
Contributor

rimrul commented Sep 28, 2024

For shiggles, I branch-deployed this to https://dscho.github.io/git-scm.com/.

Looking at that on Firefox 130.0.1 on Android it has some issues still.

The dark mode just ends appruptly.

Screenshot_20240928_082946_Firefox

Switching to light mode only toggles the text, but not the background.

Screenshot_20240928_083019_Firefox

@rimrul
Copy link
Contributor

rimrul commented Sep 28, 2024

But I do think the parts that work look quite nice.

@dscho
Copy link
Member Author

dscho commented Sep 28, 2024

The dark mode just ends appruptly.

Right, that's one thing I could not figure out. You will notice the height: 200% thing I tried, but it only worked around it on a few pages (and only on Desktop). I guess the real fix will be to have it repeat, just like the isometric grid on the front page.

But I do think the parts that work look quite nice.

Thank you!

There's still a ton of work to do, in particular cleaning up the CSS and then consolidating the remaining hard-coded colors into CSS custom properties à la --main-bg. Want to join the fun?

It makes more sense to put them all in the same location.

Signed-off-by: Johannes Schindelin <[email protected]>
The 'layout.scss' file is meant to be included only from the top-level
`application.scss` file; It does not contain fancy reusable things like
variables or mixins.

Signed-off-by: Johannes Schindelin <[email protected]>
They have not been used since 92a2ad8 (Kill compass, 2015-03-24).

Besides, their names are slightly misleading, suggesting that they refer
to a color, when they actually have Boolean values.

Removing them will make the next step easier, where we want to bulk
convert the actual `*-color` variables from SCSS to CSS custom
properties.

Signed-off-by: Johannes Schindelin <[email protected]>
CSS custom properties are wildly popular, and have been supported by
every major browser (except Internet Explorer, but you're not supposed
to use it anymore, even Microsoft says so in
https://blogs.windows.com/windowsexperience/2021/05/19/the-future-of-internet-explorer-on-windows-10-is-in-microsoft-edge/
because Internet Explorer has been unsupported since June 15, 2022).

There is a slight complication here, though: We use the `darken()` and
`lighten()` functions extensively, and they are SASS preprocessor
functions, i.e. they are evaluated at compile time, while the actual CSS
custom properties are evaluated in the browser.

Therefore, we need to move every `darken()`/`lighten()` call into an
individual CSS custom property that is evaluated at compile time.

This is the first step in preparation for supporting dark mode on
https://git-scm.com/.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho dscho force-pushed the dark-mode branch 3 times, most recently from 199f2ce to a5dc1e9 Compare October 20, 2024 18:14
Many operating systems let users choose between light and dark themes
directly in the system settings. Web browsers can react to that via a
@media query, which we do here.

This commit draws heavily on the excellent guidance provided in
https://css-tricks.com/a-complete-guide-to-dark-mode-on-the-web/ as well
as on SASS' `@mixin` feature (for more details, see
https://sass-lang.com/documentation/at-rules/mixin/).

Signed-off-by: Johannes Schindelin <[email protected]>
In the preceding commit, I added support for a dark mode, heeding the
operating system's indicated preference.

However, many users will want to be able to switch from/to dark/light
mode without switching the rest of their operating system.

Guided by the excellent advice provided in
https://css-tricks.com/a-complete-guide-to-dark-mode-on-the-web/ and in
https://dev.to/ayc0/light-dark-mode-avoid-flickering-on-reload-1567,
this commit introduces a button to toggle dark/light mode.

To avoid flickering, the idea is to:

- store the user preference in the local storage,

- using Javascript, in the `<head>` section of the HTML page (so that it
  is executed _before_ anything is displayed), heed that stored user
  preference (if any) by setting the `theme` attribute in the
  `document`'s `dataset`, and finally

- use that `theme` via CSS:

  :root[data-theme="dark"] {
    ...
  }

Signed-off-by: Johannes Schindelin <[email protected]>
In dark mode, we do not actually want the textured background. Also, the
sidebar needs to be adjusted, it had a hard-coded, very bright
background.

Signed-off-by: Johannes Schindelin <[email protected]>
As per the guidance provided in
https://css-tricks.com/a-complete-guide-to-dark-mode-on-the-web/#aa-dark-mode-images
wht this commit we tone down the images in dark mode, applying
techniques to filter background images as described in
https://css-tricks.com/apply-a-filter-to-a-background-image/.

Signed-off-by: Johannes Schindelin <[email protected]>
WCAG 2 AA (the standard for accessible websites) recommends a  minimum
color contrast of 3:1 for links, if color is the only discerning visual
marker.

In dark mode, that would be a bit too bright, so let's add a wavy
underline instead of increasing the color contrast of the links.

Signed-off-by: Johannes Schindelin <[email protected]>
The verse blocks of the manual pages are unlike any other visual
element, and therefore need special care to look good in dark mode.

Signed-off-by: Johannes Schindelin <[email protected]>
This is a step in the right direction, there is one more instance of
`#fff` (active drop-down titles) but it will be handled differently
because `--main-bg` would be too dark.

Signed-off-by: Johannes Schindelin <[email protected]>
The front page is the first page users see, therefore we need to make
sure that it looks good in dark mode.

Signed-off-by: Johannes Schindelin <[email protected]>
The links in the navigation bar are supposed to be colored dark gray
unless the current page is in the corresponding section.

In dark mode, however, it must be a bit lighter than the very dark
background.

So let's stop hard-coding that color and override it accordingly in dark
mode with a nice, pleasant light gray.

Signed-off-by: Johannes Schindelin <[email protected]>
The color contrast was not large enough; Let's just use a different
variable to define the color.

In light mode, it is similar enough: previously it was #4e443c, now it
is #403f3c instead, i.e. a _tad_ darker. But it looks nice.

Signed-off-by: Johannes Schindelin <[email protected]>
This includes the search box as well as the search results page.

Signed-off-by: Johannes Schindelin <[email protected]>
It would stick out too much in dark mode otherwise.

Signed-off-by: Johannes Schindelin <[email protected]>
The light mode colors in the versions and the translation drop-down
boxes are waaaay to bright in dark mode.

Signed-off-by: Johannes Schindelin <[email protected]>
@dscho
Copy link
Member Author

dscho commented Oct 20, 2024

I've decided to punt on the CSS clean-up for now ;-)

@rimrul @To1ne I've updated this branch and deployed it to https://dscho.github.io/git-scm.com/; Would you mind giving it another go?

For the record, here are a couple of Firefox Mobile screenshots:

Front page

image

About - Branching and Merging

image

Sidebar

image

Documentation

image

A manual page

image

Versions drop-down of a manual page

image

Book, chapter 1 section 1

image

Downloads

Here, I present both the dark mode and the light mode to demonstrate that the monitor is cut off in both, therefore that's a bug that was not introduced by this PR:

Dark Mode Light mode
image image

@dscho dscho marked this pull request as ready for review October 21, 2024 23:02
@To1ne
Copy link
Contributor

To1ne commented Nov 6, 2024

@dscho I've had a more thorough look at it. Overall I'm quite impressed!

Shout outs:

  • It's nice to see it works without Javascript.
  • It's also very nice it remembers your preference (when JS is enabled). Although I don't know if you can reset this state to follow the browser/OS again?
  • I like the overall aesthetic. Colors feels comfy and familiar and contrast looks decent.

Few things:

  • The contrast of the Git logo is too low. Text should be white(ish).
  • Same about the logo at the bottom.
  • We should remove the background from the brand logos at the bottom of the landing page.
  • I'm not sure I like dashed underline on links. But this could become our aesthetic, but should light mode have it as well?
  • The color of the underline looks weird in some cases, when text color is different from underline, like here:
    image
  • Search menu looks bad with underline hyperlinks:
    20241106_07h02m02s_grim
  • I'm not a fan of the sun/moon animation (when you load the page)
  • I think I would move the sun/moon to the top right of the screen, it seems to be more common done like this. And then it doesn't clash with the "scroll to top" button. You could even move it right next to the search box.
  • Some of our images on /about/* should be updated to work better with dark mode. But that can be done in a later iteration. These images might need an overhaul anyway.
  • Border on <code> feels too heavy:
    image
  • I'm not sure we should apply the brightness filter to all images, like for example on /downloads/logos or /about/data-assurance
  • The color of <span class=""> on /community looks, well, not good:
    image
  • I'm not a fan of the aesthetic of the sun & moon. I think monochrome icons would fit in better, but it's cool it just works with emoji, so I get it.

All this is based on visual review, I didn't review any of the code. I can, if you like to?

If you like I can submit some patches with suggestions?

@dscho
Copy link
Member Author

dscho commented Nov 6, 2024

If you like I can submit some patches with suggestions?

I would love that!

@To1ne
Copy link
Contributor

To1ne commented Nov 8, 2024

If you like I can submit some patches with suggestions?

I would love that!

Maybe next week.

@dscho
Copy link
Member Author

dscho commented Nov 8, 2024

I just noticed that the selected OS in https://dscho.github.io/git-scm.com/downloads/guis?os=windows is not visibly discernible from non-selected ones in Dark Mode; That's also something that still needs to be fixed (leaving this here as a reminder).

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.

3 participants