fix: improve onboarding form slug generation UX#3967
fix: improve onboarding form slug generation UX#3967naaa760 wants to merge 6 commits intounkeyed:mainfrom naaa760:slug-enhance
Conversation
Update workspace name field to generate slug in real-time as user types, replacing onBlur behavior with onChange for better user experience.
naaa760
requested review from
chronark,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@naaa760 is attempting to deploy a commit to the Unkey Team on Vercel. A member of the Team first needs to authorize it. |
|
neha seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Warning Rate limit exceeded@naaa760 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 16 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAuto-generate workspace slug as the user types the workspace name (unless the slug was manually edited); mark manual slug edits to stop auto-generation. Also clamp session cookie maxAge to a non-negative value. No public API/signature changes. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant Form as Onboarding Form
participant Hook as use-workspace-step
participant Val as Form Validation
U->>Form: type workspace name (onChange)
Form->>Hook: handleNameChange(name)
alt slug not manually edited
Hook->>Hook: slug = slugify(name)
Hook->>Form: setValue("slug", slug)
Hook->>Val: trigger("slug")
else manually edited
Hook->>Form: no auto-update
end
U->>Form: type in slug input (onChange)
Form->>Hook: set slugManuallyEdited = true
Hook->>Val: trigger("slug")
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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. Comment |
|
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
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/new/hooks/use-workspace-step.tsx (2)
179-191: Bug: overriding RHF’s onChange breaks slug field state/validation.Prop-level
onChangeafter{...register("slug")}replaces RHF’s handler; form won’t see slug edits. Also make the “manual edit” flag sticky.- <FormInput - {...form.register("slug")} + <FormInput + {...form.register("slug", { + onChange: () => { + // Sticky: once edited manually, never auto-update again + setSlugManuallyEdited(true); + }, + })} placeholder="enter-a-handle" label="Workspace URL handle" required error={form.formState.errors.slug?.message} prefix="app.unkey.com/" maxLength={64} - onChange={(evt) => { - const v = evt.currentTarget.value; - setSlugManuallyEdited(v.length > 0); - }} />
219-227: Optional: handle diacritics for better international slugs (keep inline).Small enhancement: strip diacritics before filtering. Keeping it inline aligns with mcstepp’s earlier preference for POCs.
-const slugify = (text: string): string => { - return text - .toLowerCase() - .trim() - .replace(/[^a-z0-9\s-]/g, "") // Remove special chars except lowercase letters, numbers, spaces, and hyphens - .replace(/\s+/g, "-") // Replace spaces with hyphens - .replace(/-+/g, "-") // Replace multiple hyphens with single - .replace(/^-|-$/g, ""); // Remove leading/trailing hyphens -}; +const slugify = (text: string): string => { + return text + .normalize("NFD") // split base chars and diacritics + .replace(/\p{Diacritic}/gu, "") // drop diacritics (requires modern runtime) + .toLowerCase() + .trim() + .replace(/[^a-z0-9\s-]/g, "") // keep lowercase letters, numbers, spaces, hyphens + .replace(/\s+/g, "-") // spaces -> hyphens + .replace(/-+/g, "-") // collapse hyphens + .replace(/^-|-$/g, ""); // trim leading/trailing hyphens +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/app/new/hooks/use-workspace-step.tsx(5 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.
Applied to files:
apps/dashboard/app/new/hooks/use-workspace-step.tsx
📚 Learning: 2024-10-04T17:27:09.821Z
Learnt from: chronark
PR: unkeyed/unkey#2146
File: apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx:74-75
Timestamp: 2024-10-04T17:27:09.821Z
Learning: In `apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx`, the hidden `<input>` elements for `workspaceId` and `keyAuthId` work correctly without being registered with React Hook Form.
Applied to files:
apps/dashboard/app/new/hooks/use-workspace-step.tsx
🔇 Additional comments (2)
apps/dashboard/app/new/hooks/use-workspace-step.tsx (2)
26-26: Validation message clarity LGTM.
170-172: Registering onChange via RHF options is correct.Once the handler above is fixed, this wiring preserves RHF semantics.
Please verify the slug updates on every keystroke in “Workspace name” until the slug input is manually edited.
| const handleNameChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const name = e.target.value; | ||
| const currentSlug = form.getValues("slug"); | ||
| const isSlugDirty = form.formState.dirtyFields.slug; | ||
|
|
||
| // Only auto-generate if slug is empty, not dirty, and hasn't been manually edited | ||
| if (!currentSlug && !isSlugDirty && !slugManuallyEdited) { | ||
| form.setValue("slug", slugify(name), { | ||
| shouldValidate: true, | ||
| }); | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Auto-slug stops after the first update; keep syncing until manual edit.
The !currentSlug guard prevents further updates once the slug is first set. Also mark the field dirty.
- const handleNameChange = (e: React.ChangeEvent<HTMLInputElement>) => {
- const name = e.target.value;
- const currentSlug = form.getValues("slug");
- const isSlugDirty = form.formState.dirtyFields.slug;
-
- // Only auto-generate if slug is empty, not dirty, and hasn't been manually edited
- if (!currentSlug && !isSlugDirty && !slugManuallyEdited) {
- form.setValue("slug", slugify(name), {
- shouldValidate: true,
- });
- }
- };
+ const handleNameChange = (e: ChangeEvent<HTMLInputElement>) => {
+ const name = e.target.value;
+ if (!slugManuallyEdited) {
+ form.setValue("slug", slugify(name), {
+ shouldValidate: true,
+ shouldDirty: true,
+ });
+ }
+ };Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/dashboard/app/new/hooks/use-workspace-step.tsx around lines 103 to 115,
the auto-slug logic stops updating after the first auto-fill because of the
`!currentSlug` guard and doesn't mark the field as dirty; remove the
`!currentSlug` check so the slug continues to sync on every name change until
the user manually edits it, and when calling form.setValue for "slug" include
the option to mark the field dirty (e.g., shouldDirty: true) so react-hook-form
knows it was programmatically modified.
There was a problem hiding this comment.
@perkinsjr Thank you for addressing the auto-slug synchronization issue! I'm glad the logic has been updated to continue syncing the slug with the workspace name until the user manually edits it.
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
|
Hi @naaa760 Please run pnpm fmt from the root of the Unkey monorepo. Also please check the Coderabbit comments for potential bugs. |
|
Hi @perkinsjr
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/dashboard/lib/auth/cookies.ts (1)
67-88: Bug: cookie not deleted when value is falsy and no reason provided.Per the docstring, the cookie should always be cleared if
valueis null/undefined. Currently it only deletes whenreasonis set.if (value) { await setCookie({ name: cookieName, value: value, options: { httpOnly: true, secure: true, sameSite: "strict", }, }) return } - if (reason) { - console.error("Session refresh failed:", reason) - await deleteCookie(cookieName) - } + if (reason) { + console.error("Session refresh failed:", reason) + } + await deleteCookie(cookieName)apps/dashboard/app/new/hooks/use-workspace-step.tsx (2)
189-193: Potential RHF onChange override: wire through register to avoid stomping.Passing
onChangealongside{...form.register("slug")}can override RHF’s handler depending on howFormInputspreads props. Safer to attach viaregister’sonChangeso both fire.- <FormInput - {...form.register("slug")} + <FormInput + {...form.register("slug", { + onChange: (evt) => { + const v = (evt as React.ChangeEvent<HTMLInputElement>).currentTarget.value + setSlugManuallyEdited(v.length > 0) + }, + })} placeholder="enter-a-handle" label="Workspace URL handle" required error={form.formState.errors.slug?.message} prefix="app.unkey.com/" maxLength={64} - onChange={(evt) => { - const v = evt.currentTarget.value; - setSlugManuallyEdited(v.length > 0); - }} />Also applies to: 182-193
221-229: Mirror schema limit: enforce 64-char cap in slugify.Prevents transient validation churn when programmatically setting long slugs.
const slugify = (text: string): string => { - return text + return text .toLowerCase() .trim() .replace(/[^a-z0-9\s-]/g, "") // Remove special chars except lowercase letters, numbers, spaces, and hyphens .replace(/\s+/g, "-") // Replace spaces with hyphens .replace(/-+/g, "-") // Replace multiple hyphens with single - .replace(/^-|-$/g, ""); // Remove leading/trailing hyphens + .replace(/^-|-$/g, "") // Remove leading/trailing hyphens + .slice(0, 64); };
♻️ Duplicate comments (1)
apps/dashboard/app/new/hooks/use-workspace-step.tsx (1)
8-9: Nit: prefer type-only event import; drop default React type.Use
ChangeEventdirectly and avoid the extra default type import. This repeats prior feedback.-import type React from "react"; -import { useRef, useState } from "react"; +import { useRef, useState, type ChangeEvent } from "react"; @@ - const handleNameChange = (e: React.ChangeEvent<HTMLInputElement>) => { + const handleNameChange = (e: ChangeEvent<HTMLInputElement>) => {Also applies to: 105-116
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
apps/dashboard/app/(app)/api/auth/refresh/route.ts(2 hunks)apps/dashboard/app/new/hooks/use-workspace-step.tsx(5 hunks)apps/dashboard/lib/auth/cookies.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-05-05T17:55:59.607Z
Learnt from: mcstepp
PR: unkeyed/unkey#3215
File: apps/dashboard/lib/auth/sessions.ts:47-51
Timestamp: 2025-05-05T17:55:59.607Z
Learning: Local auth cookies in apps/dashboard/lib/auth/sessions.ts intentionally omit HttpOnly and Secure flags to allow easier debugging during local development. This is by design as these cookies are only used in local development environments, not production.
Applied to files:
apps/dashboard/lib/auth/cookies.ts
📚 Learning: 2025-07-28T19:42:37.047Z
Learnt from: mcstepp
PR: unkeyed/unkey#3662
File: apps/dashboard/app/(app)/projects/page.tsx:74-81
Timestamp: 2025-07-28T19:42:37.047Z
Learning: In apps/dashboard/app/(app)/projects/page.tsx, the user mcstepp prefers to keep placeholder functions like generateSlug inline during POC/demonstration phases rather than extracting them to utility modules, with plans to refactor later when the feature matures beyond the proof-of-concept stage.
Applied to files:
apps/dashboard/app/new/hooks/use-workspace-step.tsx
📚 Learning: 2024-10-04T17:27:09.821Z
Learnt from: chronark
PR: unkeyed/unkey#2146
File: apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx:74-75
Timestamp: 2024-10-04T17:27:09.821Z
Learning: In `apps/dashboard/app/(app)/apis/[apiId]/settings/default-prefix.tsx`, the hidden `<input>` elements for `workspaceId` and `keyAuthId` work correctly without being registered with React Hook Form.
Applied to files:
apps/dashboard/app/new/hooks/use-workspace-step.tsx
🔇 Additional comments (9)
apps/dashboard/lib/auth/cookies.ts (4)
122-126: Good fix: clamp Max-Age to non-negative.Prevents invalid negative
Max-Agedue to clock skew or stale expirations.
131-132: LGTM: default param for options.Makes the helper easier to consume.
156-159: LGTM: clearer SameSite capitalization.
108-128: Unify session cookie policy and avoid drift.
setSessionCookie (apps/dashboard/lib/auth/cookies.ts:108–128) uses SameSite: "strict" while the refresh route writes "lax"; route all session-cookie writes (refresh and other handlers) through setSessionCookie so httpOnly, secure, sameSite, path and maxAge remain consistent and reduce CSRF risk.apps/dashboard/app/(app)/api/auth/refresh/route.ts (2)
11-14: LGTM: formatting only.
38-41: LGTM: formatting only.apps/dashboard/app/new/hooks/use-workspace-step.tsx (3)
27-28: LGTM: clearer validation copy.
60-64: Good fix: clamp Max-Age to non-negative.
172-175: LGTM: name field keeps slug in sync via register onChange.
| maxAge: Math.max( | ||
| 0, | ||
| Math.floor((expiresAt.getTime() - Date.now()) / 1000) | ||
| ), // Convert to seconds | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Good clamp; also centralize cookie creation via setSessionCookie.
Prevents negative Max-Age. To avoid attribute drift (e.g., SameSite mismatch with cookies.ts), call the shared helper.
-import { setCookie } from "@/lib/auth/cookies";
-import { UNKEY_SESSION_COOKIE } from "@/lib/auth/types";
+import { setSessionCookie } from "@/lib/auth/cookies";
@@
- await setCookie({
- name: UNKEY_SESSION_COOKIE,
- value: newToken,
- options: {
- httpOnly: true,
- secure: true,
- sameSite: "lax",
- path: "/",
- maxAge: Math.max(
- 0,
- Math.floor((expiresAt.getTime() - Date.now()) / 1000)
- ), // Convert to seconds
- },
- });
+ await setSessionCookie({ token: newToken, expiresAt });📝 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.
| maxAge: Math.max( | |
| 0, | |
| Math.floor((expiresAt.getTime() - Date.now()) / 1000) | |
| ), // Convert to seconds | |
| }, | |
| import { setSessionCookie } from "@/lib/auth/cookies"; | |
| await setSessionCookie({ token: newToken, expiresAt }); |
🤖 Prompt for AI Agents
In apps/dashboard/app/(app)/api/auth/refresh/route.ts around lines 28 to 32, the
code manually computes and sets a clamped Max-Age for the session cookie which
prevents negative values but can cause attribute drift; replace the manual
cookie creation with a call to the shared setSessionCookie helper (passing the
computed maxAge as Math.max(0, Math.floor((expiresAt.getTime() - Date.now()) /
1000))) so the cookie attributes (SameSite, Path, Secure, etc.) are centralized
and consistent across the app.
| const handleNameChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| const name = e.target.value; | ||
| const currentSlug = form.getValues("slug"); | ||
| const isSlugDirty = form.formState.dirtyFields.slug; | ||
|
|
||
| // Only auto-generate if slug hasn't been manually edited | ||
| if (!slugManuallyEdited) { | ||
| form.setValue("slug", slugify(name), { | ||
| shouldValidate: true, | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Keep RHF state accurate and clamp slug length.
Mark the field dirty so RHF treats programmatic updates as user changes. Also trim to 64 chars to mirror the schema and input maxLength.
if (!slugManuallyEdited) {
- form.setValue("slug", slugify(name), {
- shouldValidate: true,
- });
+ form.setValue("slug", slugify(name).slice(0, 64), {
+ shouldValidate: true,
+ shouldDirty: true,
+ });
}📝 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.
| const handleNameChange = (e: React.ChangeEvent<HTMLInputElement>) => { | |
| const name = e.target.value; | |
| const currentSlug = form.getValues("slug"); | |
| const isSlugDirty = form.formState.dirtyFields.slug; | |
| // Only auto-generate if slug hasn't been manually edited | |
| if (!slugManuallyEdited) { | |
| form.setValue("slug", slugify(name), { | |
| shouldValidate: true, | |
| }); | |
| } | |
| }; | |
| const handleNameChange = (e: React.ChangeEvent<HTMLInputElement>) => { | |
| const name = e.target.value; | |
| const currentSlug = form.getValues("slug"); | |
| const isSlugDirty = form.formState.dirtyFields.slug; | |
| // Only auto-generate if slug hasn't been manually edited | |
| if (!slugManuallyEdited) { | |
| form.setValue("slug", slugify(name).slice(0, 64), { | |
| shouldValidate: true, | |
| shouldDirty: true, | |
| }); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In apps/dashboard/app/new/hooks/use-workspace-step.tsx around lines 105 to 116,
the auto-generated slug is being set programmatically but not marked dirty and
can exceed the schema/input max length; update the handler to clamp the slug to
64 characters (e.g. take slugify(name) then slice to 64) and call
form.setValue("slug", clampedSlug, { shouldValidate: true, shouldDirty: true })
so RHF treats the programmatic update as a user change and validation/dirty
state remain correct.
| export async function getCookie( | ||
| name: string, | ||
| request?: NextRequest | ||
| ): Promise<string | null> { | ||
| const cookieStore = request?.cookies || cookies(); | ||
| return cookieStore.get(name)?.value ?? null; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Minor: drop unnecessary async from getCookie.
This function is synchronous; removing async tightens types and avoids implicit Promise wrapping.
-export async function getCookie(
+export function getCookie(
name: string,
request?: NextRequest
-): Promise<string | null> {
+): string | null {
const cookieStore = request?.cookies || cookies()
return cookieStore.get(name)?.value ?? null
}📝 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.
| export async function getCookie( | |
| name: string, | |
| request?: NextRequest | |
| ): Promise<string | null> { | |
| const cookieStore = request?.cookies || cookies(); | |
| return cookieStore.get(name)?.value ?? null; | |
| export function getCookie( | |
| name: string, | |
| request?: NextRequest | |
| ): string | null { | |
| const cookieStore = request?.cookies || cookies(); | |
| return cookieStore.get(name)?.value ?? null; | |
| } |
🤖 Prompt for AI Agents
In apps/dashboard/lib/auth/cookies.ts around lines 26 to 31, the getCookie
function is marked async despite being fully synchronous; remove the async
keyword and update the return type from Promise<string | null> to string | null
so the function returns a plain value and avoids unnecessary Promise wrapping,
then run TypeScript checks to ensure callers still handle the non-Promise return
(adjust call sites if they awaited the function).
perkinsjr
left a comment
perkinsjr
left a comment
There was a problem hiding this comment.
Why are there changes to the api/auth/refresh and /lib/auth?
This change should only affect the use-workspace-step.tsx
It looks like you open the file and then your IDE formatted them, please revert them.
Then run pnpm fmt and that should resolve the changes.
3 participants
What does this PR do?
Changes made:
handleNameChangefunction for real-time slug generationonBlurevent withonChangeregistration in the workspace name inputfixes #3966
How should this be tested?
/new)Summary by CodeRabbit
New Features
Bug Fixes