-
Notifications
You must be signed in to change notification settings - Fork 605
fix: improve onboarding form slug generation UX #3967
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
Changes from all commits
988ff8c
b5b5ed7
4c5efff
1c2e7b3
47c4a83
6c15e0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,7 +24,10 @@ export interface Cookie { | |||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||
| * Get a cookie value by name | ||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||
| export async function getCookie(name: string, request?: NextRequest): Promise<string | null> { | ||||||||||||||||||||||||||||
| export async function getCookie( | ||||||||||||||||||||||||||||
| name: string, | ||||||||||||||||||||||||||||
| request?: NextRequest | ||||||||||||||||||||||||||||
| ): Promise<string | null> { | ||||||||||||||||||||||||||||
| const cookieStore = request?.cookies || cookies(); | ||||||||||||||||||||||||||||
| return cookieStore.get(name)?.value ?? null; | ||||||||||||||||||||||||||||
|
Comment on lines
+27
to
32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Minor: drop unnecessary async from getCookie. This function is synchronous; removing -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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
@@ -64,7 +67,7 @@ export async function deleteCookie(name: string): Promise<void> { | |||||||||||||||||||||||||||
| export async function updateCookie( | ||||||||||||||||||||||||||||
| cookieName: string, | ||||||||||||||||||||||||||||
| value: string | null | undefined, | ||||||||||||||||||||||||||||
| reason?: string, | ||||||||||||||||||||||||||||
| reason?: string | ||||||||||||||||||||||||||||
| ): Promise<void> { | ||||||||||||||||||||||||||||
| if (value) { | ||||||||||||||||||||||||||||
| await setCookie({ | ||||||||||||||||||||||||||||
|
|
@@ -89,7 +92,7 @@ export async function updateCookie( | |||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||
| export async function setCookiesOnResponse( | ||||||||||||||||||||||||||||
| response: NextResponse, | ||||||||||||||||||||||||||||
| cookieList: Cookie[], | ||||||||||||||||||||||||||||
| cookieList: Cookie[] | ||||||||||||||||||||||||||||
| ): Promise<NextResponse> { | ||||||||||||||||||||||||||||
| for (const cookie of cookieList) { | ||||||||||||||||||||||||||||
| response.cookies.set(cookie.name, cookie.value, cookie.options); | ||||||||||||||||||||||||||||
|
|
@@ -111,14 +114,12 @@ export async function setSessionCookie(params: { | |||||||||||||||||||||||||||
| name: UNKEY_SESSION_COOKIE, | ||||||||||||||||||||||||||||
| value: token, | ||||||||||||||||||||||||||||
| options: { | ||||||||||||||||||||||||||||
| ...getDefaultCookieOptions(), | ||||||||||||||||||||||||||||
| maxAge: Math.floor((expiresAt.getTime() - Date.now()) / 1000), | ||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| export async function getCookieOptionsAsString( | ||||||||||||||||||||||||||||
| options: Partial<CookieOptions> = {}, | ||||||||||||||||||||||||||||
| options: Partial<CookieOptions> = {} | ||||||||||||||||||||||||||||
| ): Promise<string> { | ||||||||||||||||||||||||||||
| // Set defaults if not provided | ||||||||||||||||||||||||||||
| const defaultOptions: CookieOptions = getDefaultCookieOptions(); | ||||||||||||||||||||||||||||
|
|
@@ -138,7 +139,8 @@ export async function getCookieOptionsAsString( | |||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (mergedOptions.sameSite) { | ||||||||||||||||||||||||||||
| const capitalizedSameSite = | ||||||||||||||||||||||||||||
| mergedOptions.sameSite.charAt(0).toUpperCase() + mergedOptions.sameSite.slice(1); | ||||||||||||||||||||||||||||
| mergedOptions.sameSite.charAt(0).toUpperCase() + | ||||||||||||||||||||||||||||
| mergedOptions.sameSite.slice(1); | ||||||||||||||||||||||||||||
| cookieString += `; SameSite=${capitalizedSameSite}`; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.
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.
Auto-slug stops after the first update; keep syncing until manual edit.
The
!currentSlugguard prevents further updates once the slug is first set. Also mark the field dirty.🤖 Prompt for AI Agents
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.
@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!