Skip to content
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.

Sign up for GitHub

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

Update Themetoggle and Improve Footer Text Visibility #422

Merged
merged 2 commits into from
Oct 29, 2024
Merged

Update Themetoggle and Improve Footer Text Visibility #422

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f9feba2
Update Themetoggle and footer text
AnushkaChouhan25 Oct 28, 2024
4c842ae
Merge branch 'main' into main
AnushkaChouhan25 Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions frontend/src/components/Shared/footer/Content.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,9 @@ const Section2 = () => {
/>
</div>
)}
<div

<div
className={`flex ${isWide ? 'justify-between items-end' : 'flex-col items-center'
} text-white`}

} text-[#004D43] dark:text-white`}
>
<div>
<h1
Expand Down
48 changes: 33 additions & 15 deletions frontend/src/components/ThemeToggle.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,25 +7,43 @@ const ThemeSwitcher = () => {
const isDarkMode = theme === 'dark';

return (
<div className="flex flex-row justify-center items-center">
<label className="relative inline-block w-16 h-9">
<input
type="checkbox"
className="sr-only peer"
checked={isDarkMode}
onChange={() => setTheme(isDarkMode ? 'light' : 'dark')}
/>
<span className="block w-full h-full bg-blue-500 rounded-3xl border-2 border-blue-400 cursor-pointer peer-checked:bg-slate-800 peer-checked:border-blue-400 transition-colors duration-400"></span>
<div className="flex justify-center items-center mt-2">
<label className="relative inline-flex items-center cursor-pointer">
<input
type="checkbox"
className="sr-only"
checked={isDarkMode}
onChange={() => setTheme(isDarkMode ? 'light' : 'dark')}
/>

{/* Sun Icon for Light Mode */}
<span className="absolute top-1/2 left-[0.3em] w-4 h-4 bg-gradient-to-r from-pink-500 to-orange-400 rounded-full transform -translate-y-1/2 transition-all duration-400 opacity-100 peer-checked:opacity-0"></span>
{/* Track */}
<span className={`block w-16 h-8 bg-gray-300 rounded-full shadow-inner transition duration-300 ease-in-out ${isDarkMode ? 'bg-gray-800' : 'bg-gray-200'}`}></span>

{/* Moon Icon for Dark Mode */}
<span className="absolute top-1/2 left-[calc(100%-1.3em)] w-4 h-4 bg-transparent rounded-full transform -translate-y-1/2 transition-all duration-400 opacity-0 peer-checked:opacity-100 peer-checked:shadow-[inset_-3px_-2px_5px_-2px_#8983f7,inset_-8px_-4px_0_0_#a3dafb]"></span>
</label>
{/* Moon Icon for Light Mode */}
<FaMoon
className={`absolute left-1 w-5 h-5 text-gray-700 transform transition-all duration-300 ${
isDarkMode ? 'opacity-0 scale-0' : 'opacity-100 scale-100'
}`}
/>

{/* Sun Icon for Dark Mode */}
<FaSun
className={`absolute right-1 w-5 h-5 text-gray-100 transform transition-all duration-300 ${
isDarkMode ? 'opacity-100 scale-100' : 'opacity-0 scale-0'
}`}
/>
Comment on lines +22 to +34
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor duplicate icon animation styles.

The sun and moon icons share similar animation logic with hardcoded transition durations. Consider extracting common styles and using the CSS variable.

+const iconBaseStyles = "absolute w-5 h-5 transform transition-all duration-[var(--transition-duration)]";
+const moonIconStyles = `${iconBaseStyles} left-1 text-gray-700`;
+const sunIconStyles = `${iconBaseStyles} right-1 text-gray-100`;

-<FaMoon
-  className={`absolute left-1 w-5 h-5 text-gray-700 transform transition-all duration-300 ${
-    isDarkMode ? 'opacity-0 scale-0' : 'opacity-100 scale-100'
-  }`}
-/>
+<FaMoon
+  className={`${moonIconStyles} ${isDarkMode ? 'opacity-0 scale-0' : 'opacity-100 scale-100'}`}
+/>

-<FaSun
-  className={`absolute right-1 w-5 h-5 text-gray-100 transform transition-all duration-300 ${
-    isDarkMode ? 'opacity-100 scale-100' : 'opacity-0 scale-0'
-  }`}
-/>
+<FaSun
+  className={`${sunIconStyles} ${isDarkMode ? 'opacity-100 scale-100' : 'opacity-0 scale-0'}`}
+/>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
{/* Moon Icon for Light Mode */}
<FaMoon
className={`absolute left-1 w-5 h-5 text-gray-700 transform transition-all duration-300 ${
isDarkMode ? 'opacity-0 scale-0' : 'opacity-100 scale-100'
}`}
/>
{/* Sun Icon for Dark Mode */}
<FaSun
className={`absolute right-1 w-5 h-5 text-gray-100 transform transition-all duration-300 ${
isDarkMode ? 'opacity-100 scale-100' : 'opacity-0 scale-0'
}`}
/>
const iconBaseStyles = "absolute w-5 h-5 transform transition-all duration-[var(--transition-duration)]";
const moonIconStyles = `${iconBaseStyles} left-1 text-gray-700`;
const sunIconStyles = `${iconBaseStyles} right-1 text-gray-100`;
{/* Moon Icon for Light Mode */}
<FaMoon
className={`${moonIconStyles} ${isDarkMode ? 'opacity-0 scale-0' : 'opacity-100 scale-100'}`}
/>
{/* Sun Icon for Dark Mode */}
<FaSun
className={`${sunIconStyles} ${isDarkMode ? 'opacity-100 scale-100' : 'opacity-0 scale-0'}`}
/>


</div>
{/* Slider (Knob) */}
<span
className={`absolute w-7 h-7 bg-white rounded-full shadow-lg transition-transform duration-300 ease-in-out transform ${isDarkMode ? 'translate-x-8' : 'translate-x-0'}`}
></span>

{/* Font Awesome Icon as Logo inside a rounded circle */}
<div className={`absolute w-8 h-8 bg-white rounded-full flex justify-center items-center shadow-lg transition-all duration-300 ease-in-out transform ${isDarkMode ? 'translate-x-8' : 'translate-x-0'}`}>
{isDarkMode ? <FaMoon className="w-5 h-5 text-gray-800" /> : <FaSun className="w-5 h-5 text-gray-800" />}
</div>
</label>
</div>
);
};

Expand Down
9 changes: 9 additions & 0 deletions frontend/src/index.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,12 @@
background-color: #004d43;
border-radius: 10px;
}
:root {
--transition-duration: 500ms;
}
Comment on lines +45 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Remove unused CSS variable --transition-duration

