feat: improve mobile responsiveness and auth UX (closes #413)#851
Open
singhanurag0317-bit wants to merge 1 commit into
Open
feat: improve mobile responsiveness and auth UX (closes #413)#851singhanurag0317-bit wants to merge 1 commit into
singhanurag0317-bit wants to merge 1 commit into
Conversation
📝 WalkthroughWalkthroughThis PR implements dark mode theming across the frontend with Tailwind CSS and localStorage persistence, integrates Google OAuth login, updates post-signup routing to handle pending email verification states, redesigns authentication pages with new UI styling, and refines responsive design breakpoints. ChangesDark Mode Theme and Authentication Infrastructure
Authentication Pages Redesign and Google OAuth Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Frontend/src/pages/Signup.jsx (1)
251-307:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe required company picker is mouse-only.
The trigger and options are plain
divs with click handlers, so keyboard users can't properly open, navigate, or select a company. Since this field is required, that blocks signup. Please switch this to an actual button/listbox-combobox pattern or a native/select-based control.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frontend/src/pages/Signup.jsx` around lines 251 - 307, The company picker uses plain divs and is inaccessible to keyboard users; replace the clickable div trigger and option divs with an accessible control (either a native <select> or an ARIA Listbox/Combobox) and add keyboard handling/focus management. Specifically, change the trigger that currently uses setIsDropdownOpen/isDropdownOpen and selectedCompany into a button or Listbox trigger with aria-haspopup, aria-expanded and aria-required, ensure the search input (companySearch/setCompanySearch) stops propagation but is focusable, render each filteredCompanies item with role="option"/tabindex and aria-selected and wire keyboard handlers (Enter/Space to select via setSelectedCompany, ArrowUp/ArrowDown to move focus, Escape to close via setIsDropdownOpen(false)). Also ensure the option onClick behavior (setSelectedCompany, setIsDropdownOpen, setCompanySearch) is mirrored for keyboard activation and that focus moves appropriately after selection.Frontend/src/pages/ForgotPassword.jsx (1)
98-103:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInconsistent password length requirement with ResetPassword page.
This validation requires a minimum of 6 characters, while
ResetPassword.jsx(line 28) requires 8 characters. Users might encounter different password requirements depending on which recovery flow they use, which can be confusing.Recommend standardizing the minimum password length across both password reset flows.
🔧 Suggested fix to align with ResetPassword.jsx
- if (!newPassword || newPassword.length < 6) { - setError("Password must be at least 6 characters long."); + if (!newPassword || newPassword.length < 8) { + setError("Password must be at least 8 characters long."); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frontend/src/pages/ForgotPassword.jsx` around lines 98 - 103, The password minimum length in the ForgotPassword flow is inconsistent: update the validation inside handleUpdatePassword to require the same minimum as ResetPassword (use 8 characters) so both flows match; locate handleUpdatePassword in ForgotPassword.jsx and change the check/validation message from 6 to 8 characters to mirror ResetPassword.jsx's requirement and ensure any UI/error text there is updated accordingly.
🧹 Nitpick comments (2)
Frontend/src/admin/components/AdminHeader.jsx (1)
23-38: ⚡ Quick winExtract theme toggle logic to a shared custom hook.
The theme toggle implementation (state initialization, DOM class sync, localStorage persistence) is duplicated verbatim in
TopNav.jsx(lines 20-36). Consider extracting this logic into a custom hook likeuseTheme()to eliminate duplication and ensure consistent behavior across both components.♻️ Example custom hook
Create
Frontend/src/hooks/useTheme.js:import { useState, useEffect } from 'react'; export const useTheme = () => { const [isDark, setIsDark] = useState(() => document.documentElement.classList.contains('dark') ); useEffect(() => { setIsDark(document.documentElement.classList.contains('dark')); }, []); const toggleTheme = () => { const nextDark = !isDark; setIsDark(nextDark); if (nextDark) { document.documentElement.classList.add('dark'); localStorage.setItem('theme', 'dark'); } else { document.documentElement.classList.remove('dark'); localStorage.setItem('theme', 'light'); } }; return { isDark, toggleTheme }; };Then replace lines 23-38 in both
AdminHeader.jsxandTopNav.jsxwith:-const [isDark, setIsDark] = useState(false); -useEffect(() => { - setIsDark(document.documentElement.classList.contains('dark')); -}, []); - -const toggleTheme = () => { - const nextDark = !isDark; - setIsDark(nextDark); - if (nextDark) { - document.documentElement.classList.add('dark'); - localStorage.setItem('theme', 'dark'); - } else { - document.documentElement.classList.remove('dark'); - localStorage.setItem('theme', 'light'); - } -}; +const { isDark, toggleTheme } = useTheme();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frontend/src/admin/components/AdminHeader.jsx` around lines 23 - 38, Extract the duplicated theme logic into a shared custom hook named useTheme: create and export a hook (export const useTheme) that initializes isDark from document.documentElement.classList.contains('dark'), keeps DOM sync in a useEffect, and exposes a toggleTheme that flips isDark, updates document.documentElement.classList and writes 'theme' to localStorage; then remove the local useState/useEffect/toggleTheme blocks from AdminHeader.jsx and TopNav.jsx and replace them with an import of useTheme and use the returned { isDark, toggleTheme } values.Frontend/src/pages/ForgotPassword.jsx (1)
241-284: 💤 Low valueConsider using Tailwind color utilities for disabled state.
Line 268 uses hardcoded hex colors for the disabled button state (
disabled:bg-[#e5e7eb] dark:disabled:bg-[#223c2f]), which is inconsistent with the rest of the codebase that uses Tailwind's color scale (e.g.,bg-emerald-600,text-slate-400).Consider using standard Tailwind utilities:
♻️ Suggested refactor for consistency
- className="w-full rounded-2xl py-4 font-bold transition-all flex items-center justify-center gap-2 text-white bg-gradient-to-r from-emerald-600 to-emerald-500 shadow-lg shadow-emerald-600/20 disabled:bg-[`#e5e7eb`] dark:disabled:bg-[`#223c2f`] disabled:text-[`#9ca3af`] disabled:shadow-none" + className="w-full rounded-2xl py-4 font-bold transition-all flex items-center justify-center gap-2 text-white bg-gradient-to-r from-emerald-600 to-emerald-500 shadow-lg shadow-emerald-600/20 disabled:bg-gray-200 dark:disabled:bg-gray-800 disabled:text-gray-400 disabled:shadow-none"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Frontend/src/pages/ForgotPassword.jsx` around lines 241 - 284, Replace the hardcoded hex disabled colors on the submit button in the STEP 2 OTP form with Tailwind color utilities for consistency: in the button that uses loading/otp/timerExpired state (the type="submit" button rendering "Verify Identity Signature") swap disabled:bg-[`#e5e7eb`] for a Tailwind utility like disabled:bg-slate-200 and dark:disabled:bg-slate-800, and replace disabled:text-[`#9ca3af`] with disabled:text-slate-400 (keep disabled:shadow-none). This change ensures consistency with existing classes (e.g., bg-emerald-600, text-slate-400) and touches the JSX element that references loading, otp, timerExpired and uses onMouseEnter/onMouseLeave.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Frontend/src/pages/Login.jsx`:
- Around line 101-108: The Google login catch currently sets a generic message;
update handleGoogleLogin to normalize errors the same way as the
password/magic-link handlers by reusing the same normalization logic (e.g. call
the same helper used in those handlers or replicate its checks) before calling
setError, so failures like fetch/AdBlock or Supabase-specific errors are
detected and produce the same diagnostic text; locate handleGoogleLogin and
loginWithGoogle and replace the catch body to pass err through the existing
normalization function (or inline its checks for fetch failures and Supabase
error fields) and then call setError(normalizedMessage).
In `@Frontend/src/pages/Signup.jsx`:
- Around line 172-179: handleGoogleSignup currently calls loginWithGoogle()
which completes OAuth but drops the signup form context (fullName,
selectedCompany, role "user", company_id), allowing users to bypass required
signup data; update handleGoogleSignup to preserve and forward the signup fields
after the OAuth flow (or block completion until they are provided): after
loginWithGoogle resolves, call the profile-creation path that accepts the OAuth
token plus fullName, selectedCompany (or its id as company_id), and role "user"
(e.g., a helper like createProfileWithGoogle / completeSignupWithOAuth), or
redirect back to the Signup form with the OAuth result so the user must submit
those fields before finalizing account creation. Ensure you reference and
populate the unique symbols fullName, selectedCompany/company_id and role when
creating the user profile.
In `@Frontend/src/store/authStore.js`:
- Around line 49-53: The upgrade path that calls get().updateProfile({ status:
'pending_approval' }) when user.email_confirmed_at && dbProfile.status ===
'pending_email_verification' must be wrapped in a try/catch: inside the try call
updateProfile and return the updated result if truthy; in catch log the error
(including context like user.id or email) and fall back to returning dbProfile
so calling sites (e.g., getCurrentUser and the initialize auth listener) don’t
get an unhandled rejection; ensure you reference the existing dbProfile and
updateProfile symbols and preserve the existing console message when successful.
---
Outside diff comments:
In `@Frontend/src/pages/ForgotPassword.jsx`:
- Around line 98-103: The password minimum length in the ForgotPassword flow is
inconsistent: update the validation inside handleUpdatePassword to require the
same minimum as ResetPassword (use 8 characters) so both flows match; locate
handleUpdatePassword in ForgotPassword.jsx and change the check/validation
message from 6 to 8 characters to mirror ResetPassword.jsx's requirement and
ensure any UI/error text there is updated accordingly.
In `@Frontend/src/pages/Signup.jsx`:
- Around line 251-307: The company picker uses plain divs and is inaccessible to
keyboard users; replace the clickable div trigger and option divs with an
accessible control (either a native <select> or an ARIA Listbox/Combobox) and
add keyboard handling/focus management. Specifically, change the trigger that
currently uses setIsDropdownOpen/isDropdownOpen and selectedCompany into a
button or Listbox trigger with aria-haspopup, aria-expanded and aria-required,
ensure the search input (companySearch/setCompanySearch) stops propagation but
is focusable, render each filteredCompanies item with role="option"/tabindex and
aria-selected and wire keyboard handlers (Enter/Space to select via
setSelectedCompany, ArrowUp/ArrowDown to move focus, Escape to close via
setIsDropdownOpen(false)). Also ensure the option onClick behavior
(setSelectedCompany, setIsDropdownOpen, setCompanySearch) is mirrored for
keyboard activation and that focus moves appropriately after selection.
---
Nitpick comments:
In `@Frontend/src/admin/components/AdminHeader.jsx`:
- Around line 23-38: Extract the duplicated theme logic into a shared custom
hook named useTheme: create and export a hook (export const useTheme) that
initializes isDark from document.documentElement.classList.contains('dark'),
keeps DOM sync in a useEffect, and exposes a toggleTheme that flips isDark,
updates document.documentElement.classList and writes 'theme' to localStorage;
then remove the local useState/useEffect/toggleTheme blocks from AdminHeader.jsx
and TopNav.jsx and replace them with an import of useTheme and use the returned
{ isDark, toggleTheme } values.
In `@Frontend/src/pages/ForgotPassword.jsx`:
- Around line 241-284: Replace the hardcoded hex disabled colors on the submit
button in the STEP 2 OTP form with Tailwind color utilities for consistency: in
the button that uses loading/otp/timerExpired state (the type="submit" button
rendering "Verify Identity Signature") swap disabled:bg-[`#e5e7eb`] for a Tailwind
utility like disabled:bg-slate-200 and dark:disabled:bg-slate-800, and replace
disabled:text-[`#9ca3af`] with disabled:text-slate-400 (keep
disabled:shadow-none). This change ensures consistency with existing classes
(e.g., bg-emerald-600, text-slate-400) and touches the JSX element that
references loading, otp, timerExpired and uses onMouseEnter/onMouseLeave.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 878ae2eb-26f7-4144-9925-11f0e5966f24
📒 Files selected for processing (12)
Frontend/src/App.jsxFrontend/src/admin/components/AdminHeader.jsxFrontend/src/admin/layout/AdminLayout.jsxFrontend/src/components/shared/BugReportWidget.jsxFrontend/src/pages/AdminSignup.jsxFrontend/src/pages/ForgotPassword.jsxFrontend/src/pages/Login.jsxFrontend/src/pages/ResetPassword.jsxFrontend/src/pages/Signup.jsxFrontend/src/store/authStore.jsFrontend/src/user/UserLayout.jsxFrontend/src/user/components/TopNav.jsx
Comment on lines
+101
to
+108
| const handleGoogleLogin = async () => { | ||
| try { | ||
| setError(""); | ||
| await loginWithGoogle(); | ||
| } catch (err) { | ||
| console.error("Google login error:", err); | ||
| setError(err.message || "Google Sign-In failed."); | ||
| } |
There was a problem hiding this comment.
Normalize Google OAuth fetch failures the same way as the other auth paths.
This new handler falls back to a generic error, so the Google flow loses the ad-blocker/Supabase diagnostic you already added for password and magic-link sign-in.
Suggested fix
const handleGoogleLogin = async () => {
try {
setError("");
await loginWithGoogle();
} catch (err) {
console.error("Google login error:", err);
- setError(err.message || "Google Sign-In failed.");
+ let errMsg = err.message || "Google Sign-In failed.";
+ if (errMsg.toLowerCase().includes("failed to fetch")) {
+ errMsg = "Network Error: Failed to fetch. This usually happens if your browser's ad-blocker is blocking Supabase requests. Please try disabling your ad-blocker for this site and refresh!";
+ }
+ setError(errMsg);
}
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Frontend/src/pages/Login.jsx` around lines 101 - 108, The Google login catch
currently sets a generic message; update handleGoogleLogin to normalize errors
the same way as the password/magic-link handlers by reusing the same
normalization logic (e.g. call the same helper used in those handlers or
replicate its checks) before calling setError, so failures like fetch/AdBlock or
Supabase-specific errors are detected and produce the same diagnostic text;
locate handleGoogleLogin and loginWithGoogle and replace the catch body to pass
err through the existing normalization function (or inline its checks for fetch
failures and Supabase error fields) and then call setError(normalizedMessage).
Comment on lines
+172
to
+179
| const handleGoogleSignup = async () => { | ||
| try { | ||
| setError(""); | ||
| await loginWithGoogle(); | ||
| } catch (err) { | ||
| console.error("Google signup error:", err); | ||
| setError(err.message || "Google Sign-up failed."); | ||
| } |
There was a problem hiding this comment.
This Google path bypasses the required signup data.
loginWithGoogle() starts the same OAuth flow as login, but this page's signup contract depends on fullName, selectedCompany, role "user", and company_id being carried into profile creation. Right now a user can leave this screen through Google without any of that context, so the pending-approval/company-association flow can’t be completed reliably.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Frontend/src/pages/Signup.jsx` around lines 172 - 179, handleGoogleSignup
currently calls loginWithGoogle() which completes OAuth but drops the signup
form context (fullName, selectedCompany, role "user", company_id), allowing
users to bypass required signup data; update handleGoogleSignup to preserve and
forward the signup fields after the OAuth flow (or block completion until they
are provided): after loginWithGoogle resolves, call the profile-creation path
that accepts the OAuth token plus fullName, selectedCompany (or its id as
company_id), and role "user" (e.g., a helper like createProfileWithGoogle /
completeSignupWithOAuth), or redirect back to the Signup form with the OAuth
result so the user must submit those fields before finalizing account creation.
Ensure you reference and populate the unique symbols fullName,
selectedCompany/company_id and role when creating the user profile.
Comment on lines
+49
to
+53
| if (user.email_confirmed_at && dbProfile.status === 'pending_email_verification') { | ||
| console.log("Email confirmed! Upgrading status in database to pending_approval."); | ||
| const updated = await get().updateProfile({ status: 'pending_approval' }); | ||
| if (updated) return updated; | ||
| } |
There was a problem hiding this comment.
Add error handling for the profile status upgrade.
When updateProfile is called here to upgrade pending_email_verification to pending_approval, errors are not caught. Since getProfile is invoked without await in getCurrentUser (line 94) and the initialize auth listener (line 308), a thrown error from updateProfile will result in an unhandled promise rejection in those code paths.
Wrap the upgrade call in a try/catch block to prevent console errors and ensure the function gracefully falls back to returning dbProfile if the update fails.
🛡️ Proposed fix
if (user.email_confirmed_at && dbProfile.status === 'pending_email_verification') {
console.log("Email confirmed! Upgrading status in database to pending_approval.");
- const updated = await get().updateProfile({ status: 'pending_approval' });
- if (updated) return updated;
+ try {
+ const updated = await get().updateProfile({ status: 'pending_approval' });
+ if (updated) return updated;
+ } catch (err) {
+ console.warn("Profile status upgrade failed, continuing with DB profile:", err);
+ }
}📝 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
| if (user.email_confirmed_at && dbProfile.status === 'pending_email_verification') { | |
| console.log("Email confirmed! Upgrading status in database to pending_approval."); | |
| const updated = await get().updateProfile({ status: 'pending_approval' }); | |
| if (updated) return updated; | |
| } | |
| if (user.email_confirmed_at && dbProfile.status === 'pending_email_verification') { | |
| console.log("Email confirmed! Upgrading status in database to pending_approval."); | |
| try { | |
| const updated = await get().updateProfile({ status: 'pending_approval' }); | |
| if (updated) return updated; | |
| } catch (err) { | |
| console.warn("Profile status upgrade failed, continuing with DB profile:", err); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Frontend/src/store/authStore.js` around lines 49 - 53, The upgrade path that
calls get().updateProfile({ status: 'pending_approval' }) when
user.email_confirmed_at && dbProfile.status === 'pending_email_verification'
must be wrapped in a try/catch: inside the try call updateProfile and return the
updated result if truthy; in catch log the error (including context like user.id
or email) and fall back to returning dbProfile so calling sites (e.g.,
getCurrentUser and the initialize auth listener) don’t get an unhandled
rejection; ensure you reference the existing dbProfile and updateProfile symbols
and preserve the existing console message when successful.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request resolves Issue #413 ("Improve Mobile Responsiveness and Authentication UX") by modernizing the layouts, responsive styles, navigation capabilities, and widgets in the frontend client.
Key Improvements
Authentication Store & Session Sync:
loginWithGoogleinsideauthStore.jsleveraging Supabase OAuth with a dynamic origin redirect target.getProfileto automatically detect confirmed user sessions (user.email_confirmed_at) and dynamically update profiles frompending_email_verificationtopending_approvallocally and in the database to prevent redirection loops.Global Themes & Adaptive Headers:
localStoragewith system default fallback (prefers-color-scheme) insideApp.jsx.TopNav.jsx(both desktop header and mobile navigation menu) andAdminHeader.jsx.UserLayout.jsxandAdminLayout.jsx) with theme-awaretransition-colorsbackgrounds (dark:bg-[#102219] dark:text-slate-100).Refactored Onboarding Screens:
Login.jsx,Signup.jsx,ForgotPassword.jsx,ResetPassword.jsx, andAdminSignup.jsx.AdminSignup.jsx).Responsive Diagnostics Widget:
BugReportWidget.jsxtext-badge into a minimal circular icon button on screen sizes smaller thanmd(hidden md:block), freeing up precious screen space.bottom-4 right-4on mobile andmd:bottom-6 md:right-6on desktop.Verification
npm run build) inFrontendsuccessfully:dist/index.html 1.69 kB dist/assets/index-DMZR6E-d.css 130.32 kB dist/assets/index-BTaNHvCa.js 2,223.31 kB ✓ built in 27.52sSummary by CodeRabbit
Release Notes
New Features
Style