Conversation
Co-authored-by: Universe <universe@grida.co>
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
WalkthroughTwo new sidebar components— Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
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 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 113030edf6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <ResourceTypeIcon type="connect" className="size-4" /> | ||
| Connections | ||
| </SidebarMenuLink> | ||
| <SidebarMenuLink href={`${basePath}/ciam`}> |
There was a problem hiding this comment.
With the new usePathname-based highlighting, both CIAM entries now become active on /[org]/[proj]/ciam, so the sidebar shows two highlighted destinations for the same page. This ambiguity is introduced by adding route-aware SidebarMenuLink behavior while keeping duplicate ciam links in both the Customer and Advanced groups, and it undermines the goal of a single clear active navigation state.
Useful? React with 👍 / 👎.
![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
🤖 Fix all issues with AI agents
In
`@editor/app/`(workbench)/[org]/[proj]/(console)/(resources)/console-resources-sidebar.tsx:
- Around line 98-101: There is a duplicated SidebarMenuLink for CIAM (the block
using SidebarMenuLink with ShieldCheckIcon and href `${basePath}/ciam`) present
in both the "Customer" group and the "Advanced" group; remove the redundant
occurrence so only one CIAM SidebarMenuLink remains (either keep it under
"Customer" or under "Advanced") and ensure no other code references the removed
duplicate; search for the SidebarMenuLink + ShieldCheckIcon + href
`${basePath}/ciam` snippet and delete the duplicate block accordingly.
🧹 Nitpick comments (3)
editor/app/(workbench)/[org]/[proj]/(console)/(resources)/console-resources-sidebar.tsx (1)
27-52: Consider reusing existingSidebarMenuLinkfromeditor/components/sidebar/index.tsx.There's an existing
SidebarMenuLinkcomponent ateditor/components/sidebar/index.tsx(lines 154-176) with similar path-matching logic. The implementations differ slightly (layoutprop vsmatchSubpaths, and this version wraps withSidebarMenuItem), but consolidating into a shared component would reduce duplication.If the API differences are intentional, consider extending the existing component or creating a shared base utility for path matching.
editor/app/(workbench)/[org]/[proj]/(console)/(campaign)/campaigns/[campaign]/campaign-sidebar.tsx (2)
37-62: DuplicateSidebarMenuLinkimplementation.This is identical to the
SidebarMenuLinkinconsole-resources-sidebar.tsx. Consider extracting this to a shared location (e.g.,editor/components/sidebar/) to avoid maintaining multiple copies.♻️ Suggested approach
Create a shared component that both sidebars can import:
// editor/components/sidebar/sidebar-menu-link.tsx "use client"; import type { ReactNode } from "react"; import { SidebarMenuItem, SidebarMenuButton } from "@/components/ui/sidebar"; import Link from "next/link"; import { usePathname } from "next/navigation"; type SidebarMenuLinkProps = { href: string; children: ReactNode; size?: "default" | "sm" | "lg"; matchSubpaths?: boolean; }; export function SidebarMenuLink({ href, children, size = "sm", matchSubpaths = true, }: SidebarMenuLinkProps) { const pathname = usePathname(); const isActive = matchSubpaths ? pathname === href || pathname.startsWith(`${href}/`) : pathname === href; return ( <SidebarMenuItem> <SidebarMenuButton asChild size={size} isActive={isActive}> <Link href={href}>{children}</Link> </SidebarMenuButton> </SidebarMenuItem> ); }
89-89: Icon size inconsistency between sidebars.This sidebar uses
size-3for icons whileConsoleResourcesSidebarusessize-4. If intentional (campaign sidebar has denser navigation), this is fine. Otherwise, consider aligning for visual consistency across the application.Also applies to: 93-93, 97-97, 101-101, 105-105, 116-116, 120-120
| <SidebarMenuLink href={`${basePath}/ciam`}> | ||
| <ShieldCheckIcon className="size-4" /> | ||
| CIAM | ||
| </SidebarMenuLink> |
There was a problem hiding this comment.
Duplicate CIAM menu item.
CIAM appears in both the "Customer" group (lines 98-101) and the "Advanced" group (lines 143-146) with the same href. This appears to be an unintentional duplicate.
🐛 Proposed fix: Remove the duplicate from one section
If CIAM belongs under "Advanced", remove it from "Customer":
<SidebarMenu>
<SidebarMenuLink href={`${basePath}/customers`}>
<ResourceTypeIcon type="customer" className="size-4" />
Customers
</SidebarMenuLink>
<SidebarMenuLink href={`${basePath}/tags`}>
<ResourceTypeIcon type="tag" className="size-4" />
Tags
</SidebarMenuLink>
- <SidebarMenuLink href={`${basePath}/ciam`}>
- <ShieldCheckIcon className="size-4" />
- CIAM
- </SidebarMenuLink>
</SidebarMenu>Or if CIAM belongs under "Customer", remove it from "Advanced":
<SidebarGroup>
<SidebarGroupLabel>Advanced</SidebarGroupLabel>
<SidebarGroupContent>
<SidebarMenu>
<SidebarMenuLink href={`${basePath}/integrations`}>
<ResourceTypeIcon type="connect" className="size-4" />
Connections
</SidebarMenuLink>
- <SidebarMenuLink href={`${basePath}/ciam`}>
- <ShieldCheckIcon className="size-4" />
- CIAM
- </SidebarMenuLink>
</SidebarMenu>
</SidebarGroupContent>
</SidebarGroup>Also applies to: 143-146
🤖 Prompt for AI Agents
In
`@editor/app/`(workbench)/[org]/[proj]/(console)/(resources)/console-resources-sidebar.tsx
around lines 98 - 101, There is a duplicated SidebarMenuLink for CIAM (the block
using SidebarMenuLink with ShieldCheckIcon and href `${basePath}/ciam`) present
in both the "Customer" group and the "Advanced" group; remove the redundant
occurrence so only one CIAM SidebarMenuLink remains (either keep it under
"Customer" or under "Advanced") and ensure no other code references the removed
duplicate; search for the SidebarMenuLink + ShieldCheckIcon + href
`${basePath}/ciam` snippet and delete the duplicate block accordingly.
2 participants
Make console sidebars route-aware to highlight the current page.
This PR extracts the console resources and campaign-specific sidebars into dedicated components, implementing route-awareness to highlight the active menu item based on the current URL. The campaign sidebar's "Home" link is specifically configured not to highlight on sub-routes.
Summary by CodeRabbit
Release Notes