carpentries / varnish

Template for pkgdown site
https://carpentries.github.io/varnish/
Other
7 stars 25 forks source link

Implement dark mode #124

Closed astroDimitrios closed 2 months ago

astroDimitrios commented 6 months ago

What: Implement a dark mode to the template and a toggle for the theme which defaults to Auto ie. the users preference.

Why: Accessibility / Some people prefer dark mode!

Requires - #123 Updating Bootstrap to 5.3.2 which has support for colour modes/themes. (This change also contains the changes in #123)

Overview

Implements a new dark mode which you change via a theme toggle. The theme toggle is in the header for large screens and the sidebar for small/phone screens. It is in the correct tab order and I have checked with the latest Chrome and Firefox that the correct default theme is loaded and the contrast passes the accessibility checker. Images have their brightness and saturation reduced in dark mode. Based off Bootstrap 5.3s Color Modes.

There are lots of subtle colour differences for all the carpentries (software, library etc) and the production tags (beta, stable etc) so I have taken screenshots and placed them in this folder. Where possible I have simply taken the default light mode / carpentry variant colours and used bootstrap to shade-color, mix with black to darken, or tint-color, mix with white to lighten. Example of one dark mode and the toggle below.

Dark mode showing a stable carpentries template in dark mode: carp-doi-dark

The theme toggle in light mode with the main button outlined because I tabbed to it using keyboard navigation: toggle-light

Summary of Changes

Template files

A few of the inst/pkgdown/templates had to be altered:

head.html

header.html

layout.html

navbar.html

Logos

I had to alter the data carpentries logo as the space between the D and the C was white not transparent. In dark mode I use a filter to flip the svg colour to white and this extra white area messed up the logo/there was no separation between the D and the C. These new files have been passed through an optimiser to reduce their file size.

Javascript: source/javascripts/custom/themetoggle.js

The javascript to control the theme toggle button. This loads before the css. It gets the users theme preference for the session/browser based on the prefers-color-scheme css media feature. Modified from the bootstrap version as their toggle button is in a navbar that re-sizes/is visible at all screen sizes whereas we have two theme toggle buttons. One in the main header for large screen sizes and one in the sidebar for small/phone screens - one of which is always hidden but still needs the js to update is attributes.

SCSS

source/stylesheets/dark.scss

The dark theme css stylesheet. I have added this all into one file so it is easier for people to create their own themes based on the css selectors in the stylesheet.

source/stylesheets/header.scss

Modified to center everything in the nav and adjust some colours.

source/stylesheets/sidenav.scss

Turn off transitions for the background colour in part of the nav which caused flashing when changing theme.

source/stylesheets/styles.css.scss

source/stylesheets/themetoggle.scss

The css styling for the theme toggle button for light mode.

squash-a-script.sh

Modified to also squash and copy the javascript for the themtoggle into its own file since this needs to be loaded separately before the css (see head.html changes).

Last Comments

astroDimitrios commented 6 months ago

You can now see this change in action in the incubator lesson: Introduction to Modern Fortran, as well as how it integrates with Mermaid Diagrams #125 (on branch https://github.com/carpentries-incubator/intro-to-modern-fortran/tree/learner_profiles) and tabs and group-tabs https://github.com/carpentries/sandpaper/pull/571 (coming soon via https://github.com/carpentries-incubator/intro-to-modern-fortran/issues/10).

ErinBecker commented 3 months ago

Thank you @astroDimitrios for taking the time to implement this new dark mode! I'm impressed with the thoroughness of your work, including even finding the issue with the Data Carpentry logo. πŸ˜‚ After discussing, @froggleston and I agree that this provides a good alternative for those who prefer to look at content in dark mode.

In spot checking, @froggleston noticed that there is an issue with the contrast between the pre-alpha banner and site background.

screenshot of pre-alpha lesson in dark mode

This is probably a relatively simple fix, but it makes me want to do a more comprehensive accessibility testing by running a set of rendered lessons through the WAVE Web Accessibility Evaluation Tool. I've tried to do this through forking a lesson and setting the varnish variable in the lesson's config page but something about the build actions isn't working properly on my fork.

With that in mind, we'd like to go ahead and merge this PR, as it represents an overall improvement in accessibility. I've made an issue to do further accessibility testing as described above. Please feel free to lend assistance on that issue as time allows! πŸ˜‚

I think the only thing needed before I can merge this PR is to clean up the conflicts. There are three conflicts in css files, which are all showing up as a single line and impossible to diff. 😡 (Example: https://github.com/carpentries/varnish/pull/124/conflicts). I know some of our latest changes included updates to the css (e.g. https://github.com/carpentries/varnish/commit/bc84585522ac89117fdb47770ff2ddde04c6bd0c), but because the diff is all a single line, I have no idea how to isolate the changes we want to keep and integrate into this PR. Is there a tool for working with css that would help isolate these changes?

astroDimitrios commented 3 months ago

Hi @ErinBecker ! Thanks for your and @froggleston 's feedback. The contrast should be a quick fix and I think it might be best for me to try and merge/rebase my branch to resolve the conflicts. Unfortunately I am on leave now for around 3 weeks so won't get to this anytime soon. Will look again when I'm back :)

froggleston commented 3 months ago

This is looking great! As far as I'm aware, you'll always have CSS conflicts due to the way the CSS is built - you squash the CSS and CSS map to aggregate and minify the styles, so there's no way to resolve this easily. @astroDimitrios I guess the check to do would be to make sure your dark mode CSS is aligned with the current main CSS, and then check your edits still hold?

froggleston commented 3 months ago

Hey @astroDimitrios ! I've been trying to merge in your changes but it's a little tricky with the discrepancy between this PR and the updated varnish release code (mainly the changes @ErinBecker made to the search button in the header). Please could you update your branch to the 1.0.3.9000 pre-release version of varnish to merge in your commits in this PR? We can then merge this PR back into main and release! Many thanks!

astroDimitrios commented 3 months ago

Hey @ErinBecker @froggleston. I think it's done! I have updated so there are no conflicts (added in a few lines for the new search button) and tinted the pre-alpha button background for contrast. Let me know if there is anything else :)

froggleston commented 2 months ago

This all works as expected. Thank you again @astroDimitrios for a fantastic contribution! πŸš€