-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: Add signup page and update navbar #1012
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
Conversation
WalkthroughIntroduces a new signup page and client-side SignupForm with email, Google, and SSO flows, plus billing handoff. Updates navbar to surface login/signup appropriately. Adds a login-page sign-up prompt and adjusts spacing. One component receives formatting-only changes. No backend/auth logic changes beyond integrating existing providers. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant P as /signup page (server)
participant C as SignupForm (client)
participant A as Auth (NextAuth)
participant B as Billing API
participant N as Next Router
U->>P: GET /signup
P->>A: getCurrentUser()
alt Session exists
P-->>U: redirect to /dashboard
else No session
P-->>U: Render SignupForm
U->>C: Choose flow
alt Email signup
C->>A: signIn('email', {email, redirect:false, callbackUrl})
A-->>C: success/failure
opt On success
C->>N: navigate to /verify-otp?email=...&next=...&lastSent=...
opt Pending price in localStorage
C->>B: POST /api/settings/billing/subscribe
B-->>C: { redirectUrl? }
alt redirectUrl provided
C->>N: navigate to redirectUrl
end
end
end
else Google signup
C->>A: signIn('google', {callbackUrl?})
else SSO signup
U->>C: Enter Organization ID
C->>A: signIn('workos', {organization, connection})
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
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.
Actionable comments posted: 4
🧹 Nitpick comments (7)
apps/web/components/ReadyToGetStarted.tsx (1)
8-16: File naming: switch to kebab-case.
Guidelines call for kebab-case file names. Suggest renaming to apps/web/components/ready-to-get-started.tsx and updating imports.apps/web/app/(site)/Navbar.tsx (2)
162-179: Invalid Tailwind class “focus:text-8”.
Likely intended “focus:text-blue-8”. Fix to avoid dead class.- "px-2 py-0 text-sm font-medium hover:text-blue-9 focus:text-8", + "px-2 py-0 text-sm font-medium hover:text-blue-9 focus:text-blue-8",
103-103: File naming: kebab-case preferred.
Consider renaming Navbar.tsx → navbar.tsx per guidelines (update imports).apps/web/app/(org)/signup/form.tsx (4)
247-261: Extract resend wait into a constant.
Minor readability and single-source-of-truth for rate limit window.- const waitTime = 30000; // 30 seconds - if (timeSinceLastRequest < waitTime) { + const RESEND_WAIT_MS = 30_000; // 30 seconds + if (timeSinceLastRequest < RESEND_WAIT_MS) { const remainingSeconds = Math.ceil( - (waitTime - timeSinceLastRequest) / 1000, + (RESEND_WAIT_MS - timeSinceLastRequest) / 1000, );
321-356: Add rel="noopener noreferrer" to target="_blank" links.
Prevents opener leaks; even for same-origin it’s a safe default.- <Link - href="/terms" - target="_blank" - className="text-xs font-semibold text-gray-12 hover:text-blue-300" - > + <Link + href="/terms" + target="_blank" + rel="noopener noreferrer" + className="text-xs font-semibold text-gray-12 hover:text-blue-300" + > ... - <Link - href="/privacy" - target="_blank" - className="text-xs font-semibold text-gray-12 hover:text-blue-300" - > + <Link + href="/privacy" + target="_blank" + rel="noopener noreferrer" + className="text-xs font-semibold text-gray-12 hover:text-blue-300" + >
112-131: Disable SSO submit while looking up org.
Reuse loading state to prevent double submissions.- const handleOrganizationLookup = async (e: React.FormEvent) => { + const handleOrganizationLookup = async (e: React.FormEvent) => { e.preventDefault(); if (!organizationId) { toast.error("Please enter an organization ID"); return; } try { + setLoading(true); const data = await getOrganizationSSOData(organizationId); setOrganizationName(data.name); signIn("workos", undefined, { organization: data.organizationId, connection: data.connectionId, }); } catch (error) { console.error("Lookup Error:", error); toast.error("Organization not found or SSO not configured"); + } finally { + setLoading(false); } };Also pass loading to SignupWithSSO and set Button disabled={loading}.
- <SignupWithSSO + <SignupWithSSO handleOrganizationLookup={handleOrganizationLookup} organizationId={organizationId} setOrganizationId={setOrganizationId} - organizationName={organizationName} + organizationName={organizationName} + loading={loading} />And in SignupWithSSO props and Button:
-}: { +}: { handleOrganizationLookup: (e: React.FormEvent) => void; organizationId: string; setOrganizationId: (organizationId: string) => void; - organizationName: string | null; + organizationName: string | null; + loading: boolean; }) => { ... - <Button type="submit" variant="dark" className="w-full max-w-full"> + <Button type="submit" variant="dark" className="w-full max-w-full" disabled={loading}>
77-104: Shared logic duplicated with LoginForm (billing handoff, error toasts).
Consider a small hook (e.g., useAuthBillingHandoff, useAuthErrorToasts) to DRY up both forms.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/(org)/login/form.tsx(1 hunks)apps/web/app/(org)/signup/form.tsx(1 hunks)apps/web/app/(org)/signup/page.tsx(1 hunks)apps/web/app/(site)/Navbar.tsx(3 hunks)apps/web/components/ReadyToGetStarted.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for client-side server state and data fetching in the web app
Mutations should call Server Actions and perform precise cache updates with setQueryData/setQueriesData, avoiding broad invalidations
Prefer Server Components for initial data and pass initialData to client components for React Query hydration
Files:
apps/web/app/(org)/login/form.tsxapps/web/app/(org)/signup/page.tsxapps/web/components/ReadyToGetStarted.tsxapps/web/app/(org)/signup/form.tsxapps/web/app/(site)/Navbar.tsx
{apps/web,packages/ui}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
{apps/web,packages/ui}/**/*.{ts,tsx}: Use Tailwind CSS exclusively for styling in the web app and shared React UI components
Component naming: React components in PascalCase; hooks in camelCase starting with 'use'
Files:
apps/web/app/(org)/login/form.tsxapps/web/app/(org)/signup/page.tsxapps/web/components/ReadyToGetStarted.tsxapps/web/app/(org)/signup/form.tsxapps/web/app/(site)/Navbar.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; prefer shared types from packages
**/*.{ts,tsx}: Use Biome to format/lint TypeScript with a 2-space indent
TypeScript file names should be kebab-case (e.g., user-menu.tsx)
Files:
apps/web/app/(org)/login/form.tsxapps/web/app/(org)/signup/page.tsxapps/web/components/ReadyToGetStarted.tsxapps/web/app/(org)/signup/form.tsxapps/web/app/(site)/Navbar.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
React/Solid components should be named using PascalCase
Files:
apps/web/app/(org)/login/form.tsxapps/web/app/(org)/signup/page.tsxapps/web/components/ReadyToGetStarted.tsxapps/web/app/(org)/signup/form.tsxapps/web/app/(site)/Navbar.tsx
🧬 Code graph analysis (3)
apps/web/app/(org)/signup/page.tsx (1)
apps/web/app/(org)/signup/form.tsx (1)
SignupForm(27-361)
apps/web/components/ReadyToGetStarted.tsx (1)
apps/web/data/homepage-copy.ts (1)
homepageCopy(110-318)
apps/web/app/(org)/signup/form.tsx (2)
packages/ui/src/components/icons/LogoBadge.tsx (1)
LogoBadge(1-28)apps/web/actions/organization/get-organization-sso-data.ts (1)
getOrganizationSSOData(7-34)
⏰ 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 Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Analyze (rust)
🔇 Additional comments (7)
apps/web/components/ReadyToGetStarted.tsx (1)
5-5: Import reorder is fine.
No functional impact; consistent with style.apps/web/app/(org)/login/form.tsx (2)
440-451: Nice UX touch: sign-up prompt on login.
Copy, link target, and styling look good.
452-456: Spacing tweak LGTM.
Explicit mt-4/mb-4 maintains previous layout.apps/web/app/(site)/Navbar.tsx (2)
96-102: Auth link config is clear and scoped.
Good separation; renders only when unauthenticated.
249-256: Auth-aware CTA is correct.
Unauthenticated users see “Sign Up” to /signup; authenticated see Dashboard.apps/web/app/(org)/signup/page.tsx (2)
8-14: Correct redirect gate.
Authenticated users are redirected server-side to /dashboard; dynamic rendering prevents stale cache.
16-27: Page scaffold looks good.
Clear back-to-home affordance and composes the client SignupForm cleanly.
| const next = searchParams?.get("next"); | ||
| const [email, setEmail] = useState(""); |
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.
Sanitize callbackUrl to prevent open redirects.
Validate next before passing to NextAuth. Use only same-origin relative paths.
- const next = searchParams?.get("next");
+ const next = searchParams?.get("next");
+ const isSafeRelativePath = (p: string) =>
+ typeof p === "string" && p.startsWith("/") && !p.startsWith("//") && !p.includes("://");
+ const safeNext = next && isSafeRelativePath(next) ? next : undefined;📝 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 next = searchParams?.get("next"); | |
| const [email, setEmail] = useState(""); | |
| - const next = searchParams?.get("next"); | |
| const next = searchParams?.get("next"); | |
| const isSafeRelativePath = (p: string | null | undefined): p is string => | |
| typeof p === "string" && | |
| p.startsWith("/") && | |
| !p.startsWith("//") && | |
| !p.includes("://"); | |
| const safeNext = isSafeRelativePath(next) ? next : undefined; | |
| const [email, setEmail] = useState(""); |
🤖 Prompt for AI Agents
In apps/web/app/(org)/signup/form.tsx around lines 30-31, the raw next from
searchParams is used directly which can enable open redirects; validate and
sanitize it before passing to NextAuth by allowing only same-origin relative
paths: if next is missing or invalid, replace with a safe default (e.g., "/");
reject values that start with "//" or contain an absolute scheme (http:, https:)
or a different origin; implement the check by ensuring the value begins with a
single "/" and does not contain "://" or by constructing a URL and comparing its
origin to location.origin, then use the validated/sanitized path variable
instead of the raw searchParams value.
| const handleGoogleSignIn = () => { | ||
| trackEvent("auth_started", { method: "google", is_signup: true }); | ||
| signIn("google", { | ||
| ...(next && next.length > 0 ? { callbackUrl: next } : {}), | ||
| }); | ||
| }; |
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.
Also use the sanitized next for Google sign-up.
- signIn("google", {
- ...(next && next.length > 0 ? { callbackUrl: next } : {}),
- });
+ signIn("google", {
+ ...(safeNext ? { callbackUrl: safeNext } : {}),
+ });📝 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 handleGoogleSignIn = () => { | |
| trackEvent("auth_started", { method: "google", is_signup: true }); | |
| signIn("google", { | |
| ...(next && next.length > 0 ? { callbackUrl: next } : {}), | |
| }); | |
| }; | |
| const handleGoogleSignIn = () => { | |
| trackEvent("auth_started", { method: "google", is_signup: true }); | |
| signIn("google", { | |
| ...(safeNext ? { callbackUrl: safeNext } : {}), | |
| }); | |
| }; |
🤖 Prompt for AI Agents
In apps/web/app/(org)/signup/form.tsx around lines 105 to 110, the Google
sign-in handler currently uses the raw next value for the callbackUrl; change it
to use the already-computed sanitizedNext instead. Update the signIn call to
pass callbackUrl: sanitizedNext when sanitizedNext is non-empty (matching the
existing pattern used elsewhere), ensuring you reference the correct sanitized
variable in scope and preserve the trackEvent and other options.
| email, | ||
| redirect: false, | ||
| ...(next && next.length > 0 | ||
| ? { callbackUrl: next } | ||
| : {}), | ||
| }) |
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.
Sanitize callbackUrl for email flow too.
Use safeNext computed above.
- ...(next && next.length > 0
- ? { callbackUrl: next }
- : {}),
+ ...(safeNext ? { callbackUrl: safeNext } : {}),📝 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.
| email, | |
| redirect: false, | |
| ...(next && next.length > 0 | |
| ? { callbackUrl: next } | |
| : {}), | |
| }) | |
| email, | |
| redirect: false, | |
| ...(safeNext ? { callbackUrl: safeNext } : {}), | |
| }) |
🤖 Prompt for AI Agents
In apps/web/app/(org)/signup/form.tsx around lines 269 to 274, the email sign-in
call is passing the raw next value as callbackUrl instead of using the sanitized
safeNext; update the spread to conditionally include callbackUrl only when
safeNext is set (i.e. replace usage of next with safeNext or check
safeNext.length) so the email flow uses the precomputed sanitized URL.
| <Button | ||
| variant="gray" | ||
| href="/pricing" | ||
| size="lg" | ||
| className="w-full font-medium sm:w-auto" | ||
| > | ||
| {homepageCopy.readyToGetStarted.buttons.secondary} | ||
| </Button> | ||
| <Button | ||
| variant="blue" | ||
| href="/download" | ||
| size="lg" | ||
| className="w-full font-medium sm:w-auto" | ||
| > | ||
| {homepageCopy.readyToGetStarted.buttons.primary} | ||
| </Button> |
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.
CTA label/URL pairing looks reversed (Download
Primary label “Upgrade to Cap Pro” should link to /pricing; secondary “Download for free” should link to /download. Apply swap to avoid user confusion.
- <Button
- variant="gray"
- href="/pricing"
+ <Button
+ variant="gray"
+ href="/download"
size="lg"
className="w-full font-medium sm:w-auto"
>
{homepageCopy.readyToGetStarted.buttons.secondary}
</Button>
<Button
variant="blue"
- href="/download"
+ href="/pricing"
size="lg"
className="w-full font-medium sm:w-auto"
>
{homepageCopy.readyToGetStarted.buttons.primary}
</Button>📝 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.
| <Button | |
| variant="gray" | |
| href="/pricing" | |
| size="lg" | |
| className="w-full font-medium sm:w-auto" | |
| > | |
| {homepageCopy.readyToGetStarted.buttons.secondary} | |
| </Button> | |
| <Button | |
| variant="blue" | |
| href="/download" | |
| size="lg" | |
| className="w-full font-medium sm:w-auto" | |
| > | |
| {homepageCopy.readyToGetStarted.buttons.primary} | |
| </Button> | |
| <Button | |
| variant="gray" | |
| href="/download" | |
| size="lg" | |
| className="w-full font-medium sm:w-auto" | |
| > | |
| {homepageCopy.readyToGetStarted.buttons.secondary} | |
| </Button> | |
| <Button | |
| variant="blue" | |
| href="/pricing" | |
| size="lg" | |
| className="w-full font-medium sm:w-auto" | |
| > | |
| {homepageCopy.readyToGetStarted.buttons.primary} | |
| </Button> |
🤖 Prompt for AI Agents
In apps/web/components/ReadyToGetStarted.tsx around lines 24 to 39 the CTA
label/URL pairing is reversed: the primary button currently links to /download
and the secondary to /pricing. Swap the hrefs so the primary (“Upgrade to Cap
Pro” / homepageCopy.readyToGetStarted.buttons.primary) uses /pricing and the
secondary (“Download for free” /
homepageCopy.readyToGetStarted.buttons.secondary) uses /download; you can either
swap the href props between the two Button components or reorder the Button
elements so labels and URLs align accordingly.
Adds a signup page (duplicate of /login pretty much) and updates the navbar to include the signup.
Summary by CodeRabbit