-
Notifications
You must be signed in to change notification settings - Fork 125
add "auto" state to theme toggle #154
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
Conversation
|
Thanks for the PR! Is this setting used on any other sites? I haven't really seen it anywhere, which is why I feel like it might be a bit confusing. |
|
My idea was:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a three-state theme toggle system, allowing users to choose between "always light", "always dark", and "auto" (system preference) modes. Previously, the toggle only supported two states: light and dark.
Key Changes:
- Modified the toggle cycle to include an intermediate "auto" state between dark and light modes
- Added a new auto icon to visually represent the auto theme state
- Implemented system theme preference detection and automatic updates when in auto mode
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| templates/partials/nav.html | Added auto-icon image element to support the new three-state toggle UI |
| static/js/themetoggle.js | Implemented auto mode logic, system preference detection, and event listener for system theme changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if (localStorage.getItem("theme-storage") === "dark") { | ||
| setTheme("auto"); | ||
| updateItemToggleTheme(); | ||
| } else { |
Copilot
AI
Oct 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic uses getSavedTheme() at line 61 but directly calls localStorage.getItem(\"theme-storage\") here. Consider using getSavedTheme() consistently for better maintainability and to avoid potential issues if the saved theme is null/undefined.
| autoIcon.style.display = (mode === "auto") ? "block" : "none"; | ||
|
|
||
| if (mode === "auto") { | ||
| autoIcon.style.filter = getSystemPrefersDark() ? "invert(1)" : "invert(0)"; |
Copilot
AI
Oct 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The auto icon filter is only updated when mode is 'auto', but the filter persists when switching modes. This could cause visual issues if the icon is reused. Consider resetting the filter when switching away from auto mode or moving this logic outside the conditional.
| autoIcon.style.filter = getSystemPrefersDark() ? "invert(1)" : "invert(0)"; | |
| autoIcon.style.filter = getSystemPrefersDark() ? "invert(1)" : "invert(0)"; | |
| } else { | |
| autoIcon.style.filter = "none"; |
|
Ohhh, yeah. I've seen the "Auto" field sometimes. I guess it could be useful. Will merge this in a sec. |
|
Thanks for the PR, it's merged! de3f539 |
When using "toggle" mode for theme, instead of toggling between "always dark" and "always light" modes, a three-state toggle is employed: "always light", "always dark" and "auto". That way user has total autonomy over how they want the theme to behave.
Because of that, toggle icon needs to display currently selected mode: sun for light, moon for dark, half circle for auto.
Working example: https://mtsz.pl/
Firefox extension to quickly change browser theme with single button: https://addons.mozilla.org/firefox/addon/toggley/