Redesign auth as single split-screen route#45
Merged
Conversation
Replace the separate landing / sign-in / sign-up screens with a single /auth route that hosts sign-in, sign-up, forgot-password, and email verification behind a segmented toggle. Wide-web shows the merged landing as a side panel; iOS and narrow web collapse to a compact header above the card. Apple, Google, and GitHub OAuth buttons are coded in but gated by an OAUTH_ENABLED flag so they ship disabled on day one. Extracts the shared clerkErrorMessage / sanitizeRedirect helpers into lib/clerk-errors.ts. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughConsolidates landing/sign-in/sign-up into a single multi‑phase /auth screen; adds reusable auth UI components and Clerk error/redirect helpers; updates routing and callers to /auth; and reformats a couple of event type exports. ChangesAuth Screen Consolidation
Sequence DiagramsequenceDiagram
participant User
participant AuthScreen
participant EmailPasswordForm
participant VerificationForm
participant ClerkAPI
participant Router
User->>AuthScreen: Navigate to /auth?mode=sign-in&redirect=/home
AuthScreen->>EmailPasswordForm: Render email/password form
User->>EmailPasswordForm: Enter email and password
EmailPasswordForm->>AuthScreen: onSubmit({email, password})
AuthScreen->>ClerkAPI: useSignIn.create({email, password})
alt Second Factor Required
ClerkAPI-->>AuthScreen: status=needs_second_factor (pendingEmail)
AuthScreen->>VerificationForm: Render code entry
User->>VerificationForm: Enter verification code
VerificationForm->>AuthScreen: onSubmit(code)
AuthScreen->>ClerkAPI: useSignIn.attemptEmailCodeVerification({code})
else Sign-In Complete
ClerkAPI-->>AuthScreen: status=complete
end
ClerkAPI-->>AuthScreen: Session created
AuthScreen->>ClerkAPI: setActive({session})
AuthScreen->>Router: router.replace(/home)
Router-->>User: Navigate to redirect destination
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
app/app/auth.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. app/components/auth/OAuthRow.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. Comment |
![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
🧹 Nitpick comments (1)
app/components/auth/OAuthRow.tsx (1)
32-32: ⚡ Quick winMake disabled styling conditional on
OAUTH_ENABLED.Line [32] keeps
opacity-50even when OAuth is enabled, so the buttons still look disabled after the flag is flipped.Proposed diff
- className="h-11 flex-row items-center justify-center rounded-lg border border-border bg-background opacity-50" + className={`h-11 flex-row items-center justify-center rounded-lg border border-border bg-background ${ + OAUTH_ENABLED ? "" : "opacity-50" + }`}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/components/auth/OAuthRow.tsx` at line 32, The OAuthRow component currently always applies the "opacity-50" class in the className string (making buttons look disabled) even when OAUTH_ENABLED is true; update OAuthRow (the JSX that includes className="... opacity-50") to conditionally include "opacity-50" only when the OAUTH_ENABLED flag is falsy (or when OAuth is actually disabled). Implement the condition by building the className dynamically (e.g., template literal or classnames helper) so that "opacity-50" is appended only when !OAUTH_ENABLED, leaving the rest of the classes unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/app/auth.tsx`:
- Around line 209-319: The memoized `card` subtree (const card = useMemo(...))
is capturing stale handler closures because `isLoaded` and the auth handler
functions (e.g., handleVerify, handleForgotVerify, handleResendVerify,
handleResendForgot, handleForgotStart, handleSignIn, handleSignUp, switchMode,
reset) are not included in its dependency array; update the memo to either
remove useMemo entirely (so handlers always reflect latest state) or include
`isLoaded` plus all referenced handler functions in the dependency array so the
JSX is recomputed when loading state or handlers change, ensuring handlers like
handleVerify/handleForgotVerify no longer early-return incorrectly.
---
Nitpick comments:
In `@app/components/auth/OAuthRow.tsx`:
- Line 32: The OAuthRow component currently always applies the "opacity-50"
class in the className string (making buttons look disabled) even when
OAUTH_ENABLED is true; update OAuthRow (the JSX that includes className="...
opacity-50") to conditionally include "opacity-50" only when the OAUTH_ENABLED
flag is falsy (or when OAuth is actually disabled). Implement the condition by
building the className dynamically (e.g., template literal or classnames helper)
so that "opacity-50" is appended only when !OAUTH_ENABLED, leaving the rest of
the classes unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 32bc7c5f-e5cd-4ec1-a53b-5f3c06b92cd4
📒 Files selected for processing (16)
app/app/_layout.tsxapp/app/auth.tsxapp/app/cli-auth.tsxapp/app/landing.tsxapp/app/shares/invite/[ownerUserId]/[token]/accept.tsxapp/app/sign-in.tsxapp/app/sign-up.tsxapp/components/AuthMenu.tsxapp/components/auth/BrandPanel.tsxapp/components/auth/EmailPasswordForm.tsxapp/components/auth/ForgotPasswordForm.tsxapp/components/auth/OAuthRow.tsxapp/components/auth/VerificationForm.tsxapp/lib/clerk-errors.tsapp/lib/events.tsapp/lib/heartbeat.ts
💤 Files with no reviewable changes (3)
- app/app/sign-up.tsx
- app/app/landing.tsx
- app/app/sign-in.tsx
CodeRabbit flagged a stale-closure risk: when Clerk hadn't loaded yet on first render, the memoized JSX retained handlers closing over isLoaded=false, causing later auth actions to silently no-op. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/app/auth.tsx (1)
276-284:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDisable mode switches while an auth request is in flight.
modedecides which verification branch runs later, but these two controls stay clickable duringloading. If a user starts sign-up and flips to sign-in before Clerk responds, the request can still land inphase="verify"withmode="sign-in", andhandleVerifywill call the sign-in path against a sign-up attempt.Suggested fix
function switchMode(next: AuthMode) { + if (loading) return; setMode(next); setPhase("form"); reset(); }<SegmentedControl fullWidth options={[ { value: "sign-in", label: "Sign in" }, { value: "sign-up", label: "Sign up" }, ]} value={mode} onChange={switchMode} /> @@ {phase === "form" ? ( <Button className="mt-4" variant="ghost" label={ mode === "sign-in" ? "Don’t have an account? Sign up" : "Have an account? Sign in" } onPress={() => switchMode(mode === "sign-in" ? "sign-up" : "sign-in")} + disabled={loading} /> ) : null}Also applies to: 337-345
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/app/auth.tsx` around lines 276 - 284, Disable the SegmentedControl (and the similar control around lines 337-345) while an auth request is in flight by wiring its disabled prop to the request loading state; specifically, update the SegmentedControl instances that use value={mode} and onChange={switchMode} to also accept disabled={loading} (or the relevant boolean like isLoading) so users cannot toggle mode during an ongoing Clerk request, and ensure switchMode and handleVerify still correctly read the mode/phase values after the request completes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@app/app/auth.tsx`:
- Around line 276-284: Disable the SegmentedControl (and the similar control
around lines 337-345) while an auth request is in flight by wiring its disabled
prop to the request loading state; specifically, update the SegmentedControl
instances that use value={mode} and onChange={switchMode} to also accept
disabled={loading} (or the relevant boolean like isLoading) so users cannot
toggle mode during an ongoing Clerk request, and ensure switchMode and
handleVerify still correctly read the mode/phase values after the request
completes.
- Disable the sign-in/sign-up mode toggles while a Clerk request is in flight so users can't flip mode mid-request and land in the wrong verify branch. - Tie the OAuth row's opacity-50 to !OAUTH_ENABLED so flipping the flag visually enables the buttons too. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Owner
Author
|
Addressed CodeRabbit's out-of-diff feedback in 260d4fe:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/landing,/sign-in,/sign-upwith a single/authroute hosting sign-in, sign-up, forgot-password, and email verification behind a segmented toggle.OAUTH_ENABLEDflag (ClerkuseSSOwiring ready behind the gate).reset_password_email_code.clerkErrorMessage+sanitizeRedirectintolib/clerk-errors.ts, drop hand-rolled<TextInput>styling in favour of theInputprimitive, add show/hide password toggle, iOS keychainautoComplete/textContentType.Test plan
yarn typecheckclean (one unrelatedexpo-notificationstypes miss is pre-existing).yarn lintclean for new files.yarn dev:web:relaywithEXPO_PUBLIC_CLERK_PUBLISHABLE_KEYset and confirm the wide-web split layout, segmented toggle, and email + password sign-in.yarn dev:ios:relay: layout, keyboard avoidance, password toggle.soontag and ignore taps.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Refactor
Removed