Conversation
…oard sidebar - Add react-feather dependency - Replace all emojis with Feather icons in dashboard, api-docs, and search pages - Replace inline SVGs with Feather icons in landing page components - Add collapsible sidebar with navigation (Home, API Keys, Setup, Settings) - Create dashboard layout with auth protection - Add new pages: - /dashboard/api-keys - key generation and rate limit analytics - /dashboard/settings - account details and subscription info - /dashboard/setup - Python setup guide with code examples - Refactor main dashboard to be overview page with quick actions
📝 WalkthroughWalkthroughThis PR adds react-feather icons to the frontend, replacing inline SVGs and emoji across multiple components. It introduces a new dashboard structure with dedicated pages for API keys, settings, and setup, each with authentication checks and TRPC integration for data fetching. A new sidebar navigation component is added to support dashboard layout. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant DashboardLayout
participant Logto
participant DashboardClient
participant TRPC
User->>Browser: Navigate to /dashboard
Browser->>DashboardLayout: Render layout.tsx
DashboardLayout->>Logto: getLogtoContext()
Logto-->>DashboardLayout: auth status & claims
alt Unauthenticated
DashboardLayout-->>Browser: Render sign-in prompt
Browser-->>User: Display sign-in UI
else Authenticated
DashboardLayout->>DashboardClient: Render with logtoId, email, name
DashboardClient->>TRPC: Query user info & usage stats
TRPC-->>DashboardClient: Return user data & limits
DashboardClient-->>Browser: Render welcome + quick actions
Browser-->>User: Display dashboard
rect rgba(100, 200, 100, 0.1)
note over User,Browser: Navigation to sub-pages
User->>Browser: Click "API Keys"
Browser->>DashboardLayout: Navigate to /dashboard/api-keys
DashboardLayout->>Logto: Verify authentication
Logto-->>DashboardLayout: Auth confirmed
DashboardLayout->>DashboardClient: Render ApiKeysClient
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/kal-frontend/src/components/landing/Navbar.tsx (1)
51-56: Add aria-label to mobile menu button for accessibility.The mobile menu toggle button lacks an accessible label for screen readers.
Recommended fix
<button onClick={() => setMobileMenuOpen(!mobileMenuOpen)} + aria-label={mobileMenuOpen ? "Close menu" : "Open menu"} + aria-expanded={mobileMenuOpen} className="md:hidden p-2 text-content-secondary hover:text-content-primary" > {mobileMenuOpen ? <X size={24} /> : <Menu size={24} />} </button>packages/kal-frontend/src/app/search/page.tsx (1)
515-520: Add aria-label to modal close button for accessibility.The close button in the food detail modal lacks an accessible label for screen readers.
Recommended fix
<button onClick={() => setSelectedFood(null)} + aria-label="Close details" className="text-content-muted hover:text-content-primary transition-colors" > <X size={20} /> </button>packages/kal-frontend/src/app/api-docs/client.tsx (1)
359-364: Add aria-label to mobile menu button for accessibility.The mobile menu toggle button lacks an accessible label, same issue as in Navbar.tsx.
Recommended fix
<button onClick={() => setMobileMenuOpen(!mobileMenuOpen)} + aria-label={mobileMenuOpen ? "Close menu" : "Open menu"} + aria-expanded={mobileMenuOpen} className="md:hidden p-2 text-content-secondary hover:text-content-primary" > {mobileMenuOpen ? <X size={24} /> : <Menu size={24} />} </button>
🧹 Nitpick comments (4)
packages/kal-frontend/src/components/landing/Features.tsx (1)
10-38: Consider adding ARIA labels for icon accessibility.The icons lack semantic meaning for screen readers. While they are decorative in this context (paired with descriptive text), adding
aria-hidden="true"would explicitly mark them as decorative.Example implementation
{ - icon: <Zap className="w-6 h-6" />, + icon: <Zap className="w-6 h-6" aria-hidden="true" />, title: "Instant Search", description: "Find any food in milliseconds with our fast search engine", },Apply similarly to all icon instances.
packages/kal-frontend/src/app/dashboard/setup/client.tsx (1)
51-113: Consider using actual user API key in code examples.The code examples use a placeholder
"YOUR_API_KEY_HERE"on Line 61. Since you're fetching the user's actual API keys, consider pre-populating the examples with their real key for a better user experience.Implementation approach
+ const actualApiKey = apiKeys?.[0] ? `${apiKeys[0].keyPrefix}...` : "YOUR_API_KEY_HERE"; + const codeSnippets = [ { title: "1. Install the requests library", code: "pip install requests", language: "bash", }, { title: "2. Search for foods", code: `import requests -API_KEY = "YOUR_API_KEY_HERE" # Replace with your actual API key +API_KEY = "${actualApiKey}" # Your API key from the API Keys page BASE_URL = "https://kalori-api.my" ...This provides a more seamless experience while keeping the examples safe (showing only the key prefix).
packages/kal-frontend/src/app/dashboard/api-keys/client.tsx (2)
64-75: Add error handling to TRPC mutations.The mutations lack
onErrorhandlers, which could leave users without feedback if the operation fails.Recommended implementation
const generateMutation = trpc.apiKeys.generate.useMutation({ onSuccess: (data) => { setGeneratedKey(data.key); refetchKeys(); }, + onError: (error) => { + alert(`Failed to generate API key: ${error.message}`); + }, }); const revokeMutation = trpc.apiKeys.revoke.useMutation({ onSuccess: () => { refetchKeys(); }, + onError: (error) => { + alert(`Failed to revoke API key: ${error.message}`); + }, });
101-105: Consider using a custom modal instead of window.confirm.The
window.confirmdialog is not customizable and doesn't match the app's design system. Consider using a custom modal component for consistency.You could create a reusable confirmation modal component similar to the generate key modal pattern already used in this file.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
packages/kal-frontend/package.jsonpackages/kal-frontend/src/app/api-docs/client.tsxpackages/kal-frontend/src/app/dashboard/api-keys/client.tsxpackages/kal-frontend/src/app/dashboard/api-keys/page.tsxpackages/kal-frontend/src/app/dashboard/client.tsxpackages/kal-frontend/src/app/dashboard/layout.tsxpackages/kal-frontend/src/app/dashboard/page.tsxpackages/kal-frontend/src/app/dashboard/settings/client.tsxpackages/kal-frontend/src/app/dashboard/settings/page.tsxpackages/kal-frontend/src/app/dashboard/setup/client.tsxpackages/kal-frontend/src/app/dashboard/setup/page.tsxpackages/kal-frontend/src/app/search/page.tsxpackages/kal-frontend/src/components/dashboard/Sidebar.tsxpackages/kal-frontend/src/components/landing/Features.tsxpackages/kal-frontend/src/components/landing/HowItWorks.tsxpackages/kal-frontend/src/components/landing/Navbar.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
packages/kal-frontend/src/app/dashboard/settings/page.tsx (5)
packages/kal-frontend/src/app/dashboard/api-keys/page.tsx (1)
metadata(8-11)packages/kal-frontend/src/app/dashboard/page.tsx (1)
metadata(8-11)packages/kal-frontend/src/app/dashboard/setup/page.tsx (1)
metadata(8-11)packages/kal-frontend/src/lib/logto.ts (1)
getLogtoConfig(17-32)packages/kal-frontend/src/app/dashboard/settings/client.tsx (1)
SettingsClient(15-22)
packages/kal-frontend/src/app/dashboard/setup/client.tsx (2)
packages/kal-frontend/src/lib/auth-context.tsx (2)
AuthUpdater(50-68)useAuth(19-21)packages/kal-frontend/src/lib/trpc.ts (1)
trpc(5-5)
packages/kal-frontend/src/app/dashboard/page.tsx (1)
packages/kal-frontend/src/app/dashboard/client.tsx (1)
DashboardClient(16-23)
packages/kal-frontend/src/app/dashboard/layout.tsx (3)
packages/kal-db/migrate-mongo-config.js (1)
config(33-56)packages/kal-frontend/src/lib/logto.ts (1)
getLogtoConfig(17-32)packages/kal-frontend/src/components/dashboard/Sidebar.tsx (1)
DashboardLayout(115-137)
packages/kal-frontend/src/app/dashboard/setup/page.tsx (1)
packages/kal-frontend/src/lib/logto.ts (1)
getLogtoConfig(17-32)
packages/kal-frontend/src/app/dashboard/settings/client.tsx (3)
packages/kal-frontend/src/lib/auth-context.tsx (2)
AuthUpdater(50-68)useAuth(19-21)packages/kal-frontend/src/lib/trpc.ts (1)
trpc(5-5)packages/kal-shared/src/types/index.ts (2)
RATE_LIMITS(24-28)User(6-14)
⏰ 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: Build
🔇 Additional comments (3)
packages/kal-frontend/src/components/landing/HowItWorks.tsx (1)
1-27: LGTM! Clean icon replacement.The migration from inline SVGs to react-feather icons is well-executed. The icons are appropriately sized and semantically paired with descriptive text.
packages/kal-frontend/src/app/dashboard/api-keys/client.tsx (1)
262-286: Excellent security practice for API key display.The one-time display of the generated API key with a clear warning is a solid security pattern. The copy functionality and visual feedback enhance the user experience.
packages/kal-frontend/package.json (1)
22-22: react-feather 2.0.10 is compatible with React 19.react-feather declares a peerDependency of
"react": ">=16.8.6", which explicitly supports React 19. While the library is unmaintained (last release 2021), this does not prevent it from working with React 19. If type definitions or ongoing maintenance are concerns, consider alternatives likelucide-reactorreact-icons.Likely an incorrect or invalid review comment.
Comment on lines
+85
to
+91
| const handleCopyKey = () => { | ||
| if (generatedKey) { | ||
| navigator.clipboard.writeText(generatedKey); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 2000); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling for clipboard API.
Same issue as in setup/client.tsx: the clipboard API needs error handling for secure contexts and permission failures.
Recommended fix
const handleCopyKey = () => {
if (generatedKey) {
- navigator.clipboard.writeText(generatedKey);
- setCopied(true);
- setTimeout(() => setCopied(false), 2000);
+ navigator.clipboard.writeText(generatedKey)
+ .then(() => {
+ setCopied(true);
+ setTimeout(() => setCopied(false), 2000);
+ })
+ .catch((err) => {
+ console.error('Failed to copy to clipboard:', err);
+ alert('Failed to copy key. Please copy it manually.');
+ });
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const handleCopyKey = () => { | |
| if (generatedKey) { | |
| navigator.clipboard.writeText(generatedKey); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 2000); | |
| } | |
| }; | |
| const handleCopyKey = () => { | |
| if (generatedKey) { | |
| navigator.clipboard.writeText(generatedKey) | |
| .then(() => { | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 2000); | |
| }) | |
| .catch((err) => { | |
| console.error('Failed to copy to clipboard:', err); | |
| alert('Failed to copy key. Please copy it manually.'); | |
| }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In packages/kal-frontend/src/app/dashboard/api-keys/client.tsx around lines 85
to 91, the clipboard write call lacks error handling for non-secure contexts and
permission failures; wrap navigator.clipboard.writeText in an async try/catch
(or Promise.then/.catch), first checking for navigator.clipboard existence and
secure context, only setCopied(true) on successful write, reset after timeout,
and in the catch branch log the error and surface a user-facing fallback (e.g.,
show an error toast or copy the key into a selectable input and focus it) so
failures are handled gracefully.
Comment on lines
+15
to
+24
| if (!isAuthenticated) { | ||
| return ( | ||
| <div className="min-h-screen bg-dark flex items-center justify-center"> | ||
| <div className="text-center"> | ||
| <p className="text-content-secondary mb-4">Please sign in to access the dashboard.</p> | ||
| <a href="/" className="text-accent hover:underline">Go to Home</a> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Unreachable authentication guard — child pages redirect first.
The sign-in prompt rendered here is unreachable because all child dashboard pages (setup, settings, api-keys, and the main dashboard page) perform their own authentication checks and redirect to "/" before this layout component renders. This creates dead code and an inconsistent authentication pattern.
🔎 Recommended fix: centralize authentication in the layout
Remove individual page-level auth checks and redirect here instead:
// Redirect to home if not authenticated
if (!isAuthenticated) {
+ redirect('/');
- return (
- <div className="min-h-screen bg-dark flex items-center justify-center">
- <div className="text-center">
- <p className="text-content-secondary mb-4">Please sign in to access the dashboard.</p>
- <a href="/" className="text-accent hover:underline">Go to Home</a>
- </div>
- </div>
- );
}Then remove the authentication checks from all child pages (setup/page.tsx, settings/page.tsx, api-keys/page.tsx, and page.tsx) since the layout will handle authentication centrally.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/kal-frontend/src/app/dashboard/layout.tsx around lines 15 to 24, the
authentication guard rendering a sign-in prompt is dead code because child pages
perform their own redirects first; centralize auth here by replacing the current
guard with a server/client-side check that redirects unauthenticated users to
"/" (or uses next/navigation redirect) and remove all duplicate auth checks and
redirects from the child pages
(packages/kal-frontend/src/app/dashboard/setup/page.tsx,
packages/kal-frontend/src/app/dashboard/settings/page.tsx,
packages/kal-frontend/src/app/dashboard/api-keys/page.tsx, and
packages/kal-frontend/src/app/dashboard/page.tsx) so the layout alone enforces
authentication for the entire dashboard route.
Comment on lines
+45
to
+49
| const copyToClipboard = (text: string, index: number) => { | ||
| navigator.clipboard.writeText(text); | ||
| setCopiedIndex(index); | ||
| setTimeout(() => setCopiedIndex(null), 2000); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add error handling for clipboard API.
The clipboard API may fail in insecure contexts or if the user denies permission. Add error handling to improve user experience.
Recommended implementation
const copyToClipboard = (text: string, index: number) => {
- navigator.clipboard.writeText(text);
- setCopiedIndex(index);
- setTimeout(() => setCopiedIndex(null), 2000);
+ navigator.clipboard.writeText(text)
+ .then(() => {
+ setCopiedIndex(index);
+ setTimeout(() => setCopiedIndex(null), 2000);
+ })
+ .catch((err) => {
+ console.error('Failed to copy to clipboard:', err);
+ alert('Failed to copy to clipboard. Please copy manually.');
+ });
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const copyToClipboard = (text: string, index: number) => { | |
| navigator.clipboard.writeText(text); | |
| setCopiedIndex(index); | |
| setTimeout(() => setCopiedIndex(null), 2000); | |
| }; | |
| const copyToClipboard = (text: string, index: number) => { | |
| navigator.clipboard.writeText(text) | |
| .then(() => { | |
| setCopiedIndex(index); | |
| setTimeout(() => setCopiedIndex(null), 2000); | |
| }) | |
| .catch((err) => { | |
| console.error('Failed to copy to clipboard:', err); | |
| alert('Failed to copy to clipboard. Please copy manually.'); | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In packages/kal-frontend/src/app/dashboard/setup/client.tsx around lines 45 to
49, the copyToClipboard function calls navigator.clipboard.writeText without
handling rejection; update it to await the writeText Promise inside a try/catch
(or use .then/.catch), only call setCopiedIndex and setTimeout on success, and
in the catch block provide a fallback (e.g., create a hidden textarea, select &
execCommand('copy')) and surface an error to the user (toast or console.error)
so failures are handled gracefully.
Comment on lines
+27
to
+113
| export function Sidebar({ onSignOut }: SidebarProps) { | ||
| const [collapsed, setCollapsed] = useState(false); | ||
| const pathname = usePathname(); | ||
|
|
||
| return ( | ||
| <aside | ||
| className={` | ||
| fixed left-0 top-0 h-full bg-dark-surface border-r border-dark-border | ||
| transition-all duration-300 ease-in-out z-40 | ||
| ${collapsed ? "w-16" : "w-64"} | ||
| `} | ||
| > | ||
| <div className="flex flex-col h-full"> | ||
| {/* Logo */} | ||
| <div className="h-16 flex items-center px-4 border-b border-dark-border"> | ||
| <Link href="/" className="flex items-center gap-2 group"> | ||
| <div className="w-3 h-3 rounded-full bg-accent group-hover:scale-110 transition-transform flex-shrink-0" /> | ||
| {!collapsed && ( | ||
| <span className="text-xl font-bold text-content-primary">Kal</span> | ||
| )} | ||
| </Link> | ||
| </div> | ||
|
|
||
| {/* Navigation */} | ||
| <nav className="flex-1 py-4"> | ||
| {navItems.map((item) => { | ||
| const isActive = pathname === item.href; | ||
| const Icon = item.icon; | ||
|
|
||
| return ( | ||
| <Link | ||
| key={item.href} | ||
| href={item.href} | ||
| className={` | ||
| flex items-center gap-3 px-4 py-3 mx-2 rounded-lg | ||
| transition-all duration-200 | ||
| ${isActive | ||
| ? "bg-accent/10 text-accent border border-accent/30" | ||
| : "text-content-secondary hover:bg-dark-elevated hover:text-content-primary" | ||
| } | ||
| `} | ||
| title={collapsed ? item.label : undefined} | ||
| > | ||
| <Icon size={20} className="flex-shrink-0" /> | ||
| {!collapsed && <span className="font-medium">{item.label}</span>} | ||
| </Link> | ||
| ); | ||
| })} | ||
| </nav> | ||
|
|
||
| {/* Bottom actions */} | ||
| <div className="border-t border-dark-border py-4"> | ||
| {onSignOut && ( | ||
| <button | ||
| onClick={onSignOut} | ||
| className="flex items-center gap-3 px-4 py-3 mx-2 rounded-lg | ||
| text-content-secondary hover:bg-dark-elevated hover:text-red-400 | ||
| transition-all duration-200 w-[calc(100%-1rem)]" | ||
| title={collapsed ? "Sign Out" : undefined} | ||
| > | ||
| <LogOut size={20} className="flex-shrink-0" /> | ||
| {!collapsed && <span className="font-medium">Sign Out</span>} | ||
| </button> | ||
| )} | ||
|
|
||
| {/* Collapse toggle */} | ||
| <button | ||
| onClick={() => setCollapsed(!collapsed)} | ||
| className="flex items-center gap-3 px-4 py-3 mx-2 rounded-lg | ||
| text-content-muted hover:bg-dark-elevated hover:text-content-primary | ||
| transition-all duration-200 w-[calc(100%-1rem)]" | ||
| title={collapsed ? "Expand" : "Collapse"} | ||
| > | ||
| {collapsed ? ( | ||
| <ChevronRight size={20} className="flex-shrink-0" /> | ||
| ) : ( | ||
| <> | ||
| <ChevronLeft size={20} className="flex-shrink-0" /> | ||
| <span className="font-medium">Collapse</span> | ||
| </> | ||
| )} | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </aside> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Significant code duplication between Sidebar and SidebarWithState.
The Sidebar component (lines 27-113) and SidebarWithState component (lines 139-232) contain nearly identical JSX (~90 lines duplicated). The only difference is that Sidebar manages its own collapsed state internally, while SidebarWithState accepts it as a prop. This creates a maintenance burden where UI changes must be applied to both components.
Additionally, DashboardLayout only uses SidebarWithState, which suggests Sidebar may be unused or redundant.
🔎 Proposed refactor to eliminate duplication
Extract the shared sidebar rendering logic into a single internal component:
+function SidebarContent({
+ collapsed,
+ onSignOut,
+ onToggleCollapse
+}: {
+ collapsed: boolean;
+ onSignOut?: () => Promise<void>;
+ onToggleCollapse: () => void;
+}) {
+ const pathname = usePathname();
+
+ return (
+ <aside
+ className={`
+ fixed left-0 top-0 h-full bg-dark-surface border-r border-dark-border
+ transition-all duration-300 ease-in-out z-40
+ ${collapsed ? "w-16" : "w-64"}
+ `}
+ >
+ <div className="flex flex-col h-full">
+ {/* Logo */}
+ <div className="h-16 flex items-center px-4 border-b border-dark-border">
+ <Link href="/" className="flex items-center gap-2 group">
+ <div className="w-3 h-3 rounded-full bg-accent group-hover:scale-110 transition-transform flex-shrink-0" />
+ {!collapsed && (
+ <span className="text-xl font-bold text-content-primary">Kal</span>
+ )}
+ </Link>
+ </div>
+
+ {/* Navigation */}
+ <nav className="flex-1 py-4">
+ {navItems.map((item) => {
+ const isActive = pathname === item.href;
+ const Icon = item.icon;
+
+ return (
+ <Link
+ key={item.href}
+ href={item.href}
+ className={`
+ flex items-center gap-3 px-4 py-3 mx-2 rounded-lg
+ transition-all duration-200
+ ${isActive
+ ? "bg-accent/10 text-accent border border-accent/30"
+ : "text-content-secondary hover:bg-dark-elevated hover:text-content-primary"
+ }
+ `}
+ title={collapsed ? item.label : undefined}
+ >
+ <Icon size={20} className="flex-shrink-0" />
+ {!collapsed && <span className="font-medium">{item.label}</span>}
+ </Link>
+ );
+ })}
+ </nav>
+
+ {/* Bottom actions */}
+ <div className="border-t border-dark-border py-4">
+ {onSignOut && (
+ <button
+ onClick={onSignOut}
+ className="flex items-center gap-3 px-4 py-3 mx-2 rounded-lg
+ text-content-secondary hover:bg-dark-elevated hover:text-red-400
+ transition-all duration-200 w-[calc(100%-1rem)]"
+ title={collapsed ? "Sign Out" : undefined}
+ >
+ <LogOut size={20} className="flex-shrink-0" />
+ {!collapsed && <span className="font-medium">Sign Out</span>}
+ </button>
+ )}
+
+ {/* Collapse toggle */}
+ <button
+ onClick={onToggleCollapse}
+ className="flex items-center gap-3 px-4 py-3 mx-2 rounded-lg
+ text-content-muted hover:bg-dark-elevated hover:text-content-primary
+ transition-all duration-200 w-[calc(100%-1rem)]"
+ title={collapsed ? "Expand" : "Collapse"}
+ >
+ {collapsed ? (
+ <ChevronRight size={20} className="flex-shrink-0" />
+ ) : (
+ <>
+ <ChevronLeft size={20} className="flex-shrink-0" />
+ <span className="font-medium">Collapse</span>
+ </>
+ )}
+ </button>
+ </div>
+ </div>
+ </aside>
+ );
+}
export function Sidebar({ onSignOut }: SidebarProps) {
const [collapsed, setCollapsed] = useState(false);
- const pathname = usePathname();
- return (
- <aside ...>
- {/* 85 lines of JSX */}
- </aside>
- );
+ return (
+ <SidebarContent
+ collapsed={collapsed}
+ onSignOut={onSignOut}
+ onToggleCollapse={() => setCollapsed(!collapsed)}
+ />
+ );
}
export function DashboardLayout({
children,
onSignOut
}: {
children: React.ReactNode;
onSignOut?: () => Promise<void>;
}) {
const [collapsed, setCollapsed] = useState(false);
return (
<div className="min-h-screen bg-dark">
- <SidebarWithState collapsed={collapsed} setCollapsed={setCollapsed} onSignOut={onSignOut} />
+ <SidebarContent
+ collapsed={collapsed}
+ onSignOut={onSignOut}
+ onToggleCollapse={() => setCollapsed(!collapsed)}
+ />
<main
className={`
transition-all duration-300 ease-in-out
${collapsed ? "ml-16" : "ml-64"}
`}
>
{children}
</main>
</div>
);
}
-function SidebarWithState({ ... }) {
- // Remove this entire component (93 lines)
-}This refactoring:
- Eliminates ~90 lines of duplication
- Creates a single source of truth for sidebar rendering
- Makes future UI changes easier to maintain
- Keeps both public APIs (
SidebarandDashboardLayout) unchanged
Also applies to: 139-232
🤖 Prompt for AI Agents
packages/kal-frontend/src/components/dashboard/Sidebar.tsx around lines 27-113
and SidebarWithState at 139-232 contain nearly identical JSX (duplication ~90
lines); extract the shared sidebar rendering into a single internal component
(e.g., SidebarView) that accepts props for collapsed, onToggle, onSignOut,
pathname, and navItems, then have Sidebar keep its internal state and render
SidebarView, and have SidebarWithState forward its props to SidebarView; update
imports/exports accordingly and remove duplicated JSX so both components share
the same rendering logic while preserving existing public APIs.
This was referenced Dec 26, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📝 Description
Brief description of what this PR does.
🔗 Related Issue
Fixes #(issue number)
🏷️ Type of Change
✅ Checklist
dev(notmain)pnpm lint:fixpnpm typecheck📸 Screenshots (if applicable)
Add screenshots to help explain your changes.
🧪 How to Test
Steps to test this PR:
📝 Additional Notes
Any additional information reviewers should know.
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.