Add Email/Password Authentication to AuthScreen#16
Conversation
- Add email and password inputs to AuthScreen - Integrate with Supabase signUp and signInWithPassword - Add form validation, loading states, and error handling - Update Build_plan.md
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAuthScreen adds email/password sign-in and sign-up (email, password, authMode, emailLoading), input validation (non-empty email, password ≥6), a keyboard-aware scrollable form with an "or" divider and toggle, a submit button with spinner, and handlers calling Supabase signInWithPassword/signUp. Theme design tokens (Colors, Spacing, Sizes, BorderWidths) were extended. Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (UI)"
participant AuthScreen as "AuthScreen"
participant Supabase as "Supabase Auth"
participant AppAlert as "Alert"
User->>AuthScreen: Enter email & password, choose mode (signin/signup)
User->>AuthScreen: Tap Submit
AuthScreen->>AuthScreen: Trim inputs, validate (email non-empty, pw ≥6)
alt validation fails
AuthScreen->>AppAlert: show validation error
else validation passes
AuthScreen->>Supabase: signInWithPassword / signUp
Supabase-->>AuthScreen: success / error
alt error
AuthScreen->>AppAlert: show error alert
else signUp success
AuthScreen->>AppAlert: show confirmation alert
AuthScreen->>AuthScreen: set authMode = "signin"
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
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)
Review rate limit: 1/3 review remaining, refill in 38 minutes and 27 seconds. Comment |
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 (4)
src/screens/AuthScreen.tsx (4)
149-272:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the auth form keyboard-aware.
The new email/password section sits in a centred static layout, so the keyboard can cover the inputs or CTA on smaller devices. Wrap this part in
KeyboardAvoidingView/ScrollViewso the form remains usable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/AuthScreen.tsx` around lines 149 - 272, The email/password form (the View with styles.emailForm containing the two TextInput fields and the primary button) can be obscured by the keyboard on small devices; wrap that section in a KeyboardAvoidingView (import from 'react-native') and a ScrollView so inputs and CTA stay visible when the keyboard opens. Replace the outer <View style={styles.emailForm}> with a <KeyboardAvoidingView behavior={Platform.OS === 'ios' ? 'padding' : undefined} keyboardVerticalOffset={/* adjust if you have a header e.g. 64 */}><ScrollView contentContainerStyle={styles.emailForm} keyboardShouldPersistTaps="handled">...existing inputs and buttons...</ScrollView></KeyboardAvoidingView>, keep existing handlers (setEmail, setPassword, handleEmailAuth), loading state and styles, and ensure Platform is imported.
9-28: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winMove the Google icon sizes and fills into theme tokens.
This SVG still hardcodes its dimensions and brand colours, which breaks the repo rule that design values must come from
theme.ts. If you need these exact colours, add matching tokens rather than inlining the hex values here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/AuthScreen.tsx` around lines 9 - 28, The GoogleIcon component currently hardcodes SVG width/height and hex fills; update GoogleIcon to read size and color tokens from the theme instead: replace the literal width="18" height="18" with a token (e.g., theme.tokens.icon.googleSize or theme.tokens.icon.small) and replace each fill="#4285F4", "#34A853", "#FBBC05", "#EA4335" with theme color tokens (e.g., theme.colors.brand.googleBlue, googleGreen, googleYellow, googleRed) by importing the theme (or useTheme()) at the top of the file and using those tokens in the Svg and Path props; if those tokens don’t exist yet, add matching tokens to theme.ts using the exact brand hex values so the component references theme tokens only.
49-55: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConfirm these
EXPO_PUBLIC_*values are intended for client bundling.These environment variables are inlined into the app bundle at build time, so this pattern should only be used for values that are deliberately public. If any of them are sensitive, move them out of the screen-level env access.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/AuthScreen.tsx` around lines 49 - 55, The code is currently reading EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID and EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID directly in AuthScreen (variables webClientId and iosClientId) which will inline them into the client bundle; confirm these are intentionally public and, if they are sensitive, remove this screen-level env access and instead fetch them from a secure runtime source (e.g., server endpoint or runtime config) before calling GoogleSignin.configure, or use a secure secrets manager and pass only non-sensitive tokens to the client; update the logic that sets setGoogleConfigError and the call to GoogleSignin.configure to use the new secure source (or keep envs only if explicitly intended to be public).
275-427: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winReplace the remaining hardcoded spacing and overlay values with theme constants.
There are still literal margins, paddings, gaps, sizes, radii, and the translucent overlay colour in this block. Pull them from
theme.tsso the screen stays aligned with the design system.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/screens/AuthScreen.tsx` around lines 275 - 427, Several style entries still use hardcoded numeric values and a literal overlay color; update these to use theme constants from theme.ts. Replace fixed margins/paddings/gaps (e.g., header.marginBottom: 48, tagline.marginTop: 10, buttonContainer.gap/other gaps, dividerRow.gap: 12, dividerRow.marginVertical: 20, finePrint.marginTop: 16, toggleRow.marginTop: 4) with appropriate Spacing tokens (Spacing.xl/Spacing.md/Spacing.sm etc.), swap hardcoded heights and paddings (appleButton.height, googleButton.height, input.paddingVertical, primaryButton.padding) for size/spacing tokens, replace numeric radii and borders (googleButton.borderRadius: 14, loadingOverlay.borderRadius, input.borderWidth) with Radius and Border tokens, and change the translucent loadingOverlay.backgroundColor 'rgba(0, 0, 0, 0.3)' to the semantic Colors.overlay/Colors.backdrop value; update styles referenced above (container, header, tagline, buttonContainer, appleButton, googleButton, googleIconContainer, loadingOverlay, finePrint, dividerRow, input, primaryButton, toggleRow) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/screens/AuthScreen.tsx`:
- Around line 91-99: In handleEmailAuth: stop trimming the password before
authentication—keep trimming only the email (use trimmedEmail = email.trim())
and use the original password variable (or a new untrimmed variable) for
validation and auth; update the empty-check and min-length check to use the
untrimmed password (e.g., check password instead of trimmedPassword) and pass
the untrimmed password into the auth call so leading/trailing spaces remain part
of the credential.
---
Outside diff comments:
In `@src/screens/AuthScreen.tsx`:
- Around line 149-272: The email/password form (the View with styles.emailForm
containing the two TextInput fields and the primary button) can be obscured by
the keyboard on small devices; wrap that section in a KeyboardAvoidingView
(import from 'react-native') and a ScrollView so inputs and CTA stay visible
when the keyboard opens. Replace the outer <View style={styles.emailForm}> with
a <KeyboardAvoidingView behavior={Platform.OS === 'ios' ? 'padding' : undefined}
keyboardVerticalOffset={/* adjust if you have a header e.g. 64 */}><ScrollView
contentContainerStyle={styles.emailForm}
keyboardShouldPersistTaps="handled">...existing inputs and
buttons...</ScrollView></KeyboardAvoidingView>, keep existing handlers
(setEmail, setPassword, handleEmailAuth), loading state and styles, and ensure
Platform is imported.
- Around line 9-28: The GoogleIcon component currently hardcodes SVG
width/height and hex fills; update GoogleIcon to read size and color tokens from
the theme instead: replace the literal width="18" height="18" with a token
(e.g., theme.tokens.icon.googleSize or theme.tokens.icon.small) and replace each
fill="#4285F4", "#34A853", "#FBBC05", "#EA4335" with theme color tokens (e.g.,
theme.colors.brand.googleBlue, googleGreen, googleYellow, googleRed) by
importing the theme (or useTheme()) at the top of the file and using those
tokens in the Svg and Path props; if those tokens don’t exist yet, add matching
tokens to theme.ts using the exact brand hex values so the component references
theme tokens only.
- Around line 49-55: The code is currently reading
EXPO_PUBLIC_GOOGLE_WEB_CLIENT_ID and EXPO_PUBLIC_GOOGLE_IOS_CLIENT_ID directly
in AuthScreen (variables webClientId and iosClientId) which will inline them
into the client bundle; confirm these are intentionally public and, if they are
sensitive, remove this screen-level env access and instead fetch them from a
secure runtime source (e.g., server endpoint or runtime config) before calling
GoogleSignin.configure, or use a secure secrets manager and pass only
non-sensitive tokens to the client; update the logic that sets
setGoogleConfigError and the call to GoogleSignin.configure to use the new
secure source (or keep envs only if explicitly intended to be public).
- Around line 275-427: Several style entries still use hardcoded numeric values
and a literal overlay color; update these to use theme constants from theme.ts.
Replace fixed margins/paddings/gaps (e.g., header.marginBottom: 48,
tagline.marginTop: 10, buttonContainer.gap/other gaps, dividerRow.gap: 12,
dividerRow.marginVertical: 20, finePrint.marginTop: 16, toggleRow.marginTop: 4)
with appropriate Spacing tokens (Spacing.xl/Spacing.md/Spacing.sm etc.), swap
hardcoded heights and paddings (appleButton.height, googleButton.height,
input.paddingVertical, primaryButton.padding) for size/spacing tokens, replace
numeric radii and borders (googleButton.borderRadius: 14,
loadingOverlay.borderRadius, input.borderWidth) with Radius and Border tokens,
and change the translucent loadingOverlay.backgroundColor 'rgba(0, 0, 0, 0.3)'
to the semantic Colors.overlay/Colors.backdrop value; update styles referenced
above (container, header, tagline, buttonContainer, appleButton, googleButton,
googleIconContainer, loadingOverlay, finePrint, dividerRow, input,
primaryButton, toggleRow) accordingly.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d621b606-aefc-4c5d-acb4-e21a5bfffacd
⛔ Files ignored due to path filters (1)
Build_plan.mdis excluded by!**/*.md
📒 Files selected for processing (1)
src/screens/AuthScreen.tsx
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 1 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/screens/AuthScreen.tsx`:
- Around line 120-123: The alert title is hard-coded to "Sign in failed" inside
the catch block in AuthScreen.tsx; update that Alert.alert call to use a
mode-specific title based on the local authMode (e.g., use "Sign up failed" when
authMode === 'signup' and "Sign in failed" otherwise) so failures reflect the
current flow; ensure you reference the same authMode variable in scope where the
catch runs (e.g., in the email submit/handleEmailAuth function) and keep the
existing error message logic unchanged.
- Around line 367-425: The style block uses hardcoded numeric spacing/sizing
(e.g., dividerRow.gap, dividerRow.marginVertical, dividerLine.height,
input.paddingHorizontal, input.paddingVertical, primaryButton.padding,
toggleRow.marginTop) which must be replaced with theme spacing/radius tokens;
import Spacing from src/constants/theme.ts (and use existing
Radius/FontSizes/Fonts/Colors) and swap those numeric literals for appropriate
Spacing tokens (e.g., Spacing.xs/sm/md) and ensure any border/height uses theme
tokens instead of raw numbers in the styles: dividerRow, dividerLine, emailForm,
input, primaryButton, toggleRow, toggleTextMuted, toggleTextAction.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9b2d337d-c111-4710-b079-131e3883e11c
📒 Files selected for processing (1)
src/screens/AuthScreen.tsx
| } catch (error: unknown) { | ||
| const message = error instanceof Error ? error.message : 'Something went wrong.'; | ||
| Alert.alert('Sign in failed', message); | ||
| } finally { |
There was a problem hiding this comment.
Use mode-specific failure copy for email auth errors.
At Line 122, the alert title is always “Sign in failed”, which is misleading when authMode === 'signup'. Use a conditional title so sign-up failures are labelled correctly.
Suggested patch
- Alert.alert('Sign in failed', message);
+ Alert.alert(authMode === 'signin' ? 'Sign in failed' : 'Sign up failed', message);📝 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.
| } catch (error: unknown) { | |
| const message = error instanceof Error ? error.message : 'Something went wrong.'; | |
| Alert.alert('Sign in failed', message); | |
| } finally { | |
| } catch (error: unknown) { | |
| const message = error instanceof Error ? error.message : 'Something went wrong.'; | |
| Alert.alert(authMode === 'signin' ? 'Sign in failed' : 'Sign up failed', message); | |
| } finally { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/screens/AuthScreen.tsx` around lines 120 - 123, The alert title is
hard-coded to "Sign in failed" inside the catch block in AuthScreen.tsx; update
that Alert.alert call to use a mode-specific title based on the local authMode
(e.g., use "Sign up failed" when authMode === 'signup' and "Sign in failed"
otherwise) so failures reflect the current flow; ensure you reference the same
authMode variable in scope where the catch runs (e.g., in the email
submit/handleEmailAuth function) and keep the existing error message logic
unchanged.
…eyboard support and theme-based styling
This PR adds email and password authentication to
AuthScreen.tsxas requested.email,password,authMode, andemailLoadingstate.handleEmailAuthfunction to validate inputs, interact with Supabase (signInWithPasswordandsignUp), display anAlerton success or failure, and toggle states accordingly.signinandsignupmodes.theme.tsconstants for colors, spacing, and styling ensuring consistent UI representation.Build_plan.md(renamed based on Unix-case sensitivity requirement) to reflect the task's completion.PR created automatically by Jules for task 12826351048971014011 started by @FLAiRistaken
Summary by CodeRabbit
New Features
Usability
Style