refactor: replace breadcrumbs with new top navbar #2753
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 introduces a comprehensive redesign of the dashboard's navigation and layout structure across multiple components. The changes primarily focus on implementing a consistent navigation pattern using new Changes
Suggested Labels
Suggested Reviewers
Possibly Related PRs
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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! 🙏 |
|
From Oz ‣ yeey no more breadcrumbs |
|
This PR make pages slightly faster because we don't fetch details twice in pages anymore. Also, some of the queries are updated with joins. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🔭 Outside diff range comments (3)
apps/dashboard/app/(app)/apis/loading.tsx (1)
Line range hint
1-42: Consider creating a shared loading componentThis file is identical to
ratelimits/loading.tsx. Consider extracting a shared loading component that can be configured for both APIs and rate limits pages.Create a new shared component:
// components/loading/CardGridSkeleton.tsx interface CardGridSkeletonProps { count: number; header?: boolean; } export function CardGridSkeleton({ count, header = true }: CardGridSkeletonProps) { return ( <PageContent> {header && ( <div className="flex flex-col pt-2"> <Skeleton className="w-32 h-6" /> <Skeleton className="w-28 h-3 mt-4" /> <Skeleton className="w-full h-[1px] mt-6 md:mt-9 lg:mt-[52px]" /> </div> )} <div className="flex max-sm:flex-col md:flex-row mt-6 gap-4"> <Skeleton className="h-8 w-full" /> <Skeleton className="sm:basis-full md:basis-48 lg:basis-[168px] h-8" /> </div> <div className="flex lg:flex-row flex-col mt-4 gap-6"> {Array.from({ length: count }).map((_, i) => ( <Skeleton key={i} className="h-44 w-full p-6 bg-transparent border border-round border-border"> <Skeleton className="w-[130px] h-6" /> <Skeleton className="w-[215px] h-3 mt-4" /> <div className="w-full h-full mt-11 flex flex-row"> <Skeleton className="w-14 h-3" /> <Skeleton className="w-3 h-3 ml-auto" /> </div> </Skeleton> ))} </div> </PageContent> ); }Usage in both files:
// apis/loading.tsx and ratelimits/loading.tsx import { CardGridSkeleton } from "@/components/loading/CardGridSkeleton"; export default function Loading() { return <CardGridSkeleton count={3} />; }apps/dashboard/app/(app)/authorization/roles/page.tsx (1)
Line range hint
45-48: Consider handling soft-deleted keys in the database queryInstead of filtering soft-deleted keys in memory, consider adding the condition to the database query for better performance:
keys: { with: { key: { + where: (table, { isNull }) => isNull(table.deletedAt), columns: { deletedAt: true, }, }, }, },apps/dashboard/app/(app)/settings/billing/page.tsx (1)
Based on the complete file content, I can now provide the final response:
The edge case concern in
forecastUsageis valid and requires attentionThe
forecastUsagefunction is used in the billing page to estimate future usage and costs, specifically in:
- Calculating estimated total price for verifications
- Displaying forecasted usage in the metered line items UI
The current implementation could indeed produce extreme values early in the billing cycle when
passedis very small (close to 0), as it uses1 / passedwithout any bounds. This could lead to unrealistic forecasts and potentially confusing UI for users.The suggested fix to add a minimum threshold is appropriate and should be implemented to prevent unrealistic projections.
🔗 Analysis chain
Line range hint
356-366: Handle edge case in usage forecastingThe
forecastUsagefunction might produce extreme values when called very early in the billing cycle due to the division by a small number.function forecastUsage(currentUsage: number): number { const t = new Date(); t.setUTCDate(1); t.setUTCHours(0, 0, 0, 0); const start = t.getTime(); t.setUTCMonth(t.getUTCMonth() + 1); const end = t.getTime() - 1; const passed = (Date.now() - start) / (end - start); - return currentUsage * (1 + 1 / passed); + // Add minimum threshold to avoid extreme projections + const minPassedThreshold = 0.1; + const adjustedPassed = Math.max(passed, minPassedThreshold); + return currentUsage * (1 + 1 / adjustedPassed); }
The search results show that
forecastUsageis used in three places within the same file, specifically for estimating total price and in a component's usage calculation. Let me verify the full context of these usages to better understand the impact.
Let me try again with proper path escaping:
Let me try a different approach to get the file content:
Let me try one last search approach:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usage of forecastUsage to verify impact rg "forecastUsage" -A 5Length of output: 1548
Script:
#!/bin/bash # Get more context around the usage of forecastUsage rg "forecastUsage" -B 5 -A 10 apps/dashboard/app/(app)/settings/billing/page.tsxLength of output: 263
Script:
#!/bin/bash # Get more context around the usage of forecastUsage rg "forecastUsage" -B 5 -A 10 "apps/dashboard/app/\(app\)/settings/billing/page.tsx"Length of output: 177
Script:
#!/bin/bash # Find the billing page file first fd "page.tsx" --full-path "*/settings/billing"Length of output: 152
Script:
#!/bin/bash # Find the billing page and its content fd "page.tsx" | grep -i billing cat apps/dashboard/app/\(app\)/settings/billing/page.tsx 2>/dev/null || trueLength of output: 18823
🧹 Nitpick comments (37)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/constants.ts (1)
1-23: Consider memoizing the navigation configurationThe current implementation creates a new array on every call. For better performance, consider memoizing the result.
Here's a suggested implementation using
useMemo:+import { useMemo } from 'react'; + -export const navigation = (namespaceId: string) => [ +export const useNavigation = (namespaceId: string) => useMemo( + () => [ { label: "Overview", href: `/ratelimits/${namespaceId}`, segment: "overview", }, // ... rest of the items -]; + ], + [namespaceId] +);apps/dashboard/app/(app)/apis/[apiId]/layout.tsx (1)
Line range hint
1-29: Consider adding error boundariesWhile the authentication and API validation logic remains solid, the simplified layout structure might benefit from error boundaries to gracefully handle rendering failures in child components, especially during this transition.
Consider wrapping the children with an error boundary component:
+ import { ErrorBoundary } from '@/components/ErrorBoundary'; export default async function ApiPageLayout(props: Props) { const tenantId = getTenantId(); const api = await db.query.apis.findFirst({ where: (table, { eq, and, isNull }) => and(eq(table.id, props.params.apiId), isNull(table.deletedAt)), with: { workspace: true, }, }); if (!api || api.workspace.tenantId !== tenantId) { return notFound(); } - return props.children; + return ( + <ErrorBoundary fallback={<div>Something went wrong. Please try again.</div>}> + {props.children} + </ErrorBoundary> + ); }apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/[overrideId]/page.tsx (2)
43-70: Consider adding tooltips for better UX.The navigation structure is well-implemented, but consider adding tooltips to the namespace ID badge to explain its purpose to users.
<Badge key="namespaceId" variant="secondary" - className="flex justify-between w-full gap-2 font-mono font-medium ph-no-capture" + className="flex justify-between w-full gap-2 font-mono font-medium ph-no-capture group relative" > + <span className="absolute invisible group-hover:visible bg-black text-white p-2 rounded text-sm -top-8 left-1/2 transform -translate-x-1/2"> + Namespace Identifier + </span> {override.namespace.id} <CopyButton value={override.namespace.id} /> </Badge>
73-77: Optimize PageHeader styling.The
m-0class appears redundant when used withpx-1. Consider simplifying the styling.<PageHeader - className="m-0 px-1" + className="px-1" title="Identifier" description="Edit identifier configuration"apps/dashboard/app/(app)/apis/client.tsx (1)
Line range hint
39-47: Optimize search functionalityConsider the following improvements to enhance performance and user experience:
- Add debouncing to prevent excessive filtering on each keystroke
- Make the search case-insensitive by converting both strings to lowercase once
+ const debouncedSearch = useMemo( + () => + debounce((query: string) => { + const searchTerm = query.toLowerCase(); + const filtered = apis.filter((a) => + a.name.toLowerCase().includes(searchTerm) + ); + setLocalData(filtered); + }, 300), + [apis] + ); <input className="placeholder:text-content-subtle flex-grow bg-transparent focus-visible:outline-none disabled:cursor-not-allowed disabled:opacity-50 " placeholder="Search.." - onChange={(e) => { - const filtered = apis.filter((a) => - a.name.toLowerCase().includes(e.target.value.toLowerCase()), - ); - setLocalData(filtered); - }} + onChange={(e) => debouncedSearch(e.target.value)} />apps/dashboard/app/(app)/identities/[identityId]/page.tsx (1)
224-229: Consider using Intl.DateTimeFormat for consistent date formatting.The current implementation uses
toUTCString()which might not be ideal for all locales. Consider usingIntl.DateTimeFormatfor better internationalization support.Here's an example implementation:
-<span className="text-content-subtle"> - {new Date(lastUsed).toUTCString()} -</span> +<span className="text-content-subtle"> + {new Intl.DateTimeFormat('en-US', { + dateStyle: 'medium', + timeStyle: 'long', + timeZone: 'UTC' + }).format(new Date(lastUsed))} +</span>apps/dashboard/app/(app)/ratelimits/loading.tsx (1)
Line range hint
15-41: Consider extracting repeated skeleton card structureThere are three identical skeleton card structures that could be extracted into a reusable component to reduce duplication.
+ // Create a new component RateLimitCardSkeleton + function RateLimitCardSkeleton() { + return ( + <Skeleton className="h-44 w-full p-6 bg-transparent border border-round border-border"> + <Skeleton className="w-[130px] h-6" /> + <Skeleton className="w-[215px] h-3 mt-4" /> + <div className="w-full h-full mt-11 flex flex-row"> + <Skeleton className="w-14 h-3" /> + <Skeleton className="w-3 h-3 ml-auto" /> + </div> + </Skeleton> + ); + } // Use in the main component <div className="flex lg:flex-row flex-col mt-4 gap-6"> + <RateLimitCardSkeleton /> + <RateLimitCardSkeleton /> + <RateLimitCardSkeleton /> </div>apps/dashboard/app/(app)/apis/[apiId]/loading.tsx (1)
Line range hint
6-58: Consider adding ARIA labels to improve accessibility.While the loading UI is well-structured, adding ARIA labels would improve the experience for screen reader users during loading states.
Consider adding aria-label attributes to key skeleton sections:
- <div className="grid grid-cols-2 max-sm:gap-2 gap-4 md:grid-cols-3"> + <div className="grid grid-cols-2 max-sm:gap-2 gap-4 md:grid-cols-3" aria-label="Loading API statistics"> - <Skeleton className="h-[104px] p-6 max-md:mr-2 max-md:mb-2 max-sm:mb-2 bg-transparent border border-round border-border"> + <Skeleton className="h-[104px] p-6 max-md:mr-2 max-md:mb-2 max-sm:mb-2 bg-transparent border border-round border-border" aria-label="Loading API metric">apps/dashboard/app/(app)/settings/root-keys/[keyId]/page-layout.tsx (2)
4-4: Useimport typefor type-only imports to reduce bundle sizeSince
Keyis only used as a type, prefacing the import withtypehelps the compiler remove the import during transpilation, optimizing the bundle size.Apply this diff:
-import { Key } from "@unkey/db"; +import type { Key } from "@unkey/db";🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
43-50: Avoid displaying negative durations for expired keysWhen a key has expired,
key.expires.getTime() - Date.now()results in a negative duration, which may display confusing negative time strings to the user. Consider handling expired keys explicitly to improve user experience.Apply this diff:
value={ - key.expires ? ms(key.expires.getTime() - Date.now()) : "-" + key.expires + ? key.expires.getTime() > Date.now() + ? ms(key.expires.getTime() - Date.now()) + : "-" + : "-" }apps/dashboard/app/(app)/authorization/layout.tsx (1)
23-23: Consider incorporating theNavbarcomponent to maintain consistent navigationBy returning only
children, theAuthorizationLayoutremoves the previous navigation structure. To ensure a consistent user experience across the application, consider adding theNavbarcomponent here as in other layouts.apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/page.tsx (1)
77-92: Consider consolidating namespace ID displayThere might be redundancy in displaying the namespace ID twice with separate copy buttons (in the Navbar badge and in the Card component). Consider consolidating this information to avoid confusion.
- <Card> - <CardHeader> - <CardTitle>Namespace ID</CardTitle> - <CardDescription> - This is your namespace id. It's used in some API calls. - </CardDescription> - </CardHeader> - <CardContent> - <Code className="flex items-center justify-between w-full h-8 max-w-sm gap-4"> - <pre>{namespace.id}</pre> - <div className="flex items-start justify-between gap-4"> - <CopyButton value={namespace.id} /> - </div> - </Code> - </CardContent> - </Card>apps/dashboard/app/(app)/apis/page.tsx (1)
55-88: Consider extracting the unpaid state UI to a separate componentThe unpaid state UI logic (lines 56-84) contains significant markup that could be extracted into a reusable component for better maintainability.
Consider creating a component like:
type UnpaidStateProps = { onUpgrade: () => void; }; function UnpaidState({ onUpgrade }: UnpaidStateProps) { return ( <div> <section className="flex flex-col gap-4 my-4 md:flex-row md:items-center"> {/* ... search input ... */} </section> <div className="mt-10 flex min-h-[400px] flex-col items-center justify-center space-y-6 rounded-lg border border-dashed px-4 md:mt-24"> {/* ... upgrade message ... */} </div> </div> ); }apps/dashboard/app/(app)/ratelimits/page.tsx (1)
61-94: Consider adding loading states for better UXWhile Suspense is used for RatelimitCard, the empty state could benefit from a loading indicator during the initial data fetch.
Consider adding a loading state:
{workspace.ratelimitNamespaces.length > 0 ? ( <Suspense fallback={<LoadingGrid />}> <ul className="grid grid-cols-1 gap-x-6 gap-y-8 lg:grid-cols-2"> {/* ... existing code ... */} </ul> </Suspense> ) : ( <Suspense fallback={<LoadingPlaceholder />}> <EmptyPlaceholder className="my-4"> {/* ... existing code ... */} </EmptyPlaceholder> </Suspense> )}apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx (1)
86-86: Consider moving SubMenu into PageContent componentThe SubMenu navigation could be encapsulated within the PageContent component for better component organization.
Consider updating the PageContent component to handle secondary navigation:
type PageContentProps = { subNavigation?: { items: NavigationItem[]; segment: string; }; children: React.ReactNode; }; function PageContent({ subNavigation, children }: PageContentProps) { return ( <div> {subNavigation && ( <SubMenu navigation={subNavigation.items} segment={subNavigation.segment} /> )} {children} </div> ); }apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx (1)
57-81: Consider adding aria-label to improve accessibilityThe Navbar implementation is clean and well-structured. Consider adding an aria-label to the copy button for better accessibility.
- <CopyButton value={namespace.id} /> + <CopyButton value={namespace.id} aria-label={`Copy namespace ID ${namespace.id}`} />apps/dashboard/app/(app)/authorization/roles/[roleId]/page.tsx (1)
129-137: Remove redundant div wrapperThere's an unnecessary nested div that can be removed to simplify the DOM structure.
- <div className="flex items-center gap-2"> <div className="flex items-center gap-2"> <UpdateRole role={role} trigger={<Button>Update Role</Button>} /> <DeleteRole role={role} trigger={<Button variant="destructive">Delete Role</Button>} /> </div> - </div>apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx (2)
58-58: Consider increasing the truncation widthThe current truncation width of 120px for the key ID might be too restrictive for longer IDs. Consider increasing it or making it responsive based on viewport width.
74-74: Consider extracting Tailwind classes to a constantThe className contains multiple Tailwind utility classes. Consider extracting these to a constant or a CSS module for better maintainability.
apps/dashboard/app/(app)/authorization/permissions/[permissionId]/page.tsx (1)
95-95: Consider simplifying the tooltip implementationThe current tooltip implementation with nested divs could be simplified. Consider using the tooltip directly on the truncated text:
-<Badge className="w-40 font-mono font-medium ph-no-capture"> +<Badge className="max-w-[10rem] font-mono font-medium ph-no-capture"> <Tooltip> - <TooltipTrigger asChild> - <div className="flex items-center justify-between gap-2 w-full truncate"> - <span className="truncate">{permission.name}</span> - <div> - <CopyButton value={permission.name} /> - </div> - </div> - </TooltipTrigger> + <TooltipTrigger className="flex items-center justify-between gap-2 w-full"> + <span className="truncate">{permission.name}</span> + <CopyButton value={permission.name} /> + </TooltipTrigger> {shouldShowTooltip && ( <TooltipContent> <span className="text-xs font-medium">{permission.name}</span> </TooltipContent> )} </Tooltip> </Badge>Also applies to: 97-111
apps/dashboard/app/(app)/authorization/roles/page.tsx (2)
71-74: Consider improving the limit displayDisplaying "∞" as the limit might be confusing. Consider either:
- Removing the limit display if there truly is no limit
- Showing the actual limit if one exists
- Adding a tooltip explaining the unlimited nature
-{Intl.NumberFormat().format(workspace.roles.length)} /{" "} -{Intl.NumberFormat().format(Number.POSITIVE_INFINITY)} used{" "} +{Intl.NumberFormat().format(workspace.roles.length)} roles
105-105: Consider using semantic class names for grid layoutInstead of using raw grid-cols-12, consider creating semantic class names that describe the purpose of each column:
-className="grid items-center grid-cols-12 px-4 py-2 duration-250 hover:bg-background-subtle" +className="grid items-center grid-[6fr_3fr_3fr] px-4 py-2 duration-250 hover:bg-background-subtle"This makes the layout more maintainable and self-documenting.
apps/dashboard/app/(app)/authorization/permissions/page.tsx (3)
71-74: Consider improving the infinity badge displayThe badge displays "X / ∞ used" which might be confusing for users. Consider either removing the infinity symbol or providing more context about what this represents.
- {Intl.NumberFormat().format(workspace.permissions.length)} /{" "} - {Intl.NumberFormat().format(Number.POSITIVE_INFINITY)} used{" "} + {Intl.NumberFormat().format(workspace.permissions.length)} permissions
105-105: Fix inconsistent spacing in classNameThe className contains extra spaces that should be removed for consistency.
- className="grid items-center grid-cols-12 px-4 py-2 duration-250 hover:bg-background-subtle " + className="grid items-center grid-cols-12 px-4 py-2 duration-250 hover:bg-background-subtle"
107-107: Fix inconsistent spacing in classNameSimilar spacing issue in another className.
- <div className="flex flex-col items-start col-span-6 "> + <div className="flex flex-col items-start col-span-6">apps/dashboard/app/(app)/settings/root-keys/[keyId]/page.tsx (2)
63-74: Improve permission reduction logic readabilityThe permission reduction logic could be more readable with destructuring and early returns.
- const permissionsByApi = permissions.reduce((acc, permission) => { - if (!permission.name.startsWith("api.")) { - return acc; - } - const [_, apiId, _action] = permission.name.split("."); - - if (!acc[apiId]) { - acc[apiId] = []; - } - acc[apiId].push(permission); - return acc; - }, {} as { [apiId: string]: Permission[] }); + const permissionsByApi = permissions.reduce<{ [apiId: string]: Permission[] }>((acc, permission) => { + if (!permission.name.startsWith("api.")) return acc; + + const [_, apiId] = permission.name.split("."); + acc[apiId] = acc[apiId] || []; + acc[apiId].push(permission); + return acc; + }, {});
125-130: Consider combining API filtering operationsThe consecutive filter operations could be combined for better performance.
- const apisWithActivePermissions = apis.filter( - (api) => api.hasActivePermissions - ); - const apisWithoutActivePermissions = apis.filter( - (api) => !api.hasActivePermissions - ); + const [apisWithActivePermissions, apisWithoutActivePermissions] = apis.reduce( + (acc, api) => { + acc[api.hasActivePermissions ? 0 : 1].push(api); + return acc; + }, + [[], []] as [typeof apis, typeof apis] + );apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/page.tsx (2)
88-95: Consider improving the namespace ID displayThe namespace ID display could be more user-friendly with a label.
<Badge key="namespaceId" variant="secondary" - className="flex justify-between w-full gap-2 font-mono font-medium ph-no-capture" + className="flex items-center justify-between w-full gap-2 font-mono font-medium ph-no-capture" > + <span className="text-content-subtle">Namespace:</span> {props.params.namespaceId} <CopyButton value={props.params.namespaceId} /> </Badge>
103-111: Consider adding loading state feedbackThe loading state could be more informative to users.
<Suspense fallback={ <EmptyPlaceholder> <EmptyPlaceholder.Icon> <Loading /> </EmptyPlaceholder.Icon> + <EmptyPlaceholder.Title>Loading logs...</EmptyPlaceholder.Title> + <EmptyPlaceholder.Description>Please wait while we fetch the data.</EmptyPlaceholder.Description> </EmptyPlaceholder> } >apps/dashboard/app/(app)/ratelimits/[namespaceId]/page.tsx (2)
153-169: Consider improving grid responsivenessThe grid layout for metrics could benefit from better responsive behavior on smaller screens.
Consider updating the grid classes:
- <CardContent className="grid grid-cols-3 divide-x"> + <CardContent className="grid grid-cols-1 sm:grid-cols-2 md:grid-cols-3 divide-y sm:divide-y-0 sm:divide-x">
182-242: Maintain consistency in empty state messagesThe empty state message "Ratelimit something or change the range" could be more consistent with other empty states in the application.
Consider using a more consistent message:
- Ratelimit something or change the range + No data available for the selected rangeapps/dashboard/app/(app)/audit/[bucket]/page.tsx (2)
130-172: Consider memoizing filter componentsThe filter components could benefit from memoization to prevent unnecessary rerenders, especially when handling multiple filter changes.
Consider wrapping the filter components with
memo:import { memo } from 'react'; const MemoizedFilter = memo(Filter); const MemoizedUserFilter = memo(UserFilter); const MemoizedRootKeyFilter = memo(RootKeyFilter);
175-191: Enhance loading state feedbackThe loading state could provide more context about what's being loaded.
Consider adding a loading message:
<EmptyPlaceholder> <EmptyPlaceholder.Icon> <Loading /> </EmptyPlaceholder.Icon> + <EmptyPlaceholder.Title>Loading audit logs...</EmptyPlaceholder.Title> </EmptyPlaceholder>apps/dashboard/app/(app)/apis/[apiId]/page.tsx (1)
170-189: Consider adding tooltips to metricsThe metrics would benefit from tooltips explaining what each metric represents, especially for new users.
Consider adding a title attribute or a dedicated tooltip component:
- <Metric label="Total Keys" value={formatNumber(keySpace?.sizeApprox ?? 0)} /> + <Metric + label="Total Keys" + value={formatNumber(keySpace?.sizeApprox ?? 0)} + title="Total number of API keys created for this API" + />apps/dashboard/app/(app)/settings/billing/page.tsx (3)
376-377: Consider extracting number formatting logic into a utility functionThe number formatting logic is repeated in multiple places. Consider extracting it into a reusable utility function to improve maintainability.
+const formatCompactNumber = (value: number): string => { + return Intl.NumberFormat("en-US", { notation: "compact" }).format(value); +}; -Intl.NumberFormat("en-US", { notation: "compact" }).format(included) +formatCompactNumber(included) -Intl.NumberFormat("en-US", { notation: "compact" }).format(props.used) +formatCompactNumber(props.used) -Intl.NumberFormat("en-US", { notation: "compact" }).format(forecast) +formatCompactNumber(forecast)Also applies to: 404-407, 465-468, 472-475
537-541: Consider improving date handling robustnessThe current date handling could be enhanced to handle edge cases and provide consistent formatting.
-invoice.custom_fields?.find((f) => f.name === "Billing Period")?.value ?? - (invoice.due_date ? new Date(invoice.due_date * 1000).toDateString() : null) +const getBillingPeriod = (invoice: Stripe.Invoice): string => { + const customField = invoice.custom_fields?.find((f) => f.name === "Billing Period")?.value; + if (customField) return customField; + + if (invoice.due_date) { + const date = new Date(invoice.due_date * 1000); + return date.toLocaleDateString('en-US', { + year: 'numeric', + month: 'long', + day: 'numeric' + }); + } + + return 'N/A'; +}; + +getBillingPeriod(invoice)
416-432: Enhance tooltip accessibilityWhile the tooltip implementation is good, it could benefit from additional accessibility attributes.
<TooltipTrigger style={{ width: percentage(props.used - tier.firstUnit, max), }} + aria-label={`Usage tier ${tier.firstUnit} to ${tier.lastUnit ?? '∞'}`} >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/keys/[keyAuthId]/new/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/keys/[keyAuthId]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/settings/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/apis/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/audit/[...path]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/audit/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/authorization/permissions/[permissionId]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/authorization/permissions/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/authorization/roles/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/debug/[...path]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/debug/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/default.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/identities/[identityId]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/identities/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/logs/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/monitors/[...path]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/monitors/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/overview/[...path]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/overview/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/ratelimits/[namespaceId]/logs/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/ratelimits/[namespaceId]/overrides/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/ratelimits/[namespaceId]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/ratelimits/[namespaceId]/settings/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/ratelimits/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/secrets/[...path]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/secrets/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/semantic-cache/[...path]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/semantic-cache/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/settings/[...path]/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/settings/page.tsx(0 hunks)apps/dashboard/app/(app)/@breadcrumb/success/page.tsx(0 hunks)apps/dashboard/app/(app)/apis/[apiId]/constants.ts(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx(7 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/page.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/page.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/layout.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/loading.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/page.tsx(3 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx(2 hunks)apps/dashboard/app/(app)/apis/client.tsx(1 hunks)apps/dashboard/app/(app)/apis/loading.tsx(2 hunks)apps/dashboard/app/(app)/apis/page.tsx(2 hunks)apps/dashboard/app/(app)/audit/[bucket]/page.tsx(3 hunks)apps/dashboard/app/(app)/authorization/constants.ts(1 hunks)apps/dashboard/app/(app)/authorization/layout.tsx(1 hunks)apps/dashboard/app/(app)/authorization/permissions/[permissionId]/page.tsx(2 hunks)apps/dashboard/app/(app)/authorization/permissions/page.tsx(2 hunks)apps/dashboard/app/(app)/authorization/roles/[roleId]/page.tsx(2 hunks)apps/dashboard/app/(app)/authorization/roles/[roleId]/tree.tsx(1 hunks)apps/dashboard/app/(app)/authorization/roles/page.tsx(2 hunks)apps/dashboard/app/(app)/identities/[identityId]/page.tsx(3 hunks)apps/dashboard/app/(app)/identities/layout.tsx(0 hunks)apps/dashboard/app/(app)/identities/page.tsx(5 hunks)apps/dashboard/app/(app)/layout.tsx(1 hunks)apps/dashboard/app/(app)/logs/logs-page.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/constants.ts(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/layout.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/page.tsx(3 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/[overrideId]/page.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/page.tsx(3 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/page.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/loading.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/page.tsx(2 hunks)apps/dashboard/app/(app)/settings/billing/page.tsx(18 hunks)apps/dashboard/app/(app)/settings/billing/plans/constants.ts(1 hunks)apps/dashboard/app/(app)/settings/billing/plans/page.tsx(2 hunks)apps/dashboard/app/(app)/settings/constants.ts(1 hunks)apps/dashboard/app/(app)/settings/general/page.tsx(2 hunks)apps/dashboard/app/(app)/settings/layout.tsx(0 hunks)apps/dashboard/app/(app)/settings/loading.tsx(2 hunks)apps/dashboard/app/(app)/settings/root-keys/[keyId]/history/loading.tsx(0 hunks)apps/dashboard/app/(app)/settings/root-keys/[keyId]/history/page.tsx(0 hunks)apps/dashboard/app/(app)/settings/root-keys/[keyId]/page-layout.tsx(3 hunks)apps/dashboard/app/(app)/settings/root-keys/[keyId]/page.tsx(7 hunks)apps/dashboard/app/(app)/settings/root-keys/[keyId]/permissions/loading.tsx(0 hunks)
⛔ Files not processed due to max files limit (18)
- apps/dashboard/app/(app)/settings/root-keys/new/page.tsx
- apps/dashboard/app/(app)/settings/root-keys/page.tsx
- apps/dashboard/app/(app)/settings/team/page.tsx
- apps/dashboard/app/(app)/settings/user/page.tsx
- apps/dashboard/app/(app)/settings/vercel/page.tsx
- apps/dashboard/app/(app)/workspace-navigations.tsx
- apps/dashboard/components/dashboard/breadcrumb-skeleton.tsx
- apps/dashboard/components/dashboard/navbar.tsx
- apps/dashboard/components/dashboard/page-header.tsx
- apps/dashboard/components/navbar.tsx
- apps/dashboard/components/page-content.tsx
- apps/dashboard/components/ui/breadcrumb.tsx
- apps/dashboard/components/ui/metric.tsx
- internal/icons/src/icons/gauge.tsx
- internal/icons/src/icons/gear.tsx
- internal/icons/src/icons/input-search.tsx
- internal/icons/src/icons/shield-key.tsx
- internal/icons/src/index.ts
💤 Files with no reviewable changes (41)
- apps/dashboard/app/(app)/@breadcrumb/semantic-cache/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/logs/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/default.tsx
- apps/dashboard/app/(app)/@breadcrumb/overview/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/monitors/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/secrets/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/debug/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/success/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/ratelimits/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/monitors/[...path]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/audit/[...path]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/audit/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/apis/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/authorization/roles/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/semantic-cache/[...path]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/authorization/permissions/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/settings/[...path]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/authorization/permissions/[permissionId]/page.tsx
- apps/dashboard/app/(app)/settings/root-keys/[keyId]/history/loading.tsx
- apps/dashboard/app/(app)/@breadcrumb/debug/[...path]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/identities/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/overview/[...path]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/secrets/[...path]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/settings/page.tsx
- apps/dashboard/app/(app)/settings/layout.tsx
- apps/dashboard/app/(app)/identities/layout.tsx
- apps/dashboard/app/(app)/@breadcrumb/authorization/roles/[roleId]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/ratelimits/[namespaceId]/logs/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/keys/[keyAuthId]/new/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/settings/page.tsx
- apps/dashboard/app/(app)/settings/root-keys/[keyId]/permissions/loading.tsx
- apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/ratelimits/[namespaceId]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/page.tsx
- apps/dashboard/app/(app)/settings/root-keys/[keyId]/history/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/identities/[identityId]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/ratelimits/[namespaceId]/settings/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/apis/[apiId]/keys/[keyAuthId]/page.tsx
- apps/dashboard/app/(app)/@breadcrumb/ratelimits/[namespaceId]/overrides/page.tsx
✅ Files skipped from review due to trivial changes (2)
- apps/dashboard/app/(app)/authorization/roles/[roleId]/tree.tsx
- apps/dashboard/app/(app)/authorization/constants.ts
🧰 Additional context used
🪛 Biome (1.9.4)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx
[error] 90-90: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
apps/dashboard/app/(app)/identities/[identityId]/page.tsx
[error] 136-136: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 173-173: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
apps/dashboard/app/(app)/settings/root-keys/[keyId]/page-layout.tsx
[error] 4-4: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
🔇 Additional comments (44)
apps/dashboard/app/(app)/ratelimits/[namespaceId]/layout.tsx (2)
30-30: LGTM! Clean and focused layout implementation
The simplified layout correctly maintains its core responsibilities:
- Authorization check via
getTenantId() - Namespace existence verification
- Proper error handling with
notFound() - Clean rendering of child components
This change aligns well with the single responsibility principle.
Line range hint 1-30: Verify navigation implementation in parent layouts
The removal of the navigation structure from this layout aligns with the PR's objective to replace breadcrumbs with a top navbar. However, we should verify that the navigation is properly handled by parent layouts to maintain a consistent user experience.
Let's check the parent layout files:
✅ Verification successful
Navigation implementation is properly handled by parent layouts
The removal of navigation from the rate limits namespace layout is correct as navigation is comprehensively implemented in the parent app layout through:
- A responsive navigation system with both desktop and mobile views
DesktopSidebarcomponent providing workspace and resource navigationMobileSideBarcomponent for mobile-friendly navigation- Proper breadcrumb support via the layout's
breadcrumbprop
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check parent layout files for navigation implementation
# Expected: Find Navbar component usage in parent layouts
# Find all layout files in the app directory
fd "layout.tsx" apps/dashboard/app
# Check for Navbar component usage in layout files
rg -A 5 "Navbar" --type tsx
Length of output: 482
Script:
#!/bin/bash
# Let's check the parent layouts content, particularly focusing on the app layout
# which is most likely to contain the navbar implementation
# Check the main app layout
cat "apps/dashboard/app/(app)/layout.tsx"
# Also check for any navigation-related components in the components directory
fd -e tsx -e ts "nav|header" apps/dashboard/components
Length of output: 2932
Script:
#!/bin/bash
# Let's check the content of the navbar components and the mobile/desktop sidebars
cat "apps/dashboard/components/dashboard/navbar.tsx"
cat "apps/dashboard/app/(app)/desktop-sidebar.tsx"
cat "apps/dashboard/app/(app)/mobile-sidebar.tsx"
Length of output: 10824
apps/dashboard/app/(app)/ratelimits/[namespaceId]/constants.ts (1)
1-23: Verify the completeness of navigation items
The navigation structure seems to align with the deleted breadcrumb files mentioned in the summary. However, let's verify that all necessary routes are covered.
✅ Verification successful
Based on the search results, I can verify that all the necessary routes are covered in the navigation structure. The fd command shows the following page components under the ratelimits section:
/ratelimits/[namespaceId]/logs/page.tsx/ratelimits/[namespaceId]/overrides/[overrideId]/page.tsx/ratelimits/[namespaceId]/overrides/page.tsx/ratelimits/[namespaceId]/page.tsx(Overview)/ratelimits/[namespaceId]/settings/page.tsx
These exactly match the navigation items defined in the constants file:
- Overview (
/ratelimits/${namespaceId}) - Settings (
/ratelimits/${namespaceId}/settings) - Logs (
/ratelimits/${namespaceId}/logs) - Overrides (
/ratelimits/${namespaceId}/overrides)
Navigation structure is complete and accurate
The navigation items in the constants file perfectly align with all the page components found in the ratelimits section of the codebase. Each route has a corresponding page implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all page components under ratelimits to ensure we haven't missed any routes
fd -e tsx -e ts "page\.(tsx|ts)$" "apps/dashboard/app/(app)/ratelimits"
# Search for any remaining breadcrumb references that might need updating
rg -g "!*.{log,lock}" -i "breadcrumb" "apps/dashboard"
Length of output: 22720
apps/dashboard/app/(app)/apis/[apiId]/layout.tsx (1)
29-29: Verify the removal of layout structure
The layout has been significantly simplified to only return child components. While this aligns with the PR's objective to replace breadcrumbs with a top navbar, we should verify that:
- The navigation functionality is properly handled by the new top navbar
- The removed components (PageHeader, CreateKeyButton, etc.) are appropriately relocated
Let's verify the new navigation structure is in place:
✅ Verification successful
Navigation structure has been properly replaced
The removal of the layout structure in apps/dashboard/app/(app)/apis/[apiId]/layout.tsx is safe as the navigation functionality has been properly handled by:
-
A new top-level navigation system using two navbar components:
apps/dashboard/components/dashboard/navbar.tsxapps/dashboard/components/navbar.tsx
-
The breadcrumb functionality is now properly integrated into the new navbar component with full support for:
- Hierarchical navigation
- Active state tracking
- Icon support
- Proper accessibility attributes
-
The parent layout at
apps/dashboard/app/(app)/layout.tsxprovides the necessary structure with:- Mobile and desktop navigation support
- Proper workspace context
- Content area with appropriate styling and layout
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the new navigation implementation exists and the old components are properly relocated
# Check if there's a new top-level navbar component
echo "Checking for new navbar implementation..."
fd -e tsx -e ts navbar
# Verify the relocated components
echo "Checking where the removed components are now used..."
rg "CreateKeyButton|PageHeader|Badge" -t tsx -t ts
# Check for any remaining breadcrumb references that might need cleanup
echo "Checking for any remaining breadcrumb references..."
rg -i "breadcrumb" -t tsx -t ts
Length of output: 572
Script:
#!/bin/bash
# Let's modify our search approach to avoid the file type issue
# Check the content of the navbar components to verify they handle navigation
echo "=== Checking navbar implementation ==="
cat apps/dashboard/components/dashboard/navbar.tsx apps/dashboard/components/navbar.tsx 2>/dev/null
# Search for removed components without file type restrictions
echo -e "\n=== Checking relocated components ==="
rg "CreateKeyButton|PageHeader|Badge"
# Search for breadcrumb references without file type restrictions
echo -e "\n=== Checking breadcrumb references ==="
rg -i "breadcrumb"
# Check the parent layout to understand the new structure
echo -e "\n=== Checking parent layout structure ==="
cat apps/dashboard/app/\(app\)/layout.tsx 2>/dev/null
Length of output: 57923
apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/[overrideId]/page.tsx (3)
2-4: LGTM! Import statements are well-organized.
The new imports align with the PR objective of implementing a new navigation structure.
Also applies to: 8-8
34-34: LGTM! Appropriate data fetching enhancement.
The addition of the namespace relation is necessary for the new navigation structure and is properly implemented.
42-96: Verify breadcrumb replacement across all routes.
The implementation looks good, but let's ensure consistency across all routes that previously used breadcrumbs.
✅ Verification successful
Breadcrumb implementation is consistently updated across all routes
The search results confirm that all routes in the application are using the new Navbar.Breadcrumbs component consistently. There are no instances of old breadcrumb implementations remaining, and the new navigation pattern is properly implemented across all pages including:
- Rate limits section
- Identity management
- Settings pages
- API management
- Authorization
- Audit pages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining breadcrumb implementations that need to be updated
# Search for old breadcrumb components or implementations
rg -l 'Breadcrumb|breadcrumb' 'apps/dashboard/app/(app)' --type tsx
# Search for routes that might need the new Navbar
fd . 'apps/dashboard/app/(app)' -e tsx -E '**/components/**' --exec grep -L 'Navbar'
Length of output: 6666
Script:
#!/bin/bash
# Let's try a different approach to find breadcrumb usage with ripgrep
# Using -t tsx to specify TypeScript/TSX files
rg -i 'breadcrumb' apps/dashboard/app --type-add 'tsx:*.tsx' -t tsx
# Also check for the new Navbar.Breadcrumbs pattern to see migration progress
rg -i 'Navbar\.Breadcrumbs' apps/dashboard/app --type-add 'tsx:*.tsx' -t tsx
Length of output: 40829
apps/dashboard/app/(app)/apis/client.tsx (1)
36-36: Verify layout spacing with new navbar
The removal of top margin (my-4 → mb-4) appears to be related to the new navigation structure. Please ensure this spacing works well with the new top navbar across all viewport sizes.
✅ Verification successful
Based on the context gathered, I can now generate the final response:
Margin change is correct with new navbar structure
The removal of top margin (my-4 → mb-4) is correct as the component is now properly positioned under the new <Navbar> component. The layout structure shows the navbar is consistently implemented across pages, providing the necessary top spacing, making the top margin redundant.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent margin changes across other similar components
# that might have been affected by the navbar changes
# Look for other components that might need similar margin adjustments
rg -l 'className=".*my-4.*"' --type tsx
Length of output: 69
Script:
#!/bin/bash
# Let's try with a more generic search approach without file type restriction
# First, let's look for similar margin classes in the codebase
rg 'className="[^"]*\bm[ytb]-4\b[^"]*"'
# Also check the specific file to understand the component's context
cat "apps/dashboard/app/(app)/apis/client.tsx"
Length of output: 14629
Script:
#!/bin/bash
# Let's check the navbar implementation and surrounding layout structure
# to understand the context of the margin change
# Check the app layout file that might contain the navbar
cat "apps/dashboard/app/(app)/layout.tsx"
# Also check for any recent changes to the navbar component
rg -l "navbar" --type tsx apps/dashboard/
Length of output: 2754
Script:
#!/bin/bash
# Let's search for navbar-related files without file type restriction
rg -l "navbar"
# Also check the specific page layout to understand the component's position
cat "apps/dashboard/app/(app)/apis/page.tsx"
Length of output: 5279
apps/dashboard/app/(app)/identities/[identityId]/page.tsx (3)
22-24: LGTM: Clean import structure for new navigation components.
The new imports align well with the PR objective of replacing breadcrumbs with a top navbar.
60-202: Overall implementation looks solid.
The new navigation structure is well-implemented while maintaining all existing functionality. The code is clean, well-organized, and follows a consistent pattern.
🧰 Tools
🪛 Biome (1.9.4)
[error] 136-136: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 173-173: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
61-75: Verify truncation behavior for long identity IDs.
The implementation looks good, but let's ensure the truncation at 200px width works well for various identity ID lengths.
apps/dashboard/app/(app)/settings/loading.tsx (2)
1-1: LGTM: Clean PageContent integration
The PageContent component is properly imported and integrated, maintaining the loading state structure while adopting the new navigation pattern.
Also applies to: 6-6, 31-31
Line range hint 7-30: Verify loading state matches actual content structure
The skeleton structure appears to mirror a settings page layout with three main sections. Please ensure this matches the actual content layout of the settings page.
Consider extracting these skeleton sections into reusable components if they're shared across different settings pages.
apps/dashboard/app/(app)/ratelimits/loading.tsx (1)
Line range hint 7-14: LGTM: Well-structured responsive layout
Good use of responsive utility classes for different breakpoints, ensuring a smooth transition from mobile to desktop views.
apps/dashboard/app/(app)/apis/loading.tsx (1)
1-1: Verify PageContent component implementation
The PageContent component is consistently implemented across all loading states. Let's verify its implementation and styling to ensure it properly integrates with the new top navbar.
✅ Verification successful
Let me gather more information about the PageContent component implementation since the previous search didn't show its definition.
PageContent component is correctly implemented and consistently used
The PageContent component is a simple wrapper that adds padding to its children and is consistently imported and used across all pages and loading states in the dashboard app. Its implementation aligns well with the new top navbar as it only provides basic padding without any conflicting styles.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find PageContent component implementation and its usage
ast-grep --pattern 'export $_ PageContent = $$$'
# Find all loading state implementations using PageContent
fd -e tsx -e ts -e jsx -e js | xargs ast-grep --pattern 'import { PageContent } from "@/components/page-content"'
Length of output: 7252
Script:
#!/bin/bash
# Search for PageContent component definition
fd page-content
rg -A 5 "export.*PageContent" --type tsx --type jsx
Length of output: 142
Script:
#!/bin/bash
# Let's examine the PageContent component implementation
cat apps/dashboard/components/page-content.tsx
# Also check for any usage patterns
rg -l "PageContent" apps/dashboard/app
Length of output: 2173
apps/dashboard/app/(app)/apis/[apiId]/loading.tsx (1)
1-3: LGTM! Imports and component declaration are well-structured.
The component follows Next.js conventions with proper imports and async loading component declaration.
apps/dashboard/app/(app)/settings/billing/plans/constants.ts (1)
1-53: 🛠️ Refactor suggestion
Consider improvements for maintainability and security
A few suggestions to enhance this constants file:
- Add TypeScript types for better maintainability:
type Tier = {
name: string;
href: string;
price: number | string;
description: string;
buttonText: string;
features: string[];
footnotes: string[];
};
export const tiers: Record<'free' | 'pro' | 'custom', Tier> = {
// ... existing tiers
};- Move sensitive values to environment variables:
export const tiers = {
free: {
// ... other properties
price: 0,
features: [
`${process.env.NEXT_PUBLIC_FREE_TIER_API_KEYS} API keys`,
`${process.env.NEXT_PUBLIC_FREE_TIER_VERIFICATIONS} successful verifications / month`,
// ... other features
],
},
pro: {
// ... other properties
price: process.env.NEXT_PUBLIC_PRO_TIER_PRICE,
// ...
},
};- Consider implementing rate limiting or validation for the external calendar link to prevent potential abuse.
apps/dashboard/app/(app)/settings/billing/plans/page.tsx (2)
24-33: Navigation implementation aligns with PR objectives
The new Navbar component with breadcrumbs is implemented correctly, maintaining navigation context while upgrading the UI.
43-133: Enhance accessibility and performance of pricing tiers
Several improvements could be made to the pricing tiers section:
- Extract the pricing tier card to a separate component for better maintainability:
type PricingTierProps = {
tier: keyof typeof tiers;
workspace: typeof workspace;
};
function PricingTierCard({ tier, workspace }: PricingTierProps) {
const tierData = tiers[tier];
return (
<div className="border border-border flex w-full flex-col justify-between rounded-lg bg-white dark:bg-black p-8 lg:w-1/3 xl:p-10">
{/* ... existing markup ... */}
</div>
);
}- Improve accessibility:
- <div className="flex items-center justify-between gap-x-4">
+ <div className="flex items-center justify-between gap-x-4" role="heading" aria-level={2}>- Add proper aria-labels to the checkmark SVGs:
- aria-hidden="true"
+ aria-label={`Feature included: ${feature}`}- Consider using Next.js Image component for better performance:
import Image from 'next/image';
// ... in the checkmark section:
<Image
src="/icons/checkmark.svg"
width={20}
height={24}
alt=""
aria-hidden="true"
/>✅ Verification successful
Accessibility and component structure improvements recommended
The suggested improvements are valid and align with the codebase's patterns:
-
The pricing tier card extraction is beneficial as there are no existing dedicated pricing components, promoting better code organization.
-
Accessibility improvements are consistent with the codebase's practices, as evidenced by extensive use of
aria-labelattributes in other components. -
SVG accessibility enhancement is important as the codebase shows careful attention to accessibility in other interactive elements.
-
However, the Next.js Image suggestion should be reconsidered since the checkmark is a simple SVG icon that's better served inline for this use case.
Specific recommendations:
- Extract the pricing tier card as suggested
- Add
role="heading"andaria-level={2}to the tier headings - Add descriptive
aria-labelto the checkmark SVGs - Keep the SVG implementation but enhance its accessibility
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for similar pricing tier implementations that might need consolidation
rg -l "tier.*price.*features" --type typescript
# Look for other SVG implementations that might need accessibility improvements
rg -A 5 "<svg.*aria-hidden" --type typescript
Length of output: 167
Script:
#!/bin/bash
# Check for similar pricing tier implementations that might need consolidation
rg -l "tier.*price.*features"
# Look for other SVG implementations that might need accessibility improvements
rg -A 5 "<svg.*aria-hidden"
# Check for existing component structure
fd "pricing|tier" --type f
# Look for existing accessibility patterns
rg "aria-label|role=\"heading\""
Length of output: 3319
apps/dashboard/app/(app)/apis/[apiId]/constants.ts (1)
1-17: LGTM!
The navigation constants are well-defined and correctly construct the URLs, enhancing navigation throughout the application.
apps/dashboard/app/(app)/logs/logs-page.tsx (2)
18-23: LGTM: Clean implementation of the new navigation structure
The Navbar implementation with breadcrumbs follows the new navigation pattern consistently, providing clear hierarchy and navigation with the Layers3 icon.
29-30: Verify the necessity of negative margin
The comment suggests this is a workaround for tooltip display. Consider if this could be addressed more systematically through the chart component itself.
Consider addressing the chart spacing issue at the component level rather than using negative margins as a workaround.
✅ Verification successful
Based on the search results, I can see that the negative margin is being used in the context of the LogsChart component to compensate for tooltip display space. The code shows that this is a specific implementation detail where the chart component needs extra space for tooltip rendering, and the negative margin is used to reclaim that space in the layout.
The negative margin is necessary and intentionally implemented
The comment accurately describes the purpose of the negative margin - it's compensating for extra space needed by the chart's tooltip functionality. This is a valid approach given that:
- The chart component (LogsChart) uses Recharts with tooltips that need overflow space
- The negative margin specifically targets the chart's extra space without affecting other components
- The comment clearly documents the reason for this implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar negative margin patterns that might indicate a systemic issue
rg "-mt-\d+" --type tsx
Length of output: 422
Script:
#!/bin/bash
# Search for similar negative margin patterns that might indicate a systemic issue
rg "'-mt-\d+'" --type tsx
# Also search for the chart component usage to understand the context
rg "Chart" -A 5 -B 5 --type tsx
# Look for any tooltip-related styles or components
rg "tooltip" -i --type tsx
Length of output: 176
Script:
#!/bin/bash
# Search for similar negative margin patterns that might indicate a systemic issue
rg "'-mt-\d+'"
# Also search for the chart component usage to understand the context
rg "Chart" -A 5 -B 5
# Look for any tooltip-related styles or components
rg "tooltip" -i
Length of output: 66679
apps/dashboard/app/(app)/settings/general/page.tsx (2)
39-58: LGTM: Well-structured content layout
The content layout is clean and well-organized with appropriate spacing and consistent use of Card components for displaying workspace information.
30-38: Verify the necessity of dual navigation components
The implementation uses both Navbar and SubMenu components. While this provides detailed navigation, verify if this pattern is consistent across similar pages and doesn't create confusion.
Consider documenting this navigation pattern in the UI guidelines if it's intended to be a reusable pattern across settings pages.
✅ Verification successful
The previous regex had special characters that needed escaping. Let's try a simpler search to understand the navigation patterns in the codebase.
Let's try without specifying the file type flag and instead use the extension patterns that are more commonly supported.
The output shows that SubMenu with navigation prop is used consistently across many pages in the dashboard app. Let's examine a couple of these files to understand if the dual navigation pattern (Navbar + SubMenu) is indeed a standard pattern.
The dual navigation pattern is a consistent design pattern
The implementation of both Navbar and SubMenu components is a deliberate and consistent pattern across the application. This is evidenced by:
- The pattern is used consistently across settings pages, API pages, and other sections
Navbarprovides top-level navigation with breadcrumbs and actionsSubMenuhandles contextual navigation within specific sections- The hierarchy is clear:
Navbarfor global context,SubMenufor section-specific navigation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances of dual navigation
rg "SubMenu.*navigation={navigation}" --type tsx
Length of output: 186
Script:
#!/bin/bash
# Search for SubMenu usage patterns in TSX files
rg "SubMenu" -A 2 --type tsx
# Also search for files containing both Navbar and SubMenu
rg -l "Navbar.*SubMenu|SubMenu.*Navbar" --type tsx
Length of output: 137
Script:
#!/bin/bash
# Search for SubMenu usage patterns in all files
rg "SubMenu" -A 2 -t ts -t tsx -t jsx
# Find files containing both Navbar and SubMenu components
rg -l "Navbar.*SubMenu|SubMenu.*Navbar" -t ts -t tsx -t jsx
# Also look for the specific navigation pattern implementation
fd -e ts -e tsx -e jsx --exec grep -l "SubMenu.*navigation="
Length of output: 1090
Script:
#!/bin/bash
# Let's examine one of these files to understand the pattern
cat "./apps/dashboard/app/(app)/settings/user/page.tsx"
# Also check the content of a different section to confirm consistency
cat "./apps/dashboard/app/(app)/apis/[apiId]/page.tsx"
Length of output: 14713
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/page.tsx (2)
34-48: LGTM: Well-implemented hierarchical navigation
The breadcrumb implementation provides clear navigation hierarchy with appropriate use of the isIdentifier prop for the API name.
51-58: Verify the purpose of ph-no-capture class
The Badge component includes a ph-no-capture class which might be related to analytics or security.
✅ Verification successful
The ph-no-capture class is used to prevent PostHog analytics capture
Based on the evidence found in the codebase:
- The project uses PostHog for analytics (multiple PostHog imports and configurations found)
- The
ph-no-captureclass is consistently used on elements containing sensitive information like IDs and codes - This naming convention (
ph-no-capture) aligns with PostHog's pattern for excluding elements from analytics capture
The class is correctly used here to prevent sensitive API IDs from being captured in analytics data.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances of ph-no-capture to understand its usage
rg "ph-no-capture" --type tsx --type css
# Search for any documentation about this class
fd -t f "ph-no-capture" -x grep -l "ph-no-capture" {} \;
Length of output: 126
Script:
#!/bin/bash
# Search for ph-no-capture using ripgrep without file type restrictions
rg "ph-no-capture"
# Search for any files containing ph-no-capture
fd . -t f -x grep -l "ph-no-capture" {} \;
Length of output: 4759
Script:
#!/bin/bash
# Search for any PostHog related imports or configurations to verify if ph-no-capture is related to PostHog analytics
rg -i "posthog"
# Search for any documentation or comments mentioning analytics capture or related terms
rg -i "analytics.*capture|capture.*analytics"
Length of output: 7401
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/page.tsx (2)
39-63: LGTM! Navigation implementation enhances user experience
The new Navbar implementation provides clear hierarchical navigation while maintaining important context and actions. The breadcrumb path (APIs > API Name > Keys) is intuitive, and the addition of the API ID badge with copy functionality improves usability.
65-71: Verify SubMenu navigation items
The navigation structure looks good, but let's verify the navigation items passed to the SubMenu component.
✅ Verification successful
Navigation structure is correctly implemented
The navigation items passed to the SubMenu component are properly defined in the constants file. The structure includes:
- Overview link pointing to
/apis/${apiId} - Keys link pointing to
/apis/${apiId}/keys/${keyAuthId}
Both items have the correct segments and href patterns matching the routing structure.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the navigation items defined in the constants file
rg -A 10 "export const navigation" "apps/dashboard/app/(app)/apis/[apiId]/constants.ts"
Length of output: 335
apps/dashboard/app/(app)/layout.tsx (1)
46-48: Verify padding consistency across pages
The removal of padding classes (p-4 lg:p-8) from the children container could affect content spacing. Ensure that the PageContent component consistently handles padding across all pages.
✅ Verification successful
Let me gather more information about the PageContent component's implementation to verify the padding consistency.
Padding is consistently handled by PageContent component
The removal of padding from the layout's children container is not a concern because:
- The PageContent component consistently applies
p-4padding to all its children - All pages in the dashboard use the PageContent component as evidenced by the extensive usage across multiple routes
- The padding responsibility has been properly centralized in the PageContent component
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check PageContent component implementation and usage
rg -l "PageContent" "apps/dashboard"
ast-grep --pattern 'className="$_"' "apps/dashboard/components/page-content.tsx"
Length of output: 2082
Script:
#!/bin/bash
# Check PageContent component implementation
cat apps/dashboard/components/page-content.tsx
# Check a few example pages to see how PageContent is used
cat apps/dashboard/app/\(app\)/apis/page.tsx
cat apps/dashboard/app/\(app\)/settings/general/page.tsx
Length of output: 5933
apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/page.tsx (1)
51-71: LGTM! Navigation implementation is consistent with other pages
The Navbar implementation follows the same pattern as other pages, providing clear hierarchical navigation and context through the namespace ID badge.
apps/dashboard/app/(app)/apis/page.tsx (1)
46-54: LGTM: Clean navigation implementation
The new Navbar implementation provides a clear hierarchical structure with proper icon and action button placement.
apps/dashboard/app/(app)/ratelimits/page.tsx (1)
52-59: LGTM: Consistent navigation pattern
The Navbar implementation maintains consistency with the APIs page, following the same pattern for breadcrumbs and actions.
apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx (1)
59-83: LGTM: Well-structured navigation with proper hierarchy
The Navbar implementation effectively handles complex navigation with proper breadcrumbs hierarchy and API identifier display.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/overrides/page.tsx (1)
Line range hint 1-56: LGTM! Clean implementation of new navigation imports
The imports and component setup are well-structured, properly introducing the new navigation components while maintaining existing functionality.
apps/dashboard/app/(app)/identities/page.tsx (3)
31-55: LGTM! Robust implementation of workspace and feature checks
The implementation properly handles workspace existence and beta feature access, with clean error paths.
57-81: LGTM! Clean and consistent navigation implementation
The Navbar implementation follows the established pattern and maintains simplicity for top-level pages.
97-100: LGTM! Improved query condition formatting
The or condition formatting enhances code readability while maintaining the search functionality.
apps/dashboard/app/(app)/authorization/roles/[roleId]/page.tsx (2)
Line range hint 1-111: LGTM! Clean integration of navigation components
The implementation successfully integrates the new navigation components while maintaining the complex permission handling logic.
140-152: LGTM! Well-structured layout implementation
The PageContent implementation provides a clean and consistent layout structure with proper spacing and organization.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx (1)
84-116: LGTM! Well-organized settings layout
The settings components are well-organized with a clear visual hierarchy and proper spacing.
apps/dashboard/app/(app)/ratelimits/[namespaceId]/page.tsx (1)
127-149: Well-structured navigation implementation!
The new navigation bar provides a clear hierarchy and improved user experience with:
- Intuitive breadcrumb navigation
- Accessible namespace identifier
- Convenient copy functionality
apps/dashboard/app/(app)/audit/[bucket]/page.tsx (1)
123-127: Clean and consistent navigation implementation!
The navigation maintains consistency with other pages while keeping a simpler structure appropriate for the audit page.
apps/dashboard/app/(app)/apis/[apiId]/page.tsx (1)
144-165: Excellent navigation implementation with rich features!
The navigation provides:
- Clear context with API information
- Quick access to key creation
- Easy API ID reference and copying
apps/dashboard/app/(app)/settings/billing/page.tsx (1)
9-13: LGTM: Clean navigation structure implementation
The new navigation structure with Navbar and SubMenu components provides a consistent and improved user experience, aligning well with the PR's objective of replacing breadcrumbs.
Also applies to: 26-30, 47-57
apps/dashboard/app/(app)/authorization/permissions/[permissionId]/page.tsx
Show resolved
Hide resolved
apps/dashboard/app/(app)/ratelimits/[namespaceId]/logs/page.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/page.tsx
Outdated
Show resolved
Hide resolved
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/page.tsx
Outdated
Show resolved
Hide resolved
|
The breadcrumbs should be: we'll remove some breadcrumbs from the middle, rather than the end keep the beginning and end |
I thought about that then decided not to do it because there is plenty of space on the screen. I think we can keep it as is, wdyt? |
it's not about space on the screen, it's about showing the most important parts |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
apps/dashboard/app/(app)/authorization/permissions/page.tsx (1)
Line range hint
47-52: Consider moving soft delete filtering to database queryThe comment suggests manual filtering is used due to Drizzle limitations. However, this could be handled in the database query using a join condition.
const workspace = await db.query.workspaces.findFirst({ where: (table, { and, eq, isNull }) => and(eq(table.tenantId, tenantId), isNull(table.deletedAt)), with: { permissions: { orderBy: [asc(permissions.name)], with: { keys: { + where: (table, { isNull }) => isNull(table.key.deletedAt), with: { key: { columns: { deletedAt: true, }, }, }, }, // ... rest of the query }, }, }, }); -workspace.permissions = workspace.permissions.map((permission) => { - permission.keys = permission.keys.filter(({ key }) => key.deletedAt === null); - return permission; -});
♻️ Duplicate comments (3)
apps/dashboard/app/(app)/apis/[apiId]/page.tsx (1)
238-245: 🛠️ Refactor suggestionRefactor time granularity logic to eliminate duplication
The logic for determining
timeGranularityis duplicated between theStackedColumnChartandAreaChartcomponents. Consider extracting this logic into a reusable function to improve maintainability.Also applies to: 283-289
apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx (1)
91-104: 🛠️ Refactor suggestionRemove duplicate API ID display to avoid redundancy
The API ID is already shown in the navigation bar (lines 70-80). Displaying it again in a separate card creates redundancy. Consider removing the duplicate Card component and keeping only the Badge in the navigation bar.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx (1)
331-331:⚠️ Potential issueFix incorrect metric value for disabled verifications
The metric for disabled verifications is incorrectly using
stats.validinstead ofstats.disabled.Apply this fix:
-<Metric label="Disabled" value={formatNumber(stats.valid)} /> +<Metric label="Disabled" value={formatNumber(stats.disabled)} />
🧹 Nitpick comments (8)
apps/dashboard/app/(app)/authorization/roles/page.tsx (2)
81-84: Consider simplifying the layout structureThe nested div structure with multiple margin and overflow classes could be simplified by moving these styles to the
PageContentcomponent or creating a dedicated layout component.- <PageContent> - <SubMenu navigation={navigation} segment="roles" /> - <div className="mt-8 mb-20 overflow-x-auto"> - <div className="flex flex-col gap-8 mb-20 "> + <PageContent className="mt-8 mb-20 overflow-x-auto"> + <SubMenu navigation={navigation} segment="roles" /> + <div className="flex flex-col gap-8">
114-119: Consider memoizing NumberFormat instancesThe
Intl.NumberFormatinstances are recreated for each item in the list. Consider creating a shared formatter instance outside the component for better performance.+const compactFormatter = new Intl.NumberFormat(undefined, { + notation: "compact", +}); export default async function RolesPage() { // ... existing code ... - {Intl.NumberFormat(undefined, { - notation: "compact", - }).format(r.permissions.length)} + {compactFormatter.format(r.permissions.length)}Also applies to: 122-127
apps/dashboard/app/(app)/authorization/permissions/page.tsx (1)
91-93: Consider reusing the create button componentThe create button appears both in the navbar and empty state. Consider extracting it to avoid duplication and ensure consistent behavior.
// Extract to a shared component or variable const CreatePermissionButton = ( <CreateNewPermission trigger={<Button variant="primary">Create New Permission</Button>} /> ); // Use in both locations <Navbar.Actions> <Badge variant="secondary" className="h-8"> {/* ... */} </Badge> {CreatePermissionButton} </Navbar.Actions> <EmptyPlaceholder> {/* ... */} {CreatePermissionButton} </EmptyPlaceholder>apps/dashboard/app/(app)/apis/[apiId]/page.tsx (1)
5-7: Clarify the aliasing ofNavbarasSubMenuThe
Navbarcomponent from"@/components/dashboard/navbar"is imported asSubMenu, while anotherNavbarcomponent is imported from"@/components/navbar". This may cause confusion due to similar naming. Consider renamingSubMenuto something more descriptive to improve code readability.apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx (1)
95-108: Remove duplicate Key ID display to avoid redundancyThe Key ID is already displayed in the navigation bar (lines 67-76). Showing it again in a separate card creates redundancy. Consider removing the duplicate Card component and keeping only the Badge in the navigation bar.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx (2)
262-268: Add aria-label to improve badge accessibilityThe badge containing the key ID should have an aria-label for better screen reader support.
<Badge variant="secondary" className="flex justify-between w-full gap-2 font-mono font-medium ph-no-capture" + aria-label={`API Key ID: ${key.id}`} > {key.id} <CopyButton value={key.id} /> </Badge>
344-350: Simplify time granularity logicThe nested ternary operators make the code harder to read. Consider using a more declarative approach.
-timeGranularity={ - granularity >= 1000 * 60 * 60 * 24 * 30 - ? "month" - : granularity >= 1000 * 60 * 60 * 24 - ? "day" - : "hour" -} +timeGranularity={getTimeGranularity(granularity)} // Add this helper function: +function getTimeGranularity(granularity: number) { + const DAY = 1000 * 60 * 60 * 24; + const MONTH = DAY * 30; + + if (granularity >= MONTH) return "month"; + if (granularity >= DAY) return "day"; + return "hour"; +}apps/dashboard/app/(app)/audit/[bucket]/page.tsx (1)
127-129: Extract breadcrumb label logic for better readabilityConsider extracting the complex inline logic for determining the breadcrumb label into a separate function.
+const getBucketLabel = (bucket: string, ratelimitNamespaces: Array<{ id: string, name: string }>) => { + return ratelimitNamespaces.find((ratelimit) => ratelimit.id === bucket)?.name ?? bucket; +}; <Navbar.Breadcrumbs.Link href={`/audit/${props.params.bucket}`} active isIdentifier> - {workspace.ratelimitNamespaces.find((ratelimit) => ratelimit.id === props.params.bucket) - ?.name ?? props.params.bucket} + {getBucketLabel(props.params.bucket, workspace.ratelimitNamespaces)} </Navbar.Breadcrumbs.Link>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx(7 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/settings/page.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/page.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/page.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/layout.tsx(0 hunks)apps/dashboard/app/(app)/apis/[apiId]/page.tsx(3 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx(2 hunks)apps/dashboard/app/(app)/audit/[bucket]/bucket-select.tsx(1 hunks)apps/dashboard/app/(app)/audit/[bucket]/filter.tsx(3 hunks)apps/dashboard/app/(app)/audit/[bucket]/page.tsx(3 hunks)apps/dashboard/app/(app)/authorization/permissions/[permissionId]/page.tsx(3 hunks)apps/dashboard/app/(app)/authorization/permissions/page.tsx(2 hunks)apps/dashboard/app/(app)/authorization/roles/[roleId]/page.tsx(2 hunks)apps/dashboard/app/(app)/authorization/roles/page.tsx(3 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/layout.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- apps/dashboard/app/(app)/apis/[apiId]/layout.tsx
- apps/dashboard/app/(app)/ratelimits/[namespaceId]/layout.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/dashboard/app/(app)/audit/[bucket]/bucket-select.tsx
- apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/page.tsx
- apps/dashboard/app/(app)/authorization/roles/[roleId]/page.tsx
- apps/dashboard/app/(app)/audit/[bucket]/filter.tsx
🔇 Additional comments (5)
apps/dashboard/app/(app)/authorization/roles/page.tsx (2)
2-4: LGTM! Well-structured component organization
The new imports establish a clear separation of concerns between navigation components (Navbar, SubMenu) and layout (PageContent), promoting reusability across the application.
Also applies to: 8-8, 13-13
Line range hint 59-78: Consider moving badge styling to the Navbar.Actions component
The badge styling (h-8) could be encapsulated within the Navbar.Actions component for better reusability and consistency across pages.
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/page.tsx (1)
7-9: Clarify the aliasing of Navbar as SubMenu
The Navbar component from "@/components/dashboard/navbar" is imported as SubMenu, while another Navbar component is imported from "@/components/navbar". This may cause confusion due to similar naming. Consider renaming SubMenu to something more descriptive to improve code readability.
apps/dashboard/app/(app)/apis/[apiId]/settings/page.tsx (1)
3-5: Clarify the aliasing of Navbar as SubMenu
The Navbar component from "@/components/dashboard/navbar" is imported as SubMenu, while another Navbar component is imported from "@/components/navbar". This may cause confusion due to similar naming. Consider renaming SubMenu to something more descriptive to improve code readability.
apps/dashboard/app/(app)/authorization/permissions/[permissionId]/page.tsx (1)
139-141: Fix potential date formatting issue
The toDateString() call before format is unnecessary and might cause unexpected behavior.
-format(permission.updatedAt?.toDateString(), "PPPP")
+format(permission.updatedAt, "PPPP")
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
NavbarandPageContentfor various settings and audit pages.Bug Fixes
Refactor
Chores