Add Dark Mode Toggle Feature (#85-10)#95
Conversation
- Implement dark mode toggle button in top-right corner - Add comprehensive dark mode CSS styles - Persist user preference in localStorage - Toggle displays moon icon (🌙) in light mode, sun icon (☀️) in dark mode - Dark theme uses comfortable #1e1e1e background with proper contrast - Smooth transitions between light and dark modes - Mobile-responsive toggle button - Accessible with aria-labels and title attributes Features: - Dark mode preference persists across sessions - All UI elements properly themed (cards, filters, header, footer) - Links maintain proper color contrast in both modes - Smooth animations for mode transitions
Summary of ChangesHello @Subham-KRLX, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new dark mode feature, providing users with the ability to toggle between a light and dark theme for an enhanced viewing experience. The implementation ensures that the chosen theme persists across browsing sessions and applies consistently across all user interface components, improving overall usability and accessibility. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a well-implemented dark mode feature. The theme state is correctly persisted in localStorage, and the UI updates accordingly. The code is clean and well-commented. I've provided a few suggestions to enhance maintainability and follow best practices, such as using CSS variables for theming, centralizing initialization logic, and avoiding magic strings and redundant DOM queries in the JavaScript code. Overall, this is a great addition to the project.
Code Quality Improvements: - Add constants for localStorage keys (DARK_MODE_KEY, THEME_ENABLED, THEME_DISABLED) - Cache darkModeToggleElement to avoid redundant DOM queries - Centralize dark mode initialization by calling initializeDarkMode() from initializeApp() - Remove duplicate DOMContentLoaded event listener CSS Best Practices: - Introduce CSS Custom Properties (variables) for theming - Define light theme colors in :root - Override colors in body.dark-mode for dark theme - Consolidate dark mode styles using variables instead of hardcoded values - Improves maintainability and makes future theme changes easier Benefits: - Reduced code duplication - Better performance with cached DOM elements - Centralized initialization logic - Easier to extend with additional themes - Single source of truth for color values
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a well-implemented dark mode feature. The use of CSS variables for theming and localStorage for persistence is a great approach. The JavaScript code is clean and efficient. I've found a few minor issues in the CSS related to a z-index bug, a redundant rule, and a hardcoded color value. Addressing these will improve the robustness and maintainability of the new theme. Overall, great work!
- Fix z-index bug: Change dark mode toggle z-index from 999 to 100001 Prevents toggle button from being hidden behind GitHub ribbon (z-index: 100000) - Remove redundant CSS rule: body.dark-mode background/color already inherit from variables - Replace hardcoded color: Use var(--text-color) instead of #fff in details-box strong Maintains consistency with theme variables for easier future updates
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a dark mode toggle feature, which is a great addition for user experience. The implementation uses CSS variables for theming and localStorage to persist the user's choice, which are good practices.
My review includes two main suggestions for improvement:
- In
docs/script.js, I've pointed out a potential "flash of incorrect theme" and suggested a solution using an inline script to apply the theme before the page renders. - In
docs/styles.css, I've recommended refactoring the base styles to make better use of the new CSS variables. This will make the stylesheet more maintainable and concise.
Overall, the feature is well-implemented, and these changes would further enhance the code quality and user experience.
Prevent Flash of Incorrect Theme (FOIT): - Add inline script in <head> to apply dark mode before page renders - Prevents white flash when user has dark mode enabled - Script runs synchronously before page content is rendered JavaScript Improvements: - Update initializeDarkMode() to check documentElement instead of reading localStorage - Use document.documentElement for theme application (not body) - Simplify init logic - theme is already applied by inline script CSS Refactoring: - Change all body.dark-mode selectors to html.dark-mode - Refactor base styles to use CSS variables: * h1: use var(--text-color) instead of #333 * .card: use var(--card-bg) instead of #fff * .details-box strong: use var(--text-color) * .details-box span: use var(--text-secondary) * .stats-row border: use var(--card-border) * .stat a: use var(--link-color) and var(--link-hover) - Eliminates need for many dark-mode override rules - More maintainable and concise CSS - Automatic theme application without duplication Result: - Smooth theme switching without flicker - Cleaner, more maintainable codebase - Better user experience overall
|
Please fix the conflicts |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a dark mode toggle, which is a great feature for user experience. The implementation is well-structured, using CSS variables for theming and localStorage to persist the user's choice. The use of an inline script to prevent a "flash of incorrect theme" is a thoughtful touch.
I've found a couple of critical syntax errors in both the JavaScript and CSS files that would prevent the page from working correctly. I've also pointed out an unused variable and a CSS transition that won't behave as smoothly as intended. After addressing these points, the feature will be in great shape.
|
This branch has conflicts that must be resolved |
|
Here's the code health analysis summary for commits Analysis Summary
|
jbampton
left a comment
jbampton
left a comment
There was a problem hiding this comment.
Search by name etc is messed up in the wrong position and the light and dark button is over the fork ribbon. This is in Chromium
|
The buttons works and seems functional 
|
|
1- This branch has conflicts that must be resolved 2- You need to move switch theme button from top - right into bottom-right. 3- as John reported above: Search by name etc is messed up in the wrong position and the light and dark button is over the fork ribbon. This is in Chromium |
|
This PR is open for a while, please finalize and do the task sooner so we can merge and have it up/ready. Thanks, and Happy Yalda Night. |
|
This branch has conflicts that must be resolved |
|
Can you fix all of the the above issues sooner? I want to merge it. |
…rk ribbon overlap
|
Hi @BaseMax Thank you for the reminders! My exams are currently going on so I haven’t been able to fix the issues yet Once my exams are over I will fix this PR for sure Please give me a bit more time. |
|
This branch has conflicts that must be resolved Please make sure you pull from latest version of main and fix all conflicts. Best |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces dark mode functionality across the application. The changes involve adding JavaScript logic in docs/script.js to initialize and toggle dark mode, save user preferences to localStorage, and update the dark mode toggle button's appearance. The docs/styles.css file has been refactored to use CSS variables for theming, with a new set of variables defined for dark mode, and includes styling for the new dark mode toggle button. The layouts/layout.html file incorporates the dark mode toggle button and an inline script to prevent a 'Flash of Incorrect Theme' (FOIT). A review comment points out an unresolved merge conflict in the hideLoadingState function within docs/script.js, noting that the HEAD version of the conflict incorrectly prevents the display of results information after the loading state is hidden.
- Properly show results info after hiding loading state - Fix indentation to use consistent 2-space formatting - Resolves critical bug where results info was not displayed - Addresses gemini-code-assist bot review feedback
- Merge recent changes from upstream/main - Include both resultsInfoDesktop and new Featured Developer section - Maintain dark mode toggle button functionality
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a well-implemented dark mode feature, which is a great enhancement for user experience. The use of CSS variables for theming and an inline script to prevent the 'flash of incorrect theme' are excellent practices. The code is generally clean and well-structured. I've identified a couple of areas for improvement: a potential runtime error when accessing localStorage and a z-index issue that affects the visibility of the new toggle button. Addressing these will make the feature more robust and reliable.
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| z-index: 100001; |
There was a problem hiding this comment.
The z-index of the .dark-mode-toggle button is 100001, which is lower than the z-index of .filters-aside (1000000). This causes the dark mode toggle to be hidden behind the filter panel when it's open, making it inaccessible. To fix this, the toggle button's z-index should be higher than the filter panel's.
| z-index: 100001; | |
| z-index: 1000001; |
| function toggleDarkMode() { | ||
| const isDarkMode = document.documentElement.classList.toggle('dark-mode'); | ||
| localStorage.setItem(DARK_MODE_KEY, isDarkMode ? THEME_ENABLED : THEME_DISABLED); | ||
| updateDarkModeIcon(isDarkMode); | ||
| } |
There was a problem hiding this comment.
Accessing localStorage can throw an exception in some browser environments, such as in private browsing mode or if the user has disabled it. This would cause the toggleDarkMode function to crash. It's best practice to wrap localStorage operations in a try...catch block to handle these edge cases gracefully and ensure the rest of the function (like updating the icon) still executes.
function toggleDarkMode() {
const isDarkMode = document.documentElement.classList.toggle('dark-mode');
try {
localStorage.setItem(DARK_MODE_KEY, isDarkMode ? THEME_ENABLED : THEME_DISABLED);
} catch (error) {
console.error('Failed to save dark mode preference:', error);
}
updateDarkModeIcon(isDarkMode);
}
|
@jbampton I don’t think I am able to resolve this issue with this PR I have been working on it for a long time and tried my best but I can’t complete it right now so I am closing this PR I would love to contribute to another issue instead. Thanks for the feedback. |
3 participants
Add Dark Mode Toggle (#85)
What's New
Key Features
✅ One-click toggle between light (#f0f2f5) and dark (#1e1e1e) modes
✅ All UI elements properly themed (cards, filters, header, footer, links)
✅ Smooth CSS transitions
✅ ARIA labels and keyboard accessible
✅ No performance impact
Files Changed
docs/styles.css- Dark mode CSSdocs/script.js- Toggle functionality & localStoragelayouts/header.html- Toggle buttonUsage
Click the moon icon (🌙) to switch to dark mode. Your preference is automatically saved!
closes #96