feat: add new nested routes #3027
Conversation
ogzhanolguncu
requested review from
perkinsjr,
chronark,
mcstepp and
MichaelUnkey
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request implements multiple changes across the dashboard. The Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Sidebar as AppSidebar
participant Hook as useApiNavigation / useRatelimitNavigation
participant TRPC as TRPC Server
User->>Sidebar: Interact with sidebar/navigation
Sidebar->>Hook: Request enhanced navigation items
Hook->>TRPC: Fetch paginated API/ratelimit data
TRPC-->>Hook: Return navigation data with pagination info
Hook-->>Sidebar: Return enhanced navigation items (including "Load More")
Sidebar->>User: Render updated sidebar menu
Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (16)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 0
🧹 Nitpick comments (24)
apps/dashboard/app/(app)/apis/_components/api-list-card.tsx (1)
29-31: Performance improvement: Eliminated loading state for multiple chartsReplacing the dynamic loading state with a hardcoded
falsevalue is a good optimization for scenarios with multiple charts. The comment clearly explains the rationale behind this change.However, consider implementing a staggered loading approach if users experience a perception of unresponsiveness with no loading indicators.
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/toggle-sidebar-button.tsx (1)
20-20: Consider adding size fallback for iconWhile using the
size="xl-medium"prop is good, consider implementing a fallback or default size in case the icon component doesn't support this specific size prop.- {toggleNavItem.icon && <toggleNavItem.icon size="xl-medium" />} + {toggleNavItem.icon && <toggleNavItem.icon size="xl-medium" className="size-5" />}apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-link.tsx (1)
17-23: Enhance accessibility for load more buttonThe button implementation for load more functionality should include accessibility attributes to improve screen reader support.
- <button type="button" onClick={onClick} className="w-full text-left"> + <button + type="button" + onClick={onClick} + className="w-full text-left" + aria-label="Load more items" + > {children} </button>apps/dashboard/app/(app)/apis/actions.ts (1)
63-88: Improved data transformation with detailed key informationThe transformation logic now includes key details and maintains backward compatibility with the previous structure. Consider adding error handling for any potential failures during the Promise.all execution.
const apiList = await Promise.all( apiItems.map(async (api) => { - const keyspaceId = api.keyAuth?.id || null; + try { + const keyspaceId = api.keyAuth?.id || null; // Transform the keys data for the new keyDetails field const keyDetails = api.keyAuth?.keys?.map((key) => ({ id: key.id, name: key.name, })) || []; return { id: api.id, name: api.name, keyspaceId, keyDetails, // Include count for backward compatibility keys: [ { count: api.keyAuth?.sizeApprox || 0, }, ], }; + } catch (error) { + console.error(`Error transforming API data for ${api.id}:`, error); + // Return a minimal object with required fields + return { + id: api.id, + name: api.name, + keyspaceId: null, + keyDetails: [], + keys: [{ count: 0 }], + }; + } }), );apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/utils.ts (1)
13-25: SVG path utility needs bounds checkingWhile the implementation of
getPathForSegmentworks, it's missing validation for out-of-range indices, which could lead to undefined being returned.export function getPathForSegment(index: number) { const paths = [ "M13.162,3.82c-.148,0-.299-.044-.431-.136-.784-.552-1.662-.915-2.61-1.08-.407-.071-.681-.459-.61-.867,.071-.408,.459-.684,.868-.61,1.167,.203,2.248,.65,3.216,1.33,.339,.238,.42,.706,.182,1.045-.146,.208-.378,.319-.614,.319Z", "M16.136,8.5c-.357,0-.675-.257-.738-.622-.163-.942-.527-1.82-1.082-2.608-.238-.339-.157-.807,.182-1.045,.34-.239,.809-.156,1.045,.182,.683,.97,1.132,2.052,1.334,3.214,.07,.408-.203,.796-.611,.867-.043,.008-.086,.011-.129,.011Z", "M14.93,13.913c-.148,0-.299-.044-.431-.137-.339-.238-.42-.706-.182-1.045,.551-.784,.914-1.662,1.078-2.609,.071-.408,.466-.684,.867-.611,.408,.071,.682,.459,.611,.867-.203,1.167-.65,2.25-1.33,3.216-.146,.208-.378,.318-.614,.318Z", "M10.249,16.887c-.357,0-.675-.257-.738-.621-.07-.408,.202-.797,.61-.868,.945-.165,1.822-.529,2.608-1.082,.34-.238,.807-.156,1.045,.182,.238,.338,.157,.807-.182,1.045-.968,.682-2.05,1.13-3.214,1.333-.044,.008-.087,.011-.13,.011Z", "M7.751,16.885c-.043,0-.086-.003-.13-.011-1.167-.203-2.249-.651-3.216-1.33-.339-.238-.42-.706-.182-1.045,.236-.339,.702-.421,1.045-.183,.784,.551,1.662,.915,2.61,1.08,.408,.071,.681,.459,.61,.868-.063,.364-.381,.621-.738,.621Z", "M3.072,13.911c-.236,0-.469-.111-.614-.318-.683-.97-1.132-2.052-1.334-3.214-.07-.408,.203-.796,.611-.867,.403-.073,.796,.202,.867,.61,.163,.942,.527,1.82,1.082,2.608,.238,.339,.157,.807-.182,1.045-.131,.092-.282,.137-.431,.137Z", "M1.866,8.5c-.043,0-.086-.003-.129-.011-.408-.071-.682-.459-.611-.867,.203-1.167,.65-2.25,1.33-3.216,.236-.339,.703-.422,1.045-.182,.339,.238,.42,.706,.182,1.045-.551,.784-.914,1.662-1.078,2.609-.063,.365-.381,.622-.738,.622Z", "M4.84,3.821c-.236,0-.468-.111-.614-.318-.238-.338-.157-.807,.182-1.045,.968-.682,2.05-1.13,3.214-1.333,.41-.072,.797,.202,.868,.61,.07,.408-.202,.797-.61,.868-.945,.165-1.822,.529-2.608,1.082-.131,.092-.282,.137-.431,.137Z", ]; - return paths[index]; + if (index >= 0 && index < paths.length) { + return paths[index]; + } + console.warn(`Invalid path index: ${index}. Expected 0-${paths.length - 1}`); + return paths[0]; // Default to first path as fallback }apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/animated-loading-spinner.tsx (1)
8-33: Consider using a CSS-in-JS solution for handling styles.The direct DOM manipulation to inject styles, while appropriately guarded against duplication with the
STYLE_IDcheck, could be replaced with a more React-friendly approach like CSS modules or a CSS-in-JS library that's already in use in the project.- // Add styles only once when module is loaded - if (typeof document !== "undefined" && !document.getElementById(STYLE_ID)) { - const style = document.createElement("style"); - style.id = STYLE_ID; - style.textContent = ` - @media (prefers-reduced-motion: reduce) { - [data-prefers-reduced-motion="respect-motion-preference"] { - animation: none !important; - transition: none !important; - } - } - - @keyframes spin-slow { - from { - transform: rotate(0deg); - } - to { - transform: rotate(360deg); - } - } - - .animate-spin-slow { - animation: spin-slow 1.5s linear infinite; - } - `; - document.head.appendChild(style); - }Consider moving these styles to a CSS module or using your project's existing styling solution.
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx (2)
87-88: Remove TypeScript ignore comment and fix the type issue.The code includes a
@ts-expect-errorwith a comment "will fix that later", which should be addressed now rather than left for future work.- //@ts-expect-error will fix that later - label: <div className="font-normal decoration-dotted underline ">More</div>, + label: <div className="font-normal decoration-dotted underline ">More</div> as React.ReactNode,Alternatively, update the
NavItemtype to properly type thelabelproperty to accept React nodes.
57-95: Ensure consistent implementation between "More" items across hooks.In this hook, the "More" button is styled with an underline, while in the
useRatelimitNavigationhook, it's a plain text. Consider standardizing the UI for consistency.apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx (2)
85-86: Standardize styling for "More" buttons across navigation hooks.Unlike in
useApiNavigation, this hook uses a plain string for the "More" button without styling. Ensure consistent styling across similar UI elements.- label: "More", + label: <div className="font-normal decoration-dotted underline ">More</div>,
77-92: Add showSubItems property for consistency with useApiNavigation.In
useApiNavigation, you setshowSubItems: truefor the APIs section, but this property is missing here for the ratelimits section. For consistency, consider adding it.if (ratelimitsItemIndex !== -1) { const ratelimitsItem = { ...items[ratelimitsItemIndex] }; + // Always set showSubItems to true for the Ratelimits section + ratelimitsItem.showSubItems = true; ratelimitsItem.items = [...(ratelimitsItem.items || []), ...ratelimitNavItems];apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/flat-nav-item.tsx (1)
50-50: Simplify conditional rendering of loading spinner or icon.The current implementation has a nested ternary expression which can be simplified for better readability.
- {showLoader ? <AnimatedLoadingSpinner /> : Icon ? <Icon size="xl-medium" /> : null} + {showLoader ? <AnimatedLoadingSpinner /> : Icon && <Icon size="xl-medium" />}apps/dashboard/components/ui/sidebar.tsx (1)
525-546: Consider validatingdepthandmaxDepthinputs.Currently, negative or otherwise unexpected values for
depthormaxDepthmay cause unpredictable styles or logic if not handled. While this scenario may be unlikely, adding basic validation or clamping the values can prevent subtle bugs, especially in dynamic sidebar configurations.apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx (4)
36-40: Ensure keyboard accessibility for chevron toggle.Handling click events is fine, but users navigating via keyboard and screen readers may need a keypress handler or an accessible element (e.g., a button). Consider adding a role="button" and handling Enter/Space keys for inclusive design.
42-54: Addnoopener noreferrerfor external links.When opening external links in a new tab with
_blank, setrel="noopener noreferrer"to avoid potential security vulnerabilities where the new page can accesswindow.opener.- window.open(item.href, "_blank"); + window.open(item.href, "_blank", "noopener,noreferrer");
56-129: Revisit the 300ms timeout after navigation.The fixed 300ms delay in resetting the sub-item loading state might become brittle. For instance, if the actual navigation or rendering takes longer than 300ms, your UI might revert to a non-loading state prematurely. Consider using route change events or callbacks to ensure your loader state aligns with actual navigation completion.
131-175: Improve accessibility for nested items.Embedding the chevron click area inside the main button is thorough, but ensure that nested sub-items and deeper collapsibles remain keyboard-accessible. You might provide a dedicated keyboard handler on the chevron section or unify the toggling with the main button to meet WCAG guidelines.
Would you like help creating an accessible wrapper or reworking the click event for this nested structure?
apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx (2)
45-56: Unify load-more actions for clarity.Currently, load-more logic is bifurcated between APIs and Ratelimits. Although it works, consider extracting a shared handler or naming them distinctly (e.g.,
loadMoreApisvs.loadMoreRatelimits) to avoid confusion. This can future-proof the code for additional resource types.
72-90: Add a tooltip for the hide icon when sidebar is not collapsed.For consistency, you could show a quick tooltip or label for the icon at line 83 to communicate its action—for users who may not recognize the icon at a glance. This minor UX improvement can help discoverability.
apps/dashboard/lib/trpc/routers/ratelimit/query-keys/schemas.ts (3)
1-27: Schema structure looks good, consider adding documentation.The Zod schemas for ratelimit namespaces are well-structured, with appropriate types and validations. Consider adding JSDoc comments to document the purpose of each schema and its fields for better developer experience.
You could enhance this file with documentation like:
+/** + * Represents a ratelimit namespace with unique identifier and display name + */ export const ratelimitNamespace = z.object({ id: z.string(), name: z.string(), }); +/** + * Pagination cursor used for fetching subsequent pages + */ const Cursor = z.object({ id: z.string(), });
14-19: Consider exposing Cursor schema for consistency.While the
Cursorschema is used internally within this file, it might be useful to export it for reuse elsewhere, especially since it's referenced in multiple schemas.-const Cursor = z.object({ +export const Cursor = z.object({ id: z.string(), });
23-26: Validate cursor id if cursor is provided.The current schema doesn't validate the cursor id when a cursor is provided. Consider adding a refinement to ensure the cursor id is not empty when it's present.
export const queryRatelimitNamespacesPayload = z.object({ limit: z.number().min(1).max(18).default(9), - cursor: Cursor.optional(), + cursor: Cursor.optional().refine( + (data) => !data || data.id.trim().length > 0, + { message: "Cursor ID cannot be empty if cursor is provided" } + ), });apps/dashboard/lib/trpc/routers/ratelimit/query-keys/index.ts (2)
22-25: Consider structured error logging.The current error logging concatenates the error object as a JSON string, which might not capture all error properties correctly, especially for non-serializable properties.
- console.error( - "Something went wrong when fetching ratelimit namespaces", - JSON.stringify(error), - ); + console.error( + "Something went wrong when fetching ratelimit namespaces", + error instanceof Error ? { message: error.message, stack: error.stack } : error, + );
45-55: Consider optimizing the total count query.The separate query for the total count could be inefficient for large datasets. Consider using a single query with a count window function if your database supports it, or implementing cursor-based pagination without requiring the total count.
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
173-186: Consider updating resourcesNavigation to match new NavItem type.The
resourcesNavigationarray items should be updated to explicitly set the newly added properties to maintain consistency, even if they're optional.export const resourcesNavigation: NavItem[] = [ { icon: BookBookmark, href: "https://unkey.dev/docs", external: true, label: "Docs", + items: undefined, + showSubItems: undefined, }, { icon: DiscordIcon, href: "https://www.unkey.com/discord", external: true, label: "Discord", + items: undefined, + showSubItems: undefined, }, ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
apps/dashboard/app/(app)/apis/_components/api-list-card.tsx(2 hunks)apps/dashboard/app/(app)/apis/actions.ts(3 hunks)apps/dashboard/app/(app)/layout.tsx(1 hunks)apps/dashboard/app/(app)/settings/root-keys/new/client.tsx(1 hunks)apps/dashboard/components/app-sidebar.tsx(0 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/animated-loading-spinner.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/flat-nav-item.tsx(1 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(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/toggle-sidebar-button.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/utils.ts(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-link.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-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(1 hunks)apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx(4 hunks)apps/dashboard/components/stats-card/components/chart/stats-chart.tsx(1 hunks)apps/dashboard/components/ui/sidebar.tsx(1 hunks)apps/dashboard/lib/trpc/routers/api/overview/query-overview/schemas.ts(2 hunks)apps/dashboard/lib/trpc/routers/index.ts(2 hunks)apps/dashboard/lib/trpc/routers/ratelimit/query-keys/index.ts(1 hunks)apps/dashboard/lib/trpc/routers/ratelimit/query-keys/schemas.ts(1 hunks)internal/icons/src/icons/caret-right.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- apps/dashboard/components/app-sidebar.tsx
🧰 Additional context used
🧬 Code Definitions (9)
apps/dashboard/lib/trpc/routers/index.ts (1)
apps/dashboard/lib/trpc/routers/ratelimit/query-keys/index.ts (1)
queryRatelimitNamespaces(10-31)
apps/dashboard/app/(app)/apis/_components/api-list-card.tsx (1)
apps/dashboard/app/(app)/apis/_components/hooks/use-query-timeseries.ts (1)
useFetchVerificationTimeseries(8-73)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsx (3)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/flat-nav-item.tsx (1)
FlatNavItem(10-57)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx (1)
NestedNavItem(18-176)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/toggle-sidebar-button.tsx (2)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/utils.ts (1)
getButtonStyles(3-11)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/animated-loading-spinner.tsx (1)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/utils.ts (1)
getPathForSegment(13-25)
apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx (5)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (3)
createWorkspaceNavigation(59-171)NavItem(16-29)resourcesNavigation(173-186)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx (1)
useApiNavigation(11-111)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx (1)
useRatelimitNavigation(11-108)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/toggle-sidebar-button.tsx (1)
ToggleSidebarButton(5-25)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsx (1)
NavItems(5-19)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/flat-nav-item.tsx (4)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-link.tsx (1)
NavLink(3-35)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/utils.ts (1)
getButtonStyles(3-11)apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/animated-loading-spinner.tsx (1)
AnimatedLoadingSpinner(46-91)
apps/dashboard/app/(app)/apis/actions.ts (1)
internal/clickhouse/src/index.ts (1)
api(177-196)
apps/dashboard/lib/trpc/routers/ratelimit/query-keys/index.ts (1)
apps/dashboard/lib/trpc/routers/ratelimit/query-keys/schemas.ts (3)
queryRatelimitNamespacesPayload(23-26)ratelimitNamespacesResponse(14-19)RatelimitNamespacesResponse(21-21)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (38)
apps/dashboard/components/stats-card/components/chart/stats-chart.tsx (1)
46-46: Performance optimization with debounce parameterAdding the debounce parameter to the ResponsiveContainer is a good improvement. This will reduce the number of re-renders when resizing the window by waiting 1000ms before triggering a re-render, which can help improve performance especially on slower devices.
apps/dashboard/lib/trpc/routers/index.ts (2)
40-40: LGTM: New ratelimit namespaces importImporting the queryRatelimitNamespaces function to support the new nested routes feature.
151-151: Added support for ratelimit namespace queriesGood addition of the namespace query endpoint to the ratelimit router. This supports the new nested routes feature by enabling retrieval of ratelimit namespace data.
apps/dashboard/app/(app)/layout.tsx (1)
1-1: Updated import path to new sidebar structureThe import path change reflects the restructured sidebar organization. This adaptation is needed for the new nested routes feature.
apps/dashboard/app/(app)/settings/root-keys/new/client.tsx (1)
282-282: Enhanced dialog styling with proper dark mode supportThe DialogContent styling has been improved to explicitly support both light and dark modes. The addition of
bg-whitefor light mode and maintainingdark:bg-grayA-1for dark mode ensures consistent appearance across theme settings.apps/dashboard/app/(app)/apis/_components/api-list-card.tsx (1)
15-15: Performance optimization: Removed loading state from hook destructuringThe
isLoadingstate has been removed from the destructured hook return, aligning with the hardcodedfalsevalue passed to the chart component.apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/toggle-sidebar-button.tsx (1)
1-25: LGTM: Well-implemented sidebar toggle button componentThis component effectively handles sidebar toggling functionality with proper type definitions and follows the project's UI patterns. The implementation correctly uses the provided utility functions and component hierarchy.
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-link.tsx (1)
3-35: Versatile navigation link component with conditional renderingThis component elegantly handles both standard navigation and "load more" functionality, providing appropriate behavior for each use case. The external link handling is also properly implemented with target="_blank" and prefetch control.
apps/dashboard/lib/trpc/routers/api/overview/query-overview/schemas.ts (3)
3-8: Well-structured key detail schema implementationThe new
keyDetailschema is cleanly defined with appropriate types for the required properties.
13-20: Good backward compatibility handling with new schema fieldMaking
keyDetailsoptional while maintaining the existingkeysarray with a clear comment about backward compatibility is a thoughtful approach to schema evolution.
42-43: Properly exported type for wider usageExporting the
KeyDetailtype derived from the schema is good practice for maintaining type consistency across the codebase.apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/index.tsx (1)
1-19: Well-structured component with clear conditional rendering logic!This implementation cleanly handles both flat and nested navigation items. The component properly forwards the
onLoadMorecallback, which is essential for pagination functionality in navigation lists.internal/icons/src/icons/caret-right.tsx (3)
13-16: Good enhancement to support dynamic sizing!The update to include
sizeMapand extract dynamic values improves the component's flexibility.
18-24: Properly implemented dynamic dimensions while maintaining fixed viewBoxThe SVG now uses dynamic height and width from the size map while maintaining a fixed viewBox, which is a good approach for scalable icons.
32-32: Dynamic stroke width implementationUsing the dynamic
strokeWidthfrom the size map ensures consistent proportions across different icon sizes.apps/dashboard/app/(app)/apis/actions.ts (1)
22-51: Enhanced query with more detailed key informationThe updated query now includes the keyspace ID and fetches actual keys, providing richer data for the navigation components. This will be valuable for the nested routes feature.
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/utils.ts (1)
3-11: Well-implemented button styling utilityThe
getButtonStylesfunction provides a clean way to generate consistent button styles based on active and loading states.apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/animated-loading-spinner.tsx (3)
35-44: Good use of descriptive comments for segment positions.The comments clearly document the position of each segment in the spinner, which enhances code readability.
49-56: Proper cleanup of interval in useEffect.The interval is correctly cleared in the useEffect cleanup function, which prevents memory leaks when the component unmounts.
58-90: Good accessibility implementation with reduced motion preferences.The component respects user preferences for reduced motion using the
data-prefers-reduced-motionattribute, which is excellent for accessibility.apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx (2)
11-23: Implementation of infinite query for API data looks good.The hook properly sets up an infinite query with pagination handling using TRPC. The use of
getNextPageParamto extract the cursor from the response is appropriate.
25-71: Well-structured navigation item creation with clear comments.The code effectively transforms API data into navigation items with appropriate sub-items. The comments explaining the purpose of each section enhance readability.
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx (2)
11-23: Well-implemented infinite query for ratelimit namespaces.The hook correctly sets up an infinite query using TRPC with appropriate pagination handling.
25-71: Good structure for namespace navigation items with multiple sub-items.The creation of ratelimit namespace navigation items with logs, settings, and overrides sub-items is well-organized and clearly commented.
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/flat-nav-item.tsx (3)
17-19: Good use of delayed loading indicator for better UX.The combination of React's
useTransitionwith a customuseDelayLoaderhook creates a smoother user experience by preventing flickering of loading indicators during quick transitions.
24-35: Well-structured navigation handling with support for load more action.The click handler properly differentiates between load more buttons and regular navigation items, with appropriate transition handling for internal navigation.
37-55: Good component composition with appropriate props passing.The component properly composes
SidebarMenuItem,NavLink, andSidebarMenuButtonwith appropriate props, making the code modular and maintainable.apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx (2)
1-16: Imports look organized.The introduced imports are consistent with the rest of the code and appear to be used properly. No issues identified here.
18-35: AlignmaxDepthdefaults across components.You are setting
maxDepth = 1here, whereas theSidebarMenuSubinsidebar.tsxdefaults tomaxDepth = 2. Consider whether different defaults are intended or if keeping them in sync would ensure consistent behavior across nested items.apps/dashboard/components/navigation/sidebar/app-sidebar/index.tsx (2)
28-37: Good modular approach forAppSidebar.Your function properly composes pieces from the workspace navigation, API, and rate limit hooks, keeping concerns separate. This is a well-structured entry point to the new nested routing logic.
94-121: Consider adding test coverage for toggling and load-more flows.Your code modifies the navigation items depending on user interaction. Automated tests covering the toggle button (lines 100-102) and the dynamic load-more process (lines 103-109) could help prevent regressions, especially as you refine or add more nested resources.
Would you like to generate a basic test suite to confirm that toggling, load-more calls, and the resulting UI match expectations?
apps/dashboard/lib/trpc/routers/ratelimit/query-keys/index.ts (3)
10-31: Well-structured TRPC procedure with proper error handling.The
queryRatelimitNamespacesprocedure is well-implemented with appropriate input/output schemas and error handling. The use ofrateLimitedProcedureensures rate limiting is applied correctly.
57-72: Good use of database query builders for pagination.The implementation correctly uses the query builder pattern to construct a paginated query with proper conditions based on the cursor. The ordering and limit handling for pagination is implemented correctly.
74-87: Well-implemented cursor-based pagination logic.The pagination logic correctly determines if there are more items by fetching one extra item, and properly calculates the next cursor based on the last item in the result set.
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (4)
16-29: Good update to NavItem type to support nested navigation.The NavItem type has been enhanced to support nested navigation with the addition of
items,loadMoreAction, andshowSubItemsproperties. The change to allownullfor icons is appropriate for sub-items that don't need icons.
69-69: APIs navigation item prepared for future sub-items.The
showSubItems: falseproperty suggests that the APIs navigation might have nested items in the future that can be toggled. This is consistent with the PR's goal of adding nested routes.
82-96: Well-structured nested navigation for Authorization section.The Authorization section now has a clear hierarchical structure with Roles and Permissions as sub-items. The implementation correctly sets the active state based on the URL segments.
137-169: Comprehensive nested navigation for Settings section.The Settings section now has a well-organized structure with multiple sub-items (Team, Root Keys, Billing, Vercel Integration, User). The implementation correctly sets the active state based on the URL segments.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/dashboard/components/app-sidebar.tsx (2)
159-203: Consider moving AnimatedLoadingSpinner to a separate fileI notice that this component appears to be duplicated, as the relevant code snippets show that it already exists in
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/animated-loading-spinner.tsx. Duplicating components can lead to maintenance issues if changes are needed in the future.-const AnimatedLoadingSpinner = memo(() => { - const [segmentIndex, setSegmentIndex] = useState(0); - - useEffect(() => { - // Animate the segments in sequence - const timer = setInterval(() => { - setSegmentIndex((prevIndex) => (prevIndex + 1) % SEGMENTS.length); - }, 125); // 125ms per segment = 1s for full rotation - return () => clearInterval(timer); - }, []); - - return ( - <svg - xmlns="http://www.w3.org/2000/svg" - width="18" - height="18" - viewBox="0 0 18 18" - className="animate-spin-slow" - data-prefers-reduced-motion="respect-motion-preference" - > - <g> - {SEGMENTS.map((id, index) => { - const distance = (SEGMENTS.length + index - segmentIndex) % SEGMENTS.length; - const opacity = distance <= 4 ? 1 - distance * 0.2 : 0.1; - return ( - <path - key={id} - id={id} - style={{ - fill: "currentColor", - opacity: opacity, - transition: "opacity 0.12s ease-in-out", - }} - d={getPathForSegment(index)} - /> - ); - })} - <path - d="M9,6.5c-1.379,0-2.5,1.121-2.5,2.5s1.121,2.5,2.5,2.5,2.5-1.121,2.5-2.5-1.121-2.5-2.5-2.5Z" - style={{ fill: "currentColor", opacity: 0.6 }} - /> - </g> - </svg> - ); -});And add the import at the top:
+import { AnimatedLoadingSpinner } from "@/components/navigation/sidebar/app-sidebar/components/nav-items/animated-loading-spinner";
357-381: Consider moving CSS styles to a stylesheetDynamically injecting styles via JavaScript is harder to maintain and could cause flashes of unstyled content. Consider moving these styles to a CSS or SCSS file for better maintainability and performance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/components/app-sidebar.tsx(4 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/dashboard/components/app-sidebar.tsx (1)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/animated-loading-spinner.tsx (1)
AnimatedLoadingSpinner(46-91)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
🔇 Additional comments (4)
apps/dashboard/components/app-sidebar.tsx (4)
241-241: Improved error handling for icon renderingThe conditional rendering pattern here prevents potential runtime errors by checking if the Icon exists before attempting to render it. This is a good defensive programming practice.
267-267: Good implementation of consistent icon rendering patternThis follows the same robust pattern as in the FlatNavItem component, ensuring proper icon rendering with appropriate fallbacks.
339-340: Well-structured icon assignment from propsExtracting the icon into a local variable makes the code more readable and consistent with the pattern used in other components.
349-349: Improved conditional icon renderingThe conditional check before rendering the icon prevents potential runtime errors if the icon is undefined or null.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx (1)
53-54: LGTM: Enhancing data consistency after mutation.The addition of
revalidate("/ratelimits")and cache invalidation ensures UI data remains consistent after creating a namespace. This is an excellent improvement for user experience.Consider adding error handling for the revalidation operation:
- await revalidate("/ratelimits"); + try { + await revalidate("/ratelimits"); + } catch (error) { + console.error("Failed to revalidate ratelimits:", error); + // Optional: Show a toast warning that some data might not be updated + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx(3 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx (3)
apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)apps/dashboard/lib/trpc/routers/index.ts (1)
router(78-202)apps/dashboard/app/(app)/authorization/roles/[roleId]/page.tsx (1)
revalidate(8-8)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Test Go API Local / Test (Shard 7/8)
- GitHub Check: Test Go API Local / Test (Shard 8/8)
- GitHub Check: Test Packages / Test ./internal/keys
- GitHub Check: Test Packages / Test ./internal/id
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/dashboard/app/(app)/ratelimits/_components/create-namespace-button.tsx (3)
3-3: LGTM: Good addition for route revalidation.The import statement for
revalidateis essential for triggering Next.js route revalidation after namespace creation.
36-37: LGTM: Proper setup for cache invalidation.Extracting the
ratelimitutils fromtrpc.useUtils()provides access to cache invalidation methods, which is a good practice for keeping UI data in sync after mutations.
50-50: LGTM: Making the handler async.Adding
asyncto theonSuccesshandler is required since we'll be awaiting therevalidatefunction call.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx (5)
1-8: Use a single icon library for stylistic consistency (optional).The code imports icons from both
@unkey/iconsandlucide-react, which may result in inconsistent icon sizes or styles in the UI. Consider consolidating icon usage under a single library for a more uniform look and feel.
9-10: Consider makingDEFAULT_LIMITconfigurable.Using a hard-coded limit might be restrictive if the app requires different pagination parameters. Exposing
DEFAULT_LIMITvia environment variables or function arguments could offer more flexibility.
11-22: Add error state handling for the infinite query.Currently, the component handles loading states but not errors. If the request fails, a user might be confused by the missing data. Consider returning or displaying an error state to improve user feedback.
80-106: Remove//@ts-expect-errorby adjusting theNavItemtype.You’re using a React node for
label(line 95) while theNavIteminterface declares it as a string. Remove thets-expect-errorcomment by expanding theNavItemtype to acceptReact.ReactNode. For example, inworkspace-navigations.tsx:// Proposed snippet for updating the NavItem definition: export type NavItem = { disabled?: boolean; tooltip?: string; icon: React.ElementType | null; href: string; external?: boolean; label: string | React.ReactNode; active?: boolean; tag?: React.ReactNode; hidden?: boolean; items?: NavItem[]; loadMoreAction?: boolean; showSubItems?: boolean; };
113-118: Optionally handle or expose error states alongsideisLoading.If you need to display specific UI based on errors, consider returning or consuming an
isErroror similar state to inform the user if the data fails to load.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx(1 hunks)apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx(1 hunks)internal/icons/src/icons/arrow-dotted-rotate-anticlockwise.tsx(1 hunks)internal/icons/src/icons/arrow-opposite-direction-y.tsx(1 hunks)internal/icons/src/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/components/navigation/sidebar/app-sidebar/components/nav-items/nested-nav-item.tsx
- apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-ratelimit-navigation.tsx
🧰 Additional context used
🧬 Code Definitions (3)
internal/icons/src/icons/arrow-opposite-direction-y.tsx (1)
internal/icons/src/props.ts (2)
IconProps(30-35)sizeMap(7-28)
internal/icons/src/icons/arrow-dotted-rotate-anticlockwise.tsx (1)
internal/icons/src/props.ts (2)
IconProps(30-35)sizeMap(7-28)
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx (4)
apps/dashboard/components/navigation/sidebar/workspace-navigations.tsx (1)
NavItem(16-29)apps/dashboard/lib/trpc/server.ts (1)
trpc(7-14)internal/icons/src/icons/gear.tsx (1)
Gear(15-197)internal/icons/src/icons/arrow-opposite-direction-y.tsx (1)
ArrowOppositeDirectionY(15-68)
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: Test Go API Local / Test (Shard 4/8)
- GitHub Check: Test Go API Local / Test (Shard 8/8)
- GitHub Check: Test Go API Local / Test (Shard 6/8)
- GitHub Check: Test Go API Local / Test (Shard 3/8)
- GitHub Check: Test Go API Local / Test (Shard 5/8)
- GitHub Check: Test Packages / Test ./internal/resend
- GitHub Check: Test Go API Local / Test (Shard 2/8)
- GitHub Check: Test Go API Local / Test (Shard 7/8)
- GitHub Check: Test Packages / Test ./packages/nextjs
- GitHub Check: Test Packages / Test ./internal/encryption
- GitHub Check: Test Packages / Test ./internal/clickhouse
- GitHub Check: Test Agent Local / test_agent_local
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
internal/icons/src/icons/arrow-dotted-rotate-anticlockwise.tsx (1)
15-57: Implementation looks good!The component follows the established icon pattern with proper typing, default props, and consistent SVG structure.
internal/icons/src/index.ts (1)
71-72: Icon exports correctly addedNew exports follow the established pattern and make the icons available to consumers.
internal/icons/src/icons/arrow-opposite-direction-y.tsx (1)
15-68: Implementation looks good!The component correctly implements the icon pattern and properly uses both the size and strokeWidth parameters from sizeMap.
apps/dashboard/components/navigation/sidebar/app-sidebar/hooks/use-api-navigation.tsx (2)
24-79: Well-structured transformation of API data into nav items.Your approach of mapping API data to main and sub-navigation items is clear and follows a logical structure. Great job!
107-111: TheloadMorehandler is straightforward and effective.Your check on
!isFetchingNextPage && hasNextPageis sufficient to prevent redundant calls and ensure correct pagination.
Showing the loader only for items that has icon will prevent layout shift
What does this PR do?
Fixes # (issue)
If there is not an issue for this, please create one first. This is used to tracking purposes and also helps use understand why this PR exists
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
FlatNavItem,NavLink,NestedNavItem,ToggleSidebarButton, andAnimatedLoadingSpinner.StatsTimeseriesBarChartwith improved responsiveness.ArrowDottedRotateAnticlockwiseicon and anArrowOppositeDirectionYicon.Bug Fixes
Style