The --transition-duration CSS variable declared in frontend/src/index.css is not being used anywhere in the codebase. While there are many transition-related properties throughout the application, they all use hardcoded duration values (e.g., 300ms, 500ms) and none reference this variable using var(--transition-duration). Consider either:

  • Removing this unused variable, or
  • Refactoring existing transitions to use this variable for consistent animation timing
🔗 Analysis chain

Verify the usage of --transition-duration variable.

The new CSS variable appears to be intended for theme transition animations, but its usage is not visible in the provided changes.

Let's check if the variable is used in any way, even without the var() syntax, and also look for transition properties in general to understand the context better.

The previous search showed the variable declaration but we need to verify its usage across both CSS and JavaScript files, as it might be used in component styles or dynamic CSS.

Let's do a broader search to ensure we don't miss any usage of this variable, including in JavaScript/TypeScript files where it might be used in template literals or style objects.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage of the transition-duration variable
rg "var\(--transition-duration\)"

Length of output: 126

Script:

#!/bin/bash
# Search for any transition-related properties in CSS files
rg "transition" --type css

# Also search for the exact variable name without var() wrapper
rg "--transition-duration" --type css

Length of output: 627

Script:

#!/bin/bash
# Search for the variable with proper escaping
rg "\-\-transition\-duration" --type css

# Let's also check JavaScript/JSX files for potential usage
rg "transition" --type js --type jsx -A 2

Length of output: 167

Script:

#!/bin/bash
# Search in all files (not just css) to catch any potential usage
rg "\-\-transition\-duration" 

# Search for transition-related properties in all frontend files
fd . frontend -t f -e js -e jsx -e tsx -e css | xargs rg "transition"

Length of output: 9617


/* Enable dark mode */
html.dark {
background-color: #1a202c; /* Dark slate background */
color: #f0e7db; /* Light text */
}
Comment on lines +49 to +53
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add transitions for smooth theme switching.

The dark mode colors provide excellent contrast (14:1 ratio) for accessibility. However, consider adding transitions for a smoother theme switch experience.

 html.dark {
+  transition: background-color var(--transition-duration) ease-in-out,
+              color var(--transition-duration) ease-in-out;
   background-color: #1a202c; /* Dark slate background */
   color: #f0e7db; /* Light text */
 }
+
+html {
+  transition: background-color var(--transition-duration) ease-in-out,
+              color var(--transition-duration) ease-in-out;
+}

Committable suggestion was skipped due to low confidence.

Loading