🎨 Palette: Improve Login UX and Navbar accessibility#36
🎨 Palette: Improve Login UX and Navbar accessibility#36BilalkhanIO wants to merge 1 commit intomainfrom
Conversation
- Added isLoading state and cleanup to Login.jsx - Improved login button feedback with "Logging in..." text and disabled state - Added :disabled styles to .form__btn in Login.css and signup.css - Prevented hover effects on disabled buttons using :not(:disabled) - Added aria-label to Navbar cart link and aria-hidden to its contents - Added cleanup logic to Signup.jsx for consistency 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. |
📝 WalkthroughWalkthroughThis pull request introduces async state cleanup patterns using React hooks (useRef and useEffect), implements disabled button styling across login and signup components, adds accessibility enhancements to icon-only links, and documents UX learning patterns. Changes span CSS styling updates, component logic modifications, and accessibility improvements across the authentication and navigation components. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
fy-project/src/components/Signup/Signup.jsx (1)
44-54:⚠️ Potential issue | 🟠 MajorRemove credential logging from the client.
Logging password and confirmPassword is a security/PII risk and should be removed or masked.🛡️ Suggested fix
- console.log( - "Signing up with:", - { - name, - email, - password, - confirmPassword - } - ); + // Avoid logging credentials in client logs + console.log("Signing up with:", { name, email });
🤖 Fix all issues with AI agents
In `@fy-project/src/components/Login/Login.jsx`:
- Around line 33-37: Remove the plaintext credential logging in the simulated
network request: in the setTimeout callback where timeoutRef.current is set,
eliminate the console.log that prints email and password (or if you need an
audit, only log non-sensitive info and mask the email/password, e.g., show email
domain or replace password with fixed asterisks). Update the block that calls
setIsLoading(false) inside the timeout to no longer reference or print the
password (remove console.log(email, password) and, if needed, replace with a
safe message like "Login simulated" or a masked email via the existing email
variable).
In `@fy-project/src/components/Navbar/Navbar.jsx`:
- Around line 24-26: Replace the hard-coded cart count in Navbar.jsx with a
derived value (e.g., a prop or state like cartCount) and reuse that single
variable for both the aria-label and the visible badge; update the anchor's
aria-label to include `${cartCount} items` and render the same cartCount inside
the <span className='navbar__cart_span'> so they stay synchronized (ensure the
component that renders Navbar passes or computes cartCount and that
FiShoppingBag keeps aria-hidden="true" while the span is exposed to assistive
tech).
| // Simulate a network request | ||
| timeoutRef.current = setTimeout(() => { | ||
| setIsLoading(false); | ||
| console.log("Logging in with email:", email, " and password:", password); | ||
| }, 1500); // Simulate a 1.5-second delay |
There was a problem hiding this comment.
Remove credential logging from the client.
Logging passwords (even in dev) is a security/PII risk and can leak via console capture tools. Please remove or mask sensitive fields.
🛡️ Suggested fix
- console.log("Logging in with email:", email, " and password:", password);
+ // Avoid logging credentials in client logs
+ console.log("Logging in with email:", email);📝 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.
| // Simulate a network request | |
| timeoutRef.current = setTimeout(() => { | |
| setIsLoading(false); | |
| console.log("Logging in with email:", email, " and password:", password); | |
| }, 1500); // Simulate a 1.5-second delay | |
| // Simulate a network request | |
| timeoutRef.current = setTimeout(() => { | |
| setIsLoading(false); | |
| // Avoid logging credentials in client logs | |
| console.log("Logging in with email:", email); | |
| }, 1500); // Simulate a 1.5-second delay |
🤖 Prompt for AI Agents
In `@fy-project/src/components/Login/Login.jsx` around lines 33 - 37, Remove the
plaintext credential logging in the simulated network request: in the setTimeout
callback where timeoutRef.current is set, eliminate the console.log that prints
email and password (or if you need an audit, only log non-sensitive info and
mask the email/password, e.g., show email domain or replace password with fixed
asterisks). Update the block that calls setIsLoading(false) inside the timeout
to no longer reference or print the password (remove console.log(email,
password) and, if needed, replace with a safe message like "Login simulated" or
a masked email via the existing email variable).
| <a href='#addToCart' className='navbar__cart' aria-label="View shopping cart, 3 items"> | ||
| <FiShoppingBag className="navbar__cart-icon" aria-hidden="true" /> | ||
| <span className='navbar__cart_span' aria-hidden="true">3</span> |
There was a problem hiding this comment.
Avoid hard‑coding cart count in the aria‑label and badge.
If the cart count changes, the label and badge will become inaccurate for users and assistive tech. Consider deriving the count from state/props and reusing it for both the label and the badge.
🔧 Suggested direction
- <a href='#addToCart' className='navbar__cart' aria-label="View shopping cart, 3 items">
- <FiShoppingBag className="navbar__cart-icon" aria-hidden="true" />
- <span className='navbar__cart_span' aria-hidden="true">3</span>
- </a>
+ {/* e.g. const cartCount = props.cartCount; */}
+ <a
+ href='#addToCart'
+ className='navbar__cart'
+ aria-label={`View shopping cart, ${cartCount} items`}
+ >
+ <FiShoppingBag className="navbar__cart-icon" aria-hidden="true" />
+ <span className='navbar__cart_span' aria-hidden="true">{cartCount}</span>
+ </a>🤖 Prompt for AI Agents
In `@fy-project/src/components/Navbar/Navbar.jsx` around lines 24 - 26, Replace
the hard-coded cart count in Navbar.jsx with a derived value (e.g., a prop or
state like cartCount) and reuse that single variable for both the aria-label and
the visible badge; update the anchor's aria-label to include `${cartCount}
items` and render the same cartCount inside the <span
className='navbar__cart_span'> so they stay synchronized (ensure the component
that renders Navbar passes or computes cartCount and that FiShoppingBag keeps
aria-hidden="true" while the span is exposed to assistive tech).
1 participant
This PR implements several micro-UX and accessibility improvements:
💡 What:
:disabledstyling for form buttons.🎯 Why:
♿ Accessibility:
aria-label="View shopping cart, 3 items"to the cart link.aria-hidden="true"to decorative icons and redundant text badges.PR created automatically by Jules for task 17783112272834294221 started by @BilalkhanIO
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation