fix: update slug as workspace name is entered#4151
Conversation
MichaelUnkey
requested review from
chronark,
mcstepp,
ogzhanolguncu and
perkinsjr
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughReworked workspace creation hook: added hydration-safe rendering ( Changes
Sequence Diagram(s)sequenceDiagram
participant UI as User Input
participant Hook as useWorkspaceStep Hook
participant Validator as Validation Logic
note over Hook,Validator #DDEBF7: Hydration-safe, onChange-driven generation
UI->>Hook: mount -> Hook sets isMounted = true
UI->>Hook: change workspaceName
Hook->>Validator: validate workspaceName
alt workspaceName.length >= 3 AND NOT slugManuallyEdited
Hook->>Hook: generate slug = slugify(workspaceName)
Hook->>Validator: validate slug
else
Hook->>Hook: do not auto-generate slug
end
sequenceDiagram
participant UI as User Input
participant Hook as useWorkspaceStep Hook
participant Validator as Validation Logic
note over Hook,Validator #F7F3DD: Slug normalization & manual-edit tracking
UI->>Hook: change slug (user types)
Hook->>Hook: clear slug errors
Hook->>Hook: slug = slugify(input)
Hook->>Hook: set slugManuallyEdited = input.length > 0
Hook->>Validator: validate slug
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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). (5)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/dashboard/app/new/hooks/use-workspace-step.tsx (1)
157-185: Critical: onChange prop overrides registered onChange handler.The slug field has two onChange handlers: one registered via
form.register("slug", { onChange: ... })at lines 157-168, and a separateonChangeprop at lines 175-185. React Hook Form spreads the registered props first, then the separateonChangeprop overrides the registered handler, causing the logic at lines 159-167 to never execute.Additionally, this creates code duplication:
- Error clearing: lines 159-161 and 176-178
- Flag setting: lines 163 and 180
- Validation trigger: lines 166 and 184
Remove the registered onChange and keep only the prop onChange, eliminating the override conflict and duplication:
- <FormInput - {...form.register("slug", { - onChange: (evt) => { - // If we don't clear the manually set error, it will persist even if the user clears - // or changes the input - form.clearErrors("slug"); - const v = evt.currentTarget.value; - setSlugManuallyEdited(v.length > 0); - - // Re-validate on change to update validFieldCount - form.trigger("slug"); - }, - })} + <FormInput + {...form.register("slug")} placeholder="enter-a-handle" label="Workspace URL handle" required
🧹 Nitpick comments (1)
apps/dashboard/app/new/hooks/use-workspace-step.tsx (1)
181-184: Remove redundant validation trigger.Line 181 calls
form.setValue("slug", slugify(v), { shouldValidate: true }), which already triggers validation for the slug field. The subsequentform.trigger("slug")at line 184 is redundant and causes double validation on every keystroke.Apply this diff to remove the redundant trigger:
form.setValue("slug", slugify(v), { shouldValidate: true, }); - form.trigger("slug");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/dashboard/app/new/hooks/use-workspace-step.tsx(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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
⏰ 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
ogzhanolguncu
left a comment
ogzhanolguncu
left a comment
There was a problem hiding this comment.
Screen.Recording.2025-10-24.at.16.15.16.mov
I think we have a problem clearing it
…-enhancement-improve-onboarding-form-slug-generation
…-enhancement-improve-onboarding-form-slug-generation
chronark
left a comment
chronark
left a comment
There was a problem hiding this comment.
there is some flickering as the slug input loads, otherwise it works well
Screen Recording 2025-10-28 at 06.23.47.mov (uploaded via Graphite)
…-enhancement-improve-onboarding-form-slug-generation
ogzhanolguncu
left a comment
ogzhanolguncu
left a comment
There was a problem hiding this comment.
LGTM. I didn't see any flicker or any other weirdness 😄
…-enhancement-improve-onboarding-form-slug-generation
|
|
perkinsjr
deleted the
eng-2076-enhancement-improve-onboarding-form-slug-generation
branch
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (11/04/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
4 participants
What does this PR do?
Fixes # (issue)
ENG-2076
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