🎨 Palette: Enhance login feedback and navbar accessibility#35
🎨 Palette: Enhance login feedback and navbar accessibility#35BilalkhanIO wants to merge 1 commit intomainfrom
Conversation
- Implemented a loading state in Login.jsx with a 2-second simulation. - Added styles for disabled buttons in Login.css and signup.css. - Improved accessibility for the Navbar shopping cart using ARIA attributes. - Refactored the login button and Forgot Password link to ensure better reliability. Co-authored-by: BilalkhanIO <48455259+BilalkhanIO@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThe pull request adds loading state management to the Login component with visual feedback via button state changes, applies disabled button styling to Login and Signup components, enhances Navbar accessibility with ARIA attributes, updates the test script configuration, and documents a UX learning regarding form structure for reliable submit button handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@fy-project/src/components/Login/Login.jsx`:
- Around line 20-28: In handleSubmit, remove the console.log that prints the
password (avoid logging credentials—log only non-sensitive info like email or a
generic "login attempted" message) and prevent the potential memory leak by
storing the timeout ID returned from setTimeout (e.g., in a ref) and clearing it
when the component unmounts (add a useEffect cleanup that calls clearTimeout on
that ref or guard state updates with an isMounted flag) so setIsLoading(false)
is not called after unmount.
🧹 Nitpick comments (1)
fy-project/src/components/Signup/signup.css (1)
65-70: Consistent disabled styling across forms.The disabled state styling matches the Login form's approach, maintaining UI consistency. Consider also suppressing the hover effect when disabled to prevent visual confusion:
💅 Optional: suppress hover on disabled state
/* disabled state for submit button */ .form__btn:disabled { opacity: 0.7; cursor: not-allowed; } + +.form__btn:disabled:hover { + background-color: var(--color-golden); +}
| const handleSubmit = (event) => { | ||
| event.preventDefault(); | ||
| // Perform login validation | ||
| console.log("Logging in with email:", email, " and password:", password); | ||
| setIsLoading(true); | ||
|
|
||
| // Simulate a network request | ||
| setTimeout(() => { | ||
| setIsLoading(false); | ||
| console.log("Logging in with email:", email, " and password:", password); | ||
| }, 2000); |
There was a problem hiding this comment.
Security concern: avoid logging passwords, and clean up the timeout.
Two issues with the submit handler:
-
Security: Line 27 logs the password to the console. Even for development/simulation code, this creates a bad pattern and could leak credentials in production logs.
-
Memory leak: If the component unmounts during the 2-second timeout, React will warn about updating state on an unmounted component.
🔒 Proposed fix
+import React, { useState, useEffect, useRef } from "react";
...
function Login() {
const [email, setEmail] = useState("");
const [password, setPassword] = useState("");
const [isLoading, setIsLoading] = useState(false);
+ const timeoutRef = useRef(null);
+
+ useEffect(() => {
+ return () => {
+ if (timeoutRef.current) {
+ clearTimeout(timeoutRef.current);
+ }
+ };
+ }, []);
const handleSubmit = (event) => {
event.preventDefault();
setIsLoading(true);
// Simulate a network request
- setTimeout(() => {
+ timeoutRef.current = setTimeout(() => {
setIsLoading(false);
- console.log("Logging in with email:", email, " and password:", password);
+ console.log("Login submitted for email:", email);
+ // TODO: Replace with actual authentication API call
}, 2000);
};🤖 Prompt for AI Agents
In `@fy-project/src/components/Login/Login.jsx` around lines 20 - 28, In
handleSubmit, remove the console.log that prints the password (avoid logging
credentials—log only non-sensitive info like email or a generic "login
attempted" message) and prevent the potential memory leak by storing the timeout
ID returned from setTimeout (e.g., in a ref) and clearing it when the component
unmounts (add a useEffect cleanup that calls clearTimeout on that ref or guard
state updates with an isMounted flag) so setIsLoading(false) is not called after
unmount.
💡 What
isLoadingstate and a simulated 2-second network request to the Login form.:disabledvisual styles (reduced opacity andnot-allowedcursor) for primary form buttons across Login and Signup pages.aria-labeland marking the internal icon and badge asaria-hidden="true".<li>elements.🎯 Why
♿ Accessibility
aria-label="Shopping cart, 3 items"to ensure the cart's state is clear to assistive technologies.aria-hidden="true"on the icon and numeric badge to prevent redundant announcements.📸 Verification
aria-labelis correctly applied.PR created automatically by Jules for task 18129730110871075199 started by @BilalkhanIO
Summary by CodeRabbit
New Features
Improvements
Chores