Enhance Friends, Groups, and Profile Management Features #206
Conversation
- Refactored Friends component to include group breakdown for each friend. - Added loading skeletons while fetching friends data. - Implemented expandable friend rows to show detailed group balances. - Improved error handling and user feedback during data fetching. feat: improve GroupDetails with member management and settings tabs - Added functionality to leave groups and kick members with confirmation prompts. - Introduced settings tabs for group information, members, and danger actions. - Enhanced UI for inviting members and managing group settings. feat: create Profile page for user account management - Implemented profile editing functionality with image upload and name change. - Added modal for editing profile details and handling image selection. - Integrated logout functionality and menu items for account settings. feat: update API service with new endpoints for profile and group management - Added API calls for updating user profiles and managing group memberships. - Included Google login functionality in the authentication service. chore: add Firebase service for Google authentication - Set up Firebase configuration and authentication methods for Google sign-in.
WalkthroughAdds Firebase Google sign-in flow, new Profile page with edit/avatar upload, new API wrappers (loginWithGoogle, updateProfile, leaveGroup, removeMember), exposes updateUserInContext in AuthContext, and applies theme-aware UI/animation updates across multiple pages and components. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks✅ Passed checks (3 passed)
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 |
✅ Deploy Preview for split-but-wiser ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #206 +/- ##
=======================================
Coverage 72.79% 72.79%
=======================================
Files 17 17
Lines 1669 1669
Branches 154 154
=======================================
Hits 1215 1215
Misses 400 400
Partials 54 54
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 21
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
web/components/layout/Sidebar.tsx (1)
55-68: Share image URL validation with Profile to avoid driftThe avatar logic (URL/data‑URI check with fallback initial) looks good, but it duplicates the image validation used in
Profile.tsx. Consider extracting a small shared helper (e.g.,isValidImageUrl) and reusing it here to keep behavior consistent if you ever tighten/relax the validation rules.web/contexts/AuthContext.tsx (1)
5-57: Normalize user shape inupdateUserInContext(align withinitAuth/ mobile)
initAuthmapsid→_id, butupdateUserInContext(andlogin) currently storeuserDataas‑is. If the backend returns onlyid, consumers expecting_idwill see inconsistent shapes depending on how the user was loaded/updated.Suggest normalizing here, similar to the mobile context:
const normalizeUser = (u: any): User | null => { if (!u) return null; return u._id ? u : u.id ? { ...u, _id: u.id } : u; }; const login = (token: string, userData: User) => { localStorage.setItem('access_token', token); setUser(normalizeUser(userData)); }; const updateUserInContext = (userData: User) => { setUser(normalizeUser(userData)); };This keeps the user shape stable regardless of load path and matches the mobile implementation.
web/pages/Groups.tsx (4)
48-50: Avoid usinganytype; use proper typing.The
g: anyweakens type safety. SinceGroupBalanceSummaryis already imported, use it directly.const getGroupBalance = (groupId: string): GroupBalanceSummary | undefined => { - return balanceSummary?.groupsSummary?.find((g: any) => g.groupId === groupId); + return balanceSummary?.groupsSummary?.find((g: GroupBalanceSummary) => g.groupId === groupId); };
59-61: Consider replacingalert()with toast notifications.Using
alert()blocks the UI and provides a poor user experience. Consider using a toast/notification system for error feedback.
222-226: Form validation bypassed by direct button onClick.The
requiredattribute on the Input (line 236) won't prevent submission because the Button'sonClickdirectly callshandleCreateGroupinstead of triggering form submission. Either attachonSubmitto the form and usetype="submit"on the button, or add manual validation in the handler.- <form id="createGroupForm" className="space-y-4"> + <form id="createGroupForm" className="space-y-4" onSubmit={handleCreateGroup}>And in the footer, change the button to submit type or keep as is if using form's onSubmit:
- <Button onClick={handleCreateGroup}>Create Group</Button> + <Button type="submit" form="createGroupForm">Create Group</Button>
246-250: Same form validation issue in Join Group modal.The Join Group form has the same validation bypass issue. The
requiredattribute on the Input won't be enforced.Apply a similar fix:
- <form className="space-y-4"> + <form id="joinGroupForm" className="space-y-4" onSubmit={handleJoinGroup}>- <Button onClick={handleJoinGroup}>Join Group</Button> + <Button type="submit" form="joinGroupForm">Join Group</Button>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (14)
web/App.tsx(2 hunks)web/components/layout/Sidebar.tsx(2 hunks)web/components/ui/Button.tsx(3 hunks)web/components/ui/Modal.tsx(3 hunks)web/contexts/AuthContext.tsx(2 hunks)web/package.json(1 hunks)web/pages/Auth.tsx(2 hunks)web/pages/Dashboard.tsx(4 hunks)web/pages/Friends.tsx(1 hunks)web/pages/GroupDetails.tsx(1 hunks)web/pages/Groups.tsx(6 hunks)web/pages/Profile.tsx(1 hunks)web/services/api.ts(2 hunks)web/services/firebase.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (11)
web/components/layout/Sidebar.tsx (1)
mobile/context/AuthContext.js (1)
user(13-13)
web/App.tsx (1)
web/pages/Profile.tsx (1)
Profile(13-275)
web/contexts/AuthContext.tsx (2)
web/types.ts (1)
User(1-8)mobile/context/AuthContext.js (4)
user(13-13)updateUserInContext(165-173)AuthContext(10-10)AuthContext(10-10)
web/pages/Profile.tsx (8)
web/contexts/AuthContext.tsx (1)
useAuth(62-68)web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/api.ts (1)
updateProfile(50-50)web/constants.ts (1)
THEMES(1-4)web/types.ts (1)
User(1-8)web/components/ui/Modal.tsx (1)
Modal(15-89)web/components/ui/Button.tsx (1)
Button(10-55)web/components/ui/Input.tsx (1)
Input(10-28)
web/services/api.ts (1)
mobile/api/groups.js (4)
leaveGroup(40-41)leaveGroup(40-41)removeMember(46-47)removeMember(46-47)
web/pages/Groups.tsx (6)
web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/constants.ts (1)
THEMES(1-4)web/components/ui/Button.tsx (1)
Button(10-55)web/components/ui/Skeleton.tsx (1)
Skeleton(9-23)web/components/ui/Modal.tsx (1)
Modal(15-89)web/components/ui/Input.tsx (1)
Input(10-28)
web/components/ui/Button.tsx (1)
web/contexts/ThemeContext.tsx (1)
useTheme(43-49)
web/pages/Dashboard.tsx (2)
web/constants.ts (1)
THEMES(1-4)web/components/ui/Card.tsx (1)
Card(12-35)
web/pages/Auth.tsx (6)
web/contexts/AuthContext.tsx (1)
useAuth(62-68)web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/firebase.ts (1)
signInWithGoogle(21-26)web/services/api.ts (2)
loginWithGoogle(23-23)login(21-21)web/components/ui/Button.tsx (1)
Button(10-55)web/components/ui/Input.tsx (1)
Input(10-28)
web/components/ui/Modal.tsx (1)
web/constants.ts (1)
THEMES(1-4)
web/pages/Friends.tsx (3)
web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/api.ts (2)
getFriendsBalance(49-49)getGroups(27-27)web/constants.ts (1)
THEMES(1-4)
🪛 Biome (2.1.2)
web/pages/Profile.tsx
[error] 127-127: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 127-127: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 237-237: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 237-237: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 182-187: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
web/pages/Auth.tsx
[error] 166-167: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
web/pages/Friends.tsx
[error] 262-265: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
web/pages/GroupDetails.tsx
[error] 578-578: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 587-587: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 648-660: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 648-660: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 740-740: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 753-753: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 884-884: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
[error] 359-359: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 380-383: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 402-405: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 408-411: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 780-783: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 786-789: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 792-795: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 843-847: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🪛 Gitleaks (8.29.1)
web/services/firebase.ts
[high] 6-6: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: preview
🔇 Additional comments (12)
web/package.json (1)
11-19: Firebase dependency aligns with new auth serviceAdding
firebaseto dependencies matches the new Firebase auth module usage and fits the existing tooling stack; no issues from this manifest change alone.web/components/ui/Button.tsx (1)
10-55: Button theming changes are consistent and non‑breakingButton still exposes the same props and behavior; the added NEOBRUTALISM styling (
font-mono, uppercase, etc.) cleanly composes with existing base and size classes.web/App.tsx (1)
2-44: Profile route integration looks correct
/profileis wired throughProtectedRouteand uses the sharedLayout, consistent with other authenticated pages. Redirect logic for login/signup and the catch‑all route remains coherent.web/components/ui/Modal.tsx (1)
1-88: Modal theme/animation tweaks are coherent and scopedThe new NEOBRUTALISM animation variant and
rounded-nonestyling integrate cleanly with the existing Modal API, keeping overlay behavior and close controls intact while only altering visuals based on theme.web/services/api.ts (1)
21-55: New auth/profile/group API wrappers match existing patterns
loginWithGoogle,updateProfile,leaveGroup, andremoveMemberall follow the established API client pattern and mirror the mobile endpoints, so they should integrate cleanly with the backend and existing interceptors.web/pages/Auth.tsx (2)
30-54: Good error handling for Google sign-in flow.The differentiation between
popup-closed-by-user, server errors, and general failures provides appropriate user feedback without exposing unnecessary error details.
73-79: Error message extraction may not handle all API response formats.The error extraction
err.response.data?.detail?.[0]?.msgassumesdetailis an array with objects containingmsg. If the API returns a different format (e.g., stringdetail), users will see the generic "Authentication failed" message.Consider a more robust extraction:
if (err.response) { - setError( - err.response.data?.detail?.[0]?.msg || 'Authentication failed' - ); + const detail = err.response.data?.detail; + const message = typeof detail === 'string' + ? detail + : Array.isArray(detail) ? detail[0]?.msg : null; + setError(message || 'Authentication failed'); } else {web/pages/GroupDetails.tsx (1)
188-239: Expense submission validation could silently fail.The
handleExpenseSubmitfunction returns early withalert()on validation failures but doesn't prevent further execution paths clearly. The early returns are correct, but consider consolidating validation logic for clarity.The validation logic correctly handles equal vs unequal splits with appropriate user feedback. The filtering of zero-amount splits at line 217 is a good defensive measure.
web/pages/Groups.tsx (4)
1-12: LGTM!Imports are well-organized and all appear to be utilized in the component.
22-26: LGTM!Good use of a derived
isNeoboolean to simplify theme-based conditionals throughout the component.
80-93: LGTM!Animation variants are well-structured for staggered list animations.
150-207: LGTM on the AnimatePresence and group card structure.The conditional rendering with
AnimatePresence mode='popLayout'is correctly set up for smooth transitions between loading and loaded states. Theme-aware styling is comprehensive and consistent.
| <h2 className={`text-3xl font-extrabold ${summary && summary.netBalance >= 0 ? (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-emerald-500') : (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-red-500')}`}> | ||
| ${summary?.netBalance.toFixed(2)} | ||
| </h2> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider extracting the complex ternary for readability.
The nested ternary for Net Balance color is difficult to parse. Consider extracting to a helper variable.
+ const netBalanceColor = summary && summary.netBalance >= 0
+ ? (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-emerald-500')
+ : (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-red-500');
+
return (
...
- <h2 className={`text-3xl font-extrabold ${summary && summary.netBalance >= 0 ? (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-emerald-500') : (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-red-500')}`}>
+ <h2 className={`text-3xl font-extrabold ${netBalanceColor}`}>📝 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.
| <h2 className={`text-3xl font-extrabold ${summary && summary.netBalance >= 0 ? (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-emerald-500') : (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-red-500')}`}> | |
| ${summary?.netBalance.toFixed(2)} | |
| </h2> | |
| const netBalanceColor = summary && summary.netBalance >= 0 | |
| ? (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-emerald-500') | |
| : (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-red-500'); | |
| return ( | |
| ... | |
| <h2 className={`text-3xl font-extrabold ${netBalanceColor}`}> | |
| ${summary?.netBalance.toFixed(2)} | |
| </h2> | |
| ) |
🤖 Prompt for AI Agents
In web/pages/Dashboard.tsx around lines 64 to 66, the long nested ternary used
to choose the net balance text color harms readability; extract the logic into a
small, clearly named helper variable (e.g., balanceColorClass) above the JSX
that computes the class by first checking summary?.netBalance >= 0 and then
style === THEMES.NEOBRUTALISM to return one of 'text-black', 'text-emerald-500',
or 'text-red-500' (handle summary undefined safely), and then replace the inline
ternary in the h2 className with that variable so the JSX reads simply
`className={\`text-3xl font-extrabold ${balanceColorClass}\`}`.
| // Your web app's Firebase configuration | ||
| const firebaseConfig = { | ||
| apiKey: "AIzaSyC4Ny4BSh3q4fNEVBGyw2u_FvLaxXukB8U", | ||
| authDomain: "splitwiser-25e34.firebaseapp.com", | ||
| projectId: "splitwiser-25e34", | ||
| storageBucket: "splitwiser-25e34.firebasestorage.app", | ||
| messagingSenderId: "323312632683", | ||
| appId: "1:323312632683:web:eef9ca7acc5c5a89ce422e", | ||
| measurementId: "G-SDY9ZRV9V4" | ||
| }; | ||
|
|
||
| // Initialize Firebase | ||
| const app = initializeApp(firebaseConfig); | ||
| const auth = getAuth(app); | ||
| const googleProvider = new GoogleAuthProvider(); | ||
|
|
||
| // Sign in with Google popup | ||
| export const signInWithGoogle = async (): Promise<string> => { | ||
| const result = await signInWithPopup(auth, googleProvider); | ||
| // Get the ID token to send to your backend | ||
| const idToken = await result.user.getIdToken(); | ||
| return idToken; | ||
| }; | ||
|
|
||
| export { auth, googleProvider }; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the file structure and locate related files
fd -type f -name "*.ts" -o -name "*.tsx" -o -name "*.env*" | head -20Repository: Devasy23/splitwiser
Length of output: 233
🏁 Script executed:
#!/bin/bash
# Find TypeScript files and configuration files
find . -type f \( -name "*.ts" -o -name "*.tsx" -o -name ".env*" -o -name "vite.config.*" \) | head -30Repository: Devasy23/splitwiser
Length of output: 697
🏁 Script executed:
#!/bin/bash
# Read the firebase.ts file to verify current state
cat -n web/services/firebase.tsRepository: Devasy23/splitwiser
Length of output: 1225
🏁 Script executed:
#!/bin/bash
# Check AuthContext.tsx for signInWithGoogle usage and error handling
cat -n web/contexts/AuthContext.tsxRepository: Devasy23/splitwiser
Length of output: 2548
🏁 Script executed:
#!/bin/bash
# Search for signInWithGoogle usage in the codebase
rg "signInWithGoogle" -n -A 5 -B 2Repository: Devasy23/splitwiser
Length of output: 1429
🏁 Script executed:
#!/bin/bash
# Read Auth.tsx to see the full error handling context
cat -n web/pages/Auth.tsx | head -70Repository: Devasy23/splitwiser
Length of output: 2765
🏁 Script executed:
#!/bin/bash
# Check vite.config.ts for environment variable configuration
cat -n web/vite.config.tsRepository: Devasy23/splitwiser
Length of output: 803
🏁 Script executed:
#!/bin/bash
# Check for .env files in web directory and search for env variable usage patterns
echo "=== Looking for .env files in web ===" && \
find web -name ".env*" 2>/dev/null && \
echo "" && \
echo "=== Searching for VITE_ environment variable usage ===" && \
rg "VITE_|import\.meta\.env" web/ -n | head -20Repository: Devasy23/splitwiser
Length of output: 156
🏁 Script executed:
#!/bin/bash
# Do a thorough search for all references to signInWithGoogle to ensure we didn't miss any
rg "signInWithGoogle" -c --type ts --type tsxRepository: Devasy23/splitwiser
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Search for all references to signInWithGoogle
rg "signInWithGoogle" -cRepository: Devasy23/splitwiser
Length of output: 110
🌐 Web query:
Firebase web API key exposure and GCP restrictions best practices
💡 Result:
Short answer: Firebase Web API keys are not secret credentials (they identify the Firebase project, not authorize access), but an exposed key still creates risk if abused — so treat them carefully: restrict and monitor keys, enforce Firebase security (Rules + App Check), move privileged work to trusted servers, and rotate + revoke keys if needed. Sources below.
Recommended checklist (actionable):
- Understand risk
- Firebase API keys identify the project but do not replace Firebase Security Rules or IAM; don’t rely on the key for authorization. [1]
- Apply API key restrictions in GCP
- Add application restrictions (HTTP referrers for web, or IPs for server keys) and API restrictions (limit which Google APIs the key can call). Create separate keys per client type. [2][3]
- Use Firebase App Check for web
- Enable App Check (reCAPTCHA Enterprise or reCAPTCHA v3 for web) and flip enforcement once rollout is validated — blocks unverified clients. [4]
- Harden backend and privileged operations
- Keep any sensitive operations (admin actions, billing, service-account-bound calls) on server-side using service accounts / OAuth, not client API keys. Bind server keys to IPs and/or service accounts. [3]
- Enforce Firebase security controls
- Implement and test Firebase Security Rules (Firestore, Realtime DB, Storage, Functions) and enable App Check enforcement to reject unauthenticated/unattested requests. [1][4]
- Monitoring, quotas, rotation, incident steps
- Enable usage/quotas, billing alerts and logging (APIs & Services) and set quotas/limits on keys. If a key is abused, restrict or delete it and issue a rotated key. Audit with Cloud Audit Logs and Firebase usage metrics. [2][3]
- Optional: API gateway / Cloud Endpoints
- For custom APIs, put an API gateway (Cloud Endpoints/ESPv2, Cloud API Gateway) in front and require API key verification and service-level checks (use fail-closed policies). [5]
Representative docs
- Firebase: API keys info and guidance. [1]
- GCP: Manage API keys & application/API restrictions. [2][3]
- Firebase App Check + enforcement (web). [4]
- Google Cloud Endpoints API-key restrictions / best practices. [5]
If you want, I can:
- give exact console steps to add HTTP referrer restrictions for a key, or
- scan your project (you share screenshots/outputs) and propose specific restrictions.
Secure Firebase API key with GCP restrictions; error handling is already in place
The Firebase initialization is sound, but the hard-coded API key in source control needs mitigation:
-
API key exposure
For Firebase web apps, the config (includingapiKey) is intentionally public, but since this repo is in Git, ensure:- The key is restricted in GCP/Firebase to allowed domains (HTTP referrers) and specific Google APIs.
- Enable Firebase App Check (reCAPTCHA Enterprise/v3) to block unverified clients.
- Rotate and revoke the key if it was ever used with broader permissions.
- Optional: Move config to environment variables (Vite supports this via
import.meta.env) for easier key rotation per environment without code changes.
-
Error handling note
signInWithGooglecurrently rejects on any error (popup cancel, network, etc.), but this is fine—Auth.tsxalready wraps it in a comprehensivetry/catchblock (lines 34–54) that handles Firebase errors, API errors, and user cancellations gracefully. No changes needed here.
🧰 Tools
🪛 Gitleaks (8.29.1)
[high] 6-6: Uncovered a GCP API key, which could lead to unauthorized access to Google Cloud services and data breaches.
(gcp-api-key)
🤖 Prompt for AI Agents
In web/services/firebase.ts around lines 4 to 29 the Firebase config (including
apiKey) is hard-coded in source; although web apiKeys are public by design, you
must restrict and protect its usage: restrict the key in GCP/Firebase to allowed
HTTP referrers and specific Google APIs, enable Firebase App Check (reCAPTCHA
Enterprise/v3) to block unverified clients, rotate/revoke the key if it had
broader permissions, and optionally move the config into environment variables
(e.g. Vite’s import.meta.env) so different environments can use different keys
and rotation doesn’t require code changes.
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/pages/Groups.tsx (1)
48-50: Avoidanytype in getGroupBalance.The function parameter uses
anywhich reduces type safety.const getGroupBalance = (groupId: string): GroupBalanceSummary | undefined => { - return balanceSummary?.groupsSummary?.find((g: any) => g.groupId === groupId); + return balanceSummary?.groupsSummary?.find((g) => g.groupId === groupId); };If
GroupBalanceSummaryis properly typed inBalanceSummary.groupsSummary, TypeScript should infer the type automatically.web/pages/Dashboard.tsx (1)
31-34: Hardcoded$currency symbol in chart data.The chart data hardcodes values without currency context. While the tooltip displays raw numbers, the overall dashboard lacks currency awareness that other pages now have.
♻️ Duplicate comments (11)
web/pages/Friends.tsx (2)
262-263: LGTM! Balance sign display fixed.The balance display now correctly prepends the sign:
+for positive,-for negative, empty for zero. Combined withformatCurrencyusingMath.abs(), this produces correct output like+$10.00or-$10.00.
43-45: UnusedgMapvariable for group names.The
gMapis built from groups butgroupNameis already provided directly infriend.breakdown. The map is only used to enrichimageUrl. Consider simplifying:- const gMap = new Map<string, { name: string; imageUrl?: string }>( - groups.map((g: { _id: string; name: string; imageUrl?: string }) => [g._id, { name: g.name, imageUrl: g.imageUrl }]) + const groupImageMap = new Map<string, string | undefined>( + groups.map((g: { _id: string; imageUrl?: string }) => [g._id, g.imageUrl]) ); ... - imageUrl: gMap.get(group.groupId)?.imageUrl + imageUrl: groupImageMap.get(group.groupId)web/pages/Groups.tsx (3)
76-79: LGTM! Filtering now memoized.The
filteredGroupscomputation is properly wrapped inuseMemowith correct dependencies[groups, searchTerm], preventing unnecessary recalculations on every render.
186-187: LGTM! Currency display fixed.The balance display now correctly uses
group.currencyinstead of a hardcoded$symbol.
105-105: External URL dependency for visual asset.The noise texture is loaded from an external service. If the service becomes unavailable, the visual effect breaks silently. Consider hosting locally or using a data URI.
web/pages/Dashboard.tsx (2)
44-46: LGTM! Nullish coalescing prevents runtime errors.The balance values now use
(summary?.field ?? 0).toFixed(2)pattern, preventing runtime errors whensummaryis null.Also applies to: 54-56, 64-66
64-65: Consider extracting complex ternary for readability.The nested ternary for Net Balance color is difficult to parse. Extract to a helper variable for clarity.
+ const netBalanceColor = summary && summary.netBalance >= 0 + ? (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-emerald-500') + : (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-red-500'); return ( ... - <h2 className={`text-3xl font-extrabold ${summary && summary.netBalance >= 0 ? (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-emerald-500') : (style === THEMES.NEOBRUTALISM ? 'text-black' : 'text-red-500')}`}> + <h2 className={`text-3xl font-extrabold ${netBalanceColor}`}>web/pages/GroupDetails.tsx (4)
74-76: MovefetchDatainside useEffect or wrap with useCallback.The
fetchDatafunction is defined outside the effect but referencesid. ESLint's exhaustive-deps rule would flag this. Move it inside the effect for correctness.useEffect(() => { + const fetchData = async () => { + if (!id) return; + setLoading(true); + try { + const [groupRes, expRes, memRes, setRes] = await Promise.all([ + getGroupDetails(id), + getExpenses(id), + getGroupMembers(id), + getOptimizedSettlements(id) + ]); + setGroup(groupRes.data); + setExpenses(expRes.data.expenses); + setMembers(memRes.data); + setSettlements(setRes.data.optimizedSettlements); + setEditGroupName(groupRes.data.name); + } catch (err) { + console.error(err); + } finally { + setLoading(false); + } + }; if (id) fetchData(); }, [id]);
314-320: Client-side unsettled balance check uses potentially stale data.This check uses the local
settlementsstate which may be stale if another user just settled. Let the server handle validation and display the server error instead.if (window.confirm(`Are you sure you want to remove ${memberName} from the group?`)) { try { - const hasUnsettled = settlements.some( - s => (s.fromUserId === memberId || s.toUserId === memberId) && (s.amount || 0) > 0 - ); - if (hasUnsettled) { - alert('Cannot remove: This member has unsettled balances in the group.'); - return; - } await removeMember(id, memberId); fetchData(); } catch (err: any) { alert(err.response?.data?.detail || "Failed to remove member"); } }
651-668: Add keyboard accessibility to member selection checkboxes.The div elements with
onClickfor member selection are not keyboard accessible. Users cannot tab to and activate these elements.<div key={m.userId} + role="checkbox" + aria-checked={isSelected} + tabIndex={0} onClick={() => { const newSet = new Set(selectedUsers); if (isSelected) newSet.delete(m.userId); else newSet.add(m.userId); setSelectedUsers(newSet); }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + const newSet = new Set(selectedUsers); + if (isSelected) newSet.delete(m.userId); + else newSet.add(m.userId); + setSelectedUsers(newSet); + } + }} className={`cursor-pointer ...`} >
742-764: Associate labels with form controls using htmlFor.The "Payer" and "Payee" labels (lines 743, 756) are not programmatically associated with their select elements, making them less accessible for screen readers.
<div className="flex flex-col gap-1"> - <label className="text-sm font-bold opacity-80">Payer</label> + <label htmlFor="payment-payer" className="text-sm font-bold opacity-80">Payer</label> <select + id="payment-payer" className={`...`} value={paymentPayerId} onChange={e => setPaymentPayerId(e.target.value)} >Apply the same pattern to "Payee" at line 756.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
web/pages/Auth.tsx(2 hunks)web/pages/Dashboard.tsx(4 hunks)web/pages/Friends.tsx(1 hunks)web/pages/GroupDetails.tsx(1 hunks)web/pages/Groups.tsx(6 hunks)web/pages/Profile.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-26T09:41:01.332Z
Learnt from: CR
Repo: Devasy23/splitwiser PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/expenses/**/*.py : Settlements must track debt resolution between users in the expense tracking logic.
Applied to files:
web/pages/GroupDetails.tsx
🧬 Code graph analysis (4)
web/pages/Groups.tsx (6)
web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/constants.ts (1)
THEMES(1-4)web/components/ui/Button.tsx (1)
Button(10-55)web/components/ui/Skeleton.tsx (1)
Skeleton(9-23)web/components/ui/Modal.tsx (1)
Modal(15-89)web/components/ui/Input.tsx (1)
Input(10-28)
web/pages/Friends.tsx (3)
web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/api.ts (2)
getFriendsBalance(49-49)getGroups(27-27)web/constants.ts (1)
THEMES(1-4)
web/pages/Auth.tsx (7)
web/contexts/AuthContext.tsx (1)
useAuth(62-68)web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/firebase.ts (1)
signInWithGoogle(21-26)web/services/api.ts (2)
loginWithGoogle(23-23)login(21-21)web/constants.ts (1)
THEMES(1-4)web/components/ui/Button.tsx (1)
Button(10-55)web/components/ui/Input.tsx (1)
Input(10-28)
web/pages/Profile.tsx (8)
web/contexts/AuthContext.tsx (1)
useAuth(62-68)web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/api.ts (1)
updateProfile(50-50)web/constants.ts (1)
THEMES(1-4)web/types.ts (1)
User(1-8)web/components/ui/Modal.tsx (1)
Modal(15-89)web/components/ui/Button.tsx (1)
Button(10-55)web/components/ui/Input.tsx (1)
Input(10-28)
🪛 Biome (2.1.2)
web/pages/GroupDetails.tsx
[error] 581-581: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 590-590: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 651-663: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 651-663: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 743-743: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 756-756: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
web/pages/Profile.tsx
[error] 135-135: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
[error] 259-259: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: preview
🔇 Additional comments (14)
web/pages/Profile.tsx (3)
130-136: LGTM! Keyboard accessibility properly implemented.The clickable avatar now correctly includes
role="button",tabIndex={0}, keyboard event handlers, andaria-label. This addresses the accessibility requirements for interactive elements.
254-260: LGTM! Image picker accessibility properly implemented.The image picker div correctly implements keyboard accessibility with
role="button",tabIndex={0},onKeyDownhandler, andaria-label.
37-65: LGTM! Profile save logic properly refactored.The implementation correctly:
- Uses inline error state (
saveError) instead of blockingalert()for validation- Uses
updateUserInContextto update user state in-place instead ofwindow.location.reload()- Handles errors gracefully with proper loading state management
web/pages/Friends.tsx (2)
8-22: LGTM! Proper TypeScript interfaces defined.The
GroupBreakdownandFriendinterfaces provide clear type definitions for the data structures, improving type safety and IDE support.
43-67: LGTM! Inline interface replacesanytypes.The
FriendBalanceDatainterface properly types the API response, and the mapping now uses explicit types instead ofany.web/pages/GroupDetails.tsx (1)
890-898: LGTM! ScaleIcon now has accessible title.The SVG now includes
role="img",aria-labelledby, and a<title>element for screen reader accessibility.web/pages/Auth.tsx (8)
1-15: LGTM!Import organization is clean. Good practice aliasing
login/signupasapiLogin/apiSignupto avoid naming conflicts with theloginfunction fromuseAuth.
17-28: LGTM!Separate loading states for email/password and Google sign-in prevent UI conflicts when one flow is in progress.
85-86: LGTM!Clean derivation of theme flag for conditional styling throughout the component.
136-141: LGTM!Good UX allowing theme switching from the authentication page. Correctly uses the reusable
Buttoncomponent.
166-184: SVG accessibility properly implemented.The Google logo SVG now includes
role="img",aria-labelledby, and a<title>element, providing proper accessibility for screen readers.
196-212: LGTM!The
AnimatePresencewithmode="wait"correctly handles the conditional name field animation during signup. Therequiredattribute is appropriately applied only when the field is rendered.
231-239: LGTM!Animated error display provides clear feedback with theme-aware styling.
246-257: LGTM!The toggle between login and signup preserves form field values, which is good UX as users don't lose their input when switching modes.
| {/* Left Side - Visuals */} | ||
| <div className="hidden lg:flex lg:w-1/2 relative overflow-hidden bg-black text-white items-center justify-center p-12"> | ||
| <div className="absolute inset-0 bg-gradient-to-br from-blue-600 to-purple-900 opacity-50" /> | ||
| <div className="absolute inset-0 bg-[url('https://grainy-gradients.vercel.app/noise.svg')] opacity-20" /> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
External asset dependency may affect reliability.
The noise texture references an external URL (grainy-gradients.vercel.app). If this service becomes unavailable, the visual effect will silently fail (acceptable degradation), but consider hosting the asset locally for reliability.
🤖 Prompt for AI Agents
In web/pages/Auth.tsx around line 92 the background image references an external
noise SVG which can be unreliable; download that SVG into the repo (e.g.
public/images/noise.svg), add the file to source control, and change the
background URL to point to the local path (e.g. url('/images/noise.svg')) so the
app serves it from our static assets; ensure the file is included in the build
and consider keeping the external URL only as an optional fallback if desired.
| {googleLoading ? ( | ||
| <div className="w-5 h-5 border-2 border-black/20 border-t-black rounded-full animate-spin" /> | ||
| ) : ( |
There was a problem hiding this comment.
Add accessibility attributes to the loading spinner.
The spinner lacks accessibility information. Screen reader users won't know the button is in a loading state.
{googleLoading ? (
- <div className="w-5 h-5 border-2 border-black/20 border-t-black rounded-full animate-spin" />
+ <div
+ className="w-5 h-5 border-2 border-black/20 border-t-black rounded-full animate-spin"
+ role="status"
+ aria-label="Signing in with Google"
+ />
) : (📝 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.
| {googleLoading ? ( | |
| <div className="w-5 h-5 border-2 border-black/20 border-t-black rounded-full animate-spin" /> | |
| ) : ( | |
| {googleLoading ? ( | |
| <div | |
| className="w-5 h-5 border-2 border-black/20 border-t-black rounded-full animate-spin" | |
| role="status" | |
| aria-label="Signing in with Google" | |
| /> | |
| ) : ( |
🤖 Prompt for AI Agents
In web/pages/Auth.tsx around lines 163 to 165, the loading spinner lacks
accessibility attributes; add role="status" and aria-live="polite" to the
spinner element and include an offscreen/plain-text label (e.g., a span with the
project's "sr-only" class containing "Loading…") so screen readers announce the
state, and also set aria-busy="true" on the parent button when googleLoading is
true to indicate the control is busy.
| const handleComingSoon = () => { | ||
| alert('This feature is coming soon!'); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Replace alert() with toast notification for consistency.
The "coming soon" handler still uses alert(), which is inconsistent with the improved UX pattern used elsewhere in the component (e.g., saveError state for validation). Consider using a toast notification system.
🤖 Prompt for AI Agents
In web/pages/Profile.tsx around lines 74-76, replace the blocking browser alert
used in handleComingSoon() with the app's toast notification pattern: import and
use the existing toast utility/component used elsewhere in the app (or reuse the
component/state used for saveError) to display "This feature is coming soon!"
with a non-blocking info/neutral variant, remove the alert() call, and ensure
any required import is added and types updated.
…ation in profile settings
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
web/pages/GroupDetails.tsx (4)
82-84: MissingfetchDatain useEffect dependencies.The effect calls
fetchDatawhich is defined outside but referencesid. ESLint's exhaustive-deps rule would flag this.Either move
fetchDatainside the effect, wrap it withuseCallback, or suppress with an ESLint comment:useEffect(() => { if (id) fetchData(); + // eslint-disable-next-line react-hooks/exhaustive-deps }, [id]);
316-335: Client-side unsettled balance check uses potentially stale data.The
settlements.some(...)check uses local state that may be stale if another user just settled. This could incorrectly block removal. The server should be the source of truth for validation.Consider removing the client-side pre-check and relying on server validation:
if (window.confirm(`Are you sure you want to remove ${memberName} from the group?`)) { try { - const hasUnsettled = settlements.some( - s => (s.fromUserId === memberId || s.toUserId === memberId) && (s.amount || 0) > 0 - ); - if (hasUnsettled) { - alert('Cannot remove: This member has unsettled balances in the group.'); - return; - } await removeMember(id, memberId); fetchData(); } catch (err: any) { alert(err.response?.data?.detail || "Failed to remove member"); }
659-677: Add keyboard accessibility to member selection checkboxes.The
divelements withonClickhandlers for member selection are not keyboard accessible. Users cannot tab to and activate these elements.<div key={m.userId} + role="checkbox" + aria-checked={isSelected} + tabIndex={0} onClick={() => { const newSet = new Set(selectedUsers); if (isSelected) newSet.delete(m.userId); else newSet.add(m.userId); setSelectedUsers(newSet); }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + const newSet = new Set(selectedUsers); + if (isSelected) newSet.delete(m.userId); + else newSet.add(m.userId); + setSelectedUsers(newSet); + } + }} className={`cursor-pointer flex items-center gap-2 p-2 border ...`} >
750-772: Associate labels with their form controls.Labels for "Payer" and "Payee" selects are not programmatically associated with their inputs, making them inaccessible to screen readers.
<div className="flex flex-col gap-1"> - <label className="text-sm font-bold opacity-80">Payer</label> + <label htmlFor="payment-payer" className="text-sm font-bold opacity-80">Payer</label> <select + id="payment-payer" className={`w-full p-3 outline-none font-bold ...`} value={paymentPayerId} onChange={e => setPaymentPayerId(e.target.value)} >Apply similarly to "Payee" at line 764.
web/pages/Profile.tsx (1)
85-87: Consider replacingalert()with toast notification.The
handleComingSoonfunction usesalert()which is inconsistent with the polished UI design. A toast notification would provide better UX.web/pages/Auth.tsx (1)
101-101: External asset dependency noted.The noise texture references an external URL (
grainy-gradients.vercel.app). While this will degrade gracefully if unavailable, consider hosting locally for reliability in production.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
web/pages/Auth.tsx(2 hunks)web/pages/Friends.tsx(1 hunks)web/pages/GroupDetails.tsx(1 hunks)web/pages/Profile.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-26T09:41:01.332Z
Learnt from: CR
Repo: Devasy23/splitwiser PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/expenses/**/*.py : Settlements must track debt resolution between users in the expense tracking logic.
Applied to files:
web/pages/GroupDetails.tsx
🧬 Code graph analysis (3)
web/pages/Auth.tsx (7)
web/contexts/AuthContext.tsx (1)
useAuth(62-68)web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/firebase.ts (1)
signInWithGoogle(21-26)web/services/api.ts (2)
loginWithGoogle(23-23)login(21-21)web/constants.ts (1)
THEMES(1-4)web/components/ui/Button.tsx (1)
Button(10-55)web/components/ui/Input.tsx (1)
Input(10-28)
web/pages/GroupDetails.tsx (8)
web/types.ts (4)
Settlement(58-68)Group(16-25)Expense(46-56)GroupMember(27-32)web/contexts/AuthContext.tsx (1)
useAuth(62-68)web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/api.ts (12)
getGroupDetails(29-29)getExpenses(36-36)getGroupMembers(32-32)getOptimizedSettlements(44-44)updateExpense(38-38)createExpense(37-37)deleteExpense(39-39)createSettlement(43-43)updateGroup(30-30)deleteGroup(31-31)leaveGroup(53-53)removeMember(54-54)web/constants.ts (1)
THEMES(1-4)web/components/ui/Button.tsx (1)
Button(10-55)web/components/ui/Modal.tsx (1)
Modal(15-89)web/components/ui/Input.tsx (1)
Input(10-28)
web/pages/Profile.tsx (8)
web/contexts/AuthContext.tsx (1)
useAuth(62-68)web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/api.ts (1)
updateProfile(50-50)web/constants.ts (1)
THEMES(1-4)web/types.ts (1)
User(1-8)web/components/ui/Modal.tsx (1)
Modal(15-89)web/components/ui/Button.tsx (1)
Button(10-55)web/components/ui/Input.tsx (1)
Input(10-28)
🪛 Biome (2.1.2)
web/pages/GroupDetails.tsx
[error] 589-589: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 598-598: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 659-671: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 659-671: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 751-751: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 764-764: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
web/pages/Profile.tsx
[error] 146-146: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
[error] 270-270: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: preview
🔇 Additional comments (13)
web/pages/GroupDetails.tsx (2)
898-907: LGTM! Accessible SVG implementation.The
ScaleIconcorrectly includesrole="img",aria-labelledby, and a<title>element for screen reader accessibility.
103-123: LGTM! Clean parallel data fetching.The
fetchDatafunction correctly usesPromise.allto fetch group details, expenses, members, and settlements in parallel, with proper error handling and loading state management.web/pages/Friends.tsx (4)
8-22: LGTM! Well-defined TypeScript interfaces.The
GroupBreakdownandFriendinterfaces provide proper type safety for the data structures used throughout the component.
43-67: LGTM! Properly typed data transformation.The data transformation now uses typed interfaces (
FriendBalanceData) instead ofany, addressing the previous type safety concerns.
236-243: Net balance display now includes sign indicator.The balance rendering correctly prefixes
+for positive and-for negative balances, consistent with the group breakdown display.
270-275: LGTM! Button has explicit type attribute.The "View Details" button correctly includes
type="button"to prevent unintended form submissions.web/pages/Profile.tsx (3)
25-46: LGTM! Image validation implemented.The
handleImagePickfunction properly validates file size (5MB limit) and file type before processing, preventing oversized base64 payloads.
65-69: LGTM! Context update instead of page reload.The profile update flow correctly uses
updateUserInContextto update local state instead of forcing a full page reload, preserving client-side state and improving UX.
203-219: LGTM! Menu buttons have explicit type attribute.The menu section buttons correctly include
type="button"to prevent unintended form submissions.web/pages/Auth.tsx (4)
30-60: LGTM! Robust Google sign-in implementation.The
handleGoogleSignInfunction includes:
- Defensive checks for
access_tokenanduserin the response- Proper handling of popup-closed-by-user scenario
- Error handling for both string and array detail formats from the backend
78-88: LGTM! Consistent error handling in email/password flow.The
handleSubmiterror handling now properly handles both string and array formats for the backenddetailfield, matching the pattern used in Google sign-in.
172-197: LGTM! Accessible loading spinner and Google logo.The loading spinner includes
role="status"andaria-labelfor screen readers. The Google logo SVG properly includes a<title>element witharia-labelledby.
208-257: LGTM! Well-structured form with animations.The email/password form correctly uses
AnimatePresencefor the conditional name field during signup, with proper form submission handling and error display.
| <div className="w-24 flex flex-col gap-1"> | ||
| <label className="text-sm font-semibold opacity-80">Currency</label> | ||
| <div className={`w-full p-3 font-bold text-center opacity-80 ${style === THEMES.NEOBRUTALISM ? 'border-2 border-black bg-white rounded-none' : 'bg-white/10 border border-white/20 rounded-lg'}`}> | ||
| {currency} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Associate currency label with its display element.
The "Currency" label at line 589 is not associated with any control.
Since this is a read-only display, consider using aria-labelledby or wrapping in a <dl> structure:
- <div className="w-24 flex flex-col gap-1">
- <label className="text-sm font-semibold opacity-80">Currency</label>
+ <div className="w-24 flex flex-col gap-1" role="group" aria-label="Currency">
+ <span className="text-sm font-semibold opacity-80">Currency</span>
<div className={`w-full p-3 font-bold text-center opacity-80 ...`}>
{currency}
</div>
</div>🧰 Tools
🪛 Biome (2.1.2)
[error] 589-589: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
🤖 Prompt for AI Agents
In web/pages/GroupDetails.tsx around lines 588 to 593, the "Currency" label is
not programmatically associated with the read-only display element; update the
markup so the label is connected to the display either by wrapping the pair in a
semantic description list (<dl> with <dt> for the label and <dd> for the value)
or by giving the value element a unique id and adding aria-labelledby to it that
references the label's id, ensuring the label text remains visible and the
display remains read-only and styled as before.
| <div> | ||
| <label className="block text-sm font-bold mb-2 opacity-80">Paid By</label> | ||
| <div className="flex flex-wrap gap-2"> | ||
| {members.map(m => ( | ||
| <button | ||
| key={m.userId} | ||
| type="button" | ||
| onClick={() => setPayerId(m.userId)} | ||
| className={`px-4 py-2 text-sm font-bold transition-all border ${payerId === m.userId | ||
| ? (style === THEMES.NEOBRUTALISM ? 'bg-black text-white border-black rounded-none' : 'bg-blue-600 border-blue-500 text-white rounded-full') | ||
| : (style === THEMES.NEOBRUTALISM ? 'bg-white text-black border-black hover:bg-gray-100 rounded-none' : 'bg-transparent border-gray-600 text-gray-400 hover:border-gray-400 rounded-full') | ||
| }`} | ||
| > | ||
| {m.userId === user?._id ? 'You' : m.user?.name} | ||
| </button> | ||
| ))} | ||
| </div> | ||
| Split Unequally | ||
| </button> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
"Paid By" label lacks association with the button group.
The label "Paid By" at line 598 is not programmatically linked to the selection controls.
Consider using a fieldset with legend for proper semantics:
- <div>
- <label className="block text-sm font-bold mb-2 opacity-80">Paid By</label>
+ <fieldset>
+ <legend className="block text-sm font-bold mb-2 opacity-80">Paid By</legend>
<div className="flex flex-wrap gap-2">
{members.map(m => (
<buttonOr use role="group" with aria-label.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 598-598: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
🤖 Prompt for AI Agents
In web/pages/GroupDetails.tsx around lines 597 to 614, the "Paid By" label is
not programmatically associated with the button group; wrap the button list in a
semantic container (preferably a <fieldset> with a <legend> containing "Paid
By") or at minimum add a container element with role="group" and
aria-label="Paid By", and ensure each toggle button exposes its state (e.g.,
aria-pressed or aria-checked) so assistive tech can identify the group and the
selected payer.
| <div | ||
| className="relative group cursor-pointer" | ||
| onClick={openEditModal} | ||
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); openEditModal(); } }} | ||
| tabIndex={0} | ||
| role="button" | ||
| aria-label="Edit profile picture" | ||
| > | ||
| <div className={`w-32 h-32 p-1 ${isNeo ? 'bg-black rounded-none' : 'bg-gradient-to-br from-blue-500 to-purple-500 rounded-full'}`}> | ||
| {isValidImageUrl ? ( | ||
| <img | ||
| src={avatarUrl} | ||
| alt={user?.name} | ||
| className={`w-full h-full object-cover border-4 border-white dark:border-gray-900 ${isNeo ? 'rounded-none' : 'rounded-full'}`} | ||
| /> | ||
| ) : ( | ||
| <div className={`w-full h-full bg-gray-100 dark:bg-gray-800 flex items-center justify-center text-4xl font-bold text-gray-500 border-4 border-white dark:border-gray-900 ${isNeo ? 'rounded-none' : 'rounded-full'}`}> | ||
| {user?.name?.charAt(0) || 'A'} | ||
| </div> | ||
| )} | ||
| </div> | ||
| <div className={`absolute inset-0 flex items-center justify-center opacity-0 group-hover:opacity-100 transition-opacity ${isNeo ? 'rounded-none' : 'rounded-full'}`}> | ||
| <div className="bg-black/50 p-2 text-white rounded-full"> | ||
| <Camera size={24} /> | ||
| </div> | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using a semantic <button> element for the avatar.
While the keyboard accessibility attributes are correctly added (tabIndex, role="button", onKeyDown), using a native <button> element would provide these behaviors automatically and is more semantically correct.
- <div
+ <button
+ type="button"
className="relative group cursor-pointer"
onClick={openEditModal}
- onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); openEditModal(); } }}
- tabIndex={0}
- role="button"
aria-label="Edit profile picture"
>
...
- </div>
+ </button>Apply similarly to the image picker at lines 265-287.
📝 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.
| <div | |
| className="relative group cursor-pointer" | |
| onClick={openEditModal} | |
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); openEditModal(); } }} | |
| tabIndex={0} | |
| role="button" | |
| aria-label="Edit profile picture" | |
| > | |
| <div className={`w-32 h-32 p-1 ${isNeo ? 'bg-black rounded-none' : 'bg-gradient-to-br from-blue-500 to-purple-500 rounded-full'}`}> | |
| {isValidImageUrl ? ( | |
| <img | |
| src={avatarUrl} | |
| alt={user?.name} | |
| className={`w-full h-full object-cover border-4 border-white dark:border-gray-900 ${isNeo ? 'rounded-none' : 'rounded-full'}`} | |
| /> | |
| ) : ( | |
| <div className={`w-full h-full bg-gray-100 dark:bg-gray-800 flex items-center justify-center text-4xl font-bold text-gray-500 border-4 border-white dark:border-gray-900 ${isNeo ? 'rounded-none' : 'rounded-full'}`}> | |
| {user?.name?.charAt(0) || 'A'} | |
| </div> | |
| )} | |
| </div> | |
| <div className={`absolute inset-0 flex items-center justify-center opacity-0 group-hover:opacity-100 transition-opacity ${isNeo ? 'rounded-none' : 'rounded-full'}`}> | |
| <div className="bg-black/50 p-2 text-white rounded-full"> | |
| <Camera size={24} /> | |
| </div> | |
| </div> | |
| </div> | |
| <button | |
| type="button" | |
| className="relative group cursor-pointer" | |
| onClick={openEditModal} | |
| aria-label="Edit profile picture" | |
| > | |
| <div className={`w-32 h-32 p-1 ${isNeo ? 'bg-black rounded-none' : 'bg-gradient-to-br from-blue-500 to-purple-500 rounded-full'}`}> | |
| {isValidImageUrl ? ( | |
| <img | |
| src={avatarUrl} | |
| alt={user?.name} | |
| className={`w-full h-full object-cover border-4 border-white dark:border-gray-900 ${isNeo ? 'rounded-none' : 'rounded-full'}`} | |
| /> | |
| ) : ( | |
| <div className={`w-full h-full bg-gray-100 dark:bg-gray-800 flex items-center justify-center text-4xl font-bold text-gray-500 border-4 border-white dark:border-gray-900 ${isNeo ? 'rounded-none' : 'rounded-full'}`}> | |
| {user?.name?.charAt(0) || 'A'} | |
| </div> | |
| )} | |
| </div> | |
| <div className={`absolute inset-0 flex items-center justify-center opacity-0 group-hover:opacity-100 transition-opacity ${isNeo ? 'rounded-none' : 'rounded-full'}`}> | |
| <div className="bg-black/50 p-2 text-white rounded-full"> | |
| <Camera size={24} /> | |
| </div> | |
| </div> | |
| </button> |
🧰 Tools
🪛 Biome (2.1.2)
[error] 146-146: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
🤖 Prompt for AI Agents
In web/pages/Profile.tsx around lines 141 to 167, the avatar wrapper is using a
div with manual keyboard handlers and role="button" which is less semantic and
duplicates built-in behavior; replace the outer div with a native <button>
(preserve className, onClick, aria-label, tabIndex can be removed since button
is focusable, and move onKeyDown logic into the button or remove because
Enter/Space are natively handled) and ensure you keep the inner layout and
Camera overlay styles; also apply the same change to the image picker at lines
265 to 287 (swap the div for a button, retain styling and handlers, and remove
redundant accessibility props).
…and GroupDetails components
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (6)
web/pages/GroupDetails.tsx (6)
82-84: MissingfetchDatain useEffect dependencies.This is a duplicate of a previous comment. The effect calls
fetchDatawhich is defined outside but the dependency array only includesid. Consider wrappingfetchDatainuseCallbackwithidas a dependency, or move the function inside the effect.
316-335: Client-side unsettled balance check is unreliable.This is a duplicate of a previous comment. The check uses potentially stale client data from
settlements. If another user just settled, this check would incorrectly prevent removal. The server should be the source of truth for this validation.
663-681: Add keyboard accessibility to member selection checkboxes.This is a duplicate of a previous comment. The div elements with
onClickhandlers are not keyboard accessible. Users cannot tab to and activate these elements. Addrole="checkbox",aria-checked,tabIndex={0}, and anonKeyDownhandler for Enter/Space keys.
592-597: Associate currency label with its display element.This is a duplicate of a previous comment. The "Currency" label is not programmatically associated with any control. Since this is read-only, consider using
aria-labelledbyor changing<label>to<span>.
601-618: "Paid By" label lacks association with the button group.This is a duplicate of a previous comment. Consider using a
<fieldset>with<legend>for proper semantics, or userole="group"witharia-label.
754-763: Associate labels with their form controls.This is a duplicate of a previous comment. Labels for "Payer" and "Payee" selects are not programmatically associated with their inputs.
<div className="flex flex-col gap-1"> - <label className="text-sm font-bold opacity-80">Payer</label> + <label htmlFor="payment-payer" className="text-sm font-bold opacity-80">Payer</label> <select + id="payment-payer" className={`w-full p-3 outline-none font-bold ...`} value={paymentPayerId} onChange={e => setPaymentPayerId(e.target.value)} >Apply similarly to "Payee" select at line 768-775.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
web/pages/Friends.tsx(1 hunks)web/pages/GroupDetails.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-26T09:41:01.332Z
Learnt from: CR
Repo: Devasy23/splitwiser PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/expenses/**/*.py : Settlements must track debt resolution between users in the expense tracking logic.
Applied to files:
web/pages/GroupDetails.tsx
🧬 Code graph analysis (2)
web/pages/GroupDetails.tsx (8)
web/types.ts (4)
Settlement(58-68)Group(16-25)Expense(46-56)GroupMember(27-32)web/contexts/AuthContext.tsx (1)
useAuth(62-68)web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/api.ts (12)
getGroupDetails(29-29)getExpenses(36-36)getGroupMembers(32-32)getOptimizedSettlements(44-44)updateExpense(38-38)createExpense(37-37)deleteExpense(39-39)createSettlement(43-43)updateGroup(30-30)deleteGroup(31-31)leaveGroup(53-53)removeMember(54-54)web/components/ui/Skeleton.tsx (1)
Skeleton(9-23)web/constants.ts (1)
THEMES(1-4)web/components/ui/Modal.tsx (1)
Modal(15-89)web/components/ui/Input.tsx (1)
Input(10-28)
web/pages/Friends.tsx (3)
web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/api.ts (1)
getGroups(27-27)web/constants.ts (1)
THEMES(1-4)
🪛 Biome (2.1.2)
web/pages/GroupDetails.tsx
[error] 449-449: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
[error] 593-593: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 602-602: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 663-675: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 663-675: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 755-755: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 768-768: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
web/pages/Friends.tsx
[error] 219-219: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: preview
🔇 Additional comments (9)
web/pages/GroupDetails.tsx (5)
1-35: LGTM!Imports are well-organized, and the local
Settlementinterface provides proper type safety as previously requested.
77-80: LGTM!The
isAdminmemoization correctly depends onmembersanduser?._id, ensuring it updates when either changes.
350-406: LGTM!The header section is well-structured with proper button types and clean animation implementation.
788-897: LGTM!The settings modal is well-organized with proper tab structure, button types, and admin-gated dangerous actions.
902-911: LGTM!The
ScaleIconcomponent now has proper accessibility withrole="img",aria-labelledby, and a<title>element as previously requested.web/pages/Friends.tsx (4)
1-22: LGTM!The imports are clean and the TypeScript interfaces
GroupBreakdownandFriendare well-defined at module level, providing good type safety for the component.
79-113: LGTM!The helper functions are well-implemented. The
formatCurrencyhelper correctly returns absolute values with the sign handled separately in the render logic, andgetAvatarContentproperly validates image URLs before rendering.
156-193: LGTM!The summary cards are well-structured with proper theme-aware styling and staggered animations for a polished UX.
275-280: I'll verify whether the "View Details" button is truly non-functional by examining the full context and searching for any onClick handlers or navigation logic.
<function_calls>
#!/bin/bashFirst, let's read the Friends.tsx file around lines 275-280 with context
wc -l web/pages/Friends.tsx
</function_calls>
<function_calls>
#!/bin/bashRead the relevant section with more context (lines 250-290)
sed -n '250,290p' web/pages/Friends.tsx | cat -n
</function_calls>
<function_calls>
#!/bin/bashSearch for onClick handlers related to "View Details" button in Friends.tsx
rg -n "View Details" web/pages/Friends.tsx -B 10 -A 5
</function_calls>
<function_calls>
#!/bin/bashSearch for any onClick handlers in the same file, especially around expanded groups
rg -n "onClick" web/pages/Friends.tsx -C 3
</function_calls>
<function_calls>
#!/bin/bashSearch for FriendDetail or friend detail routes/components in the codebase
rg -n "FriendDetail|friend.*[Dd]etail" --type=tsx --type=ts | head -20
</function_calls>
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (8)
web/pages/Friends.tsx (2)
48-54: MoveFriendBalanceDatainterface to module level.The interface is still defined inside the
useEffectcallback. This is unconventional and reduces reusability. Move it alongside the other interfaces (GroupBreakdown,Friend) at the top of the file for consistency.+interface FriendBalanceData { + userId: string; + userName: string; + userImageUrl?: string; + netBalance: number; + breakdown?: { groupId: string; groupName: string; balance: number }[]; +} + interface GroupBreakdown { groupId: string; groupName: string;Then remove lines 48-54 from inside the
useEffect.
129-129: Consider bundling the noise texture locally.The external URL
grainy-gradients.vercel.appcreates a runtime dependency on a third-party service. If unavailable, the texture won't load. Download and serve this asset locally for reliability.web/pages/GroupDetails.tsx (6)
82-85: AlignuseEffectdependencies withfetchDatadefinition (exhaustive-deps).The effect at Lines 82-85 calls
fetchData, which is defined outside the effect, but the dependency array only listsid. This will keep tripping exhaustive-deps and risks subtle bugs iffetchDataever starts capturing more values.A common fix is to memoize
fetchDatawithuseCallbackand include it in the effect deps:-import React, { useEffect, useMemo, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; - const fetchData = async () => { + const fetchData = useCallback(async () => { if (!id) return; setLoading(true); try { const [groupRes, expRes, memRes, setRes = await Promise.all([ getGroupDetails(id), getExpenses(id), getGroupMembers(id), getOptimizedSettlements(id) ]); // ... } catch (err) { console.error(err); } finally { setLoading(false); } - }; + }, [id]); useEffect(() => { - if (id) fetchData(); - }, [id]); + if (id) fetchData(); + }, [id, fetchData]);This should satisfy the linter and keep the data-fetch behavior stable.
Also applies to: 103-123
332-351: Client-side unsettled-balance check is still stale and redundant.
handleKickMemberstill inspects the localsettlementsarray to block removal when there are unsettled balances. Because this data can be stale (another member may have just settled on another client), this check can incorrectly prevent a valid removal. The server already has the authoritative state and can enforce this invariant more reliably.I’d recommend:
- Removing the
hasUnsettledpre-check, and- Letting
removeMemberrun and surfacing any server-side validation error from the catch block.if (window.confirm(`Are you sure you want to remove ${memberName} from the group?`)) { try { - const hasUnsettled = settlements.some( - s => (s.fromUserId === memberId || s.toUserId === memberId) && (s.amount || 0) > 0 - ); - if (hasUnsettled) { - alert('Cannot remove: This member has unsettled balances in the group.'); - return; - } await removeMember(id, memberId); fetchData(); } catch (err: any) { alert(err.response?.data?.detail || "Failed to remove member"); } }
608-613: Fix “Currency” label a11y by not using<label>without a control.The “Currency” text is rendered as a
<label>but not associated with any input, which Biome flags. Since this is a read-only display, it’s better treated as a labeled group or description, not a form label.For example:
- <div className="w-24 flex flex-col gap-1"> - <label className="text-sm font-semibold opacity-80">Currency</label> + <div className="w-24 flex flex-col gap-1" role="group" aria-label="Currency"> + <span className="text-sm font-semibold opacity-80">Currency</span> <div className={`w-full p-3 font-bold text-center opacity-80 ${style === THEMES.NEOBRUTALISM ? 'border-2 border-black bg-white rounded-none' : 'bg-white/10 border border-white/20 rounded-lg'}`}> {currency} </div> </div>This removes the invalid label usage while keeping the same visual structure.
617-634: Associate “Paid By” text with its controls usingfieldset/legend.The “Paid By” label is not programmatically tied to the button group, which Biome flags as a label-without-control issue. A
<fieldset>with a<legend>gives better semantics for grouped selection controls.- <div> - <label className="block text-sm font-bold mb-2 opacity-80">Paid By</label> + <fieldset> + <legend className="block text-sm font-bold mb-2 opacity-80">Paid By</legend> <div className="flex flex-wrap gap-2"> {members.map(m => ( <button key={m.userId} type="button" onClick={() => setPayerId(m.userId)} className={`px-4 py-2 text-sm font-bold transition-all border ${payerId === m.userId ? (style === THEMES.NEOBRUTALISM ? 'bg-black text-white border-black rounded-none' : 'bg-blue-600 border-blue-500 text-white rounded-full') : (style === THEMES.NEOBRUTALISM ? 'bg-white text-black border-black hover:bg-gray-100 rounded-none' : 'bg-transparent border-gray-600 text-gray-400 hover:border-gray-400 rounded-full') }`} > {m.userId === user?._id ? 'You' : m.user?.name} </button> ))} - </div> - </div> + </div> + </fieldset>
675-699: Make member selection tiles keyboard-accessible and non-static.The member selection tiles in the “Split Equally” section are clickable
<div>s without keyboard handlers or roles, which Biome flags (noStaticElementInteractions,useKeyWithClickEvents). This makes them inaccessible to keyboard users.You can keep them as divs but add checkbox semantics and keyboard support:
- <div - key={m.userId} - onClick={() => { - const newSet = new Set(selectedUsers); - if (isSelected) newSet.delete(m.userId); - else newSet.add(m.userId); - setSelectedUsers(newSet); - }} + <div + key={m.userId} + role="checkbox" + aria-checked={isSelected} + tabIndex={0} + onClick={() => { + const newSet = new Set(selectedUsers); + if (isSelected) newSet.delete(m.userId); + else newSet.add(m.userId); + setSelectedUsers(newSet); + }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + const newSet = new Set(selectedUsers); + if (isSelected) newSet.delete(m.userId); + else newSet.add(m.userId); + setSelectedUsers(newSet); + } + }} className={`cursor-pointer flex items-center gap-2 p-2 border transition-all ${isSelectedThis should resolve both Biome errors and improve a11y.
770-791: Associate “Payer”/“Payee” labels with their<select>controls.In the payment modal, the “Payer” and “Payee” labels are not linked to their
selectelements, which Biome flags (noLabelWithoutControl). AddhtmlForon labels and matchingidon the selects:<div className="flex flex-col gap-1"> - <label className="text-sm font-bold opacity-80">Payer</label> - <select + <label htmlFor="payment-payer" className="text-sm font-bold opacity-80">Payer</label> + <select + id="payment-payer" className={`w-full p-3 outline-none font-bold ${style === THEMES.NEOBRUTALISM ? 'border-2 border-black bg-white rounded-none' : 'bg-white/10 border border-white/20 rounded-lg text-white'}`} value={paymentPayerId} onChange={e => setPaymentPayerId(e.target.value)} > @@ <div className="flex flex-col gap-1"> - <label className="text-sm font-bold opacity-80">Payee</label> - <select + <label htmlFor="payment-payee" className="text-sm font-bold opacity-80">Payee</label> + <select + id="payment-payee" className={`w-full p-3 outline-none font-bold ${style === THEMES.NEOBRUTALISM ? 'border-2 border-black bg-white rounded-none' : 'bg-white/10 border border-white/20 rounded-lg text-white'}`} value={paymentPayeeId} onChange={e => setPaymentPayeeId(e.target.value)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
web/pages/Friends.tsx(1 hunks)web/pages/GroupDetails.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-26T09:41:01.332Z
Learnt from: CR
Repo: Devasy23/splitwiser PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-26T09:41:01.332Z
Learning: Applies to backend/app/expenses/**/*.py : Settlements must track debt resolution between users in the expense tracking logic.
Applied to files:
web/pages/GroupDetails.tsx
🧬 Code graph analysis (2)
web/pages/GroupDetails.tsx (5)
web/types.ts (4)
Settlement(58-68)Group(16-25)Expense(46-56)GroupMember(27-32)web/contexts/AuthContext.tsx (1)
useAuth(62-68)web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/api.ts (12)
getGroupDetails(29-29)getExpenses(36-36)getGroupMembers(32-32)getOptimizedSettlements(44-44)updateExpense(38-38)createExpense(37-37)deleteExpense(39-39)createSettlement(43-43)updateGroup(30-30)deleteGroup(31-31)leaveGroup(53-53)removeMember(54-54)web/constants.ts (1)
THEMES(1-4)
web/pages/Friends.tsx (4)
web/contexts/ThemeContext.tsx (1)
useTheme(43-49)web/services/api.ts (2)
getFriendsBalance(49-49)getGroups(27-27)mobile/utils/currency.js (2)
formatCurrency(8-16)formatCurrency(8-16)web/constants.ts (1)
THEMES(1-4)
🪛 Biome (2.1.2)
web/pages/GroupDetails.tsx
[error] 465-465: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
[error] 609-609: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 618-618: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 679-691: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 679-691: Static Elements should not be interactive.
To add interactivity such as a mouse or key event listener to a static element, give the element an appropriate role value.
(lint/a11y/noStaticElementInteractions)
[error] 771-771: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
[error] 784-784: A form label must be associated with an input.
Consider adding a for or htmlFor attribute to the label element or moving the input element to inside the label element.
(lint/a11y/noLabelWithoutControl)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: preview
🔇 Additional comments (6)
web/pages/Friends.tsx (4)
1-22: LGTM!The imports are well-organized and the module-level interfaces (
GroupBreakdown,Friend) provide clear type safety for the component's data structures.
82-116: LGTM!Helper functions are well-implemented. The
formatCurrencyreturning absolute values with signs handled at call sites is consistent throughout. ThegetAvatarContentprovides proper fallback handling with theme-aware styling.
159-196: LGTM!Summary cards are well-implemented with proper animation staggering and theme-aware styling. The visual hierarchy clearly communicates the balance summary.
248-276: LGTM!The friend card header properly uses a semantic
<button>element with appropriate accessibility attributes (type="button",aria-expanded,aria-label). The balance display now correctly shows signs for positive and negative values.web/pages/GroupDetails.tsx (2)
515-520: Nice improvement using a stable composite key for settlements.Switching from the array index to a composite key (
${s.fromUserId}-${s.toUserId}) for the settlements grid avoids React key issues when the list changes order or items are added/removed.
201-252: I'll help you verify this review comment about the expense amount validation inGroupDetails.tsx. Let me start by examining the repository structure and the specific file mentioned.
<function_calls>
find . -type f -name "GroupDetails.tsx" 2>/dev/null | head -5
</function_calls>
<function_calls>
fd -t f "GroupDetails" 2>/dev/null
</function_calls>
<function_calls>
git ls-files | grep -i "groupdetails"
</function_calls>Now let me search for the file in the web directory:
<function_calls>
find web -name "GroupDetails.tsx" 2>/dev/null
</function_calls>Let me check what's in the web directory:
<function_calls>
ls -la web/ 2>/dev/null | head -20
</function_calls>Let me try a broader search:
<function_calls>
git ls-files | head -30
</function_calls>Let me search for any TypeScript/React files:
<function_calls>
fd -t f ".(tsx|ts|jsx)$" 2>/dev/null | grep -i group
</function_calls>Let me check the repository structure:
<function_calls>
ls -la
</function_calls>Now let me search for the handleExpenseSubmit function to locate the file:
<function_calls>
rg -l "handleExpenseSubmit" 2>/dev/null
</function_calls>Let me check all TypeScript/TSX files:
<function_calls>
fd -e tsx -e ts 2>/dev/null | head -20
</function_calls>Let me try a different approach and search more broadly:
<function_calls>
rg "handleExpenseSubmit" -t ts -t tsx 2>/dev/null
</function_calls>Let me check if there are any files in the repo:
<function_calls>
git ls-files | wc -l
</function_calls>Let me try to understand the repo better:
<function_calls>
git log --oneline -1
</function_calls>Let me check the current branch:
<function_calls>
git branch -a
</function_calls>Let me list all files in the repository:
<function_calls>
git ls-files
</function_calls>Now let me read the GroupDetails.tsx file to examine the handleExpenseSubmit function:
<function_calls>
wc -l web/pages/GroupDetails.tsx
</function_calls>Let me read the relevant section around lines 201-252:
<function_calls>
sed -n '180,260p' web/pages/GroupDetails.tsx
</function_calls>Let me also check what comes before to understand the full context of the component:
<function_calls>
sed -n '1,80p' web/pages/GroupDetails.tsx
</function_calls>Let me search for the expense input field and form setup:
<function_calls>
rg -A 5 -B 5 "amount.*input|setAmount" web/pages/GroupDetails.tsx
</function_calls>Let me check for the form JSX that renders the input:
<function_calls>
sed -n '300,400p' web/pages/GroupDetails.tsx
</function_calls>Let me search for the Input component and its configuration:
<function_calls>
rg -n "Input.*amount|type.*number" web/pages/GroupDetails.tsx -C 3
</function_calls>Let me check if there's validation on the Input component:
<function_calls>
rg -n "required|disabled.*submit|validateAmount" web/pages/GroupDetails.tsx
</function_calls>Let me examine the entire file to understand the structure better:
<function_calls>
cat web/pages/GroupDetails.tsx | head -100
</function_calls>Let me check the file size and read it systematically:
<function_calls>
wc -l web/pages/GroupDetails.tsx && echo "---" && cat -n web/pages/GroupDetails.tsx | sed -n '200,270p'
</function_calls>Now let me search for the input field that takes the amount:
<function_calls>
cat -n web/pages/GroupDetails.tsx | grep -A 10 -B 10 "amount"
</function_calls>Let me extract more context around the form:
<function_calls>
cat -n web/pages/GroupDetails.tsx | sed -n '1,60p' && echo "..." && cat -n web/pages/GroupDetails.tsx | sed -n '250,350p'
</function_calls>Let me check what type of Input component is being used:
<function_calls>
rg "from.*Input|import.*Input" web/pages/GroupDetails.tsx
</function_calls>Let me look at the entire file in segments to find the form:
<function_calls>
grep -n "handleExpenseSubmit|<form|<Input|name="amount"" web/pages/GroupDetails.tsx
</function_calls>Let me read a larger section to find the form rendering:
<function_calls>
tail -n +300 web/pages/GroupDetails.tsx | head -100
</function_calls>Let me find all occurrences of amount-related state and inputs:
<function_calls>
rg -n "setAmount|amount\s*=" web/pages/GroupDetails.tsx
</function_calls>Let me look at the entire file to map out its structure:
<function_calls>
cat web/pages/GroupDetails.tsx | python3 << 'EOF'
import sys
content = sys.stdin.read()
lines = content.split('\n')
for i, line in enumerate(lines, 1):
if any(keyword in line for keyword in ['handleExpenseSubmit', '<Input', 'name="amount"', 'required', 'setAmount']):
print(f"{i:4d}: {line}")
EOF
</function_calls>Let me read the file directly:
<function_calls>
cat web/pages/GroupDetails.tsx
</function_calls>Perfect! Now I have the full file. Let me analyze the specific function and context:
<function_calls>
cat -n web/pages/GroupDetails.tsx | sed -n '195,260p'
</function_calls>Now let me search for where the amount input is rendered:
<function_calls>
cat -n web/pages/GroupDetails.tsx | grep -n "amount" -i
</function_calls>Let me look at the JSX form section:
<function_calls>
cat -n web/pages/GroupDetails.tsx | sed -n '350,450p'
</function_calls>Let me search for the expense modal form:
<function_calls>
cat -n web/pages/GroupDetails.tsx | sed -n '300,550p'
</function_calls>Let me check the API endpoints to see if there's backend validation:
<function_calls>
rg "createExpense|updateExpense" --type ts --type tsx -A 5 2>/dev/null | head -40
</function_calls>Let me search for these API functions:
<function_calls>
rg -n "export.*createExpense|const createExpense|function createExpense" 2>/dev/null
</function_calls>Let me look for API/service files:
<function_calls>
find . -name "api" -o -name "service" 2>/dev/null | grep -E ".(ts|tsx|js)$"
</function_calls>Let me check the imports in GroupDetails.tsx:
<function_calls>
head -20 web/pages/GroupDetails.tsx
</function_calls>Let me look for the API service file:
<function_calls>
find . -path "./web/lib/" -o -path "./web/services/" -o -path "./lib/*" 2>/dev/null | head -20
</function_calls>Let me check lib directory:
<function_calls>
git ls-files | grep -E "^.lib..(ts|tsx|js)$"
</function_calls>Let me search for where createExpense is defined:
<function_calls>
rg "createExpense\s*=" 2>/dev/null
</function_calls>Let me look at all files:
<function_calls>
git ls-files
</function_calls>Let me check the api file:
<function_calls>
cat web/lib/api.ts 2>/dev/null | head -100
</function_calls>Let me check if the file exists:
<function_calls>
ls -la web/lib/ 2>/dev/null
</function_calls>Let me find all TypeScript/JavaScript files:
<function_calls>
find web -type f -name ".ts" -o -name ".tsx" 2>/dev/null
</function_calls>Let me check the structure:
<function_calls>
ls -la web/
</function_calls>Now let me look at the api file:
<function_calls>
cat web/api.ts
</function_calls>Perfect! Now let me verify the expense submission and validation. Let me check around line 201-252 in GroupDetails.tsx again to see the full context:
<function_calls>
sed -n '190,270p' web/pages/GroupDetails.tsx | cat -n
</function_calls>Now let me check if there's an Input component with validation:
<function_calls>
grep -n "Input|input" web/pages/GroupDetails.tsx | head -20
</function_calls>Let me look at the entire render section:
<function_calls>
tail -n 150 web/pages/GroupDetails.tsx
</function_calls>Let me check the return JSX for the form:
<function_calls>
sed -n '350,500p' web/pages/GroupDetails.tsx
</function_calls>Let me find where the expense form is rendered by searching more systematically:
<function_calls>
grep -n "value={amount}" web/pages/GroupDetails.tsx
</function_calls>Let me look at the entire component to find the form:
<function_calls>
wc -l web/pages/GroupDetails.tsx && sed -n '430,500p' web/pages/GroupDetails.tsx
</function_calls>Let me check for input tags:
<function_calls>
grep -n "<input|<Input" web/pages/GroupDetails.tsx
</function_calls>Let me look at lines around 440-470:
<function_calls>
sed -n '435,480p' web/pages/GroupDetails.tsx
</function_calls>Now let me check the entire render return statement to understand the form structure:
<function_calls>
sed -n '330,550p' web/pages/GroupDetails.tsx | cat -n
</function_calls>Now I can see the form! Let me analyze it more carefully. The amount input appears to be an HTML input element. Let me verify:
<function_calls>
sed -n '456,475p' web/pages/GroupDetails.tsx
</function_calls>Perfect! Now I have all the context. Let me analyze this systematically:
- The input field at line 469 has:
type="number" value={amount} onChange={(e) => setAmount(e.target.value)} required- The handleExpenseSubmit function parses the amount but doesn't validate it
- The
requiredattribute means the input cannot be submitted empty via browser form submission- However, the form can be bypassed or amount could be an invalid value
Let me verify if there are any other validations:
<function_calls>
rg "numAmount" web/pages/GroupDetails.tsx
</function_calls>Now let me confirm my analysis by checking the actual API behavior. Let me look at the backend:
<function_calls>
find . -type f -name ".ts" -o -name ".js" 2>/dev/null | xargs grep -l "expense" 2>/dev/null | grep -v node_modules
</function_calls>Let me check if there's a backend:
<function_calls>
ls -la
</function_calls>Let me look for any validation on the backend side by checking API responses:
<function_calls>
git ls-files | grep -v node_modules
</function_calls>Let me check if there are more files in web folder:
<function_calls>
find web -type f
</function_calls>Based on my verification of the codebase, I can now provide the rewritten review comment:
Add validation for expense amount to prevent NaN and non-positive values from being sent to the API.
handleExpenseSubmitparsesamountwithparseFloat()without validating the result. Although the input field hastype="number"andrequiredattributes (line 469), these don't guarantee a valid finite positive value—JavaScript can still allow invalid inputs, and direct programmatic calls could bypass form validation. IfnumAmountisNaNor ≤ 0, split calculations will produceNaNvalues sent to the backend, potentially causing unexpected behavior.Add an explicit guard immediately after parsing:
const handleExpenseSubmit = async (e: React.FormEvent) => { e.preventDefault(); if (!id) return; - const numAmount = parseFloat(amount); + const numAmount = parseFloat(amount); + if (!Number.isFinite(numAmount) || numAmount <= 0) { + alert('Please enter a valid amount greater than 0'); + return; + } let requestSplits: { userId: string; amount: number }[] = [];Likely an incorrect or invalid review comment.
| const [friends, setFriends] = useState<Friend[]>([]); | ||
| const [loading, setLoading] = useState(true); | ||
| const [error, setError] = useState<string | null>(null); |
There was a problem hiding this comment.
Add loading state UI.
The loading state is managed but there's no corresponding UI feedback. Users see nothing during data fetch, then content suddenly appears. Add a loading skeleton or spinner.
+ {/* Loading State */}
+ {loading && (
+ <motion.div
+ initial={{ opacity: 0 }}
+ animate={{ opacity: 1 }}
+ className="col-span-full text-center py-20"
+ >
+ <div className={`animate-spin w-12 h-12 mx-auto mb-4 border-4 border-t-transparent ${isNeo ? 'border-black' : 'border-white/50'} rounded-full`} />
+ <p className="text-xl font-bold opacity-50">Loading friends...</p>
+ </motion.div>
+ )}Place this before the Friends Grid section and conditionally render the grid only when !loading.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/pages/Friends.tsx around lines 25 to 27, the component tracks a loading
state but never renders any UI for it; add a visible loading indicator (spinner
or skeleton) that displays while loading is true and hide the Friends Grid until
loading is false. Implement a conditional render before the Friends Grid: if
(loading) return or render a placeholder container with accessible
spinner/skeleton markup and minimal styling, otherwise render the existing grid;
ensure the loading UI is keyboard/screen-reader accessible and does not depend
on fetched data.
| <button | ||
| type="button" | ||
| onClick={() => window.location.reload()} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider re-fetching data instead of full page reload.
Using window.location.reload() works but is a heavy-handed approach. Consider extracting fetchData and calling it directly for a smoother retry experience.
+ const fetchData = useCallback(async () => {
+ setLoading(true);
+ setError(null);
// ... existing fetch logic
+ }, []);
useEffect(() => {
fetchData();
- }, []);
+ }, [fetchData]);
// In retry button:
- onClick={() => window.location.reload()}
+ onClick={fetchData}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In web/pages/Friends.tsx around lines 209 to 211, the retry button currently
calls window.location.reload(), which is a full page reload; instead extract the
data-fetch logic into a reusable async function (e.g., fetchData) that sets
loading and error state and updates the component state with new results, then
replace the onClick handler to call that function to re-fetch the friends data
and handle success/error without reloading the page.
| {filteredFriends.length === 0 && !error ? ( | ||
| <motion.div | ||
| initial={{ opacity: 0 }} | ||
| animate={{ opacity: 1 }} | ||
| className="col-span-full text-center py-20 opacity-50" | ||
| > | ||
| <Users size={48} className="mx-auto mb-4 opacity-50" /> | ||
| <p className="text-xl font-bold">No friends found</p> | ||
| </motion.div> |
There was a problem hiding this comment.
Empty state displays during loading.
The condition filteredFriends.length === 0 && !error doesn't account for loading state. Users briefly see "No friends found" before data loads.
- {filteredFriends.length === 0 && !error ? (
+ {filteredFriends.length === 0 && !error && !loading ? (
<motion.div
initial={{ opacity: 0 }}
animate={{ opacity: 1 }}
className="col-span-full text-center py-20 opacity-50"
>
<Users size={48} className="mx-auto mb-4 opacity-50" />
<p className="text-xl font-bold">No friends found</p>
</motion.div>📝 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.
| {filteredFriends.length === 0 && !error ? ( | |
| <motion.div | |
| initial={{ opacity: 0 }} | |
| animate={{ opacity: 1 }} | |
| className="col-span-full text-center py-20 opacity-50" | |
| > | |
| <Users size={48} className="mx-auto mb-4 opacity-50" /> | |
| <p className="text-xl font-bold">No friends found</p> | |
| </motion.div> | |
| {filteredFriends.length === 0 && !error && !loading ? ( | |
| <motion.div | |
| initial={{ opacity: 0 }} | |
| animate={{ opacity: 1 }} | |
| className="col-span-full text-center py-20 opacity-50" | |
| > | |
| <Users size={48} className="mx-auto mb-4 opacity-50" /> | |
| <p className="text-xl font-bold">No friends found</p> | |
| </motion.div> |
🤖 Prompt for AI Agents
In web/pages/Friends.tsx around lines 225 to 233, the empty-state rendering uses
the condition filteredFriends.length === 0 && !error which does not account for
a loading state and causes "No friends found" to flash while data is being
fetched; update the conditional to also require that loading is false (e.g.,
filteredFriends.length === 0 && !error && !loading) so the empty message only
shows after loading completes (optionally render a loader or skeleton while
loading).
| <button type="button" className={`w-full mt-4 py-2 text-sm font-bold flex items-center justify-center gap-2 transition-colors ${isNeo | ||
| ? 'bg-black text-white hover:bg-gray-800 rounded-none' | ||
| : 'bg-white/10 hover:bg-white/20 rounded-xl' | ||
| }`}> | ||
| View Details <ArrowRight size={14} /> | ||
| </button> |
There was a problem hiding this comment.
"View Details" button has no click handler.
The button is rendered but lacks an onClick handler, making it non-functional. Either implement navigation/action or remove the button until the feature is ready.
- <button type="button" className={`w-full mt-4 py-2 text-sm font-bold flex items-center justify-center gap-2 transition-colors ${isNeo
+ <button
+ type="button"
+ onClick={() => { /* TODO: Navigate to friend details */ }}
+ className={`w-full mt-4 py-2 text-sm font-bold flex items-center justify-center gap-2 transition-colors ${isNeo
? 'bg-black text-white hover:bg-gray-800 rounded-none'
: 'bg-white/10 hover:bg-white/20 rounded-xl'
}`}>
View Details <ArrowRight size={14} />
</button>Would you like me to open an issue to track implementing the friend details navigation?
📝 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.
| <button type="button" className={`w-full mt-4 py-2 text-sm font-bold flex items-center justify-center gap-2 transition-colors ${isNeo | |
| ? 'bg-black text-white hover:bg-gray-800 rounded-none' | |
| : 'bg-white/10 hover:bg-white/20 rounded-xl' | |
| }`}> | |
| View Details <ArrowRight size={14} /> | |
| </button> | |
| <button | |
| type="button" | |
| onClick={() => { /* TODO: Navigate to friend details */ }} | |
| className={`w-full mt-4 py-2 text-sm font-bold flex items-center justify-center gap-2 transition-colors ${isNeo | |
| ? 'bg-black text-white hover:bg-gray-800 rounded-none' | |
| : 'bg-white/10 hover:bg-white/20 rounded-xl' | |
| }`}> | |
| View Details <ArrowRight size={14} /> | |
| </button> |
🤖 Prompt for AI Agents
In web/pages/Friends.tsx around lines 302 to 307 the "View Details" button is
rendered without an onClick handler making it non-functional; either wire it to
navigate to the friend's detail view (e.g., use next/router push or wrap with
<Link> to `/friends/[id]` passing the correct friend id from scope) or remove
the button until implemented; if adding navigation, call router.push or Link
with the friend's id, ensure the id is available in the component props/state,
and add an aria-label for accessibility.
| interface Settlement { | ||
| fromUserId: string; | ||
| fromUserName: string; | ||
| toUserId: string; | ||
| toUserName: string; | ||
| amount: number; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify local Settlement type vs shared Settlement in web/types.ts.
There’s a locally defined interface Settlement whose shape (fromUserId, toUserId, names, amount) differs from the exported Settlement in web/types.ts (with _id, payerId, payeeId, status, etc.). This name collision is confusing and makes it easy to use the wrong type by mistake.
Consider either:
- Reusing the shared
Settlementtype if the API shape is actually the same, or - Renaming this local type to something like
OptimizedSettlement(and documenting that it matchesgetOptimizedSettlements’ response).
🤖 Prompt for AI Agents
In web/pages/GroupDetails.tsx around lines 29 to 35, there is a local interface
named Settlement whose shape diverges from the shared Settlement in web/types.ts
which causes a confusing name collision; either import and use the shared
Settlement type from web/types.ts if the API response actually matches that
definition, or rename the local interface to a distinct name like
OptimizedSettlement (and add a brief comment indicating it matches the
getOptimizedSettlements response) and update all local usages to the new name to
avoid accidental mixing of types.
| const copyToClipboard = () => { | ||
| if (group?.joinCode) { | ||
| navigator.clipboard.writeText(group.joinCode) | ||
| .then(() => { | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| }) | ||
| .catch(() => { | ||
| alert('Failed to copy to clipboard'); | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Good clipboard error handling; consider guarding for navigator.clipboard availability.
Nice upgrade to handle navigator.clipboard.writeText via promises and show a failure alert; this is much more robust than a bare call.
One additional hardening step is to guard against environments where navigator.clipboard is undefined (e.g., non-secure contexts) before calling writeText:
- if (group?.joinCode) {
- navigator.clipboard.writeText(group.joinCode)
+ if (group?.joinCode && navigator.clipboard?.writeText) {
+ navigator.clipboard.writeText(group.joinCode)
.then(() => {
setCopied(true);
setTimeout(() => setCopied(false), 2000);
})
.catch(() => {
alert('Failed to copy to clipboard');
});
}This prevents a runtime error in older/unsupported browsers.
🤖 Prompt for AI Agents
In web/pages/GroupDetails.tsx around lines 125 to 136, the copyToClipboard
function calls navigator.clipboard.writeText without guarding against
environments where navigator.clipboard is undefined; add a guard that checks if
navigator and navigator.clipboard exist before calling writeText, and if not
available, fall back to a graceful alternative (e.g., show an alert/instruction
to manually copy or use a legacy fallback like creating a temporary textarea and
execCommand('copy')), ensure you return early when clipboard API is missing and
preserve the existing promise handling and copied state logic for the successful
path.
| <motion.div | ||
| layout | ||
| key={expense._id} | ||
| initial={{ opacity: 0, scale: 0.95 }} | ||
| animate={{ opacity: 1, scale: 1 }} | ||
| whileHover={{ scale: 1.02 }} | ||
| onClick={() => openEditExpense(expense)} | ||
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); openEditExpense(expense); } }} | ||
| tabIndex={0} | ||
| role="button" | ||
| aria-label={`Expense: ${expense.description}, ${group.currency} ${expense.amount.toFixed(2)}`} | ||
| className={`p-5 flex items-center gap-5 cursor-pointer group relative overflow-hidden ${style === THEMES.NEOBRUTALISM |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Use a semantic button instead of motion.div with role="button".
The expense card is keyboard-focusable and has role="button", and Biome suggests using a semantic <button> instead. This improves a11y and removes the need for manual tabIndex/role.
You can switch to motion.button:
- expenses.map((expense, idx) => (
- <motion.div
+ expenses.map((expense, idx) => (
+ <motion.button
layout
key={expense._id}
+ type="button"
initial={{ opacity: 0, scale: 0.95 }}
animate={{ opacity: 1, scale: 1 }}
whileHover={{ scale: 1.02 }}
onClick={() => openEditExpense(expense)}
onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); openEditExpense(expense); } }}
- tabIndex={0}
- role="button"
aria-label={`Expense: ${expense.description}, ${group.currency} ${expense.amount.toFixed(2)}`}
className={`p-5 flex items-center gap-5 cursor-pointer group relative overflow-hidden ${style === THEMES.NEOBRUTALISM
? 'bg-white border-2 border-black shadow-[4px_4px_0px_0px_rgba(0,0,0,1)] hover:translate-x-[2px] hover:translate-y-[2px] hover:shadow-[2px_2px_0px_0px_rgba(0,0,0,1)] rounded-none'
: 'bg-white/5 border border-white/10 rounded-2xl backdrop-blur-sm hover:bg-white/10 transition-all'
}`}
- >
+ >(Remember to update the closing tag to </motion.button>.)
🧰 Tools
🪛 Biome (2.1.2)
[error] 465-465: The elements with this role can be changed to the following elements:
For examples and more information, see WAI-ARIA Roles
(lint/a11y/useSemanticElements)
🤖 Prompt for AI Agents
In web/pages/GroupDetails.tsx around lines 456 to 467, the expense card uses a
keyboard-focusable motion.div with role="button", tabIndex and a manual
onKeyDown handler; replace it with a semantic motion.button (change opening and
closing tags to motion.button/motion.button) to improve accessibility, remove
role and tabIndex and the custom onKeyDown, add type="button" to prevent form
submit, keep the existing layout, key, initial/animate/whileHover props, onClick
handler, aria-label and className intact so visual/animation behavior is
preserved while relying on the native button keyboard behavior.
Introduce a revamped Friends page with group breakdowns and loading states, improve group management functionalities including member management and settings, and create a new Profile page for user account management. Update API services to support new profile and group management features, and integrate Firebase for Google authentication. Enhance UI components for better user experience.
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.