Refactor frontend routing to use React Router idiomatically #129
Conversation
Co-authored-by: mithun50 <[email protected]>
WalkthroughRefactors frontend routing to use react-router-dom nested routes with a ProtectedLayout. Replaces activePage/renderPage with declarative routes. Sidebar now uses NavLink-based navigation and exposes onLogout. Adds Outlet and AnimatePresence for nested route rendering and transitions. Auth gating redirects unauthenticated users to /login. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Router as React Router
participant Protected as ProtectedLayout
participant Sidebar
participant Outlet
participant Pages as Page Components
User->>Router: Navigate to /
Router->>Protected: Check auth
alt Authenticated
Protected->>Sidebar: Render (isOpen, onLogout)
Protected->>Outlet: Render nested route
Outlet->>Pages: Render Dashboard/Analytics/... based on path
User->>Sidebar: Click NavLink
Sidebar->>Router: Change URL
Router->>Outlet: Render new nested route
else Not authenticated
Router->>User: Redirect to /login
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@smokeyScraper request for review this pr |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (7)
frontend/src/components/layout/Sidebar.tsx (3)
36-41: Use functional state update and add a11y attributes to the togglePrevents potential stale state issues and improves accessibility.
- <button onClick={() => setIsOpen(!isOpen)} className="p-2 hover:bg-gray-800 rounded-lg"> + <button + onClick={() => setIsOpen((prev) => !prev)} + aria-label={isOpen ? 'Collapse sidebar' : 'Expand sidebar'} + aria-expanded={isOpen} + className="p-2 hover:bg-gray-800 rounded-lg" + >
45-57: Improve collapsed-mode accessibility and discoverability for linksWhen collapsed, the label is visually hidden. Provide an accessible name and tooltip.
<NavLink key={item.path} to={item.path} + title={item.label} + aria-label={!isOpen ? item.label : undefined} className={({ isActive }) =>
60-67: Add a11y attributes to the Logout buttonExplicitly set button type and label for accessibility.
- <button + <button + type="button" + aria-label="Logout" onClick={onLogout} className={`w-full flex items-center px-4 py-3 text-gray-300 hover:bg-gray-800 hover:text-red-500 transition-colors`} >frontend/src/App.tsx (4)
20-29: Avoid auth flicker on initial load by reading localStorage synchronouslyInitialize from localStorage in state initializer and drop the effect. Prevents a brief redirect to /login before hydration.
- const [isAuthenticated, setIsAuthenticated] = useState(false); - - // Check for existing authentication on app load - useEffect(() => { - const savedAuth = localStorage.getItem('isAuthenticated'); - if (savedAuth === 'true') { - setIsAuthenticated(true); - } - }, []); + const [isAuthenticated, setIsAuthenticated] = useState<boolean>(() => { + return localStorage.getItem('isAuthenticated') === 'true'; + });
2-2: Ensure route transition exit animations run by keying content with locationAnimatePresence won’t play exit animations reliably without a stable key per route. Use useLocation and key the Outlet container.
-import { BrowserRouter as Router, Routes, Route, Navigate, Outlet } from 'react-router-dom'; +import { BrowserRouter as Router, Routes, Route, Navigate, Outlet, useLocation } from 'react-router-dom';- const ProtectedLayout = () => ( - <div className="flex"> - <Sidebar - isOpen={isSidebarOpen} - setIsOpen={setIsSidebarOpen} - onLogout={handleLogout} - /> - <main - className={`transition-all duration-300 flex-1 ${ - isSidebarOpen ? 'ml-64' : 'ml-20' - }`} - > - <div className="p-8"> - <AnimatePresence mode="wait"> - <Outlet /> - </AnimatePresence> - </div> - </main> - </div> - ); + const ProtectedLayout = () => { + const location = useLocation(); + return ( + <div className="flex"> + <Sidebar + isOpen={isSidebarOpen} + setIsOpen={setIsSidebarOpen} + onLogout={handleLogout} + /> + <main + className={`transition-all duration-300 flex-1 ${ + isSidebarOpen ? 'ml-64' : 'ml-20' + }`} + > + <div className="p-8"> + <AnimatePresence mode="wait"> + <div key={location.pathname}> + <Outlet /> + </div> + </AnimatePresence> + </div> + </main> + </div> + ); + };Also applies to: 41-60
66-96: Add a catch-all route to handle unknown URLsRedirects unknown paths to a sensible destination depending on auth.
<Routes> <Route path="/login" element={ isAuthenticated ? ( <Navigate to="/" replace /> ) : ( <LoginPage onLogin={handleLogin} /> ) } /> <Route path="/" element={ isAuthenticated ? <ProtectedLayout /> : <Navigate to="/login" replace /> } > <Route index element={<LandingPage setRepoData={setRepoData} />} /> <Route path="dashboard" element={<Dashboard repoData={repoData} />} /> <Route path="integration" element={<BotIntegrationPage />} /> <Route path="contributors" element={<ContributorsPage repoData={repoData} />} /> <Route path="analytics" element={<AnalyticsPage repoData={repoData} />} /> <Route path="prs" element={<PullRequestsPage repoData={repoData} />} /> <Route path="support" element={<SupportPage />} /> <Route path="settings" element={<SettingsPage />} /> <Route path="profile" element={<ProfilePage />} /> </Route> + <Route + path="*" + element={<Navigate to={isAuthenticated ? '/dashboard' : '/login'} replace />} + /> </Routes>
19-19: Optional: replace any with a typed shape for repoDataDefine a RepoData interface to improve type-safety across Dashboard/Analytics/Contributors/PRs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
frontend/src/App.tsx(4 hunks)frontend/src/components/layout/Sidebar.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
frontend/src/App.tsx (2)
frontend/src/components/integration/BotIntegrationPage.tsx (1)
BotIntegrationPage(11-140)frontend/src/components/pages/SettingsPage.tsx (1)
SettingsPage(12-257)
🔇 Additional comments (2)
frontend/src/components/layout/Sidebar.tsx (1)
22-31: Good move: NavLink-based navigation with defined route pathsSwitching to a centralized navItems + NavLink removes prop-drilling and keeps URL as the source of truth. Clean and idiomatic.
frontend/src/App.tsx (1)
78-92: Router refactor is idiomatic and clearNested routes under a ProtectedLayout with URL-driven navigation is a strong improvement over manual state switching. Props wiring to pages looks consistent.
There was a problem hiding this comment.
Thanks a lot @mithun50! for the PR.
This helps a lot in protecting routes and decoupling logic.
2 participants
Closes #127
📝 Description
This pull request refactors the frontend routing to use react-router-dom idiomatically, replacing a manual, state-based routing system with a modern, declarative approach.
🔧 Changes Made
Replaced the old, manual routing logic in App.tsx with a modern, declarative system using the react-router-dom library. Instead of relying on component state to switch pages, the application now uses standard URL-based routes (e.g., /dashboard, /settings).Improved the Sidebar Navigation:
Updated the navigation links in the Sidebar from simple buttons to proper NavLink components. This allows the active page to be highlighted correctly based on the current URL.Added a Logout Feature:
Added a new logout button to the bottom of the sidebar, which was previously missing.✅ Checklist
Summary by CodeRabbit
New Features
Refactor