fix: Added tailwind solution to fix the the view and copy button missing when key longer than 87 characters. #2620
Conversation
adot-7
requested review from
perkinsjr,
chronark,
mcstepp and
MichaelUnkey
as code owners
|
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces updates to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
|
|
@akash-2004 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! 🙏 |
adot-7
changed the title
![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
🧹 Outside diff range and nitpick comments (4)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (3)
Line range hint
449-451: Fix the refill day reset logic.The comparison operator
===is used instead of the assignment operator=, causing the refill day to not be reset properly when the interval is daily.Apply this fix:
- if (refill?.interval === "daily") { - refill?.refillDay === undefined; - } + if (refill?.interval === "daily") { + refill.refillDay = undefined; + }
Line range hint
308-318: Improve form reset logic for consistency.The current implementation manually resets specific fields, which could lead to inconsistent form state. Consider using
form.reset()to ensure all fields are properly reset to their default values.Consider this approach:
- onClick={() => { - key.reset(); - form.setValue("expireEnabled", false); - form.setValue("ratelimitEnabled", false); - form.setValue("metaEnabled", false); - form.setValue("limitEnabled", false); - router.refresh(); - }} + onClick={() => { + key.reset(); + form.reset({ + prefix: defaultPrefix || undefined, + bytes: defaultBytes || 16, + expireEnabled: false, + limitEnabled: false, + metaEnabled: false, + ratelimitEnabled: false, + }); + router.refresh(); + }}
Line range hint
779-789: Consider making the metadata textarea more responsive.The textarea has a hardcoded height of 7 rows which might not be optimal for all screen sizes or content lengths. Consider making it more responsive or auto-resizing based on content.
Consider this enhancement:
<Textarea disabled={!form.watch("metaEnabled")} className="m-4 mx-auto border rounded-md shadow-sm" - rows={7} + className="m-4 mx-auto border rounded-md shadow-sm min-h-[120px] max-h-[300px]" + rows={Math.min(10, Math.max(3, field.value?.split('\n').length || 3))} placeholder={`{"stripeCustomerId" : "cus_9s6XKzkNRiz8i3"}`} {...field} value={ form.getValues("metaEnabled") ? field.value : undefined } />packages/api/src/openapi.d.ts (1)
378-383: Consider enhancing the refillDay documentation.While the documentation is good, it could be more explicit about:
- Valid day ranges (1-31)
- Behavior when specifying day 31 for months with fewer days
- Examples for different scenarios
Consider updating the documentation like this:
/** - * @description The day verifications will refill each month, when interval is set to 'monthly'. Value is not zero-indexed making 1 the first day of the month. If left blank it will default to the first day of the month. When 'daily' is set for 'interval' 'refillDay' will be set to null. + * @description The day of the month (1-31) when verifications will refill, when interval is set to 'monthly'. + * - Value is not zero-indexed (1 = first day of month) + * - Defaults to 1 if not specified + * - For months with fewer days than specified (e.g. day 31 in February), refill occurs on the last day + * - Set to null when interval is 'daily' * @default 1 * @example 15 */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx(1 hunks)packages/api/src/openapi.d.ts(7 hunks)
🔇 Additional comments (2)
apps/dashboard/app/(app)/apis/[apiId]/keys/[keyAuthId]/new/client.tsx (1)
Line range hint 288-293: LGTM! The overflow-x-auto class effectively fixes the button accessibility issue.
The implementation ensures that:
- Long keys are horizontally scrollable
- View/copy buttons remain accessible
- Responsive design is maintained for small screens
packages/api/src/openapi.d.ts (1)
Line range hint 1067-1084: LGTM! Consistent implementation across operations.
The refillDay field is consistently implemented across all relevant operations with proper type definitions and documentation.
Also applies to: 2783-2784, 2997-2998
adot-7
changed the title
|
@chronark Hey, I have created this new PR and added a simple tailwind solution as you suggested. Thank you. |
|
@perkinsjr hey would it be possible to merge this PR as hacktoberfest is coming to an end and I have made the necessary changes as suggested by other maintainers. |
|
@adot-7 without signing the CLA we can't merge this. |
I have signed it now. @perkinsjr |
|
@chronark I have signed the CLA and made the changes necessary. |
|
I have signed the CLA
…On Thu, 31 Oct 2024 at 17:18, James P ***@***.***> wrote:
@adot-7 <https://github.com/adot-7> without signing the CLA we can't
merge this.
—
Reply to this email directly, view it on GitHub
<#2620 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AONNP3XMVBEZRTOZV6TI65LZ6IKIZAVCNFSM6AAAAABQ4SFWISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBZGY3DANJRGY>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
What does this PR do?
I added className="overflow-x-auto" to pre tag in client.tsx to keep the view and copy buttons visible even if the key is longer than 87 characters as before the key was pushing the buttons out and making them inaccessible as a result.
Fixes #2401
Before
After
Type of change
How should this be tested?
Checklist
Required
pnpm buildpnpm fmtconsole.logsgit pull origin mainAppreciated
Summary by CodeRabbit
New Features
CreateKeycomponent for better user experience with a scrollable API key display.refillDay, to the key management schema for improved refill scheduling.Bug Fixes
Documentation