feat(profile): add profile page, edit profile UI, and logout flow#780
feat(profile): add profile page, edit profile UI, and logout flow#780Prashik-Sasane wants to merge 1 commit intoAOSSIE-Org:mainfrom
Conversation
|
|
WalkthroughIntroduces a user profile feature with edit and logout capabilities. Adds a new AlertDialog UI component, LogoutDialog confirmation component, ProfilePage and EditProfilePage for viewing and editing user profiles, and updates the Navbar with styling refinements and navigation link changes. Updates onboardingSlice with localStorage persistence and a new resetOnboarding action. Adds profile routes to the application. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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: 3
🧹 Nitpick comments (6)
frontend/src/pages/ProfilePage/ProfilePage.tsx (2)
19-22: Use React Router'suseNavigateinstead ofwindow.location.href.Using
window.location.hrefcauses a full page reload, which loses React state and defeats the purpose of a SPA. This is especially problematic since you're dispatching a Redux action right before navigation.+import { useNavigate } from 'react-router'; + const ProfilePage = () => { const dispatch = useDispatch(); + const navigate = useNavigate(); const { theme } = useTheme(); // ... const handleLogout = () => { dispatch(resetOnboarding()); - window.location.href = '/'; + navigate('/'); };
82-96: Use React Router's<Link>component for internal navigation.Using
<a href="...">for internal routes causes full page reloads. Replace with<Link to="...">fromreact-routerto maintain SPA behavior and preserve application state.+import { Link } from 'react-router'; + - <a - href="/profile/edit" + <Link + to="/profile/edit" className="border-border bg-muted hover:bg-accent flex items-center justify-between rounded-xl border px-4 py-3 text-sm font-medium" > <span>Edit Profile</span> <span className="text-muted-foreground text-xs">⌘E</span> - </a> + </Link> - <a - href="/settings" + <Link + to="/settings" className="border-border bg-muted hover:bg-accent flex items-center justify-between rounded-xl border px-4 py-3 text-sm font-medium" > <span>Account Settings</span> <span className="text-muted-foreground text-xs">⚙</span> - </a> + </Link>frontend/src/pages/ProfilePage/EditProfilePage.tsx (2)
16-20: Use React Router'suseNavigateinstead ofwindow.location.href.Same issue as ProfilePage—full page reload loses state and SPA benefits.
+import { useNavigate } from 'react-router'; + const EditProfilePage = () => { const dispatch = useDispatch(); + const navigate = useNavigate(); // ... const handleSave = () => { dispatch(setName(name)); dispatch(setAvatar(avatar)); - window.location.href = '/profile'; + navigate('/profile'); };
27-32: Use<Link>from React Router for internal navigation.Replace
<a href="...">with<Link to="...">to prevent full page reloads.Also applies to: 77-79
frontend/src/features/onboardingSlice.ts (1)
25-34: Side effects in reducers violate Redux principles.Calling
localStorage.setIteminside reducers is a side effect. While Redux Toolkit allows mutations via Immer, side effects should ideally be handled in middleware, thunks, or through listeners. This works but can cause issues with time-travel debugging and testing.Consider moving localStorage persistence to a Redux listener middleware or handling it in the component layer if you want stricter Redux purity. For a UI-first implementation this is acceptable, but worth noting for future backend integration.
frontend/src/components/Navigation/Navbar/Navbar.tsx (1)
89-95: Consider using<Link>for internal navigation.Similar to the profile pages, using
<Link to="/profile">instead of<a href="/profile">would maintain SPA navigation. However, I note the logo also uses<a href="/">, so this follows the existing pattern in this file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
frontend/src/components/Dialog/LogoutDialog.tsx(1 hunks)frontend/src/components/Navigation/Navbar/Navbar.tsx(4 hunks)frontend/src/components/ui/alert-dialog.tsx(1 hunks)frontend/src/constants/routes.ts(1 hunks)frontend/src/features/onboardingSlice.ts(1 hunks)frontend/src/pages/ProfilePage/EditProfilePage.tsx(1 hunks)frontend/src/pages/ProfilePage/ProfilePage.tsx(1 hunks)frontend/src/routes/AppRoutes.tsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
frontend/src/pages/ProfilePage/EditProfilePage.tsx (2)
frontend/src/features/onboardingSelectors.ts (2)
selectName(15-15)selectAvatar(13-13)frontend/src/features/onboardingSlice.ts (2)
setName(31-34)setAvatar(26-29)
frontend/src/routes/AppRoutes.tsx (1)
frontend/src/constants/routes.ts (1)
ROUTES(1-14)
frontend/src/pages/ProfilePage/ProfilePage.tsx (4)
frontend/src/contexts/ThemeContext.tsx (1)
useTheme(66-73)frontend/src/features/onboardingSelectors.ts (2)
selectName(15-15)selectAvatar(13-13)frontend/src/features/onboardingSlice.ts (1)
resetOnboarding(37-41)frontend/src/components/Dialog/LogoutDialog.tsx (1)
LogoutDialog(18-42)
frontend/src/components/Dialog/LogoutDialog.tsx (1)
frontend/src/components/ui/alert-dialog.tsx (8)
AlertDialog(146-146)AlertDialogContent(148-148)AlertDialogHeader(149-149)AlertDialogTitle(150-150)AlertDialogDescription(151-151)AlertDialogFooter(152-152)AlertDialogCancel(154-154)AlertDialogAction(153-153)
frontend/src/components/Navigation/Navbar/Navbar.tsx (3)
frontend/src/features/onboardingSelectors.ts (2)
selectName(15-15)selectAvatar(13-13)frontend/src/components/ui/input.tsx (1)
Input(21-21)frontend/src/components/Dialog/FaceSearchDialog.tsx (1)
FaceSearchDialog(25-153)
🔇 Additional comments (5)
frontend/src/constants/routes.ts (1)
12-13: LGTM!The new route constants follow the existing naming conventions and the nested route structure (
profile/editunderprofile) is logical.frontend/src/routes/AppRoutes.tsx (1)
12-13: LGTM!The new page imports and routes are correctly integrated within the
Layoutwrapper, consistent with other routes in the application.Also applies to: 28-29
frontend/src/components/Dialog/LogoutDialog.tsx (1)
18-41: LGTM!Clean implementation using the AlertDialog primitives. The
onOpenChangecallback correctly handles closing via overlay click or escape key by callingonCancel.frontend/src/components/Navigation/Navbar/Navbar.tsx (1)
20-21: LGTM!Good addition of the
displayNamefallback to 'Guest' for unauthenticated users, and the welcome message displays it cleanly.Also applies to: 85-86
frontend/src/components/ui/alert-dialog.tsx (1)
1-155: Well-structured AlertDialog component.This follows the standard shadcn/ui pattern for wrapping Radix UI primitives. Proper use of
forwardRef,displayNameassignments, and class merging withcn(). The component is ready for reuse across the application.
| /** ✅ LOGOUT / RESET */ | ||
| resetOnboarding() { | ||
| localStorage.removeItem('avatar'); | ||
| localStorage.removeItem('name'); | ||
| return initialState; | ||
| }, |
There was a problem hiding this comment.
resetOnboarding returns stale initialState captured at module load time.
initialState is computed once when the module loads, reading from localStorage at that time. After clearing localStorage in resetOnboarding, returning this cached object still contains the old values that were in localStorage when the app started.
resetOnboarding() {
localStorage.removeItem('avatar');
localStorage.removeItem('name');
- return initialState;
+ return {
+ currentStepIndex: 0,
+ currentStepName: STEP_NAMES[0],
+ stepStatus: STEP_NAMES.map(() => false),
+ avatar: null,
+ name: '',
+ };
},📝 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.
| /** ✅ LOGOUT / RESET */ | |
| resetOnboarding() { | |
| localStorage.removeItem('avatar'); | |
| localStorage.removeItem('name'); | |
| return initialState; | |
| }, | |
| /** ✅ LOGOUT / RESET */ | |
| resetOnboarding() { | |
| localStorage.removeItem('avatar'); | |
| localStorage.removeItem('name'); | |
| return { | |
| currentStepIndex: 0, | |
| currentStepName: STEP_NAMES[0], | |
| stepStatus: STEP_NAMES.map(() => false), | |
| avatar: null, | |
| name: '', | |
| }; | |
| }, |
🤖 Prompt for AI Agents
In frontend/src/features/onboardingSlice.ts around lines 36 to 41,
resetOnboarding currently returns the module-level initialState which was
computed once at module load (stale), so after removing localStorage keys you
must return a fresh state derived from current storage or default values instead
of the cached object; modify resetOnboarding to compute and return a new state
object (e.g., read localStorage for avatar/name or use empty/default values) or
call a helper getInitialState() that builds the state from current localStorage,
ensuring you do not return the original module-scoped initialState reference.
| <img | ||
| src={avatar} | ||
| className="h-24 w-24 rounded-full border object-cover" | ||
| /> |
There was a problem hiding this comment.
Add alt attribute to the image for accessibility.
The avatar image is missing an alt attribute, which is important for screen readers.
<img
src={avatar}
className="h-24 w-24 rounded-full border object-cover"
+ alt="User avatar preview"
/>📝 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.
| <img | |
| src={avatar} | |
| className="h-24 w-24 rounded-full border object-cover" | |
| /> | |
| <img | |
| src={avatar} | |
| className="h-24 w-24 rounded-full border object-cover" | |
| alt="User avatar preview" | |
| /> |
🤖 Prompt for AI Agents
In frontend/src/pages/ProfilePage/EditProfilePage.tsx around lines 39 to 42, the
avatar img element is missing an alt attribute; add a meaningful alt prop to the
img (e.g., alt={`Avatar of ${user.name}`} or a fallback like alt="User avatar")
to improve accessibility, and ensure it uses the user's display name if
available or a generic description when not.
| onChange={(e) => { | ||
| const file = e.target.files?.[0]; | ||
| if (!file) return; | ||
| setLocalAvatar(URL.createObjectURL(file)); | ||
| }} |
There was a problem hiding this comment.
Blob URLs are ephemeral and will break after page reload.
URL.createObjectURL() creates a temporary blob URL that's only valid in the current browser session. When this URL is saved to localStorage and the page reloads, the image will fail to load.
Convert to a base64 data URL for persistence:
onChange={(e) => {
const file = e.target.files?.[0];
if (!file) return;
- setLocalAvatar(URL.createObjectURL(file));
+ const reader = new FileReader();
+ reader.onloadend = () => {
+ setLocalAvatar(reader.result as string);
+ };
+ reader.readAsDataURL(file);
}}📝 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.
| onChange={(e) => { | |
| const file = e.target.files?.[0]; | |
| if (!file) return; | |
| setLocalAvatar(URL.createObjectURL(file)); | |
| }} | |
| onChange={(e) => { | |
| const file = e.target.files?.[0]; | |
| if (!file) return; | |
| const reader = new FileReader(); | |
| reader.onloadend = () => { | |
| setLocalAvatar(reader.result as string); | |
| }; | |
| reader.readAsDataURL(file); | |
| }} |
|
|
1 similar comment
|
|
1 participant
Summary
This PR introduces a dedicated Profile feature that improves user account management and navigation.
It adds a Profile page, Edit Profile UI, and a logout flow with confirmation, following existing PictoPy UI patterns and theme support.
Changes Overview
ProfilePage.tsxEditProfilePage.tsxNavbar.tsxLogoutDialog.tsxalert-dialog.tsxroutes.ts,AppRoutes.tsxonboardingSlice.tsDemo / Explanation Videos
Video:
PictoPy.mp4
Profile Page Overview
Shows the Profile page layout, avatar display, user name, and available actions.
Edit Profile Flow
Demonstrates navigation from Profile → Edit Profile and updating user details (UI-level).
Logout Confirmation
Shows the logout confirmation dialog and state reset behavior.
Video:
Notes
Future Improvements
Summary by CodeRabbit
Release Notes