-
Notifications
You must be signed in to change notification settings - Fork 121
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
Implement Dark Mode for the Website (#422) #425
Conversation
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. 😊 |
1 similar comment
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. 😊 |
❌ Deploy Preview for manageyourwaste failed. Why did it fail? →
|
WalkthroughThe changes in this pull request involve updates to Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (5)
styles.js (3)
22-22
: Consider performance optimization for DOM queries.The
querySelectorAll
operation could be expensive if there are many elements with thedarkmodetext
class.Consider using a more specific selector or caching elements by their IDs if possible:
-const whitetext = document.querySelectorAll(".darkmodetext"); +// If these elements are in specific containers, use more specific selectors +const whitetext = document.querySelectorAll("#main-content .darkmodetext");
27-28
: Improve code comment accuracy.The comment refers to
.whitetext
class but the code uses.darkmodetext
.- // Loop through each element with the .whitetext class and toggle .dark-mode3 + // Toggle dark mode styling for text elements
Missing dark mode styles for form visibility states
The verification reveals that while dark mode classes exist, there are no specific dark mode styles defined for
.button-active
and.form-register
states. This could lead to visibility and contrast issues when the dark theme is active.
- Add dark mode specific styles for
.button-active
in dark mode to ensure proper contrast and visibility- Define dark mode styles for
.form-register
to maintain form visibility in dark theme- Consider adding specific styles for
.dark-mode .button-active
and.dark-mode .form-register
🔗 Analysis chain
Line range hint
1-18
: Verify form visibility in dark mode.While the form toggling logic is sound, we should ensure that the form visibility states and button active states remain visually distinct in both light and dark modes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if there are corresponding dark mode styles for form visibility states # Look for .button-active and .form-register styling in dark mode rg -l "\.button-active" "\.form-register" | xargs rg "dark-mode.*\.button-active" "dark-mode.*\.form-register"Length of output: 235
Script:
#!/bin/bash # Let's try a different approach to find CSS styles # First, let's find all CSS files fd -e css | xargs rg -A 2 "\.button-active|\.form-register"Length of output: 1291
Script:
#!/bin/bash # Let's try to find the styles file directly and check its content rg -l "styles\." --type html rg -l "\.css" --type htmlLength of output: 504
Script:
#!/bin/bash # Let's examine the content of index.html and register.html to find the actual stylesheet references # and then check their content for the relevant styles cat index.html register.html | rg "<link.*stylesheet.*css"Length of output: 285
Script:
#!/bin/bash # Now let's check the content of styles.css for button-active and form-register styles # and their dark mode variants cat styles.css | rg -A 3 "\.button-active|\.form-register|\.dark-mode"Length of output: 215
styles.css (1)
334-337
: Consider consolidating duplicate hover effects.The same hover effect is duplicated across multiple components. Consider creating a shared class to reduce code duplication.
+.hover-scale { + transition: transform 0.3s ease; +} + +.hover-scale:hover { + transform: scale(1.05); +} .feature-card { width: 300px; background-color: #fff; border-radius: 8px; box-shadow: 0 4px 8px rgba(0, 0, 0, 0.1); padding: 15px; - transition: transform 0.3s ease; + @extend .hover-scale; } -.feature-card:hover { - transform: scale(1.05); -} .mission-item { width: 23%; background-color: #fff; padding: 15px; border-radius: 8px; box-shadow: 0 4px 8px rgba(0, 0, 0, 0.1); text-align: center; - transition: transform 0.3s ease; + @extend .hover-scale; } -.mission-item:hover { - transform: scale(1.05); -} .value-card { width: 23%; background-color: #fff; padding: 15px; border-radius: 8px; box-shadow: 0 4px 8px rgba(0, 0, 0, 0.1); - transition: transform 0.3s ease; + @extend .hover-scale; } -.value-card:hover { - transform: scale(1.05); -}Also applies to: 466-469, 492-495
index.html (1)
Line range hint
38-40
: Enhance accessibility of the theme toggle buttonThe theme toggle button needs proper ARIA labels and keyboard support for better accessibility.
-<div id="theme-toggle" aria-label="Toggle darok mde">🌓</div> +<button + id="theme-toggle" + aria-label="Toggle dark mode" + role="switch" + aria-checked="false" + tabindex="0"> + 🌓 +</button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
index.html
(8 hunks)script.js
(0 hunks)styles.css
(12 hunks)styles.js
(1 hunks)
💤 Files with no reviewable changes (1)
- script.js
🔇 Additional comments (1)
styles.css (1)
148-150
: LGTM! Border styling changes improve visual hierarchy.
The transition from background colors to bordered sections with subtle shadows creates better visual separation while maintaining a clean, modern look. The colored accents (green, blue) effectively distinguish different sections.
Also applies to: 202-203, 256-258, 628-628
@khushi-joshi-05 can you please assign label to it |
Description:
To enhance the website's usability and improve user experience, this PR introduces a dark mode feature, allowing users to toggle between light and dark themes. This change aims to reduce eye strain, particularly during nighttime or low-light browsing.
Requirements:
Acceptance Criteria:
Additional Context:
This dark mode implementation is designed to enhance accessibility and modernize the website's user interface. The dark color palette aligns with current design aesthetics and complies with accessibility standards for a seamless and enjoyable user experience.
Summary by CodeRabbit
Release Notes
New Features
Styling Improvements
Bug Fixes