Conversation
This commit also fixes broken load more logic
ogzhanolguncu
requested review from
chronark,
mcstepp and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthroughConsolidates nav props into a new exported Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Sidebar as Sidebar (App)
participant NavItems
participant Hook as useProjectNavigation
participant TRPC as TRPC (projects.list)
participant Router as Projects Router
User->>Sidebar: Open sidebar
Sidebar->>Hook: useProjectNavigation(baseNavItems)
Hook->>TRPC: useInfiniteQuery(fetch page, cursor?)
TRPC->>Router: fetch page
Router-->>TRPC: { projects, hasMore, nextCursor|null }
TRPC-->>Hook: pages
Hook-->>Sidebar: enhancedNavItems + loadMore
Sidebar->>NavItems: render with NavProps (items include Projects and "More")
User->>NavItems: Click "More" under Projects
NavItems->>Sidebar: onLoadMore(item with href "#load-more-projects")
Sidebar->>Hook: loadMore()
Hook->>TRPC: fetchNextPage(cursor)
TRPC->>Router: fetch next page
Router-->>TRPC: next page data
TRPC-->>Hook: append page
Hook-->>Sidebar: updated enhancedNavItems
Sidebar-->>User: Updated project list
sequenceDiagram
autonumber
participant NavItems
participant Sidebar as handleLoadMore
participant APIs as useApiNavigation.loadMore
participant Proj as useProjectNavigation.loadMore
participant Rate as useRatelimitNavigation.loadMore
NavItems->>Sidebar: onLoadMore(item)
alt item.href == "#load-more-projects"
Sidebar->>Proj: loadMore()
else item.href == "#load-more-apis"
Sidebar->>APIs: loadMore()
else item.href == "#load-more-ratelimits"
Sidebar->>Rate: loadMore()
else
Sidebar->>Sidebar: console.error("unknown load-more href", item.href)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx (3)
101-111: Use stable keys: prefer subItem.href over label.toString()toString() on ReactNode produces "[object Object]" and leads to duplicate keys. href is unique and stable.
- <SidebarMenuSubItem key={subItem.label?.toString() ?? index}> + <SidebarMenuSubItem key={subItem.href ?? String(index)}>
113-141: Fix pending state map: label can be ReactNode; key by hrefCurrent code uses subItem.label as object keys, which breaks when label is not a string and can cause stuck spinners. Key by subItem.href instead.
- const handleSubItemClick = () => { + const handleSubItemClick = () => { if (isLoadMoreButton && onLoadMore) { onLoadMore(subItem); return; } if (!subItem.external && subItem.href) { // Track loading state for this specific sub-item - const updatedPending = { ...subPending }; - updatedPending[subItem.label as string] = true; - setSubPending(updatedPending); + const key = subItem.href; + const updatedPending = { ...subPending }; + if (key) { + updatedPending[key] = true; + setSubPending(updatedPending); + } // Use a separate transition for sub-items // This prevents parent from showing loader const subItemTransition = () => { router.push(subItem.href); // Reset loading state after transition setTimeout(() => { - const resetPending = { ...subPending }; - resetPending[subItem.label as string] = false; - setSubPending(resetPending); + if (key) { + const resetPending = { ...updatedPending }; + resetPending[key] = false; + setSubPending(resetPending); + } }, 300); }; // Execute transition without affecting parent's isPending state subItemTransition(); } };And propagate this key usage where pending state is read (see next comment).
144-167: Align keys and pending lookups with hrefFinish the refactor by using href for React keys and pending state access.
- <SidebarMenuSubItem key={subItem.label?.toString() ?? index}> + <SidebarMenuSubItem key={subItem.href ?? String(index)}> <NavLink href={subItem.href} external={subItem.external} onClick={handleSubItemClick} isLoadMoreButton={isLoadMoreButton} > <SidebarMenuButton isActive={subItem.active} - className={getButtonStyles(subItem.active, subPending[subItem.label as string])} + className={getButtonStyles(subItem.active, subPending[subItem.href ?? ""])} > {SubIcon ? ( - subPending[subItem.label as string] ? ( + subPending[subItem.href ?? ""] ? ( <AnimatedLoadingSpinner /> ) : ( <SubIcon size="xl-medium" /> ) ) : null} <span className="truncate">{subItem.label}</span> {subItem.tag && <div className="ml-auto">{subItem.tag}</div>} </SidebarMenuButton> </NavLink> </SidebarMenuSubItem>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/flat-nav-item.tsx(2 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx(6 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx(3 hunks)apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx(1 hunks)apps/dashboard/lib/trpc/routers/deploy/project/list.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Use Biome for formatting and linting in TypeScript/JavaScript projects
Prefer named exports over default exports in TypeScript/JavaScript, except for Next.js pages
Files:
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsxapps/dashboard/lib/trpc/routers/deploy/project/list.tsapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/index.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/flat-nav-item.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow strict TypeScript configuration
Use Zod for runtime validation in TypeScript projects
Files:
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsxapps/dashboard/lib/trpc/routers/deploy/project/list.tsapps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/index.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsxapps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/flat-nav-item.tsx
**/*.{env,js,ts,go}
📄 CodeRabbit inference engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
apps/dashboard/lib/trpc/routers/deploy/project/list.ts
🧠 Learnings (2)
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3797
File: apps/dashboard/app/(app)/projects/[projectId]/deployments/components/control-cloud/index.tsx:1-4
Timestamp: 2025-08-18T10:28:47.391Z
Learning: In Next.js App Router, components that use React hooks don't need their own "use client" directive if they are rendered within a client component that already has the directive. The client boundary propagates to child components.
Applied to files:
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Applied to files:
apps/dashboard/lib/trpc/routers/deploy/project/list.ts
🧬 Code graph analysis (5)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx (1)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsx (1)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)
apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx (3)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx (1)
useProjectNavigation(7-74)apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsx (1)
NavItems(12-17)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx (1)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsx (1)
NavProps(5-11)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/flat-nav-item.tsx (1)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsx (1)
NavProps(5-11)
⏰ 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). (3)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
🔇 Additional comments (6)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx (1)
95-95: Correct anchor for API load-moreSwitching to "#load-more-apis" aligns with the sidebar dispatcher and prevents cross-wiring with other “More” actions.
Smoke test the three load-more anchors are handled:
- apis: "#load-more-apis"
- ratelimits: "#load-more-ratelimits"
- projects: "#load-more-projects"
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx (1)
38-61: Projects injection under /projects looks correctCloning, enabling sub-items on the /projects parent, appending project items, and adding a single "More" action when hasNextPage is true all look solid and side-effect free with respect to baseNavItems.
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsx (1)
12-17: LGTM: Delegation to Flat/Nested is cleanPublic API now centralizes the onLoadMore(item) shape and keeps rendering logic minimal.
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/flat-nav-item.tsx (2)
44-45: Icon rendering path is fineThe loader/icon fallback is correct. With the hook change to icon: null for load-more, this avoids calling a no-op icon component.
18-23: No need to call preventDefault for load-more items
TheNavLinkcomponent renders a<button type="button">whenisLoadMoreButtonis true, so there’s no default anchor navigation or scrolling to intercept. You can safely ignore the suggestede.preventDefault()here.Likely an incorrect or invalid review comment.
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx (1)
115-117: LGTM: onLoadMore now receives the subItemPassing the triggering item up enables centralized handling for APIs, Ratelimits, and Projects load-more flows.
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsx
Show resolved
Hide resolved
...dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx
Show resolved
Hide resolved
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx
Show resolved
Hide resolved
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx
Show resolved
Hide resolved
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-projects-navigation.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/dashboard/lib/trpc/routers/deploy/project/list.ts (2)
38-43: Unify Zod integer-ness for timestamps and cursor (minor type hygiene)nextCursor is z.number().int().nullish() while createdAt/updatedAt are z.number() (updatedAt nullable). If these are epoch millis, keep them consistently int; otherwise, drop .int() from nextCursor to avoid validation mismatches.
Two options (pick one):
- Make timestamps and cursor all int:
- createdAt: z.number(), - updatedAt: z.number().nullable(), + createdAt: z.number().int(), + updatedAt: z.number().int().nullable(),
- Or allow non-int cursor (most lenient):
- nextCursor: z.number().int().nullish(), + nextCursor: z.number().nullish(),Also applies to: 26-35
81-128: Search block is safe and performant enough; consider indexes to keep it fast at scaleThe EXISTS subquery over routes is correct and scoped by workspace. To avoid full scans as data grows, add/confirm indexes aligned to the filters:
- projects: (workspaceId, updatedAt DESC) to support the main sort/filter.
- routes: (workspaceId, projectId, hostname) for the EXISTS and the hostnames lookup.
- Optional: projects (workspaceId, name), projects (workspaceId, defaultBranch) if search on these becomes frequent.
♻️ Duplicate comments (1)
apps/dashboard/lib/trpc/routers/deploy/project/list.ts (1)
214-217: LGTM: nextCursor falls back to createdAt when updatedAt is nullThis aligns pagination with nullable updatedAt and prevents early termination. Nice.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/dashboard/lib/trpc/routers/deploy/project/list.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{js,jsx,ts,tsx}: Use Biome for formatting and linting in TypeScript/JavaScript projects
Prefer named exports over default exports in TypeScript/JavaScript, except for Next.js pages
Files:
apps/dashboard/lib/trpc/routers/deploy/project/list.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Follow strict TypeScript configuration
Use Zod for runtime validation in TypeScript projects
Files:
apps/dashboard/lib/trpc/routers/deploy/project/list.ts
**/*.{env,js,ts,go}
📄 CodeRabbit inference engine (CLAUDE.md)
All environment variables must follow the format: UNKEY_<SERVICE_NAME>_VARNAME
Files:
apps/dashboard/lib/trpc/routers/deploy/project/list.ts
🧠 Learnings (1)
📚 Learning: 2025-07-25T19:09:43.284Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/lib/trpc/routers/deployment/list.ts:11-11
Timestamp: 2025-07-25T19:09:43.284Z
Learning: In apps/dashboard/lib/trpc/routers/deployment/list.ts, the listDeployments procedure intentionally queries the versions table rather than a deployments table. The user mcstepp indicated that renaming the table would require a database migration, which was deferred for the current PR focused on UI features.
Applied to files:
apps/dashboard/lib/trpc/routers/deploy/project/list.ts
🧬 Code graph analysis (1)
apps/dashboard/lib/trpc/routers/deploy/project/list.ts (1)
internal/db/src/schema/projects.ts (1)
projects(9-31)
⏰ 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). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/dashboard/lib/trpc/routers/deploy/project/list.ts (1)
2-16: Good: imported null-aware predicates for pagination correctnessAdding isNotNull/isNull (and related db helpers) is the right move to handle nullable updatedAt safely and to keep the keyset pagination logic expressible.
|
@perkinsjr could you review that one? |
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (08/28/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
|
@chronark @perkinsjr force merge pls |
|
why do you want to force merge? |
* feat: add sidebar more and listing for project This commit also fixes broken load more logic * fix: query invalidation * chore: remove comment * chore: fmt * fix: coderabbit reviews * feat: add solo sidebar routes * fix: allow collapsing when back to menu triggered * chore: remove unused prop
3 participants
What does this PR do?
This PR
Type of change
How should this be tested?
-Do these for both APIs and Ratelimits make sure their load mores work properly.
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit