🎨 Palette: Improve form submission UX and Navbar accessibility#32
🎨 Palette: Improve form submission UX and Navbar accessibility#32BilalkhanIO wants to merge 1 commit intomainfrom
Conversation
- Add isLoading state to Login component - Add :disabled styles for form buttons - Add aria-label to Navbar cart icon - Initialize .Jules/palette.md with UX learnings 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. |
📝 WalkthroughWalkthroughThese changes implement consistent loading feedback for form submissions across Login and Signup components by introducing isLoading state management, adding disabled button styles, improving Navbar accessibility with ARIA attributes, and documenting the design pattern in a palette note. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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)
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: 0
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/Login/Login.jsx (1)
10-28:⚠️ Potential issue | 🟠 MajorAvoid logging credentials and clean up the timeout to prevent setState-after-unmount.
Line 27 logs the password in plaintext to console, which is a security risk. The setTimeout on lines 25-28 lacks cleanup and can fire after unmount, causing a memory leak warning in React. Additionally, add an early guard in handleSubmit to prevent rapid double submits before state updates propagate.
🛠️ Proposed fix
-import React, { useState } from "react"; +import React, { useEffect, useRef, useState } from "react"; import { Link } from "react-router-dom"; import Navbar from "../Navbar/Navbar"; import './Login.css' 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 handleEmailChange = (event) => { setEmail(event.target.value); }; const handlePasswordChange = (event) => { setPassword(event.target.value); }; const handleSubmit = (event) => { event.preventDefault(); + if (isLoading) return; 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"); }, 2000); };
🧹 Nitpick comments (1)
fy-project/src/components/Navbar/Navbar.jsx (1)
24-26: Keep cart count and aria-label in sync via a single source.Right now the count is duplicated in two places; if it ever becomes dynamic, it can drift. Consider deriving the label from the same count value.
♻️ Suggested refactor
function Navbar() { + const cartCount = 3; + const cartLabel = `View shopping cart, ${cartCount} ${cartCount === 1 ? "item" : "items"}`; return ( <div className='header'> @@ - <a href='#addToCart' className='navbar__cart' aria-label="View shopping cart, 3 items"> + <a href='#addToCart' className='navbar__cart' aria-label={cartLabel}> <FiShoppingBag className="navbar__cart-icon" aria-hidden="true" /> - <span className='navbar__cart_span' aria-hidden="true">3</span> + <span className='navbar__cart_span' aria-hidden="true">{cartCount}</span> </a>
1 participant
This PR introduces a few micro-UX and accessibility improvements:
isLoadingstate to the Login form. When submitting, the button now shows "Logging in..." and is disabled, providing immediate feedback and preventing double-clicks.:disabledstyles (lower opacity and 'not-allowed' cursor) to the common.form__btnclass in both Login and Signup stylesheets.aria-labelto the shopping cart icon in the Navbar, along witharia-hiddenon its children, to ensure screen reader users understand the link's purpose and the current item count.These changes ensure a more consistent and accessible experience across the application's core flows.
PR created automatically by Jules for task 7062473540464968753 started by @BilalkhanIO
Summary by CodeRabbit
New Features
Style
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.