Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdded a new async DashboardLayout that enforces auth and mounts the sidebar; removed DashboardShell wrappers from multiple pages; converted LoginPage to async; introduced a comprehensive sidebar system, navigation components, many UI primitives, hooks, and several new dashboard pages/components. Changes
Sequence DiagramsequenceDiagram
actor User
participant Browser
participant DashboardLayout
participant getCurrentSession
participant Router
participant SidebarProvider
participant AppSidebar
User->>Browser: Request /dashboard/*
Browser->>DashboardLayout: render layout
DashboardLayout->>getCurrentSession: getCurrentSession()
alt no session
getCurrentSession-->>DashboardLayout: null
DashboardLayout->>Router: redirect('/login')
Router-->>User: Redirect to login
else session exists
getCurrentSession-->>DashboardLayout: session { userEmail, userRole }
DashboardLayout->>SidebarProvider: render with session
SidebarProvider->>AppSidebar: mount / provide state
AppSidebar->>AppSidebar: compute active section from pathname
AppSidebar-->>SidebarProvider: render nav UI
SidebarProvider-->>Browser: render children inside SidebarInset
Browser-->>User: Dashboard + Sidebar displayed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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📝 Generate docstrings
🧪 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: 7
🧹 Nitpick comments (15)
apps/dashboard/app/(dashboard)/knowledge/page.tsx (2)
1-1: Remove empty line at start of file.Line 1 appears to be an empty line that was added. Consider removing it for cleaner formatting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/app/`(dashboard)/knowledge/page.tsx at line 1, Remove the stray empty first line in apps/dashboard/app/(dashboard)/knowledge/page.tsx so the file starts directly with its module content (e.g., the import statements or the Page component/default export); simply delete the blank line at the top of the file to restore clean formatting and avoid a leading newline before the file's imports/exports.
27-27: Unnecessary React fragment wrapper.Since there's only a single
<main>child element, the fragment wrapper (<>...</>) is unnecessary and can be removed.♻️ Simplified return
return ( - <> - <main className="flex-1 p-6 lg:p-8"> + <main className="flex-1 p-6 lg:p-8"> ... - </main> - </> + </main> );Also applies to: 82-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/app/`(dashboard)/knowledge/page.tsx at line 27, The component's JSX return uses an unnecessary React fragment wrapper around the single <main> element; remove the surrounding <>...</> so the <main> element is returned directly (apply the same change for the second occurrence noted near the other return at the location referenced), updating the component's return in page.tsx to have <main> as the top-level JSX node instead of being wrapped by a fragment.apps/dashboard/app/(dashboard)/conversations/page.tsx (2)
45-45: Unnecessary React fragment wrapper.Same as the knowledge page—the fragment can be removed since there's only a single
<div>child.Also applies to: 100-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/app/`(dashboard)/conversations/page.tsx at line 45, Remove the unnecessary React fragment wrappers surrounding the single top-level <div> in the component's JSX (the <> ... </> at the start and the matching closing fragment later around line range noted). Replace the fragment by returning the <div> directly so the component's render/return contains only the single <div> element (no <> or </>), and do the same for the other occurrence flagged around the 100-100 range.
22-23: Remove debug console.log statements.These logging statements appear to be debug artifacts that should be removed before merging:
- Line 22:
console.log('API URL:', ...)- Line 23:
console.log('Fetching conversations...')- Line 30:
console.log('Response status:', ...)♻️ Remove debug statements
const apiUrl = process.env.API_URL || process.env.NEXT_PUBLIC_API_URL || 'http://localhost:3001'; - console.log('API URL:', process.env.NEXT_PUBLIC_API_URL); - console.log('Fetching conversations...'); const res = await fetch(`${apiUrl}/api/conversations`, { headers: { Authorization: `Bearer ${session.access_token}`, }, cache: 'no-store', }); - console.log('Response status:', res.status); if (!res.ok) {Also applies to: 30-30
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/app/`(dashboard)/conversations/page.tsx around lines 22 - 23, Remove the debug console.log calls from the Conversations page: delete the console.log('API URL:', process.env.NEXT_PUBLIC_API_URL), console.log('Fetching conversations...'), and console.log('Response status:', ...) statements in the page component/file; if you still need runtime visibility, replace them with a proper logger or debug utility (e.g., processLogger.debug or a debug flag) rather than console.log so production builds won’t leak debug output.apps/dashboard/components/ui/sheet.tsx (1)
25-29:SheetPortalis exported but used internally bySheetContent.
SheetPortalis exported (line 143) butSheetContentalready wraps content inSheetPortalinternally (line 58). Consumers typically won't need to useSheetPortaldirectly. Consider either:
- Not exporting
SheetPortalsince it's an implementation detail- Documenting when direct
SheetPortalusage would be neededThis is a minor API surface concern.
Also applies to: 134-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/components/ui/sheet.tsx` around lines 25 - 29, SheetPortal is currently exported but only used internally by SheetContent (which already wraps content in SheetPortal); remove the public export of SheetPortal so it remains an internal implementation detail (keep the component function named SheetPortal but do not export it) and update any references to import SheetPortal elsewhere if present, ensuring SheetContent continues to render via SheetPrimitive.Portal; alternatively, if you prefer to keep it exported, add clear documentation on when consumers should use SheetPortal directly and why it is normally unnecessary.apps/dashboard/app/login/page.tsx (1)
20-22: Consider usingnext/fontfor optimized font loading.The inline
@importfor Google Fonts causes render-blocking and potential flash of unstyled content. Next.js provides built-in font optimization vianext/font/google.♻️ Suggested improvement
// At the top of the file or in a layout import { Inter } from 'next/font/google' const inter = Inter({ subsets: ['latin'], weight: ['400', '700'] }) // Then use: className={inter.className} or style={inter.style}This eliminates the inline style tag and provides automatic font optimization.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/app/login/page.tsx` around lines 20 - 22, Replace the inline `@import` CSS with Next's font optimizer: import Inter from 'next/font/google', instantiate it (e.g., const inter = Inter({ subsets: ['latin'], weight: ['400','700'] })), remove the <style> tag that injects the Google Fonts URL, and apply the optimized font via the returned inter.className or inter.style on your page component's root element (the default export in page.tsx) so the font is loaded non-blockingly.apps/dashboard/components/nav-user.tsx (1)
55-55: Avatar fallback is hardcoded.The fallback
"CN"doesn't reflect the actual user. Consider deriving initials from the user's name or email.💡 Dynamic fallback suggestion
- <AvatarFallback className="rounded-lg">CN</AvatarFallback> + <AvatarFallback className="rounded-lg"> + {user.name?.slice(0, 2).toUpperCase() || user.email?.slice(0, 2).toUpperCase() || "?"} + </AvatarFallback>Also applies to: 74-74
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/components/nav-user.tsx` at line 55, The AvatarFallback currently renders the hardcoded string "CN"; update the NavUser component to compute and render dynamic initials instead: create or use a small helper (e.g., getInitials) that derives initials from user.name or falls back to user.email, handle missing/empty values gracefully (return a default like '?'), and replace the static "CN" in both AvatarFallback usages with the computed initials so the avatar reflects the actual user.apps/dashboard/app/(dashboard)/dashboard/page.tsx (1)
10-12: Redundant authentication check.The parent
layout.tsxalready performsgetCurrentSession()and redirects to/loginif no session exists. This check will never trigger since the layout guard executes first.Consider removing this redundant check to simplify the page component, or keep it as defense-in-depth if you prefer explicit guards per page.
♻️ Optional simplification
export default async function DashboardHome() { const session = await getCurrentSession(); - - if (!session) { - redirect('/login'); - } + // Session is guaranteed by layout.tsx, but we need the session data + if (!session) return null; return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/app/`(dashboard)/dashboard/page.tsx around lines 10 - 12, Remove the redundant guard in the page component: the if (!session) { redirect('/login'); } block is unnecessary because getCurrentSession() and the redirect are already handled in the parent layout (getCurrentSession). Delete this conditional (or, if you prefer defense-in-depth, replace it with a short comment noting the layout already enforces auth) so the page no longer performs duplicate redirects; locate the check by searching for the session variable and the redirect('/login') call inside page.tsx.apps/dashboard/components/nav-projects.tsx (2)
44-44: Consider using a unique identifier for the key.Using
item.nameas the React key could cause issues if project names aren't unique. If the data model includes anidfield, prefer that.💡 Suggested change if `id` is available
- <SidebarMenuItem key={item.name}> + <SidebarMenuItem key={item.id}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/components/nav-projects.tsx` at line 44, Replace the unstable React key usage: currently SidebarMenuItem uses item.name as the key which can collide; instead use the unique identifier field from the project object (e.g., item.id) when rendering the list in the map that returns <SidebarMenuItem>, so change the key prop from item.name to item.id (or a guaranteed-unique value like `${item.id}`) and fall back to a stable unique alternative only if id is not present.
63-75: Menu actions are placeholders without handlers.The dropdown menu items (View Project, Share Project, Delete Project) don't have
onClickhandlers. This is fine for initial UI scaffolding, but ensure these are wired up before the feature is complete.Would you like me to help implement the action handlers or create an issue to track this?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/components/nav-projects.tsx` around lines 63 - 75, The dropdown menu items in nav-projects.tsx are placeholders without click handlers; add explicit handlers (e.g., onViewProject, onShareProject, onDeleteProject) and wire them to each DropdownMenuItem via onClick/onSelect so actions are executed with the current project context (pass project id or object from props/state). Implement the handler functions in the same component (or lift them to a parent and accept callbacks) to perform the intended behavior (navigate/open modal/call API) and ensure you handle confirmation for deletion (e.g., show confirm modal before calling delete). Update the DropdownMenuItem elements to call the appropriate handler names so functionality is testable and traceable.apps/dashboard/app/(dashboard)/conversations/[id]/page.tsx (1)
17-18: Consider removing debug console.log statements.Multiple debug logging statements are present throughout the data fetching functions. These should be removed or replaced with proper structured logging before production.
Also applies to: 28-28, 33-33, 43-44, 57-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/app/`(dashboard)/conversations/[id]/page.tsx around lines 17 - 18, Remove the debug console.log statements in apps/dashboard/app/(dashboard)/conversations/[id]/page.tsx (e.g. the logs with 'API URL:' and 'Fetching conversation timeline...' and the other console.log lines at the indicated ranges) and either delete them or replace them with the application's structured logger at an appropriate level (e.g., logger.debug or processLogger.debug) so production code does not emit raw console output; update any related data-fetching functions that emit console.log to use the logger API instead and ensure sensitive values are not logged.apps/dashboard/app/(dashboard)/escalations/page.tsx (1)
20-21: Consider removing debug console.log statements.These logging statements appear to be debugging artifacts. In production, they could leak internal API URLs and clutter logs.
🧹 Suggested cleanup
- console.log('API URL:', process.env.NEXT_PUBLIC_API_URL); - console.log('Fetching escalations...'); const res = await fetch(`${apiUrl}/api/conversations/escalations`, { headers: { Authorization: `Bearer ${session.access_token}`, }, cache: 'no-store', }); - console.log('Response status:', res.status);Also applies to: 28-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/app/`(dashboard)/escalations/page.tsx around lines 20 - 21, Remove the debug console.log statements that print the API URL and "Fetching escalations..." from apps/dashboard/app/(dashboard)/escalations/page.tsx; locate the top-level or component code where console.log('API URL:', process.env.NEXT_PUBLIC_API_URL) and console.log('Fetching escalations...') are called and delete those lines (or replace with a conditional debug logger behind a NODE_ENV check) to avoid leaking internal URLs and cluttering production logs.apps/dashboard/components/nav-main.tsx (1)
44-47: Use Next.jsLinkcomponent for client-side navigation.Using plain
<a href>tags causes full page reloads, bypassing Next.js client-side routing. This degrades performance and UX. Theapp-sidebar.tsxfile correctly usesnext/linkfor the same navigation pattern.♻️ Proposed fix
+"use client" + +import Link from "next/link" import { ChevronRight, type LucideIcon } from "lucide-react" ... <SidebarMenuButton asChild tooltip={item.title}> - <a href={item.url}> + <Link href={item.url}> <item.icon /> <span>{item.title}</span> - </a> + </Link> </SidebarMenuButton> ... <SidebarMenuSubButton asChild> - <a href={subItem.url}> + <Link href={subItem.url}> <span>{subItem.title}</span> - </a> + </Link> </SidebarMenuSubButton>Also applies to: 62-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/components/nav-main.tsx` around lines 44 - 47, Replace the plain anchor-based navigation with Next.js client-side routing: import Link from 'next/link' and replace each <a href={item.url}>…</a> (the occurrences rendering item.icon and <span>{item.title}</span>, e.g., in the nav rendering code that iterates over item) with <Link href={item.url}>…</Link>, keeping the same children (item.icon and span) and any classes/props; apply the same change to the second occurrence around lines 62–64 so all menu items use Link for client-side navigation.apps/dashboard/components/app-sidebar.tsx (1)
72-72: Remove unusedstatevariable.The
statevariable is destructured fromuseSidebar()but never used. This was flagged by the linter.🧹 Proposed fix
const pathname = usePathname(); - const { state } = useSidebar();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/components/app-sidebar.tsx` at line 72, Remove the unused `state` destructuring from the `useSidebar()` call: either delete the entire `const { state } = useSidebar();` line if the hook is not used elsewhere, or if you still need to invoke the hook for side effects, call it without destructuring (e.g., `useSidebar();`). Ensure you only keep references to symbols `useSidebar` and `state` as needed so the linter warning is resolved.apps/dashboard/components/nav-secondary.tsx (1)
29-32: Use Next.jsLinkcomponent for client-side navigation.Same issue as
nav-main.tsx- using plain<a href>tags causes full page reloads instead of client-side navigation.♻️ Proposed fix
+import Link from "next/link" import * as React from "react" ... <SidebarMenuButton asChild size="sm"> - <a href={item.url}> + <Link href={item.url}> <item.icon /> <span>{item.title}</span> - </a> + </Link> </SidebarMenuButton>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/components/nav-secondary.tsx` around lines 29 - 32, Replace the plain anchor with Next.js client-side navigation: import Link from 'next/link' at the top and change the <a href={item.url}>...</a> to use <Link href={item.url}> with the same children (keeping <item.icon /> and <span>{item.title}</span>) so navigation uses Next's Link component instead of a full page reload; ensure the usage matches your Next version (wrap children with an <a> if using legacy behavior or pass className directly to Link if using the newer API).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/app/`(dashboard)/layout.tsx:
- Line 19: The fallback role passed to AppSidebar uses an invalid AppRole
string; change the default in the call AppSidebar userRole={session.role ||
'operator'} to one of the valid AppRole values ('admin', 'support_agent', or
'analyst')—for example replace 'operator' with 'support_agent' (or pick the most
appropriate role for the UI) so that the session.role || <valid AppRole>
expression conforms to the AppRole type expected by AppSidebar.
In `@apps/dashboard/components/nav-user.tsx`:
- Around line 105-108: The "Log out" DropdownMenuItem currently has no action;
wire it to the existing server action handleSignOut by accepting handleSignOut
as a prop in the NavUser component and wrapping the DropdownMenuItem in a form
that posts to that action (use <form action={handleSignOut}> and set
DropdownMenuItem asChild so the inner button submits). Ensure the inner element
is a button type="submit" and preserves the LogOut icon and label so clicking
"Log out" triggers handleSignOut.
- Around line 33-41: The NavUser component is receiving a prop named "name" that
actually contains the user's role (causing a semantic mismatch); update the
contract so the prop is explicit: either change NavUser's prop signature to
expect "role" instead of "name" (and update all callers such as AppSidebar to
pass userRole into NavUser), or modify AppSidebar to pass the actual user name
into NavUser; locate the NavUser function and the AppSidebar call site and make
the names consistent (NavUser and AppSidebar) so the displayed label matches the
data (role vs name).
In `@apps/dashboard/components/ui/breadcrumb.tsx`:
- Around line 83-99: BreadcrumbEllipsis currently sets aria-hidden="true" on the
wrapper which prevents the inner sr-only text from being announced; decide
whether the ellipsis is decorative or interactive and fix accordingly: if
decorative, remove the inner <span className="sr-only">More</span> (and keep
aria-hidden on the wrapper) so no hidden text is useless; if it should be
accessible (e.g., clickable), remove aria-hidden from the outer span and ensure
the component (BreadcrumbEllipsis) exposes an appropriate accessible name/role
(e.g., include the sr-only text or aria-label) and keyboard/focus handling for
MoreHorizontal.
In `@apps/dashboard/components/ui/collapsible.tsx`:
- Around line 1-9: The component uses the React namespace for the type
React.ComponentProps but doesn't import React; add an import for React (e.g.,
import * as React from "react" or import React from "react") at the top of the
file so the type reference in the Collapsible component signature
(React.ComponentProps<typeof CollapsiblePrimitive.Root>) resolves correctly;
keep the rest of the Collapsible function and usage of CollapsiblePrimitive.Root
unchanged.
In `@apps/dashboard/components/ui/dropdown-menu.tsx`:
- Around line 76-78: The Tailwind selector string inside the className call in
dropdown-menu.tsx contains an invalid child selector
`data-[variant=destructive]:*:[svg]:text-destructive!`; update that token to use
the same descendant pattern as the other SVG selectors (i.e., replace the
malformed `*:[svg]` usage with `[&_svg]`) so the segment reads the destructive
variant applies to descendant svg elements (look for the className passed to
cn(...) in the DropdownMenu item/component and change the selector there).
In `@apps/dashboard/components/ui/skeleton.tsx`:
- Around line 1-3: Add the missing React type import used by the Skeleton
component: either import React (e.g., import React from "react") so
React.ComponentProps<"div"> resolves, or import the type directly (e.g., import
type { ComponentProps } from "react") and change the prop type to
ComponentProps<"div">; update the Skeleton function signature accordingly to
remove the unresolved React namespace.
---
Nitpick comments:
In `@apps/dashboard/app/`(dashboard)/conversations/[id]/page.tsx:
- Around line 17-18: Remove the debug console.log statements in
apps/dashboard/app/(dashboard)/conversations/[id]/page.tsx (e.g. the logs with
'API URL:' and 'Fetching conversation timeline...' and the other console.log
lines at the indicated ranges) and either delete them or replace them with the
application's structured logger at an appropriate level (e.g., logger.debug or
processLogger.debug) so production code does not emit raw console output; update
any related data-fetching functions that emit console.log to use the logger API
instead and ensure sensitive values are not logged.
In `@apps/dashboard/app/`(dashboard)/conversations/page.tsx:
- Line 45: Remove the unnecessary React fragment wrappers surrounding the single
top-level <div> in the component's JSX (the <> ... </> at the start and the
matching closing fragment later around line range noted). Replace the fragment
by returning the <div> directly so the component's render/return contains only
the single <div> element (no <> or </>), and do the same for the other
occurrence flagged around the 100-100 range.
- Around line 22-23: Remove the debug console.log calls from the Conversations
page: delete the console.log('API URL:', process.env.NEXT_PUBLIC_API_URL),
console.log('Fetching conversations...'), and console.log('Response status:',
...) statements in the page component/file; if you still need runtime
visibility, replace them with a proper logger or debug utility (e.g.,
processLogger.debug or a debug flag) rather than console.log so production
builds won’t leak debug output.
In `@apps/dashboard/app/`(dashboard)/dashboard/page.tsx:
- Around line 10-12: Remove the redundant guard in the page component: the if
(!session) { redirect('/login'); } block is unnecessary because
getCurrentSession() and the redirect are already handled in the parent layout
(getCurrentSession). Delete this conditional (or, if you prefer
defense-in-depth, replace it with a short comment noting the layout already
enforces auth) so the page no longer performs duplicate redirects; locate the
check by searching for the session variable and the redirect('/login') call
inside page.tsx.
In `@apps/dashboard/app/`(dashboard)/escalations/page.tsx:
- Around line 20-21: Remove the debug console.log statements that print the API
URL and "Fetching escalations..." from
apps/dashboard/app/(dashboard)/escalations/page.tsx; locate the top-level or
component code where console.log('API URL:', process.env.NEXT_PUBLIC_API_URL)
and console.log('Fetching escalations...') are called and delete those lines (or
replace with a conditional debug logger behind a NODE_ENV check) to avoid
leaking internal URLs and cluttering production logs.
In `@apps/dashboard/app/`(dashboard)/knowledge/page.tsx:
- Line 1: Remove the stray empty first line in
apps/dashboard/app/(dashboard)/knowledge/page.tsx so the file starts directly
with its module content (e.g., the import statements or the Page
component/default export); simply delete the blank line at the top of the file
to restore clean formatting and avoid a leading newline before the file's
imports/exports.
- Line 27: The component's JSX return uses an unnecessary React fragment wrapper
around the single <main> element; remove the surrounding <>...</> so the <main>
element is returned directly (apply the same change for the second occurrence
noted near the other return at the location referenced), updating the
component's return in page.tsx to have <main> as the top-level JSX node instead
of being wrapped by a fragment.
In `@apps/dashboard/app/login/page.tsx`:
- Around line 20-22: Replace the inline `@import` CSS with Next's font optimizer:
import Inter from 'next/font/google', instantiate it (e.g., const inter =
Inter({ subsets: ['latin'], weight: ['400','700'] })), remove the <style> tag
that injects the Google Fonts URL, and apply the optimized font via the returned
inter.className or inter.style on your page component's root element (the
default export in page.tsx) so the font is loaded non-blockingly.
In `@apps/dashboard/components/app-sidebar.tsx`:
- Line 72: Remove the unused `state` destructuring from the `useSidebar()` call:
either delete the entire `const { state } = useSidebar();` line if the hook is
not used elsewhere, or if you still need to invoke the hook for side effects,
call it without destructuring (e.g., `useSidebar();`). Ensure you only keep
references to symbols `useSidebar` and `state` as needed so the linter warning
is resolved.
In `@apps/dashboard/components/nav-main.tsx`:
- Around line 44-47: Replace the plain anchor-based navigation with Next.js
client-side routing: import Link from 'next/link' and replace each <a
href={item.url}>…</a> (the occurrences rendering item.icon and
<span>{item.title}</span>, e.g., in the nav rendering code that iterates over
item) with <Link href={item.url}>…</Link>, keeping the same children (item.icon
and span) and any classes/props; apply the same change to the second occurrence
around lines 62–64 so all menu items use Link for client-side navigation.
In `@apps/dashboard/components/nav-projects.tsx`:
- Line 44: Replace the unstable React key usage: currently SidebarMenuItem uses
item.name as the key which can collide; instead use the unique identifier field
from the project object (e.g., item.id) when rendering the list in the map that
returns <SidebarMenuItem>, so change the key prop from item.name to item.id (or
a guaranteed-unique value like `${item.id}`) and fall back to a stable unique
alternative only if id is not present.
- Around line 63-75: The dropdown menu items in nav-projects.tsx are
placeholders without click handlers; add explicit handlers (e.g., onViewProject,
onShareProject, onDeleteProject) and wire them to each DropdownMenuItem via
onClick/onSelect so actions are executed with the current project context (pass
project id or object from props/state). Implement the handler functions in the
same component (or lift them to a parent and accept callbacks) to perform the
intended behavior (navigate/open modal/call API) and ensure you handle
confirmation for deletion (e.g., show confirm modal before calling delete).
Update the DropdownMenuItem elements to call the appropriate handler names so
functionality is testable and traceable.
In `@apps/dashboard/components/nav-secondary.tsx`:
- Around line 29-32: Replace the plain anchor with Next.js client-side
navigation: import Link from 'next/link' at the top and change the <a
href={item.url}>...</a> to use <Link href={item.url}> with the same children
(keeping <item.icon /> and <span>{item.title}</span>) so navigation uses Next's
Link component instead of a full page reload; ensure the usage matches your Next
version (wrap children with an <a> if using legacy behavior or pass className
directly to Link if using the newer API).
In `@apps/dashboard/components/nav-user.tsx`:
- Line 55: The AvatarFallback currently renders the hardcoded string "CN";
update the NavUser component to compute and render dynamic initials instead:
create or use a small helper (e.g., getInitials) that derives initials from
user.name or falls back to user.email, handle missing/empty values gracefully
(return a default like '?'), and replace the static "CN" in both AvatarFallback
usages with the computed initials so the avatar reflects the actual user.
In `@apps/dashboard/components/ui/sheet.tsx`:
- Around line 25-29: SheetPortal is currently exported but only used internally
by SheetContent (which already wraps content in SheetPortal); remove the public
export of SheetPortal so it remains an internal implementation detail (keep the
component function named SheetPortal but do not export it) and update any
references to import SheetPortal elsewhere if present, ensuring SheetContent
continues to render via SheetPrimitive.Portal; alternatively, if you prefer to
keep it exported, add clear documentation on when consumers should use
SheetPortal directly and why it is normally unnecessary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce6ff062-4c73-4f2c-8a23-04c2f870b75c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (34)
apps/dashboard/app/(dashboard)/conversations/[id]/ConversationActions.tsxapps/dashboard/app/(dashboard)/conversations/[id]/page.tsxapps/dashboard/app/(dashboard)/conversations/actions.tsapps/dashboard/app/(dashboard)/conversations/page.tsxapps/dashboard/app/(dashboard)/dashboard/page.tsxapps/dashboard/app/(dashboard)/escalations/EscalationRow.tsxapps/dashboard/app/(dashboard)/escalations/actions.tsapps/dashboard/app/(dashboard)/escalations/page.tsxapps/dashboard/app/(dashboard)/knowledge/actions.tsapps/dashboard/app/(dashboard)/knowledge/components/delete-button.tsxapps/dashboard/app/(dashboard)/knowledge/components/knowledge-form.tsxapps/dashboard/app/(dashboard)/knowledge/page.tsxapps/dashboard/app/(dashboard)/layout.tsxapps/dashboard/app/(dashboard)/use-cases/[slug]/page.tsxapps/dashboard/app/(dashboard)/use-cases/[slug]/use-case.cssapps/dashboard/app/login/page.tsxapps/dashboard/components/app-sidebar.tsxapps/dashboard/components/nav-main.tsxapps/dashboard/components/nav-projects.tsxapps/dashboard/components/nav-secondary.tsxapps/dashboard/components/nav-user.tsxapps/dashboard/components/ui/avatar.tsxapps/dashboard/components/ui/breadcrumb.tsxapps/dashboard/components/ui/collapsible.tsxapps/dashboard/components/ui/dropdown-menu.tsxapps/dashboard/components/ui/input.tsxapps/dashboard/components/ui/label.tsxapps/dashboard/components/ui/separator.tsxapps/dashboard/components/ui/sheet.tsxapps/dashboard/components/ui/sidebar.tsxapps/dashboard/components/ui/skeleton.tsxapps/dashboard/components/ui/switch.tsxapps/dashboard/components/ui/tooltip.tsxapps/dashboard/hooks/use-mobile.ts
|
|
||
| return ( | ||
| <SidebarProvider> | ||
| <AppSidebar userEmail={session.email || ''} userRole={session.role || 'operator'} /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for AppRole type definition to verify valid values
rg -n "AppRole" --type ts -A5 | head -50Repository: Ryanakml/ChatFuse
Length of output: 2632
🏁 Script executed:
#!/bin/bash
# Find the `@wa-chat/shared` package location
find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "export.*type AppRole" | head -5Repository: Ryanakml/ChatFuse
Length of output: 124
🏁 Script executed:
#!/bin/bash
# Also search for the shared package directory
fd -type d "shared" | head -10Repository: Ryanakml/ChatFuse
Length of output: 231
🏁 Script executed:
#!/bin/bash
# Look for where AppRole might be defined (in packages or lib)
fd -type f -name "*.ts" | xargs grep -l "type AppRole" | grep -E "(shared|types|index)" | head -10Repository: Ryanakml/ChatFuse
Length of output: 231
🏁 Script executed:
#!/bin/bash
# Read the AppRole definition from the shared package
cat -n ./packages/shared/src/index.ts | grep -A 20 "AppRole"Repository: Ryanakml/ChatFuse
Length of output: 470
🏁 Script executed:
#!/bin/bash
# Find the APP_ROLES constant definition
cat -n ./packages/shared/src/index.ts | grep -B 5 -A 10 "APP_ROLES"Repository: Ryanakml/ChatFuse
Length of output: 668
🏁 Script executed:
#!/bin/bash
# Verify the exact code in layout.tsx around line 19
cat -n ./apps/dashboard/app/\(dashboard\)/layout.tsx | sed -n '15,25p'Repository: Ryanakml/ChatFuse
Length of output: 468
Replace 'operator' with a valid AppRole value.
The fallback session.role || 'operator' uses an invalid AppRole. According to the type definition in @wa-chat/shared, AppRole only accepts 'admin', 'support_agent', or 'analyst'. Using 'operator' will cause a type error and potential runtime issues. Choose an appropriate fallback from the valid roles.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/app/`(dashboard)/layout.tsx at line 19, The fallback role
passed to AppSidebar uses an invalid AppRole string; change the default in the
call AppSidebar userRole={session.role || 'operator'} to one of the valid
AppRole values ('admin', 'support_agent', or 'analyst')—for example replace
'operator' with 'support_agent' (or pick the most appropriate role for the UI)
so that the session.role || <valid AppRole> expression conforms to the AppRole
type expected by AppSidebar.
Comment on lines
+33
to
+41
| export function NavUser({ | ||
| user, | ||
| }: { | ||
| user: { | ||
| name: string | ||
| email: string | ||
| avatar: string | ||
| } | ||
| }) { |
There was a problem hiding this comment.
Semantic mismatch: name displays the user's role, not their name.
Based on app-sidebar.tsx, this component receives name: userRole — meaning the "name" field actually contains the user's role (e.g., "admin", "support_agent"). This creates confusing UI where the role appears where a name is expected.
Consider either renaming the prop to role for clarity, or updating AppSidebar to pass the actual user name if available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/components/nav-user.tsx` around lines 33 - 41, The NavUser
component is receiving a prop named "name" that actually contains the user's
role (causing a semantic mismatch); update the contract so the prop is explicit:
either change NavUser's prop signature to expect "role" instead of "name" (and
update all callers such as AppSidebar to pass userRole into NavUser), or modify
AppSidebar to pass the actual user name into NavUser; locate the NavUser
function and the AppSidebar call site and make the names consistent (NavUser and
AppSidebar) so the displayed label matches the data (role vs name).
Comment on lines
+83
to
+99
| function BreadcrumbEllipsis({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<"span">) { | ||
| return ( | ||
| <span | ||
| data-slot="breadcrumb-ellipsis" | ||
| role="presentation" | ||
| aria-hidden="true" | ||
| className={cn("flex size-9 items-center justify-center", className)} | ||
| {...props} | ||
| > | ||
| <MoreHorizontal className="size-4" /> | ||
| <span className="sr-only">More</span> | ||
| </span> | ||
| ) | ||
| } |
There was a problem hiding this comment.
The sr-only text is ineffective inside an aria-hidden container.
The <span className="sr-only">More</span> on line 96 will not be announced by screen readers because the parent element has aria-hidden="true". The sr-only text is effectively negated.
If this ellipsis is purely decorative, remove the sr-only span. If it should be accessible (e.g., as a clickable "show more" element), reconsider the aria-hidden placement.
Suggested fix (if purely decorative)
<span
data-slot="breadcrumb-ellipsis"
role="presentation"
aria-hidden="true"
className={cn("flex size-9 items-center justify-center", className)}
{...props}
>
<MoreHorizontal className="size-4" />
- <span className="sr-only">More</span>
</span>📝 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
| function BreadcrumbEllipsis({ | |
| className, | |
| ...props | |
| }: React.ComponentProps<"span">) { | |
| return ( | |
| <span | |
| data-slot="breadcrumb-ellipsis" | |
| role="presentation" | |
| aria-hidden="true" | |
| className={cn("flex size-9 items-center justify-center", className)} | |
| {...props} | |
| > | |
| <MoreHorizontal className="size-4" /> | |
| <span className="sr-only">More</span> | |
| </span> | |
| ) | |
| } | |
| function BreadcrumbEllipsis({ | |
| className, | |
| ...props | |
| }: React.ComponentProps<"span">) { | |
| return ( | |
| <span | |
| data-slot="breadcrumb-ellipsis" | |
| role="presentation" | |
| aria-hidden="true" | |
| className={cn("flex size-9 items-center justify-center", className)} | |
| {...props} | |
| > | |
| <MoreHorizontal className="size-4" /> | |
| </span> | |
| ) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/components/ui/breadcrumb.tsx` around lines 83 - 99,
BreadcrumbEllipsis currently sets aria-hidden="true" on the wrapper which
prevents the inner sr-only text from being announced; decide whether the
ellipsis is decorative or interactive and fix accordingly: if decorative, remove
the inner <span className="sr-only">More</span> (and keep aria-hidden on the
wrapper) so no hidden text is useless; if it should be accessible (e.g.,
clickable), remove aria-hidden from the outer span and ensure the component
(BreadcrumbEllipsis) exposes an appropriate accessible name/role (e.g., include
the sr-only text or aria-label) and keyboard/focus handling for MoreHorizontal.
Comment on lines
+1
to
+9
| "use client" | ||
|
|
||
| import { Collapsible as CollapsiblePrimitive } from "radix-ui" | ||
|
|
||
| function Collapsible({ | ||
| ...props | ||
| }: React.ComponentProps<typeof CollapsiblePrimitive.Root>) { | ||
| return <CollapsiblePrimitive.Root data-slot="collapsible" {...props} /> | ||
| } |
There was a problem hiding this comment.
Missing React import for React.ComponentProps type.
The file uses React.ComponentProps (lines 7, 13, 24) but doesn't import React. While the JSX transform handles JSX automatically, the React namespace reference still requires an import.
🔧 Proposed fix
"use client"
+import * as React from "react"
import { Collapsible as CollapsiblePrimitive } from "radix-ui"📝 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
| "use client" | |
| import { Collapsible as CollapsiblePrimitive } from "radix-ui" | |
| function Collapsible({ | |
| ...props | |
| }: React.ComponentProps<typeof CollapsiblePrimitive.Root>) { | |
| return <CollapsiblePrimitive.Root data-slot="collapsible" {...props} /> | |
| } | |
| "use client" | |
| import * as React from "react" | |
| import { Collapsible as CollapsiblePrimitive } from "radix-ui" | |
| function Collapsible({ | |
| ...props | |
| }: React.ComponentProps<typeof CollapsiblePrimitive.Root>) { | |
| return <CollapsiblePrimitive.Root data-slot="collapsible" {...props} /> | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/components/ui/collapsible.tsx` around lines 1 - 9, The
component uses the React namespace for the type React.ComponentProps but doesn't
import React; add an import for React (e.g., import * as React from "react" or
import React from "react") at the top of the file so the type reference in the
Collapsible component signature (React.ComponentProps<typeof
CollapsiblePrimitive.Root>) resolves correctly; keep the rest of the Collapsible
function and usage of CollapsiblePrimitive.Root unchanged.
Comment on lines
+76
to
+78
| className={cn( | ||
| "relative flex cursor-default items-center gap-2 rounded-sm px-2 py-1.5 text-sm outline-hidden select-none focus:bg-accent focus:text-accent-foreground data-[disabled]:pointer-events-none data-[disabled]:opacity-50 data-[inset]:pl-8 data-[variant=destructive]:text-destructive data-[variant=destructive]:focus:bg-destructive/10 data-[variant=destructive]:focus:text-destructive dark:data-[variant=destructive]:focus:bg-destructive/20 [&_svg]:pointer-events-none [&_svg]:shrink-0 [&_svg:not([class*='size-'])]:size-4 [&_svg:not([class*='text-'])]:text-muted-foreground data-[variant=destructive]:*:[svg]:text-destructive!", | ||
| className |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if this selector pattern is used elsewhere in the codebase
rg '\*:\[svg\]' --type=tsx --type=ts -C2Repository: Ryanakml/ChatFuse
Length of output: 88
🏁 Script executed:
#!/bin/bash
# First, let's find and examine the file mentioned
echo "=== Examining the file ==="
find . -name "dropdown-menu.tsx" -path "*/dashboard/components/ui/*"
# Then search for the pattern with correct rg syntax
echo ""
echo "=== Searching for *:[svg] pattern ==="
rg '\*:\[svg\]' -C2
# Also search for the file and show relevant lines
echo ""
echo "=== Content around lines 76-78 ==="
find . -name "dropdown-menu.tsx" -path "*/dashboard/components/ui/*" -exec sed -n '70,85p' {} +Repository: Ryanakml/ChatFuse
Length of output: 2039
🏁 Script executed:
#!/bin/bash
# Search for any other instances of the *:[svg] pattern
echo "=== Searching for *:[svg] pattern across codebase ==="
rg '\*:\[svg\]'
# Also verify Tailwind syntax - check similar patterns with correct [&_svg]
echo ""
echo "=== Instances of correct [&_svg] pattern ==="
rg '\[&_svg\]' | head -20Repository: Ryanakml/ChatFuse
Length of output: 3144
Fix malformed SVG child selector syntax.
The selector *:[svg] is invalid Tailwind syntax. Tailwind uses [&_svg] for descendant combinators. This pattern is inconsistent with other SVG selectors in the same file (e.g., [&_svg]:pointer-events-none, [&_svg]:shrink-0).
Replace data-[variant=destructive]:*:[svg]:text-destructive! with data-[variant=destructive]:[&_svg]:text-destructive!.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/components/ui/dropdown-menu.tsx` around lines 76 - 78, The
Tailwind selector string inside the className call in dropdown-menu.tsx contains
an invalid child selector
`data-[variant=destructive]:*:[svg]:text-destructive!`; update that token to use
the same descendant pattern as the other SVG selectors (i.e., replace the
malformed `*:[svg]` usage with `[&_svg]`) so the segment reads the destructive
variant applies to descendant svg elements (look for the className passed to
cn(...) in the DropdownMenu item/component and change the selector there).
Comment on lines
+1
to
+3
| import { cn } from "@/lib/utils" | ||
|
|
||
| function Skeleton({ className, ...props }: React.ComponentProps<"div">) { |
There was a problem hiding this comment.
Missing React import for type reference.
The component uses React.ComponentProps<"div"> but React is not imported. While the new JSX transform (React 17+) doesn't require importing React for JSX syntax, you still need the import when explicitly referencing the React namespace for types.
🐛 Proposed fix
+import * as React from "react"
import { cn } from "@/lib/utils"Alternatively, import the type directly:
+import type { ComponentProps } from "react"
import { cn } from "@/lib/utils"
-function Skeleton({ className, ...props }: React.ComponentProps<"div">) {
+function Skeleton({ className, ...props }: ComponentProps<"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.
Suggested change
| import { cn } from "@/lib/utils" | |
| function Skeleton({ className, ...props }: React.ComponentProps<"div">) { | |
| import * as React from "react" | |
| import { cn } from "@/lib/utils" | |
| function Skeleton({ className, ...props }: React.ComponentProps<"div">) { |
Suggested change
| import { cn } from "@/lib/utils" | |
| function Skeleton({ className, ...props }: React.ComponentProps<"div">) { | |
| import type { ComponentProps } from "react" | |
| import { cn } from "@/lib/utils" | |
| function Skeleton({ className, ...props }: ComponentProps<"div">) { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/components/ui/skeleton.tsx` around lines 1 - 3, Add the
missing React type import used by the Skeleton component: either import React
(e.g., import React from "react") so React.ComponentProps<"div"> resolves, or
import the type directly (e.g., import type { ComponentProps } from "react") and
change the prop type to ComponentProps<"div">; update the Skeleton function
signature accordingly to remove the unresolved React namespace.
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/dashboard/app/(dashboard)/use-cases/page.tsx (3)
10-16: Prefer CSS classes over inline styles for consistency.The brand text uses inline styles while the rest of the component relies on CSS classes. This creates inconsistency and makes future theming harder.
♻️ Suggested refactor
Define a CSS class for the brand styling:
/* in use-case.css or a shared stylesheet */ .use-case-brand-text { font-weight: 700; font-size: 1.5rem; /* 24px */ }Then update the component:
<Link href="/" className="use-case-brand text-decoration-none" aria-label="Chattiphy Home" > - <span style={{ fontWeight: 700, fontSize: 24 }}>Chattiphy</span> + <span className="use-case-brand-text">Chattiphy</span> </Link>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/app/`(dashboard)/use-cases/page.tsx around lines 10 - 16, Replace the inline style on the brand span with a CSS class for consistency: locate the Link with className "use-case-brand text-decoration-none" and the nested span that currently has style={{ fontWeight: 700, fontSize: 24 }}, remove the inline style and add a class like "use-case-brand-text", then define that class in the component stylesheet (e.g., use-case.css or shared CSS) with font-weight: 700 and font-size: 1.5rem (24px) so the branding follows the project's CSS conventions.
5-5: Consider adding page metadata for SEO.Next.js App Router pages can export a
metadataobject orgenerateMetadatafunction for SEO. Since this is a public-facing use cases page, adding metadata would improve discoverability.♻️ Example metadata export
import type { Metadata } from 'next'; export const metadata: Metadata = { title: 'Use Cases | Chattiphy', description: 'Explore how Chattiphy can help your clinic or business.', };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/app/`(dashboard)/use-cases/page.tsx at line 5, Add SEO metadata export for this page: create and export a Metadata object (or generateMetadata function) alongside the existing default export UseCasesIndexPage to provide title and description for search engines; for example export const metadata: Metadata = { title: 'Use Cases | Chattiphy', description: 'Explore how Chattiphy can help your clinic or business.' } (import type { Metadata } from 'next') and place it in the same module as the UseCasesIndexPage component.
3-3: Consider co-locating or extracting shared CSS.Importing CSS from a child route directory (
./[slug]/use-case.css) creates a fragile dependency. If the[slug]route is moved or refactored, this import breaks silently or requires updates in multiple places.Consider either:
- Moving shared styles to the parent
use-cases/directory (e.g.,./use-case.css)- Placing them in a shared styles directory
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/app/`(dashboard)/use-cases/page.tsx at line 3, The current import "import './[slug]/use-case.css';" creates a fragile child-route dependency; move the shared styles file up to the parent use-cases directory (rename to use-case.css) or into a shared styles folder, then update the import in page.tsx to reference the new location (e.g., "import './use-case.css';" or the shared path) and remove or update any other files still importing from "./[slug]/use-case.css" to avoid broken imports when the [slug] route is moved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/dashboard/app/`(dashboard)/use-cases/page.tsx:
- Around line 10-16: Replace the inline style on the brand span with a CSS class
for consistency: locate the Link with className "use-case-brand
text-decoration-none" and the nested span that currently has style={{
fontWeight: 700, fontSize: 24 }}, remove the inline style and add a class like
"use-case-brand-text", then define that class in the component stylesheet (e.g.,
use-case.css or shared CSS) with font-weight: 700 and font-size: 1.5rem (24px)
so the branding follows the project's CSS conventions.
- Line 5: Add SEO metadata export for this page: create and export a Metadata
object (or generateMetadata function) alongside the existing default export
UseCasesIndexPage to provide title and description for search engines; for
example export const metadata: Metadata = { title: 'Use Cases | Chattiphy',
description: 'Explore how Chattiphy can help your clinic or business.' } (import
type { Metadata } from 'next') and place it in the same module as the
UseCasesIndexPage component.
- Line 3: The current import "import './[slug]/use-case.css';" creates a fragile
child-route dependency; move the shared styles file up to the parent use-cases
directory (rename to use-case.css) or into a shared styles folder, then update
the import in page.tsx to reference the new location (e.g., "import
'./use-case.css';" or the shared path) and remove or update any other files
still importing from "./[slug]/use-case.css" to avoid broken imports when the
[slug] route is moved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b493a683-3b14-48ab-9586-b9263df53ec4
📒 Files selected for processing (1)
apps/dashboard/app/(dashboard)/use-cases/page.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/dashboard/components/app-sidebar.tsx (1)
95-95: Drop the unuseduseSidebar()call on Line 95.This subscribes
AppSidebarto context updates without using any sidebar state.Minimal cleanup
- useSidebar();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/components/app-sidebar.tsx` at line 95, AppSidebar currently calls useSidebar() solely to subscribe to context updates but never uses any returned state; remove the unused useSidebar() invocation from the AppSidebar component and also remove the related import if it becomes unused to avoid unnecessary re-renders and unused imports (look for useSidebar in the AppSidebar component and its import statement).apps/dashboard/components/ui/logo-chattiphy.tsx (1)
7-8: Mark this logo SVG as decorative in this usage context.Because the link already has visible text (“WA Chat Ops”), the logo can be hidden from assistive tech to avoid redundant announcements.
Accessibility-focused tweak
- <svg {...props} viewBox="0 0 199.42 41" fill="none" xmlns="http://www.w3.org/2000/svg"> + <svg + {...props} + viewBox="0 0 199.42 41" + fill="none" + xmlns="http://www.w3.org/2000/svg" + aria-hidden="true" + focusable="false" + > <image href="/chattiphy.svg" width="100%" height="100%" /> </svg>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/components/ui/logo-chattiphy.tsx` around lines 7 - 8, The SVG logo in the LogoChattiphy component should be marked decorative so screen readers ignore it; update the <svg> element in the LogoChattiphy component to include aria-hidden="true" and focusable="false" (and ensure there is no accessible title) so assistive tech won't announce the image while the link text ("WA Chat Ops") remains visible; locate the svg usage and add those attributes to the <svg {...props} ...> element (or alternatively set aria-hidden on the inner <image>), keeping the visual behaviour unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/components/app-sidebar.tsx`:
- Around line 98-103: Replace the ad-hoc startsWith logic used when computing
activeItemIndex (and the duplicated logic later) with a single helper that does
segment-boundary matching: create a function (e.g., isRouteMatch or
matchRouteSegment) that takes an item.url and the current pathname and returns
true only when the paths are exact or the pathname begins with item.url followed
by a path separator or query/hash boundary; then use that helper in the
activeItemIndex calculation over navMain and in the other occurrence instead of
directly calling startsWith to avoid false positives for similarly prefixed
routes.
---
Nitpick comments:
In `@apps/dashboard/components/app-sidebar.tsx`:
- Line 95: AppSidebar currently calls useSidebar() solely to subscribe to
context updates but never uses any returned state; remove the unused
useSidebar() invocation from the AppSidebar component and also remove the
related import if it becomes unused to avoid unnecessary re-renders and unused
imports (look for useSidebar in the AppSidebar component and its import
statement).
In `@apps/dashboard/components/ui/logo-chattiphy.tsx`:
- Around line 7-8: The SVG logo in the LogoChattiphy component should be marked
decorative so screen readers ignore it; update the <svg> element in the
LogoChattiphy component to include aria-hidden="true" and focusable="false" (and
ensure there is no accessible title) so assistive tech won't announce the image
while the link text ("WA Chat Ops") remains visible; locate the svg usage and
add those attributes to the <svg {...props} ...> element (or alternatively set
aria-hidden on the inner <image>), keeping the visual behaviour unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b0d419b-e37a-4ca1-bb8c-0b16000e8d89
⛔ Files ignored due to path filters (1)
apps/dashboard/public/chattiphy.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
apps/dashboard/app/(dashboard)/dashboard/page.tsxapps/dashboard/app/(dashboard)/tools/page.tsxapps/dashboard/components/app-sidebar.tsxapps/dashboard/components/nav-user.tsxapps/dashboard/components/ui/logo-chattiphy.tsxapps/dashboard/components/ui/sidebar.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/dashboard/app/(dashboard)/tools/page.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/dashboard/app/(dashboard)/dashboard/page.tsx
- apps/dashboard/components/nav-user.tsx
- apps/dashboard/components/ui/sidebar.tsx
Comment on lines
+98
to
+103
| const activeItemIndex = navMain.findIndex((item) => { | ||
| if (item.url === '/dashboard') { | ||
| return pathname === '/dashboard' || pathname === '/'; | ||
| } | ||
| return pathname.startsWith(item.url); | ||
| }); |
There was a problem hiding this comment.
Harden active-route matching and deduplicate the logic.
startsWith can mis-mark routes with shared prefixes (example: /conversations-archive matching /conversations). A single helper with segment-boundary matching also removes duplication.
Proposed fix
+ const isRouteActive = React.useCallback(
+ (url: string) => {
+ if (url === '/dashboard') return pathname === '/dashboard' || pathname === '/';
+ return pathname === url || pathname.startsWith(`${url}/`);
+ },
+ [pathname]
+ );
+
// Determine active item based on pathname matching
- const activeItemIndex = navMain.findIndex((item) => {
- if (item.url === '/dashboard') {
- return pathname === '/dashboard' || pathname === '/';
- }
- return pathname.startsWith(item.url);
- });
+ const activeItemIndex = navMain.findIndex((item) => isRouteActive(item.url));
const activeItem = (activeItemIndex !== -1 ? navMain[activeItemIndex] : navMain[0])!;
@@
- let isActive = false;
- if (item.url === '/dashboard') {
- isActive = pathname === '/dashboard' || pathname === '/';
- } else {
- isActive = pathname.startsWith(item.url);
- }
+ const isActive = isRouteActive(item.url);Also applies to: 141-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/components/app-sidebar.tsx` around lines 98 - 103, Replace the
ad-hoc startsWith logic used when computing activeItemIndex (and the duplicated
logic later) with a single helper that does segment-boundary matching: create a
function (e.g., isRouteMatch or matchRouteSegment) that takes an item.url and
the current pathname and returns true only when the paths are exact or the
pathname begins with item.url followed by a path separator or query/hash
boundary; then use that helper in the activeItemIndex calculation over navMain
and in the other occurrence instead of directly calling startsWith to avoid
false positives for similarly prefixed routes.
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 by CodeRabbit
New Features
Refactor