issue #439 is solved #451
issue #439 is solved #451
Conversation
|
@Hemraj-7 is attempting to deploy a commit to the bunty's projects Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
|
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. In the meantime, please ensure that your changes align with our CONTRIBUTING.md. If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
![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
🧹 Outside diff range and nitpick comments (4)
frontend/src/components/Shared/Navbar.jsx (4)
Line range hint
39-42: Add dependency array to useEffect for token managementThe useEffect hook for token management lacks a dependency array, causing it to run on every render. This is inefficient and could lead to unnecessary re-renders.
useEffect(() => { setToken(Cookies.get('authToken')); - }); + }, []); // Run only on mount
Line range hint
164-189: Enhance mobile menu accessibilityThe mobile menu button should include ARIA attributes for better accessibility support.
<button onClick={toggleMenu} + aria-label={isMenuOpen ? "Close menu" : "Open menu"} + aria-expanded={isMenuOpen} + aria-controls="mobile-menu" className={`${buttonTextClass} focus:outline-none`} >
Line range hint
232-257: Improve modal accessibility and keyboard interactionThe logout confirmation modal needs accessibility improvements for better keyboard navigation and screen reader support.
Consider these improvements:
- Add keyboard support:
{isModalOpen && ( <div + role="dialog" + aria-labelledby="modal-title" + aria-describedby="modal-description" className="fixed inset-0 flex items-center justify-center bg-black bg-opacity-60 z-50" + onKeyDown={(e) => { + if (e.key === 'Escape') setIsModalOpen(false); + }} > <div + tabIndex="-1" className="w-full max-w-md p-6 rounded-lg border-2 border-black bg-amber-100"> - <h2 className="text-3xl font-bold tracking-tighter mb-4"> + <h2 id="modal-title" className="text-3xl font-bold tracking-tighter mb-4"> Confirm Logout </h2> - <p className="text-base text-muted-foreground mb-6"> + <p id="modal-description" className="text-base text-muted-foreground mb-6">
- Add focus management hook:
useEffect(() => { if (isModalOpen) { // Store the current active element const previousActiveElement = document.activeElement; // Focus the first focusable element in modal const firstButton = modalRef.current.querySelector('button'); firstButton?.focus(); return () => { // Restore focus when modal closes previousActiveElement?.focus(); }; } }, [isModalOpen]);
Line range hint
43-57: Add error handling and loading state to logout processThe logout process should handle potential API errors and show loading state during the request.
+ const [isLoggingOut, setIsLoggingOut] = useState(false); const handleLogout = async () => { + setIsLoggingOut(true); + try { const response = await fetch(`${API_URL}/api/user/logout`, { method: 'POST', headers: { 'Content-Type': 'application/json', }, }) + if (!response.ok) throw new Error('Logout failed'); Cookies.remove('authToken'); setToken(null); setIsModalOpen(false); setIsMenuOpen(false); navigate('/login'); + } catch (error) { + console.error('Logout failed:', error); + // Show error message to user + } finally { + setIsLoggingOut(false); + } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
frontend/src/components/Shared/Navbar.jsx(1 hunks)
🔇 Additional comments (1)
frontend/src/components/Shared/Navbar.jsx (1)
226-226: Well-implemented theme toggle feature
The theme toggle is properly integrated into both desktop and mobile views, with appropriate dark mode classes throughout the component. This successfully implements the PR objective.
RamakrushnaBiswal
merged commit 92d0389
into
RamakrushnaBiswal:main
RamakrushnaBiswal
added
bug
Issue number #439 is solved.
Theme mode button added in the hamburger menu.
now the website is looking good in the mobile phone.
Before
After
Summary by CodeRabbit
New Features
ThemeTogglecomponent for easy switching between themes in both desktop and mobile menus.Improvements
These enhancements aim to improve user experience and interface interaction.