-
Notifications
You must be signed in to change notification settings - Fork 106
improvement: Add separate login / signup screens #331
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
WalkthroughA signup page was introduced, along with updates to the login form component to support both login and signup contexts. The login and signup pages now pass a context prop to the form, which conditionally changes headings and navigation prompts. Middleware was updated to allow requests to the signup path to bypass tenant domain redirection. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SignupPage
participant AuthUtil
participant LoginForm
User->>SignupPage: Visit /signup
SignupPage->>AuthUtil: Check session
alt Session exists
SignupPage->>User: Redirect to home
else No session
SignupPage->>AuthUtil: Get providers
SignupPage->>LoginForm: Render with context="signup"
LoginForm->>User: Show signup form and navigation prompt
end
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 0
🧹 Nitpick comments (4)
packages/web/src/app/signup/page.tsx (3)
10-15
: Rename interface to reflect its actual usage.The
LoginProps
interface name is misleading since this component handles signup. Consider renaming it toAuthProps
orAuthPageProps
to better reflect its dual usage.-interface LoginProps { +interface AuthProps { searchParams: { callbackUrl?: string; error?: string; } }
17-22
: Add missing page load logging for consistency.The login page logs when it loads, but this signup page doesn't. Consider adding a similar log entry for consistency and debugging purposes.
export default async function Signup({ searchParams }: LoginProps) { + logger.info("Signup page loaded"); const session = await auth();
1-48
: Consider refactoring to reduce code duplication.This signup page is nearly identical to the login page, with only the context prop being different. This creates maintenance overhead and potential for inconsistencies.
Consider creating a shared component or utility function that both pages can use, or explore if these could be combined into a single page with dynamic routing.
packages/web/src/app/login/components/loginForm.tsx (1)
159-171
: Consider updating provider button text for signup context.The provider buttons always display "Sign in with {name}" regardless of context. In the signup context, this might be confusing to users who expect "Sign up with {name}" or "Continue with {name}".
However, if the authentication flow handles both login and signup seamlessly (as suggested by the credentials provider in auth.ts), this might be an intentional UX decision.
Consider updating the button text based on context:
- Sign in with {name} + {context === "login" ? "Sign in with" : "Continue with"} {name}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/web/src/app/login/components/loginForm.tsx
(3 hunks)packages/web/src/app/login/page.tsx
(1 hunks)packages/web/src/app/signup/page.tsx
(1 hunks)packages/web/src/middleware.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/web/src/app/signup/page.tsx (4)
packages/logger/src/index.ts (1)
createLogger
(86-86)packages/web/src/auth.ts (2)
session
(151-160)getProviders
(36-129)packages/web/src/app/login/components/loginForm.tsx (1)
LoginForm
(28-143)packages/web/src/app/components/footer.tsx (1)
Footer
(4-18)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (6)
packages/web/src/app/login/page.tsx (1)
43-43
: LGTM!Clean implementation that correctly passes the login context to the shared form component.
packages/web/src/middleware.ts (1)
13-17
: LGTM!Correctly adds the signup path to bypass tenant domain redirection, which is necessary for the new signup functionality to work properly.
packages/web/src/app/login/components/loginForm.tsx (4)
25-26
: LGTM!Clean interface extension that properly types the new context prop.
28-28
: LGTM!Correctly updates the function signature to accept the new context prop.
81-85
: LGTM!Clean conditional rendering that provides appropriate headings for both login and signup contexts. The spacing adjustment with
mb-3
also improves the visual layout.
126-136
: LGTM!Well-implemented conditional navigation prompts that help users switch between login and signup flows. The links are properly styled and accessible.
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: 0
🧹 Nitpick comments (1)
CHANGELOG.md (1)
10-11
: Correct spelling and PR reference in the changelog entry.The word “seperate” should be “separate”, and the PR number should be 331, not 311.
Apply this diff:
@@ -10,2 +10,2 @@ - - Added seperate page for signup. [#311] + - Added separate signup page. [#331]🧰 Tools
🪛 LanguageTool
[duplication] ~10-~10: Possible typo: you repeated a word.
Context: ...pec/v2.0.0.html). ## [Unreleased] ### Added - Added seperate page for signup. [#311](https:...(ENGLISH_WORD_REPEAT_RULE)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
CHANGELOG.md
(1 hunks)
🧰 Additional context used
🪛 LanguageTool
CHANGELOG.md
[duplication] ~10-~10: Possible typo: you repeated a word.
Context: ...pec/v2.0.0.html). ## [Unreleased] ### Added - Added seperate page for signup. [#311](https:...
(ENGLISH_WORD_REPEAT_RULE)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
This PR adds a separate signup screen and a small "No account yet?" and "Already have an account?" dialogs.
Login:

Signup:

Summary by CodeRabbit
New Features
Bug Fixes
Style