implements signIn page - #9
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds an auth route group with a hidden-header stack, a themed sign-in screen with reusable inputs and local form state, and a home-screen pressable that navigates to the sign-in route. ChangesSign-in screen and navigation
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant SignInScreen
participant Router
User->>SignInScreen: Enter email and password
SignInScreen->>SignInScreen: Update local state
User->>SignInScreen: Press Sign In
SignInScreen->>Router: router.push("/(tabs)/home")
sequenceDiagram
participant User
participant HomeScreen
participant Router
participant SignInScreen
User->>HomeScreen: Press "Temp: Go to Sign In"
HomeScreen->>Router: router.push("/(auth)/sign-in")
Router->>SignInScreen: Open sign-in route
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (1)
apps/mobile/app/(auth)/sign-in.tsx (1)
83-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win"Forgot Password?" and "Sign Up" are non-interactive
Textelements.Both visually imply navigation/actions but aren't wrapped in
Pressable/Link, so they're currently dead UI and not reachable via touch or accessibility tools.Example fix for "Sign Up" (repeat pattern for "Forgot Password?")
- <View className="flex-row items-center justify-center gap-1"> - <Text className="text-[13px] font-sans text-icon-strong"> - Don't have an account? - </Text> - <Text className="text-[13px] font-sans-semibold text-text-primary"> - Sign Up - </Text> - </View> + <View className="flex-row items-center justify-center gap-1"> + <Text className="text-[13px] font-sans text-icon-strong"> + Don't have an account? + </Text> + <Pressable onPress={() => router.push("/(auth)/sign-up")}> + <Text className="text-[13px] font-sans-semibold text-text-primary"> + Sign Up + </Text> + </Pressable> + </View>🤖 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 `@apps/mobile/app/`(auth)/sign-in.tsx around lines 83 - 94, The “Forgot Password?” and “Sign Up” labels in the sign-in screen are plain Text elements but should behave like navigation actions. Update the sign-in UI to wrap these labels with interactive controls such as Pressable or Link, and wire them to the appropriate handlers/navigation so they are touchable and accessible; use the existing sign-in screen component structure to locate and replace the non-interactive Text nodes.
🤖 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 `@apps/mobile/app/`(auth)/sign-in.tsx:
- Around line 15-99: The SignInScreen route currently contains the full sign-in
UI inline instead of staying focused on screen composition. Extract the
repeated/large UI sections (header/logo, email/password fields, primary action,
footer links) into reusable components under a shared components area, then keep
SignInScreen limited to composing those components and wiring state/handlers.
- Around line 72-81: The Sign In CTA in the sign-in screen is missing its action
handler, so pressing it does nothing. Add an onPress to the Pressable in the
sign-in component so it triggers the existing sign-in flow, using the current
email and password state and any validation/submission logic already present in
the screen.
---
Nitpick comments:
In `@apps/mobile/app/`(auth)/sign-in.tsx:
- Around line 83-94: The “Forgot Password?” and “Sign Up” labels in the sign-in
screen are plain Text elements but should behave like navigation actions. Update
the sign-in UI to wrap these labels with interactive controls such as Pressable
or Link, and wire them to the appropriate handlers/navigation so they are
touchable and accessible; use the existing sign-in screen component structure to
locate and replace the non-interactive Text nodes.
🪄 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: 93c21bff-4f78-4a75-a47b-eeab0ee379ee
📒 Files selected for processing (3)
apps/mobile/app/(auth)/_layout.tsxapps/mobile/app/(auth)/sign-in.tsxapps/mobile/app/(tabs)/home.tsx
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 (1)
apps/mobile/app/(auth)/sign-in.tsx (1)
79-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win"Forgot Password?" and "Sign Up" are non-interactive text.
Both are styled to look like tappable links but have no
Pressable/onPressoraccessibilityRole="link", so they currently do nothing when tapped.💡 Example fix for "Sign Up"
- <Text className="text-[13px] font-sans-semibold text-text-primary"> - Sign Up - </Text> + <Pressable accessibilityRole="link" onPress={() => router.push("/(auth)/sign-up")}> + <Text className="text-[13px] font-sans-semibold text-text-primary"> + Sign Up + </Text> + </Pressable>🤖 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 `@apps/mobile/app/`(auth)/sign-in.tsx around lines 79 - 90, “Forgot Password?” and “Sign Up” are currently plain Text, so they look clickable but do not respond to taps. Update the sign-in screen to make these elements interactive by wrapping them in Pressable (or equivalent touchable) and wiring the appropriate onPress handlers for the password reset and sign-up navigation actions. While doing so, add accessibilityRole="link" to the tappable elements and keep the existing Text styling inside the interactive wrappers.
🧹 Nitpick comments (1)
apps/mobile/components/auth/AuthInput.tsx (1)
17-21: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
autoComplete/textContentTypepassthrough for credential fields.Since this component wraps email/password inputs, forwarding platform autofill hints (
autoComplete,textContentType) viainputPropsalready works structurally, but no defaults are set here — worth ensuring callers (sign-in screen) pass them for password manager support.🤖 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 `@apps/mobile/components/auth/AuthInput.tsx` around lines 17 - 21, The AuthInput wrapper already forwards inputProps into TextInput, but credential autofill hints are not being set by default, so update the sign-in usage to pass the appropriate autoComplete and textContentType values for email/password fields. Make sure the callers of AuthInput on the credential screens provide these props through inputProps so the underlying TextInput can use platform password manager support.
🤖 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 `@apps/mobile/app/`(auth)/sign-in.tsx:
- Around line 22-25: The handleSignIn callback in sign-in.tsx is still acting as
a stub by only dismissing the keyboard and routing directly to /(tabs)/home.
Update handleSignIn to perform real authentication through the Clerk sign-in
flow or, if this is temporary scaffolding, make that placeholder explicit in the
code. Keep the existing Keyboard.dismiss behavior if needed, but do not navigate
to the home route until sign-in succeeds.
---
Outside diff comments:
In `@apps/mobile/app/`(auth)/sign-in.tsx:
- Around line 79-90: “Forgot Password?” and “Sign Up” are currently plain Text,
so they look clickable but do not respond to taps. Update the sign-in screen to
make these elements interactive by wrapping them in Pressable (or equivalent
touchable) and wiring the appropriate onPress handlers for the password reset
and sign-up navigation actions. While doing so, add accessibilityRole="link" to
the tappable elements and keep the existing Text styling inside the interactive
wrappers.
---
Nitpick comments:
In `@apps/mobile/components/auth/AuthInput.tsx`:
- Around line 17-21: The AuthInput wrapper already forwards inputProps into
TextInput, but credential autofill hints are not being set by default, so update
the sign-in usage to pass the appropriate autoComplete and textContentType
values for email/password fields. Make sure the callers of AuthInput on the
credential screens provide these props through inputProps so the underlying
TextInput can use platform password manager support.
🪄 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: 5b5874e9-5b50-4982-8f71-dc272cd768e8
📒 Files selected for processing (2)
apps/mobile/app/(auth)/sign-in.tsxapps/mobile/components/auth/AuthInput.tsx
Comment on lines
+22
to
+25
| const handleSignIn = useCallback(() => { | ||
| Keyboard.dismiss(); | ||
| router.push("/(tabs)/home"); | ||
| }, [router]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== sign-in screen ==\n'
sed -n '1,220p' 'apps/mobile/app/(auth)/sign-in.tsx'
printf '\n== auth-related files ==\n'
git ls-files 'apps/mobile/**' | rg 'auth|sign-in|home|tabs|session|login|signin|signup|zustand|store'
printf '\n== search for auth flow references ==\n'
rg -n 'router\.push\("/\(tabs\)/home"|handleSignIn|sign in|sign-in|login|auth|session|password|email' apps/mobile -g '!**/node_modules/**'Repository: OumB2021/Journal
Length of output: 8347
Sign-in still bypasses authentication
handleSignIn only dismisses the keyboard and routes to /(tabs)/home, so any input (or no input) reaches the home screen without credential validation or a Clerk sign-in flow. If this is just temporary scaffolding, track it explicitly; otherwise this is a functional gap before merge.
🤖 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 `@apps/mobile/app/`(auth)/sign-in.tsx around lines 22 - 25, The handleSignIn
callback in sign-in.tsx is still acting as a stub by only dismissing the
keyboard and routing directly to /(tabs)/home. Update handleSignIn to perform
real authentication through the Clerk sign-in flow or, if this is temporary
scaffolding, make that placeholder explicit in the code. Keep the existing
Keyboard.dismiss behavior if needed, but do not navigate to the home route until
sign-in succeeds.
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 by CodeRabbit
AuthInputcomponent for consistent labeled form fields.