feat: metrics ui component #2745
Conversation
Flo4604
requested review from
perkinsjr,
chronark,
mcstepp and
ogzhanolguncu
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThe pull request introduces a new Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (11)
✅ Files skipped from review due to trivial changes (6)
🚧 Files skipped from review as they are similar to previous changes (4)
🔇 Additional comments (1)apps/dashboard/app/(app)/apis/[apiId]/settings/update-ip-whitelist.tsx (1)Line range hint The addition of the Let's verify the visual appearance in dark mode by checking for any other instances of Alert components that might need similar treatment: ✅ Verification successfulThe border styling enhancement is consistent with similar Alert components in the codebase Looking at the search results, most Alert components in the codebase either use variant-based styling (variant="alert") or have minimal className props. The addition of
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Find other Alert components that might need the border-border class
# Look for Alert components without the border-border class
# Search for Alert components
rg -U "Alert.*className(?!.*border-border)" --type tsx
Length of output: 311 Script: #!/bin/bash
# Let's try a simpler approach to find Alert components
# First, find all Alert components with className
rg -U "<Alert.*className" --type tsx
# Also search for general Alert usage to compare
rg -U "<Alert\b" --type tsx
Length of output: 124 Script: #!/bin/bash
# Search for Alert components without specifying file type
# First, find all Alert components with className
rg -U "<Alert.*className"
# Also search for general Alert usage to compare
rg -U "<Alert\b"
Length of output: 2411 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 (
|
|
@Flo4604 is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
|
auth page still has another metric component |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
apps/dashboard/components/ui/metric.tsx (1)
8-15: Consider enhancing accessibility with ARIA attributesThe component structure looks good, but could benefit from additional accessibility attributes.
const Metric = React.forwardRef<HTMLDivElement, MetricProps>(({ label, value }) => { return ( - <div className="flex flex-col items-start justify-center px-4 py-2 border-border"> + <div className="flex flex-col items-start justify-center px-4 py-2 border-border" role="group" aria-labelledby={`metric-${label.toLowerCase().replace(/\s+/g, '-')}`}> - <p className="text-sm text-content-subtle">{label}</p> + <p id={`metric-${label.toLowerCase().replace(/\s+/g, '-')}`} className="text-sm text-content-subtle">{label}</p> <div className="text-2xl font-semibold leading-none tracking-tight">{value}</div> </div> ); }); + +Metric.displayName = "Metric";apps/dashboard/app/(app)/settings/root-keys/[keyId]/layout.tsx (1)
Dark mode separator styling needs to be added
The code uses
divide-xclass for separating metrics in theCardContent, but lacks dark mode styling. This should be addressed to ensure proper visibility in dark mode, similar to how it's implemented in the billing page wheredark:divide-gray-800is used.
- In
apps/dashboard/app/(app)/settings/root-keys/[keyId]/layout.tsx, line 47: Add dark mode class to<CardContent className="flex flex-wrap justify-between divide-x [&>div:first-child]:pl-0">to includedark:divide-gray-800🔗 Analysis chain
Line range hint
41-47: Verify dark mode styling for metric separatorsThe
divide-xclass is used here which might be related to the white separator issue mentioned in the PR objectives.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any explicit color definitions for divide classes in dark mode rg -l "divide-" apps/dashboard/styles rg "divide-.*dark:" apps/dashboard/stylesLength of output: 81
Script:
#!/bin/bash # Let's check the actual file content to understand the context and styling cat "apps/dashboard/app/(app)/settings/root-keys/[keyId]/layout.tsx" # Also check for any global dark mode configurations in the project rg -g "*.{css,ts,tsx}" "dark:divide-" .Length of output: 2799
apps/dashboard/tailwind.config.js (1)
Line range hint
134-148: Consider modernizing the merge functionThe merge function could be improved using modern JavaScript features.
export function merge(obj1, obj2) { - for (const key in obj2) { - // biome-ignore lint/suspicious/noPrototypeBuiltins: don't tell me what to do - if (obj2.hasOwnProperty(key)) { - if (typeof obj2[key] === "object" && !Array.isArray(obj2[key])) { - if (!obj1[key]) { - obj1[key] = {}; - } - obj1[key] = merge(obj1[key], obj2[key]); - } else { - obj1[key] = obj2[key]; - } + return Object.entries(obj2).reduce((acc, [key, value]) => { + if (typeof value === "object" && !Array.isArray(value)) { + acc[key] = merge(acc[key] || {}, value); + } else { + acc[key] = value; } - } - return obj1; + return acc; + }, { ...obj1 }); }apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx (1)
Line range hint
315-315: Critical: Incorrect metric value for Disabled statusThe Disabled metric is incorrectly displaying the value of successful validations (
stats.valid) instead of disabled validations (stats.disabled).Apply this fix:
-<Metric label="Disabled" value={formatNumber(stats.valid)} /> +<Metric label="Disabled" value={formatNumber(stats.disabled)} />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/page.tsx(2 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/page.tsx(1 hunks)apps/dashboard/app/(app)/settings/root-keys/[keyId]/layout.tsx(2 hunks)apps/dashboard/app/ratelimit/page.tsx(1 hunks)apps/dashboard/components/ui/metric.tsx(1 hunks)apps/dashboard/tailwind.config.js(1 hunks)
🔇 Additional comments (11)
apps/dashboard/components/ui/metric.tsx (1)
3-6: LGTM! Clean and well-typed interface.
The MetricProps interface is well-defined and appropriately typed for the component's needs.
apps/dashboard/app/(app)/settings/root-keys/[keyId]/layout.tsx (1)
69-73: LGTM! Clean type declaration for LastUsed component
The type declaration is well-structured and properly typed.
apps/dashboard/app/(app)/apis/[apiId]/page.tsx (2)
121-124: LGTM: Improved code formatting
The reformatting of the insufficientPermissionsOverTime mapping improves readability while maintaining the same functionality.
4-4: Verify Metric component styling in dark mode
Since this PR aims to fix metric separators in dark mode, let's verify the styling.
✅ Verification successful
Based on the examination of the codebase, I can now provide a final response:
Dark mode styling is properly handled through Tailwind classes
The Metric component uses Tailwind's built-in dark mode system through utility classes:
- The border color uses
border-borderwhich is a theme token that automatically adapts to dark mode - The text color uses
text-content-subtlefor the label and default text styling for the value, both of which are theme tokens that handle dark mode transitions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for dark mode specific styles in the Metric component
rg -A 5 "dark:" apps/dashboard/components/ui/metric.tsx
Length of output: 55
Script:
#!/bin/bash
# First, let's check if the file exists and see its content
fd "metric.tsx" apps/dashboard/components/ui
# If not found in the expected location, let's search in other directories
fd "metric.tsx"
# Let's also check for any Metric component with dark mode styling
rg -i "dark.*metric" --type tsx
Length of output: 202
Script:
#!/bin/bash
# Let's examine the content of the Metric component
cat apps/dashboard/components/ui/metric.tsx
# Also search for dark mode related styles without file type restriction
rg -i "dark.*metric"
Length of output: 552
Script:
#!/bin/bash
# Let's check the tailwind config for dark mode border and text color definitions
rg "border-border|text-content-subtle" -A 5
# Also check for any recent changes to the Metric component
git diff HEAD~1 apps/dashboard/components/ui/metric.tsx
Length of output: 67239
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx (3)
12-12: LGTM: Clean import of Metric component
The import statement follows Next.js conventions and properly organizes UI components.
99-103: LGTM: Improved query readability
The clickhouse query formatting enhances code readability while maintaining the same functionality.
Line range hint 241-251: Verify dark mode styling for metric separators
The PR aims to fix white metric separators in dark mode. Please verify that the divide-x utility classes in both grid layouts properly handle dark mode colors.
Let's check the component's styling implementation:
Also applies to: 308-320
✅ Verification successful
Let me gather more specific information about the dark mode styling configuration and the divider utilities.
Based on the gathered information, I can now generate the final response:
The divide-x utility is properly handled for dark mode
The dark mode styling is correctly configured in the Tailwind CSS setup. The --border color variable is properly defined for both light mode (24, 6%, 83%) and dark mode (12, 6%, 15%), and all divider utilities (divide-x) inherit this color through the base layer configuration: * { @apply border-border; }. This ensures consistent styling of separators in both light and dark modes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for dark mode styling in the Metric component and related CSS
# Look for dark mode specific styles
rg -l "dark:" apps/dashboard/
# Check the Metric component implementation
cat apps/dashboard/components/ui/metric.tsx
# Look for any global dark mode styling configurations
fd -e css -e scss . apps/dashboard/
Length of output: 1706
Script:
#!/bin/bash
# Check tailwind configuration and global styles
cat apps/dashboard/styles/tailwind/tailwind.css
# Look for divider-related styles and their dark mode variants
rg "divide-" apps/dashboard/
# Check the specific file mentioned in the review for the metric usage context
cat "apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/[keyId]/page.tsx"
Length of output: 19057
apps/dashboard/app/ratelimit/page.tsx (2)
11-11: LGTM: Import statement change aligns with PR objectives
The change from require to import statement is consistent with the PR objectives.
Line range hint 164-167: Verify dark mode appearance of metric separators
The divide-x class is used for metric separation in multiple cards. Since the PR objectives mention fixing white separators in dark mode, please verify that these separators now appear correctly in dark mode.
Run the following script to check for any remaining instances of potentially problematic separator styling:
Also applies to: 199-202, 234-237
✅ Verification successful
Dark mode divider styling has been properly implemented
Based on the search results, the codebase shows proper dark mode handling for dividers:
- The billing page explicitly sets dark mode styles with
dark:divide-gray-800class - The Metric component uses
border-borderclass which inherits from the theme system - The global CSS shows proper dark mode theming with
--borderCSS variable set in the root theme
The separators will appear correctly in dark mode as they use the theme-aware border colors through Tailwind's configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for divide-x usage and related dark mode styles
# Look for divide-x classes and related dark mode styling
rg -A 3 'divide-x'
# Check for any custom separator styling in CSS/SCSS files
fd -e css -e scss -x rg -A 3 'divide|separator'
Length of output: 5461
Script:
#!/bin/bash
# Let's check for dark mode specific styling for dividers
rg -A 3 'dark:divide'
# Check the Metric component implementation for any dark mode specific styles
fd -g '*metric*.tsx' -x cat {}
# Check for any global dark mode styles that might affect dividers
fd -g 'globals.css' -x cat {}
Length of output: 15209
apps/dashboard/app/(app)/ratelimits/[namespaceId]/page.tsx (2)
6-6: LGTM: Import statement change aligns with PR objectives
The change from require to import statement is consistent with the PR objectives.
Line range hint 155-167: Verify dark mode appearance of responsive separators
The code uses responsive separator classes:
divide-y md:divide-y-0 md:divide-xin the main metrics carddivide-xin the request metrics header
Please verify that these separators appear correctly in dark mode across different screen sizes.
Run the following script to check for any remaining responsive separator styling issues:
Also applies to: 191-204
✅ Verification successful
Separators are properly styled for dark mode
The verification shows that separators are correctly styled for dark mode:
- The Separator component in the dashboard uses
bg-borderclass which automatically handles dark mode through the theme system. - The responsive dividers mentioned in the review (
divide-y md:divide-y-0 md:divide-x) are properly styled with dark mode variants as seen in the billing page example (dark:divide-gray-800).
The implementation ensures consistent appearance across different screen sizes and color schemes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for responsive separator styling
# Look for responsive divide classes
rg 'divide-[xy].*md:divide'
# Check for any media query related separator styling
fd -e css -e scss -x rg -A 3 '@media.*divide'
Length of output: 237
Script:
#!/bin/bash
# Let's check for dark mode styling configurations for separators and dividers
rg -A 3 'dark:.*divide|divide.*dark:'
# Check the global styles or theme configuration for separator styling
fd -e css -e scss -x cat {} \;
# Look for Separator component usage and its styling
rg -A 2 '<Separator'
Length of output: 34242
Script:
#!/bin/bash
# Let's check for dark mode specific styling in the theme configuration and tailwind config
rg -A 5 'dark:divide|dark.*divide' apps/dashboard
# Check for any separator-specific styling in the theme configuration
fd -e js -e ts -x rg -l 'separator' {} \; | grep -i 'theme\|tailwind'
# Look for the Separator component definition to understand its styling
ast-grep --pattern 'const Separator = $$$'
Length of output: 3208
|
Could you please run the formatter? |
I fixed the stuff manually and it didn't reformat it idk |
Might be my bad that one caught me off guard. Also can you open an issue for this one? |
actually It zed that formatted it like this but biome doesn't seem to care, I manually changed it back and it didn't reformat it so I'm not sure whats up with that. Created an issue and linked it sorry about that |
|
should be most if not all places |
|
Wow that's a lot of Metric components 😄. Great work. |
What does this PR do?
Fixes the white metric seperator things
Also changes require to import because the build doesn't work locally for me otherwise.
Fixes #2747
Type of change
How should this be tested?
Check if any place with a metric component is white in darkmode
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
Metriccomponent for enhanced display of various metrics across the dashboard.Metriccomponent for presenting API key metrics, API usage statistics, rate limit information, and more.Bug Fixes
Refactor
Metriccomponent in favor of a centralized version, streamlining code and enhancing maintainability.Style