feat: Move settings card and new inline link#3198
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis change set migrates usage of the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DashboardPage
participant @unkey/ui
participant Documentation
User->>DashboardPage: Loads settings page
DashboardPage->>@unkey/ui: Render SettingCard
DashboardPage->>@unkey/ui: Render InlineLink (for "Learn more")
User->>InlineLink: Clicks "Learn more"
InlineLink->>Documentation: Opens documentation in new tab
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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! 🙏 |
MichaelUnkey
requested review from
chronark,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/engineering/content/design/components/settings-card.mdx (2)
20-20: Fix typo in description text.There's a spelling error: "visualy" should be "visually".
-Two SettingCard components sharing a common edge. Useful when settings might be related or make sense visualy together. +Two SettingCard components sharing a common edge. Useful when settings might be related or make sense visually together.
24-24: Fix typo in section heading.There's a spelling error: "seperating" should be "separating".
-## Settings Cards With seperating border +## Settings Cards With separating borderapps/engineering/content/design/components/settings-card.example.tsx (2)
101-107: Typo & Description Clarification
There’s a typo in the title prop and a mismatch in the description:
- Title:
"Square corncers Example"→"Square corners Example".- Description says “default to rounded corners” but the example name emphasizes square edges when no
borderprop is passed. Please adjust the text to accurately convey the intended behavior.Proposed fix:
- title="Square corncers Example" - description="Not passing in border prop will default to rounded corners." + title="Square corners Example" + description="Not passing in the `border` prop will render square edges by default."
15-18: Optional DRY Refactor: Extract Repeated Layout
The wrapper<div className="flex gap-2 items-center justify-center w-full">…</div>is duplicated in all examples. Consider extracting it into a small helper component (e.g.,SettingCardActions) to improve readability and maintenance.Example addition at the top (after your imports):
+import type { FC, ReactNode } from 'react'; + +// Reusable wrapper for card action controls +const SettingCardActions: FC<{children: ReactNode}> = ({ children }) => ( + <div className="flex gap-2 items-center justify-center w-full">{children}</div> +);Then replace each block:
-<div className="flex gap-2 items-center justify-center w-full"> - <Input … /> - <Button … /> -</div> +<SettingCardActions> + <Input … /> + <Button … /> +</SettingCardActions>This reduces markup duplication and centralizes layout tweaks.
Also applies to: 34-37, 72-74, 108-110
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/copy-api-id.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-prefix.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-api.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.tsx(2 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-api-name.tsx(1 hunks)apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx(3 hunks)apps/dashboard/app/(app)/authorization/permissions/[permissionId]/settings-client.tsx(1 hunks)apps/dashboard/app/(app)/authorization/roles/[roleId]/settings-client.tsx(1 hunks)apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx(1 hunks)apps/dashboard/app/(app)/settings/billing/client.tsx(1 hunks)apps/dashboard/app/(app)/settings/billing/components/usage.tsx(1 hunks)apps/dashboard/app/(app)/settings/billing/page.tsx(1 hunks)apps/engineering/content/design/components/inline-link.example.tsx(1 hunks)apps/engineering/content/design/components/inline-link.mdx(1 hunks)apps/engineering/content/design/components/settings-card.example.tsx(1 hunks)apps/engineering/content/design/components/settings-card.mdx(1 hunks)internal/ui/src/components/inline-link.tsx(1 hunks)internal/ui/src/components/settings-card.tsx(1 hunks)internal/ui/src/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx (2)
internal/ui/src/components/inline-link.tsx (1)
InlineLink(32-56)internal/icons/src/icons/arrow-up-right.tsx (1)
ArrowUpRight(15-40)
apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.tsx (2)
internal/ui/src/components/inline-link.tsx (1)
InlineLink(32-56)internal/icons/src/icons/arrow-up-right.tsx (1)
ArrowUpRight(15-40)
apps/engineering/content/design/components/inline-link.example.tsx (2)
apps/engineering/app/components/render.tsx (1)
RenderComponentWithSnippet(12-67)internal/ui/src/components/inline-link.tsx (1)
InlineLink(32-56)
🔇 Additional comments (28)
apps/dashboard/app/(app)/settings/billing/client.tsx (1)
6-6: CentralizedSettingCardimport
The import path has been updated to pullSettingCardfrom the shared@unkey/uilibrary. This aligns with the PR’s goal of consolidating UI components. Please verify that@unkey/uiexportsSettingCardcorrectly.apps/dashboard/app/(app)/settings/billing/components/usage.tsx (1)
2-2: CentralizedSettingCardimport
The import forSettingCardnow references@unkey/ui, matching the new library structure. Confirm that the export exists and behaves identically to the previous local component.apps/dashboard/app/(app)/authorization/permissions/[permissionId]/settings-client.tsx (1)
9-9: CentralizedSettingCardimport
Updated the import source forSettingCardto the external UI package. Ensure@unkey/uiexports this component without breaking the existing UI contract.apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-prefix.tsx (1)
7-7: CentralizedSettingCardimport
The import path forSettingCardhas been changed to@unkey/ui, consistent with the rest of the settings components. Verify that no styles or props have been affected by this relocation.apps/dashboard/app/(app)/apis/[apiId]/settings/components/copy-api-id.tsx (1)
3-3: CentralizedSettingCardimport
Switching theSettingCardimport to the shared UI library is correct. Please confirm that the component renders and functions as before under@unkey/ui.apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-api.tsx (1)
9-9: SettingCard import updated correctly
The import ofSettingCardhas been switched to@unkey/uias intended, aligning with the shared UI library refactor. No changes to functionality are introduced.apps/dashboard/app/(app)/apis/[apiId]/settings/components/default-bytes.tsx (1)
7-7: SettingCard import updated correctly
SwitchingSettingCardto be imported from@unkey/uicentralizes the component in the UI library and matches other settings components. Behavior remains unchanged.apps/dashboard/app/(app)/ratelimits/[namespaceId]/settings/components/settings-client.tsx (1)
8-8: SettingCard import updated correctly
TheSettingCardimport from@unkey/uireflects the new shared component path. This is consistent with the refactor and has no impact on existing logic.apps/dashboard/app/(app)/authorization/roles/[roleId]/settings-client.tsx (1)
8-8: SettingCard import updated correctly
ImportingSettingCardfrom@unkey/uicentralizes this UI element and matches the updated pattern across settings pages. No logic changes here.apps/dashboard/app/(app)/settings/billing/page.tsx (1)
6-6: SettingCard import updated correctly
Changing the import to@unkey/uialigns billing settings with the shared UI library approach. Functionality remains the same.apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-api-name.tsx (1)
8-8: Import migration looks good.The
SettingCardimport is now sourced from the centralized@unkey/uipackage rather than a local path. This change aligns with the PR's objective of moving UI components to a shared library.internal/ui/src/components/settings-card.tsx (1)
1-3: Import adjustments follow best practices.The changes to the React import and the relative path for the
cnutility align with Biome linting rules. The explicit comment about why the React import is structured this way is helpful for future maintainers.internal/ui/src/index.ts (1)
5-5: Exports properly expose the components.The
inline-linkandsettings-cardcomponents are now correctly exported from the main UI package, making them available to consumers of@unkey/ui.Also applies to: 11-11
apps/engineering/content/design/components/inline-link.mdx (1)
1-64: Documentation is comprehensive and follows best practices.The documentation for the
InlineLinkcomponent is well-structured with:
- Clear description of purpose and usage
- Multiple examples demonstrating different configurations
- Complete props documentation with types and defaults
- Accessibility considerations
This thorough documentation will help developers understand how to properly use the component.
apps/dashboard/app/(app)/apis/[apiId]/settings/components/delete-protection.tsx (2)
8-8: Import consolidation looks good.Importing both
InlineLinkandSettingCardfrom the centralized@unkey/uipackage aligns with the PR's objective of standardizing UI components.
159-166: New InlineLink implementation improves code quality.The new
InlineLinkcomponent provides a more consistent, accessible, and maintainable approach to inline links with icons. The implementation:
- Properly opens links in a new tab with security attributes
- Consistently positions the icon
- Maintains the same visual style as other links in the application
This change successfully meets the PR objective of introducing the new InlineLink component.
apps/engineering/content/design/components/settings-card.mdx (1)
1-55: Well-structured documentation!The documentation is comprehensive and follows a clear structure with examples, features, and props section. Good job providing full details of the component's capabilities.
internal/ui/src/components/inline-link.tsx (2)
5-31: Well-documented props with JSDoc comments.The component's props are thoroughly documented with clear JSDoc comments that explain the purpose of each prop. This is excellent practice that improves code maintainability.
32-56: Accessible component implementation.Good implementation with proper accessibility considerations:
- Icon is marked with
aria-hidden="true"to indicate it's decorative- Security attributes (
rel="noopener noreferrer") when opening links in new tabs- Proper conditional rendering based on icon position
The component follows best practices for building accessible UI elements.
apps/dashboard/app/(app)/apis/[apiId]/settings/components/update-ip-whitelist.tsx (4)
9-9: Clean import refactoring.The import has been refactored to use the UI components from the shared
@unkey/uipackage, aligning with the PR objective of moving the SettingsCard component.
83-89: Good UX enhancement with inline documentation link.Adding the
InlineLinkcomponent with the "Learn more" text and external link icon improves the user experience by providing contextual access to documentation without cluttering the interface.
93-93: Simplified content width property.The
contentWidthproperty has been simplified to use a standard width class instead of responsive constraints. This change should make the component more consistent with other instances.
127-133: UI streamlining for disabled state.The disabled state UI has been streamlined by removing unnecessary width classes and the separate "Learn more" ghost button, resulting in a cleaner interface that aligns with the design system.
apps/engineering/content/design/components/inline-link.example.tsx (1)
1-66: Comprehensive examples covering all component variations.These examples effectively demonstrate the various ways the
InlineLinkcomponent can be used:
- Basic usage within text
- Icon placement options (left/right)
- External link behavior
- Custom styling
Each example is properly isolated and clearly shows a single feature, making it easy for other developers to understand how to use the component.
apps/engineering/content/design/components/settings-card.example.tsx (4)
1-4: Imports & Client Directive Look Good
The"use client"directive ensures this example runs in the browser for interactive components, and all import paths (RenderComponentWithSnippet,Clone,Button,Input,SettingCard) appear correct and consistent with your project’s conventions.
6-22: Basic Example Demonstratesborder="both"Correctly
TheSettingsCardBasiccomponent cleanly shows how to useborder="both",contentWidth, and children layout insideRenderComponentWithSnippet. This is clear and easy to follow.
24-59: Shared-Edge Example Usage Is Accurate
SettingsCardsWithSharedEdgecorrectly stacks two cards with only top and bottom borders. The sequence ofborder="top"andborder="bottom"props illustrates the shared-edge pattern clearly.
61-97: Divider Example Correctly UsesclassName="border-b"
SettingsCardsWithDividershows an explicit bottom border on the first card viaclassName="border-b". This approach effectively demonstrates how to insert a visual divider between cards.
ogzhanolguncu
left a comment
ogzhanolguncu
left a comment
There was a problem hiding this comment.
- Docs look good
- Permission and roles pages look good
- Inline links looking good
2 participants
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
Enhancements
Documentation