Skip to content

Add light mode support#690

Open
fyang151 wants to merge 40 commits intomainfrom
user/fyang151/web-light-mode
Open

Add light mode support#690
fyang151 wants to merge 40 commits intomainfrom
user/fyang151/web-light-mode

Conversation

@fyang151
Copy link
Copy Markdown
Contributor

@fyang151 fyang151 commented Nov 5, 2025

Description

This PR adds support for light mode by:

  • Keeping colours as variables in variables.css.
  • Setting dark mode in dom before paint with new function in file themeNoFlashScript.
  • Storing our theme using Redux by creating a slice in themeSlice.ts. This is intentionally done differently than our current way of implementing Redux to serve as an example for our possible migration to RTK in the future: Update to RTK #654
  • Adding a button to toggle the theme.

Added color changes and .css variables to light mode branch

change colors later

image

@SPDonaghy SPDonaghy added the web Website team label Nov 8, 2025
@fyang151 fyang151 mentioned this pull request Nov 16, 2025
@fyang151 fyang151 marked this pull request as ready for review November 22, 2025 20:16
@minjunminji minjunminji self-requested a review February 21, 2026 03:05
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this can throw in storage-restricted contexts and also toggles arbitrary class names if stored is unexpected. i think wrapping in try/catch and only apply 'dark' when stored === 'dark', then otherwise explicitly remove 'dark' for light/default

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done! 73cc919

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

for the
document.documentElement.classList.toggle(...) localStorage.setItem(...)
lines (20, 24, etc),

reducers should stay pure and this side-effect code can throw during dispatch (localStorage failures). imo should move DOM/storage writes to middleware/listener/effect, or at minimum guard writes with try/catch so toggle dispatch can't crash

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

What was I doing....
It should be fixed here: 5710eb8

>
ABOUT
</Link>
<div
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

interactive control is a div, so it's not keyboard accesible by default
could use semantic <button type="button"> with aria-label (e.g. "Toggle theme") and preserve current styling via CSS

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done! 73cc919

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

web Website team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LIGHT MODE!

4 participants