feat: remove old css stuff and use @unkey/ui #2737
Conversation
chronark
requested review from
perkinsjr,
mcstepp,
ogzhanolguncu and
MichaelUnkey
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces significant changes across several files in the dashboard application. Key modifications include the transition of button components to a new library, Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
apps/dashboard/app/(app)/apis/client.tsx (1)
90-93: Specifyvariantprop for theButtoncomponentThe
Buttoncomponent used in the "Read the docs" link does not have avariantprop specified. For consistency and to ensure the button displays with the correct styling, consider adding thevariantprop.Here's how you can update it:
<Link href="/docs" target="_blank"> - <Button> + <Button variant="secondary"> <BookOpen /> Read the docs </Button> </Link>Choose the appropriate variant (
"primary","secondary", etc.) based on your design guidelines.apps/dashboard/tailwind.config.js (2)
4-4: Consider moving more theme settings to @unkey/uiThe import of defaultTheme from @unkey/ui is good, but as noted in the comment, consider moving more theme settings (colors, font sizes) to @unkey/ui for better maintainability and consistency.
Also applies to: 15-19
Line range hint
149-164: Consider enhancing the merge functionWhile the merge function works, consider these improvements:
- Add type safety using TypeScript
- Handle arrays more explicitly
- Add protection against circular references
-export function merge(obj1, obj2) { +export function merge<T extends Record<string, any>>(obj1: T, obj2: T): T { + // Prevent circular references + const seen = new WeakSet(); + for (const key in obj2) { - // biome-ignore lint/suspicious/noPrototypeBuiltins: don't tell me what to do - if (obj2.hasOwnProperty(key)) { + if (Object.prototype.hasOwnProperty.call(obj2, key)) { if (typeof obj2[key] === "object" && !Array.isArray(obj2[key])) { + if (seen.has(obj2[key])) { + continue; // Skip circular reference + } + seen.add(obj2[key]); if (!obj1[key]) { obj1[key] = {}; } obj1[key] = merge(obj1[key], obj2[key]); } else { obj1[key] = obj2[key]; } } } return obj1; }apps/www/components/analytics/analytics-bento.tsx (1)
Line range hint
704-755: Consider extracting the API key display componentThere's duplication in the API key display UI between the header and details section. Consider extracting this into a reusable component.
type ApiKeyDisplayProps = { apiKey: string; className?: string; }; function ApiKeyDisplay({ apiKey, className }: ApiKeyDisplayProps) { return ( <div className={cn("flex items-center gap-2 px-2 py-1 font-mono text-xs rounded-lg bg-white/5 font-sm text-white/40", className)}> {apiKey} <svg xmlns="http://www.w3.org/2000/svg" width="24" height="24" viewBox="0 0 24 24" fill="none"> {/* ... copy icon path ... */} </svg> </div> ); }Usage:
- <div className="flex items-center gap-2 px-3 py-1 font-mono text-xs rounded-lg bg-white/5 font-sm text-white/40"> - api_UNWrXjYp6AF2h7Nx - <svg>...</svg> - </div> + <ApiKeyDisplay apiKey="api_UNWrXjYp6AF2h7Nx" className="px-3" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (6)
-
apps/dashboard/public/fonts/PangeaAfrikanTrial-Medium.woff2is excluded by!**/*.woff2 -
apps/dashboard/styles/Satoshi-Bold.ttfis excluded by!**/*.ttf -
apps/dashboard/styles/fonts/CalSans-SemiBold.ttfis excluded by!**/*.ttf -
apps/dashboard/styles/fonts/CalSans-SemiBold.woff2is excluded by!**/*.woff2 -
apps/dashboard/styles/fonts/Mona-Sans.var.woff2is excluded by!**/*.woff2 -
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
-
apps/dashboard/app/(app)/apis/client.tsx(2 hunks) -
apps/dashboard/app/(app)/apis/create-api-button.tsx(3 hunks) -
apps/dashboard/app/layout.tsx(2 hunks) -
apps/dashboard/package.json(1 hunks) -
apps/dashboard/styles/tailwind/base.css(0 hunks) -
apps/dashboard/styles/tailwind/components.css(0 hunks) -
apps/dashboard/styles/tailwind/tailwind.css(1 hunks) -
apps/dashboard/styles/tailwind/typography.css(0 hunks) -
apps/dashboard/styles/tailwind/utilities.css(0 hunks) -
apps/dashboard/tailwind.config.js(4 hunks) -
apps/www/components/analytics/analytics-bento.tsx(4 hunks) -
apps/www/components/shiny-card.tsx(0 hunks)
💤 Files with no reviewable changes (5)
- apps/dashboard/styles/tailwind/components.css
- apps/dashboard/styles/tailwind/utilities.css
- apps/dashboard/styles/tailwind/typography.css
- apps/dashboard/styles/tailwind/base.css
- apps/www/components/shiny-card.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
apps/dashboard/styles/tailwind/tailwind.css
[error] 122-122: Expected a string but instead found ')'.
Expected a string here.
(parse)
🔇 Additional comments (6)
apps/dashboard/app/layout.tsx (1)
8-10: Verify font imports and usage
Ensure that GeistMono and GeistSans are correctly imported from "geist/font/mono" and "geist/font/sans", and that these fonts are properly configured and loaded in the application. Double-check if additional setup is required for these fonts to work as intended.
To confirm the correct imports and usage, you can run the following script:
✅ Verification successful
Font imports and usage are correctly implemented
The imports of GeistMono and GeistSans from "geist/font/mono" and "geist/font/sans" are correctly implemented and consistently used across multiple apps in the monorepo. The fonts are properly configured in the layout files:
- The dashboard app correctly applies both fonts using CSS variables:
className={${GeistSans.variable} ${GeistMono.variable}} - Similar implementations are found in other apps (www, engineering, play) confirming this is the standard pattern
- The fonts are used appropriately with Next.js's font system
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for imports of Geist fonts and verify usage.
# Test: Find all import statements of Geist fonts.
rg 'import.*from ["\']geist/font/.*["\']' -A 2
# Expected: Imports of GeistMono and GeistSans with correct paths.
Length of output: 116
Script:
#!/bin/bash
# Search for Geist font imports with proper quoting
rg "import.*from ['\"]geist/font" -A 2
# Search for any mentions of Geist fonts in the codebase
rg "GeistMono|GeistSans"
# Check package.json for geist dependency
cat package.json | grep -A 5 -B 5 "geist"
Length of output: 2951
apps/dashboard/app/(app)/apis/create-api-button.tsx (1)
54-54:
Fix incomplete Tailwind CSS class in DialogContent
The className prop for DialogContent includes an incomplete Tailwind class "max-sm: ". The max-sm: prefix requires a utility class to follow. Please complete the class or remove it if unnecessary.
Here's a suggested fix:
- <DialogContent className="w-11/12 max-sm: ">
+ <DialogContent className="w-11/12 max-sm:w-full">Replace w-full with the intended utility class if different.
Likely invalid or redundant comment.
apps/dashboard/app/(app)/apis/client.tsx (1)
7-7: Verify the integration of the Button component from @unkey/ui
Ensure that the Button component imported from @unkey/ui is correctly integrated and that all instances of Button across the application are updated accordingly. Confirm that the buttons render as expected with the new styling.
You can run the following script to find all usages of the Button component:
✅ Verification successful
Let me search for both the imports and actual usage of the Button component to verify its integration. The first command will help us find where the Button is imported from @unkey/ui, and the second command will show us where Button components are being used in the codebase.
I see the imports are correctly set up across multiple files. Let's now verify the actual usage of the Button component in the codebase, particularly in the file we're reviewing.
Button component from @unkey/ui is correctly integrated
The verification shows that:
- The Button component is properly imported from @unkey/ui across multiple files
- In the specific file under review (apps/dashboard/app/(app)/apis/client.tsx), the Button is correctly imported and used with appropriate props in the "Read the docs" button
- The Button component is consistently used throughout the codebase with various props (variant, size, className) and appears to be working as expected with different styling options
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of the Button component.
# Test: Search for Button imports and usages.
rg 'import {.*Button.*} from ["\']@unkey/ui["\']' -A 5
# Expected: All Button components are imported from @unkey/ui and used correctly.
Length of output: 116
Script:
#!/bin/bash
# Search for Button imports from @unkey/ui
rg "import.*Button.*from.*@unkey/ui"
# Search for Button usage patterns in TSX/JSX files
rg "<Button" --type tsx
Length of output: 1013
Script:
#!/bin/bash
# Search for Button usage patterns in all files
rg "<Button"
# Let's also check the specific file we're reviewing to see its Button usage
cat "apps/dashboard/app/(app)/apis/client.tsx"
Length of output: 27507
apps/dashboard/package.json (1)
86-86: Verify Geist package version
The addition of the Geist package aligns with the transition to @unkey/ui. However, let's verify the version.
✅ Verification successful
✓ Geist package version is up-to-date and secure
The installed version (1.3.1) matches the latest version from npm registry, and there are no security vulnerabilities reported for this package.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest version and any potential issues with geist package
curl -s https://registry.npmjs.org/geist | jq -r '.["dist-tags"].latest'
# Check for any security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "geist") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 419
apps/dashboard/tailwind.config.js (1)
136-137: LGTM: Font family configuration updated for Geist
The font family configuration correctly uses the Geist font CSS variables, aligning with the PR objective.
apps/www/components/analytics/analytics-bento.tsx (1)
631-631: LGTM: Updated styling for consistency
The styling changes align with the transition to @unkey/ui and maintain consistency across the application.
| --warn: 46, 97%, 65%; | ||
| --warn-foreground: 46, 97%, 15%; |
There was a problem hiding this comment.
Fix inconsistent HSL color syntax in CSS variables
The HSL color values in the CSS variables use commas in some places and spaces in others. According to CSS syntax, spaces should separate HSL components when not using the hsl() function. Please ensure consistency by using spaces throughout.
Apply this diff to correct the syntax:
- --warn: 46, 97%, 65%;
- --warn-foreground: 46, 97%, 15%;
- --success: 152, 56%, 39%;
+ --warn: 46 97% 65%;
+ --warn-foreground: 46 97% 15%;
+ --success: 152 56% 39%;And in the .dark theme:
- --warn: 46, 100%, 45%;
- --warn-foreground: 46, 97%, 5%;
- --success: 152, 56%, 39%;
+ --warn: 46 100% 45%;
+ --warn-foreground: 46 97% 5%;
+ --success: 152 56% 39%;Also applies to: 27-27, 74-75, 80-80
| --warn-foreground: 46, 97%, 15%; | ||
|
|
||
| --alert: 0 100% 65%; | ||
| --alert-foreground: 0 100% 980%; |
There was a problem hiding this comment.
Correct invalid lightness value in --alert-foreground
The --alert-foreground CSS variable has an invalid lightness value of 980%. Lightness values in HSL should be between 0% and 100%. Please correct this value to ensure proper rendering.
Here's the fix:
- --alert-foreground: 0 100% 980%;
+ --alert-foreground: 0 100% 98%;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| --alert-foreground: 0 100% 980%; | |
| --alert-foreground: 0 100% 98%; |
| counter-increment: line; | ||
| content: counter(line); | ||
| @apply mr-8 inline-block w-4 text-right text-gray-500; | ||
| @-moz-document url-prefix() { |
There was a problem hiding this comment.
Fix syntax error in @-moz-document rule
At line 122, the @-moz-document url-prefix() rule is missing a string argument inside url-prefix(). This causes a syntax error. To target all URLs, use an empty string as the argument.
Apply this diff:
-@-moz-document url-prefix() {
+@-moz-document url-prefix("") {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @-moz-document url-prefix() { | |
| @-moz-document url-prefix("") { |
🧰 Tools
🪛 Biome (1.9.4)
[error] 122-122: Expected a string but instead found ')'.
Expected a string here.
(parse)
@ogzhanolguncu this starts using @unkey/ui components (only button for now as proof of concept) in the dashboard
it's mostly about ensuring the tailwind stuff works correctly and we have access to the components and colors
In followup PRs, we need to remove the old colors over time.
I also cleaned up a lot of orphaned css and font stuff that we no longer used
Summary by CodeRabbit
New Features
refreshprop in theShinyCardGroupcomponent for dynamic reinitialization of container dimensions.ShinyCardGroupfor improved interactivity.Bug Fixes
pythonCodeBlockof theAnalyticsBentocomponent.Style
Chores