-
Notifications
You must be signed in to change notification settings - Fork 523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: A user can collapse/expand APIs when creating a new root key #2124
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
WalkthroughWalkthroughThe changes implement a collapsible UI component within the Changes
Assessment against linked issues
Possibly related PRs
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! 🙏 |
Why do you prefer this way of collapsing over the way we do it for editing permissions? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
apps/dashboard/app/(app)/settings/root-keys/new/client.tsx (1)
168-168
: Usebreak-words
instead ofbreak-all
for better text wrappingThe
break-all
class breaks words at arbitrary points, which can affect readability. Usingbreak-words
ensures that words only break at appropriate points, improving the user interface.Apply this diff to refine the code:
- <CardTitle className="break-all">{api.name}</CardTitle> + <CardTitle className="break-words">{api.name}</CardTitle>
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/dashboard/app/(app)/settings/root-keys/new/client.tsx (5 hunks)
Additional comments not posted (3)
apps/dashboard/app/(app)/settings/root-keys/new/client.tsx (3)
11-11
: Importing Collapsible components is appropriateThe addition of
Collapsible
,CollapsibleContent
, andCollapsibleTrigger
imports is necessary for implementing the collapsible API cards feature.
26-26
: Importing ChevronRight icon for collapsible triggersIncluding the
ChevronRight
icon enhances the visual indication of collapsible sections.
154-227
: Implementation of collapsible API cards is effectiveWrapping each API card with the
Collapsible
component and managing its state enhances the user experience by reducing clutter. The use ofcardStatesMap
to track the open states is appropriate.
The permissions navigates to a new page. This one are elements within the same form - i didn't want to lose reference to form data without a more robust state management solution. This way allows me to manage the open/close state and retain the form data when creating a new root key. |
c4bcbfa
to
f81bafe
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this PR do?
Currently when a user creates a new root key, we show all the APIs by default. This PR converts those APIs cards into Collapsible cards, default collapsed. The user can expand each API card to add permissions if necessary.
Fixes #1980
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?
Manual testing.
Checklist
Required
pnpm build
pnpm fmt
console.logs
git pull origin main
Appreciated
Summary by CodeRabbit
New Features
Improvements