fix: Long secret key in success dialog#3938
Conversation
MichaelUnkey
requested review from
chronark,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughRefactors KeyCreatedSuccessDialog to a typed FC with a new props interface, makes onClose/onCreateAnother awaitable, updates confirm-close flow to await callbacks, and adjusts KeySecretSection layout by moving visibility/copy controls outside the Code block and adding overflow handling. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant D as KeyCreatedSuccessDialog
participant S as KeySecretSection
participant C as Consumer (caller)
U->>D: Click "Create another" or "Close"
alt Create another provided
D->>C: call onCreateAnother()
Note right of D#orange: handler is awaited (may be Promise)
C-->>D: resolves
end
D->>C: call onClose()
Note right of D#lightblue: onClose is awaited (void or Promise)
C-->>D: resolves
D->>S: render secret (showKeyInSnippet / masked)
S-->>D: user toggles visibility / copies
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-08-13T19:55:19.828ZApplied to files:
📚 Learning: 2025-08-13T19:55:19.828ZApplied to files:
🧬 Code graph analysis (1)apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-secret-section.tsx (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
🔇 Additional comments (6)
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx (2)
119-121: Modal min-width breaks small screens; use responsive width instead.
min-w-[760px]will overflow/clamp on mobile and can still clip long keys despite inner scrolling. Prefer a responsive width with a max cap.Apply this diff:
- <DialogContent - className="drop-shadow-2xl border-gray-4 overflow-hidden !rounded-2xl p-0 gap-0 min-w-[760px] max-h-[90vh] overflow-y-auto" + <DialogContent + className="drop-shadow-2xl border-gray-4 overflow-hidden !rounded-2xl p-0 gap-0 w-[90vw] max-w-[760px] max-h-[90vh] overflow-y-auto"
40-45: beforeunload handler won’t show a prompt in modern browsers.You need to set
e.returnValueto trigger the native confirmation.Apply this diff:
- const handleBeforeUnload = (e: BeforeUnloadEvent) => { - e.preventDefault(); - }; + const handleBeforeUnload = (e: BeforeUnloadEvent) => { + e.preventDefault(); + e.returnValue = ""; + };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3156
File: apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/index.tsx:112-0
Timestamp: 2025-04-22T11:49:06.167Z
Learning: In the CreateKeyDialog component of Unkey, section navigation is designed to always allow progression even if validation fails, as visual indicators display the validation state of each section.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Build / Build
- GitHub Check: Test API / API Test Local
- GitHub Check: Test Go API Local / Test
...oard/app/(app)/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx
Outdated
Show resolved
Hide resolved
...oard/app/(app)/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx
Outdated
Show resolved
Hide resolved
this is what it looks like for me
I tried it with a 200 byte length key |
chronark
left a comment
chronark
left a comment
There was a problem hiding this comment.
I think you need to clamp the key secret itself, not the entire dialog
eng-2057-creating-long-api-keys-cuts-off-modal-content
eng-2057-creating-long-api-keys-cuts-off-modal-content
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx (3)
13-20: Props allow async callbacks, but they aren’t awaited → unhandled rejections/ordering hazards
onClose/onCreateAnothercan return Promise, yethandleConfirmClosecalls them withoutawait, so errors won’t hit the try/catch and sequencing may race.Make
handleConfirmCloseasync and await callbacks:- const handleConfirmClose = () => { + const handleConfirmClose = async () => { if (!pendingAction) { console.error("No pending action when confirming close"); return; } setIsConfirmOpen(false); try { - // Always close the dialog first - onClose(); + // Always close the dialog first + await onClose(); // Then execute the specific action switch (pendingAction) { case "create-another": - if (onCreateAnother) { - onCreateAnother(); - } else { + if (onCreateAnother) { + await onCreateAnother(); + } else { console.warn("onCreateAnother callback not provided"); } break;Confirm no downstream code relies on fire-and-forget behavior; if so, we can wrap awaits in
voidwith explicit.catch(toast…)instead.
122-122: Fixed min-width (760px) breaks small screens; use responsive width constraintsA hard
min-w-[760px]can overflow mobile/tablet viewports. Prefer a responsive width cap:- className="drop-shadow-2xl border-gray-4 !rounded-2xl p-0 gap-0 min-w-[760px] max-h-[90vh] overflow-auto" + className="drop-shadow-2xl border-gray-4 !rounded-2xl p-0 gap-0 w-[min(92vw,760px)] max-h-[90vh] overflow-y-auto"Resize the viewport to <760px and ensure no horizontal scrolling is introduced by the dialog container.
38-52: beforeunload handler won’t reliably warn; sete.returnValueBrowsers require
e.returnValue = ""(or a string) to trigger the native unload confirmation.- const handleBeforeUnload = (e: BeforeUnloadEvent) => { - e.preventDefault(); - }; + const handleBeforeUnload = (e: BeforeUnloadEvent) => { + e.preventDefault(); + e.returnValue = ""; + };Confirm the native tab-close prompt appears while the dialog is open.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx(3 hunks)apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-secret-section.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3156
File: apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/index.tsx:112-0
Timestamp: 2025-04-22T11:49:06.167Z
Learning: In the CreateKeyDialog component of Unkey, section navigation is designed to always allow progression even if validation fails, as visual indicators display the validation state of each section.
📚 Learning: 2025-07-02T14:13:01.711Z
Learnt from: MichaelUnkey
PR: unkeyed/unkey#3425
File: apps/engineering/content/design/components/filter/control-cloud.examples.tsx:73-83
Timestamp: 2025-07-02T14:13:01.711Z
Learning: In apps/engineering/content/design/components/, when the `RenderComponentWithSnippet` component does not render code snippets correctly, use the `customCodeSnippet` prop to manually provide the correct JSX code as a string. This manual approach is necessary due to technical limitations in the automatic rendering mechanism.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-secret-section.tsx
🧬 Code graph analysis (1)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-secret-section.tsx (1)
internal/ui/src/components/buttons/copy-button.tsx (1)
CopyButton(27-81)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Go API Local / Test
- GitHub Check: Test API / API Test Local
- GitHub Check: Build / Build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx (1)
174-176: Good use oftruncateat the single-line key nameSingle-line, bounded text with a tooltip is the right place for
truncate. No change needed.
...workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx
Outdated
Show resolved
Hide resolved
...workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx
Outdated
Show resolved
Hide resolved
.../(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-secret-section.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-secret-section.tsx (1)
71-74: Inner scrolling div likely unnecessary; rely on Code’s pre for padding/scrolling.Simplify to reduce nested scroll containers and potential clipping. This is addressed in the diff above by moving
p-4topreClassNameand removing the inner wrapper.apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx (1)
122-122: Hard min-width can cause horizontal overflow on small screens.Prefer responsive sizing to keep the dialog usable on mobile while preserving the intended width on larger screens.
- className="drop-shadow-2xl border-gray-4 !rounded-2xl p-0 gap-0 min-w-[760px] max-h-[90vh] overflow-auto" + className="drop-shadow-2xl border-gray-4 !rounded-2xl p-0 gap-0 w-[calc(100vw-2rem)] sm:min-w-[760px] max-w-[min(100vw-2rem,760px)] max-h-[90vh] overflow-auto"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx(3 hunks)apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-secret-section.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3156
File: apps/dashboard/app/(app)/apis/[apiId]/_components/create-key/index.tsx:112-0
Timestamp: 2025-04-22T11:49:06.167Z
Learning: In the CreateKeyDialog component of Unkey, section navigation is designed to always allow progression even if validation fails, as visual indicators display the validation state of each section.
📚 Learning: 2025-07-02T14:13:01.711Z
Learnt from: MichaelUnkey
PR: unkeyed/unkey#3425
File: apps/engineering/content/design/components/filter/control-cloud.examples.tsx:73-83
Timestamp: 2025-07-02T14:13:01.711Z
Learning: In apps/engineering/content/design/components/, when the `RenderComponentWithSnippet` component does not render code snippets correctly, use the `customCodeSnippet` prop to manually provide the correct JSX code as a string. This manual approach is necessary due to technical limitations in the automatic rendering mechanism.
Applied to files:
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-secret-section.tsx
🔇 Additional comments (2)
apps/dashboard/app/(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx (2)
9-9: Type‑only import for FC is fine.Import hygiene looks good; using
type FCavoids bundling types at runtime.
189-196: Verify long key UX end‑to‑end.After applying the above, please validate:
- 200‑byte key renders fully without ellipsis in SecretKey; horizontal scroll works.
- Code block scrolls horizontally; overlay buttons stay aligned at top‑right and remain clickable.
- Toggling visibility updates both display and CopyButton value; copying while masked does not leak the real key.
- Dialog remains usable on narrow viewports (<760px) without horizontal page scroll.
...workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx
Show resolved
Hide resolved
...workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx
Outdated
Show resolved
Hide resolved
...workspaceSlug]/apis/[apiId]/_components/create-key/components/key-created-success-dialog.tsx
Outdated
Show resolved
Hide resolved
.../(app)/[workspaceSlug]/apis/[apiId]/_components/create-key/components/key-secret-section.tsx
Outdated
Show resolved
Hide resolved
… of https://github.com/unkeyed/unkey into eng-2057-creating-long-api-keys-cuts-off-modal-content
…-creating-long-api-keys-cuts-off-modal-content
… of https://github.com/unkeyed/unkey into eng-2057-creating-long-api-keys-cuts-off-modal-content
chronark
deleted the
eng-2057-creating-long-api-keys-cuts-off-modal-content
branch
2 participants
… snippet
What does this PR do?
Fixes # (issue)
ENG-2057
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