feat(settings): Add Settings Page with UX improvements#231
Merged
Conversation
- Profile section with email and member since date - GitHub connection status with disconnect button - Repository slots display (X/3 free tier) - Danger zone with delete repos and delete account - All actions have confirmation dialogs - Uses Card, Dialog, Alert, Badge, Separator, Skeleton components - Loading states and error handling included
- Remove delete account button (was non-functional placeholder)
- Add typing confirmation for delete repos ('delete all' required)
- Remove Settings and Global Search from sidebar (Settings in avatar)
- Replace custom dropdown with shadcn DropdownMenu (fixes click-outside)
- Cleaner sidebar with just Repositories and Documentation
|
@DevanshuNEU is attempting to deploy a commit to the Dev's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughReplaces direct DashboardHome render with React Router Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant SettingsPage as Settings Page
participant UserContext as User Context
participant GitHubHook as useGitHubRepos Hook
participant API as Backend API
User->>Browser: Navigate to /dashboard/settings
Browser->>SettingsPage: Render SettingsPage
SettingsPage->>UserContext: Read user profile
UserContext-->>SettingsPage: Return profile (email, member since)
SettingsPage->>GitHubHook: Fetch connection state & repos
GitHubHook-->>SettingsPage: Return connected state + repo list
User->>SettingsPage: Click "Delete All Repos"
SettingsPage->>User: Show confirmation dialog
User->>SettingsPage: Enter confirmation text & confirm
loop for each repo
SettingsPage->>API: DELETE /api/repos/:repoId
API-->>SettingsPage: 200 OK / error
end
SettingsPage->>SettingsPage: Clear repos state
SettingsPage->>User: Show success or error toast
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/src/pages/SettingsPage.tsx`:
- Around line 48-61: The fetchRepos function currently calls response.json()
without checking HTTP status; update fetchRepos to inspect response.ok after the
fetch to handle non-2xx responses: if response.ok parse the JSON and call
setRepos(data.repositories || []), otherwise parse or read the error body (json
or text) for logging and either setRepos([]) or leave existing state and throw
or return early so error handling in the catch block runs; ensure
setReposLoading(false) still runs in finally. Reference: fetchRepos,
session?.access_token, API_URL, setRepos, setReposLoading.
- Around line 74-94: handleDeleteAllRepos currently sends DELETEs in a loop
without checking response.ok and unconditionally clears local repos on any
exception, leaving UI out of sync on partial failures; update
handleDeleteAllRepos to check each fetch response.ok (using API_URL and
session?.access_token) and if any response is not ok either throw or collect
failed IDs, stop further deletion on error (or continue but track failures),
only call setRepos([]), setDeleteReposDialog(false) and clear deleteConfirmation
when every delete succeeded, and in case of partial failures refetch the repo
list from the server to reconcile local state and show a toast that includes the
number or IDs of failed deletions so the user knows what remained; ensure
setDeleteReposLoading(false) still runs in finally.
🧹 Nitpick comments (2)
frontend/src/pages/SettingsPage.tsx (1)
43-46: Missing dependency in useEffect.The
checkStatusfunction fromuseGitHubReposis called but not listed in the dependency array. This can cause stale closure issues ifcheckStatuschanges between renders. React's exhaustive-deps rule would flag this.♻️ Suggested fix
useEffect(() => { checkStatus() fetchRepos() - }, []) + }, [checkStatus])Alternatively, if
checkStatusis not memoized in the hook, consider wrappingfetchReposwithuseCallbackand including both, or document why the empty dependency array is intentional (e.g., only run once on mount).frontend/src/components/Dashboard.tsx (1)
9-13: Routing structure looks correct.The nested Routes within DashboardLayout correctly handles the settings path. One minor consideration: the catch-all route renders
DashboardHomeat the mismatched URL rather than redirecting. This could leave users at URLs like/dashboard/typoshowing the home content.💡 Optional: Redirect unknown paths to clean URL
+import { Routes, Route, Navigate } from 'react-router-dom' ... - <Route path="*" element={<DashboardHome />} /> + <Route path="*" element={<Navigate to="/dashboard" replace />} />
- fetchRepos: check response.ok before parsing JSON, log errors properly - handleDeleteAllRepos: track failed deletions, refetch on partial failure - Show toast with failed repo names on partial failure - Reconcile local state with server on any error
- Add checkStatus to useEffect dependency array (avoid stale closure) - Redirect unknown dashboard paths to /dashboard instead of showing home
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@frontend/src/pages/SettingsPage.tsx`:
- Line 133: The UI currently can display negative "slots available" when repo
count exceeds MAX_REPOS; locate where available slots are computed (use symbols
MAX_REPOS and the variable or expression that derives current repo count, e.g.,
repos.length or slotsAvailable) and clamp the result to zero (replace "MAX_REPOS
- currentCount" with "Math.max(0, MAX_REPOS - currentCount)") so the UI never
shows negatives; apply the same clamp to all places this calculation is used
(including the component that defines isDeleteEnabled and the code referenced
around lines for slots display).
- Around line 43-68: The effect currently only depends on checkStatus so
fetchRepos may return early when session?.access_token is not yet set and
reposLoading never gets cleared; update the hook so fetchRepos runs when the
session token appears by adding session?.access_token (or session) to the
useEffect dependency array and ensure fetchRepos always flips
setReposLoading(false) (or sets it to true before starting) even when returning
early; locate the useEffect and the fetchRepos function and modify dependencies
and loading state handling accordingly (functions: useEffect, fetchRepos,
setReposLoading, session, checkStatus).
- Add session?.access_token to useEffect deps (re-runs when auth loads) - Set reposLoading=false on early return (prevents stuck loading state) - Set reposLoading=true at start of fetch (consistent state) - Clamp available slots to 0 (prevents negative display if repos > MAX)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a Settings page accessible from the user avatar dropdown with proper UX patterns.
Changes
Settings Page (
SettingsPage.tsx)Navigation Improvements
DropdownMenuUX Improvements
Testing
Part of Polish Sprint
Summary by CodeRabbit
New Features
Navigation
UI Improvements
✏️ Tip: You can customize this high-level summary in your review settings.