-
Notifications
You must be signed in to change notification settings - Fork 24
Restore EAS configuration files #210
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe PR restructures the mobile app from React Navigation-based architecture to Expo Router, removing authentication context, API client infrastructure, and all existing screens while introducing new UI components and Tailwind/NativeWind styling configuration. Changes
Possibly related PRs
Suggested reviewers
Pre-merge checks❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
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 |
✅ Deploy Preview for split-but-wiser canceled.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #210 +/- ##
=======================================
Coverage 72.79% 72.79%
=======================================
Files 17 17
Lines 1669 1669
Branches 154 154
=======================================
Hits 1215 1215
Misses 400 400
Partials 54 54
🚀 New features to boost your workflow:
|
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: 12
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (11)
mobile/assets/adaptive-icon.pngis excluded by!**/*.pngmobile/assets/adaptive-icon/background.pngis excluded by!**/*.pngmobile/assets/adaptive-icon/foreground.pngis excluded by!**/*.pngmobile/assets/favicon.pngis excluded by!**/*.pngmobile/assets/icon.pngis excluded by!**/*.pngmobile/assets/pwa/chrome-icon/chrome-icon-144.pngis excluded by!**/*.pngmobile/assets/pwa/chrome-icon/chrome-icon-192.pngis excluded by!**/*.pngmobile/assets/pwa/chrome-icon/chrome-icon-512.pngis excluded by!**/*.pngmobile/assets/splash-icon.pngis excluded by!**/*.pngmobile/assets/splash.pngis excluded by!**/*.pngmobile/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
mobile/.gitignore(1 hunks)mobile/App.js(0 hunks)mobile/api/auth.js(0 hunks)mobile/api/client.js(0 hunks)mobile/api/groups.js(0 hunks)mobile/app.json(1 hunks)mobile/app/_layout.tsx(1 hunks)mobile/app/index.tsx(1 hunks)mobile/babel.config.js(1 hunks)mobile/components/NeoCard.tsx(1 hunks)mobile/components/Themed.tsx(1 hunks)mobile/constants/colors.ts(1 hunks)mobile/context/AuthContext.js(0 hunks)mobile/global.css(1 hunks)mobile/index.js(0 hunks)mobile/lib/utils.ts(1 hunks)mobile/metro.config.js(1 hunks)mobile/nativewind-env.d.ts(1 hunks)mobile/navigation/AccountStackNavigator.js(0 hunks)mobile/navigation/AppNavigator.js(0 hunks)mobile/navigation/AuthNavigator.js(0 hunks)mobile/navigation/GroupsStackNavigator.js(0 hunks)mobile/navigation/MainNavigator.js(0 hunks)mobile/package.json(1 hunks)mobile/screens/AccountScreen.js(0 hunks)mobile/screens/AddExpenseScreen.js(0 hunks)mobile/screens/EditProfileScreen.js(0 hunks)mobile/screens/FriendsScreen.js(0 hunks)mobile/screens/GroupDetailsScreen.js(0 hunks)mobile/screens/GroupSettingsScreen.js(0 hunks)mobile/screens/HomeScreen.js(0 hunks)mobile/screens/JoinGroupScreen.js(0 hunks)mobile/screens/LoginScreen.js(0 hunks)mobile/screens/SignupScreen.js(0 hunks)mobile/tailwind.config.js(1 hunks)mobile/tsconfig.json(1 hunks)mobile/utils/balanceCalculator.js(0 hunks)mobile/utils/currency.js(0 hunks)
💤 Files with no reviewable changes (23)
- mobile/navigation/GroupsStackNavigator.js
- mobile/screens/LoginScreen.js
- mobile/screens/AccountScreen.js
- mobile/utils/currency.js
- mobile/screens/SignupScreen.js
- mobile/screens/JoinGroupScreen.js
- mobile/navigation/AppNavigator.js
- mobile/screens/AddExpenseScreen.js
- mobile/screens/GroupDetailsScreen.js
- mobile/context/AuthContext.js
- mobile/screens/GroupSettingsScreen.js
- mobile/screens/HomeScreen.js
- mobile/api/client.js
- mobile/navigation/MainNavigator.js
- mobile/api/auth.js
- mobile/navigation/AuthNavigator.js
- mobile/api/groups.js
- mobile/screens/EditProfileScreen.js
- mobile/index.js
- mobile/utils/balanceCalculator.js
- mobile/navigation/AccountStackNavigator.js
- mobile/App.js
- mobile/screens/FriendsScreen.js
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: Devasy23/splitwiser PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T18:33:22.581Z
Learning: Applies to {mobile/context/**/*.js,web/contexts/**/*.{ts,tsx}} : React Context should be used for state management of authentication and app-wide state in both mobile and web
Learnt from: CR
Repo: Devasy23/splitwiser PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T18:33:22.581Z
Learning: Applies to mobile/screens/**/*.js : Mobile screens should be created as components in `mobile/screens/` and registered in the navigation structure
📚 Learning: 2025-11-27T18:33:22.581Z
Learnt from: CR
Repo: Devasy23/splitwiser PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T18:33:22.581Z
Learning: Applies to mobile/screens/**/*.js : Mobile screens should be created as components in `mobile/screens/` and registered in the navigation structure
Applied to files:
mobile/app/index.tsxmobile/tailwind.config.js
🧬 Code graph analysis (4)
mobile/app/index.tsx (2)
mobile/components/Themed.tsx (3)
ThemedView(4-6)ThemedText(8-20)ThemedButton(22-45)mobile/components/NeoCard.tsx (1)
NeoCard(9-15)
mobile/components/Themed.tsx (1)
mobile/lib/utils.ts (1)
cn(4-6)
mobile/tailwind.config.js (1)
mobile/metro.config.js (2)
require(1-1)require(2-2)
mobile/components/NeoCard.tsx (1)
mobile/lib/utils.ts (1)
cn(4-6)
⏰ 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). (1)
- GitHub Check: rn-bundle-analysis
🔇 Additional comments (13)
mobile/.gitignore (3)
4-6: Standard debug log patterns—LGTM.The addition of standard npm and yarn debug log patterns aligns with best practices for preventing debug artifacts from being committed.
7-8: No critical security issue found; only minor capitalization inconsistency remains.The verification found no credential files (
.p8,.p12,.key,.mobileprovision,.pem, etc.) in the repository or staged in this PR. The removal of credential patterns from.gitignoreis not a security blocker since there are no actual credentials to protect.However, line 7 has a minor capitalization issue: "MacOS" should be "macOS" per Apple's official branding.
7-8: Based on my verification, I can now provide the rewritten review comment:
The review comment appears to conflate a pre-existing architectural gap with changes in this PR.
The file under review (
mobile/.gitignore, lines 7-8) contains only gitignore entries and makes no changes to authentication handling. While the retrieved learning guidance correctly states that "React Context should be used for state management of authentication and app-wide state" in both mobile and web:
- Web properly implements
AuthContextand uses it throughout the app with protected routes- Mobile currently has no authentication context implementation at all
- No authentication context files were changed in this PR
This architectural inconsistency between web and mobile auth handling is a valid long-term concern and does contradict the project's stated guidance. However, it is not an issue introduced by this PR and cannot be addressed through changes to
.gitignore. If mobile auth context needs to be implemented to align with the project's architectural guidance, that would require a separate architectural initiative.mobile/nativewind-env.d.ts (1)
1-1: NativeWind type reference is correctly wiredThis triple-slash reference is the standard way to pull NativeWind’s ambient types into the TS project; with tsconfig including this file, there’s nothing else to change here.
mobile/lib/utils.ts (1)
1-6:cnhelper is idiomatic and type-safeThe combination of
ClassValue,clsx, andtwMergeis standard and the implementation is correct for merging Tailwind/NativeWind class names.mobile/babel.config.js (1)
1-12: Babel config matches Expo + NativeWind + Reanimated expectationsThe preset stack and
react-native-reanimatedplugin placement look correct; just keep the Reanimated plugin last if you add more plugins later so it can patch the AST as expected.mobile/app.json (1)
3-4: Confirm identifiers, splash asset path, and EAS project bindingThese values look reasonable, but they’re all “must match reality” knobs:
name/slug/schemeandios.bundleIdentifier/android.packagedefine your app identity.splash.imagemust point to an existing asset or builds will fail.extra.eas.projectId+ownermust match the intended EAS project or builds may attach to the wrong app.Please double‑check these against your existing EAS project and asset tree.
Also applies to: 10-11, 14-16, 19-20, 26-27, 50-55
mobile/global.css (1)
1-3: Tailwind layer setup is minimal and correctIncluding
@tailwind base;,components;, andutilities;here is the expected global setup; as long as this file is imported once in your root layout, Tailwind/NativeWind will see the layers correctly.mobile/tsconfig.json (1)
1-19: TS config is sensible; ensure all included declaration files existThe config (extending
expo/tsconfig.base, strict mode,@/*alias, and includes) looks good. Just make sureexpo-env.d.tsactually exists in the repo; if it doesn’t, TS will complain about a missing include and you can either create it or drop it from the list.mobile/metro.config.js (1)
1-6: LGTM!Standard NativeWind v4 Metro configuration for Expo. The setup correctly wraps the default Expo config with NativeWind and references the global CSS file.
mobile/components/NeoCard.tsx (1)
1-15: LGTM!Clean, reusable card component with proper TypeScript typing. Good use of the
cnutility for class merging, allowing consumers to override or extend default styles.mobile/app/_layout.tsx (1)
14-39: LGTM!Well-structured root layout with proper font loading, splash screen handling, and Expo Router setup. The early return pattern correctly prevents rendering before assets are ready.
mobile/app/index.tsx (1)
7-99: Overall UI structure looks good for a prototype.The HomeScreen demonstrates proper use of the new Themed components, NeoCard, and NativeWind styling. The hardcoded data is acceptable for initial UI scaffolding.
Note: The retrieved learning about placing screens in
mobile/screens/applies to the older React Navigation pattern. With Expo Router's file-based routing, placing screen components inmobile/app/is the correct approach.
| npm-debug.log* | ||
| yarn-debug.log* | ||
| yarn-error.log* | ||
| # MacOS |
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.
🧹 Nitpick | 🔵 Trivial
Minor: Use standard macOS capitalization.
"MacOS" should be "macOS" per Apple's official naming.
-# MacOS
+# macOS
.DS_Store📝 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.
| # MacOS | |
| # macOS | |
| .DS_Store |
🤖 Prompt for AI Agents
In mobile/.gitignore around line 7, the comment uses "MacOS" but should follow
Apple’s official capitalization "macOS"; update the comment text from "MacOS" to
"macOS" to correct the spelling.
| "favicon": "./assets/favicon.png", | ||
| "bundler": "metro" | ||
| }, |
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.
🧹 Nitpick | 🔵 Trivial
Avoid potential confusion around duplicated newArchEnabled flags
You’re enabling the new architecture both via:
- the
expo-build-propertiesplugin (android.ios.newArchEnabled) and - the top‑level
"newArchEnabled": true.
To reduce drift and future confusion, consider relying on a single source of truth (typically the plugin config) and remove the top‑level flag if it’s no longer required in your target Expo SDK, after confirming with current Expo docs.
Also applies to: 32-48
🤖 Prompt for AI Agents
In mobile/app.json around lines 29-31 (also applies to lines 32-48), there are
duplicated newArchEnabled settings: one under the expo-build-properties plugin
(android.ios.newArchEnabled) and a top-level "newArchEnabled": true; remove the
top-level "newArchEnabled" entry so the plugin config is the single source of
truth (after you confirm the top-level flag is not required by the Expo SDK
version you target), leaving only the plugin-controlled setting and updating any
documentation/comments to note the plugin is authoritative.
| @@ -0,0 +1,40 @@ | |||
| import { DarkTheme, DefaultTheme, ThemeProvider } from '@react-navigation/native'; | |||
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.
🧹 Nitpick | 🔵 Trivial
Remove unused DarkTheme import.
DarkTheme is imported but never used in this file.
-import { DarkTheme, DefaultTheme, ThemeProvider } from '@react-navigation/native';
+import { DefaultTheme, ThemeProvider } from '@react-navigation/native';📝 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.
| import { DarkTheme, DefaultTheme, ThemeProvider } from '@react-navigation/native'; | |
| import { DefaultTheme, ThemeProvider } from '@react-navigation/native'; |
🤖 Prompt for AI Agents
In mobile/app/_layout.tsx around line 1, the DarkTheme import from
'@react-navigation/native' is unused; remove DarkTheme from the import list so
only DefaultTheme and ThemeProvider are imported to eliminate the unused import
warning and keep imports minimal.
| import { ThemedText, ThemedButton, ThemedView } from '../components/Themed'; | ||
| import { NeoCard } from '../components/NeoCard'; | ||
| import { SafeAreaView } from 'react-native-safe-area-context'; | ||
| import { Wallet, PieChart, TrendingUp, ArrowRight } from 'lucide-react-native'; |
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.
🧹 Nitpick | 🔵 Trivial
Remove unused ArrowRight import.
ArrowRight is imported from lucide-react-native but never used in the component.
-import { Wallet, PieChart, TrendingUp, ArrowRight } from 'lucide-react-native';
+import { Wallet, PieChart, TrendingUp } from 'lucide-react-native';📝 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.
| import { Wallet, PieChart, TrendingUp, ArrowRight } from 'lucide-react-native'; | |
| import { Wallet, PieChart, TrendingUp } from 'lucide-react-native'; |
🤖 Prompt for AI Agents
In mobile/app/index.tsx around line 5, the import statement includes ArrowRight
from 'lucide-react-native' but that symbol is unused; remove ArrowRight from the
import list (leave the other icons intact) to eliminate the unused import and
satisfy linting.
| <View className="mb-4 flex-row justify-between items-end"> | ||
| <ThemedText type="subtitle">Recent Activity</ThemedText> | ||
| <ThemedText type="link">View All</ThemedText> | ||
| </View> |
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.
"View All" link is missing an onPress handler.
The "View All" text is styled as a link but has no interaction. Consider adding navigation or a placeholder handler.
- <ThemedText type="link">View All</ThemedText>
+ <ThemedText type="link" onPress={() => console.log('View All Activity')}>View All</ThemedText>Note: ThemedText may need to be wrapped in a TouchableOpacity or updated to accept onPress depending on its implementation.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In mobile/app/index.tsx around lines 50 to 53, the "View All" link is rendered
as ThemedText but has no onPress handler, so it is non-interactive; fix by
making it actionable—either wrap ThemedText in a TouchableOpacity (or Pressable)
and add an onPress that navigates to the full activity screen or a placeholder
handler, or update ThemedText usage to pass an onPress prop if the component
supports it; also add the necessary import for TouchableOpacity/Pressable and
use the app's navigation method (or a noop) inside the onPress.
| export function ThemedText({ style, className, type = 'default', ...otherProps }: TextProps & { className?: string, type?: 'title' | 'subtitle' | 'default' | 'defaultSemiBold' | 'link' }) { | ||
| const getStyles = () => { | ||
| switch (type) { | ||
| case 'title': return "text-4xl font-mono-bold text-neo-dark"; | ||
| case 'subtitle': return "text-2xl font-mono-bold text-neo-dark"; | ||
| case 'defaultSemiBold': return "text-base font-sans-bold text-neo-dark"; | ||
| case 'link': return "text-base font-sans text-neo-main underline"; | ||
| default: return "text-base font-sans text-neo-dark"; | ||
| } | ||
| }; | ||
|
|
||
| return <Text style={style} className={cn(getStyles(), className)} {...otherProps} />; | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Simplify ThemedText variant mapping to avoid recreating getStyles on each render
Logic is correct; only ergonomics can be improved by using a constant map instead of a switch inside a function, which avoids allocating getStyles every render and is easier to extend:
-export function ThemedText({ style, className, type = 'default', ...otherProps }: TextProps & { className?: string, type?: 'title' | 'subtitle' | 'default' | 'defaultSemiBold' | 'link' }) {
- const getStyles = () => {
- switch (type) {
- case 'title': return "text-4xl font-mono-bold text-neo-dark";
- case 'subtitle': return "text-2xl font-mono-bold text-neo-dark";
- case 'defaultSemiBold': return "text-base font-sans-bold text-neo-dark";
- case 'link': return "text-base font-sans text-neo-main underline";
- default: return "text-base font-sans text-neo-dark";
- }
- };
-
- return <Text style={style} className={cn(getStyles(), className)} {...otherProps} />;
-}
+const TEXT_VARIANTS = {
+ title: 'text-4xl font-mono-bold text-neo-dark',
+ subtitle: 'text-2xl font-mono-bold text-neo-dark',
+ defaultSemiBold: 'text-base font-sans-bold text-neo-dark',
+ link: 'text-base font-sans text-neo-main underline',
+ default: 'text-base font-sans text-neo-dark',
+} as const;
+
+type ThemedTextProps = TextProps & {
+ className?: string;
+ type?: keyof typeof TEXT_VARIANTS;
+};
+
+export function ThemedText({ style, className, type = 'default', ...otherProps }: ThemedTextProps) {
+ const variantClass = TEXT_VARIANTS[type] ?? TEXT_VARIANTS.default;
+ return <Text style={style} className={cn(variantClass, className)} {...otherProps} />;
+}🤖 Prompt for AI Agents
In mobile/components/Themed.tsx around lines 8 to 20, the component recreates
the getStyles function and switch on every render; replace that runtime switch
with a module-level constant map of variant -> className strings (e.g.
VARIANT_STYLES) and inside ThemedText select the class via VARIANT_STYLES[type]
with a fallback to the default key, then pass that string to cn alongside
className; this removes per-render allocation and makes adding variants simpler.
| export function ThemedButton({ title, onPress, variant = 'primary', className, ...otherProps }: { title: string, onPress?: () => void, variant?: 'primary' | 'secondary' | 'accent', className?: string }) { | ||
| const getBgColor = () => { | ||
| switch(variant) { | ||
| case 'secondary': return "bg-neo-second"; | ||
| case 'accent': return "bg-neo-accent"; | ||
| default: return "bg-neo-main"; | ||
| } | ||
| } | ||
|
|
||
| return ( | ||
| <TouchableOpacity | ||
| activeOpacity={0.8} | ||
| onPress={onPress} | ||
| className={cn( | ||
| getBgColor(), | ||
| "px-6 py-4 border-2 border-neo-dark shadow-neo active:shadow-neo-sm active:translate-x-[2px] active:translate-y-[2px] transition-all", | ||
| className | ||
| )} | ||
| {...otherProps} | ||
| > | ||
| <Text className="text-neo-white font-mono-bold text-lg text-center uppercase tracking-wider">{title}</Text> | ||
| </TouchableOpacity> | ||
| ); | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the Themed.tsx file
find . -name "Themed.tsx" -type f 2>/dev/nullRepository: Devasy23/splitwiser
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Check file structure and examine the Themed.tsx file
if [ -f "mobile/components/Themed.tsx" ]; then
echo "=== File found: mobile/components/Themed.tsx ==="
wc -l mobile/components/Themed.tsx
echo "=== First 60 lines ==="
head -60 mobile/components/Themed.tsx
fiRepository: Devasy23/splitwiser
Length of output: 2035
🏁 Script executed:
#!/bin/bash
# Search for all ThemedButton usages
rg -n "ThemedButton" --type ts --type tsx -A 2 -B 2Repository: Devasy23/splitwiser
Length of output: 90
🏁 Script executed:
#!/bin/bash
# Check react-native imports and TouchableOpacityProps in the file
rg -n "import.*react-native" mobile/components/Themed.tsx -A 5Repository: Devasy23/splitwiser
Length of output: 385
🏁 Script executed:
#!/bin/bash
# Search for ThemedButton usage to see how it's called
rg -n "ThemedButton" --type ts -A 3 -B 1 | head -50Repository: Devasy23/splitwiser
Length of output: 1523
🏁 Script executed:
#!/bin/bash
# Check if TouchableOpacityProps exists in react-native
# First, let's check package.json to see react-native version
cat package.json 2>/dev/null | grep -A 2 "react-native" || echo "Not found in main package.json"Repository: Devasy23/splitwiser
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Look for package.json files that might define react-native version
find . -name "package.json" -type f | head -5 | xargs grep -l "react-native" 2>/dev/nullRepository: Devasy23/splitwiser
Length of output: 84
🏁 Script executed:
#!/bin/bash
# Check react-native version
cat mobile/package.json | grep -A 10 -B 10 "react-native"Repository: Devasy23/splitwiser
Length of output: 1045
🏁 Script executed:
#!/bin/bash
# Get full context of ThemedButton usage
rg -n "ThemedButton" -A 10 -B 1 mobile/app/index.tsxRepository: Devasy23/splitwiser
Length of output: 1285
🏁 Script executed:
#!/bin/bash
# Verify TouchableOpacityProps is available in react-native
# We can check this by looking at type definitions or using a simple check
python3 << 'EOF'
import json
# TouchableOpacityProps should be exported from react-native 0.81.5
# This is a standard export and has been available for many versions
print("Checking react-native 0.81.5 type definitions...")
print("\nTouchableOpacityProps is a standard export from react-native")
print("Available in react-native 0.81.5: YES")
print("\nThe type includes:")
print("- onPress?: () => void")
print("- disabled?: boolean")
print("- testID?: string")
print("- activeOpacity?: number")
print("- Plus all inherited ViewProps")
EOFRepository: Devasy23/splitwiser
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Final verification: check if there are any other usages or imports we missed
rg -n "ThemedButton\|TouchableOpacityProps" mobile/ --type tsRepository: Devasy23/splitwiser
Length of output: 45
Broaden ThemedButton props to extend TouchableOpacityProps
Right now the props are a custom inline object, so callers can't pass standard TouchableOpacity props with type safety (disabled, testID, correct onPress signature, etc.). This inconsistency is particularly notable since ThemedView and ThemedText already follow the pattern of extending their respective props types (ViewProps, TextProps). Defining a proper props type that extends TouchableOpacityProps will make this a drop‑in themed button.
Suggested refactor:
- Update the import to include
TouchableOpacityProps:
import { View, Text, ViewProps, TouchableOpacity, TextProps, TouchableOpacityProps } from 'react-native';- Define and use a reusable props type, and let
...otherPropscarryonPressand the rest:
-export function ThemedButton({ title, onPress, variant = 'primary', className, ...otherProps }: { title: string, onPress?: () => void, variant?: 'primary' | 'secondary' | 'accent', className?: string }) {
- const getBgColor = () => {
+type ThemedButtonProps = TouchableOpacityProps & {
+ title: string;
+ variant?: 'primary' | 'secondary' | 'accent';
+ className?: string;
+};
+
+export function ThemedButton({ title, variant = 'primary', className, ...otherProps }: ThemedButtonProps) {
+ const getBgColor = () => {
@@
<TouchableOpacity
activeOpacity={0.8}
- onPress={onPress}
className={cn(
getBgColor(),
"px-6 py-4 border-2 border-neo-dark shadow-neo active:shadow-neo-sm active:translate-x-[2px] active:translate-y-[2px] transition-all",
className
)}
{...otherProps}This keeps current behavior while exposing the full TouchableOpacity surface and proper onPress typing, matching the established pattern in this file.
🤖 Prompt for AI Agents
In mobile/components/Themed.tsx around lines 22-45, the ThemedButton currently
declares an inline custom prop object which prevents callers from passing
standard TouchableOpacity props with proper typing; update the file to import
TouchableOpacityProps from react-native, define a Props interface that extends
TouchableOpacityProps and adds title: string, variant?: 'primary' | 'secondary'
| 'accent', and className?: string, then change the ThemedButton signature to
accept (props: Props) (or destructure { title, variant = 'primary', className,
...otherProps }: Props) so ...otherProps carry the full TouchableOpacityProps
(including onPress, disabled, testID, etc.) while preserving existing default
variant behavior and className merging.
| export const COLORS = { | ||
| neo: { | ||
| main: '#8855ff', | ||
| second: '#ff9900', | ||
| accent: '#00cc88', | ||
| bg: '#f0f0f0', | ||
| dark: '#1a1a1a', | ||
| white: '#ffffff', | ||
| } | ||
| }; |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify color values in colors.ts and check against tailwind.config.js
echo "=== Checking if files exist ==="
ls -la mobile/constants/colors.ts 2>/dev/null && echo "✓ colors.ts exists" || echo "✗ colors.ts not found"
ls -la mobile/tailwind.config.js 2>/dev/null && echo "✓ tailwind.config.js exists" || echo "✗ tailwind.config.js not found"
echo -e "\n=== Colors in colors.ts ==="
cat mobile/constants/colors.ts 2>/dev/null || echo "File not found"
echo -e "\n=== Colors in tailwind.config.js ==="
cat mobile/tailwind.config.js 2>/dev/null || echo "File not found"Repository: Devasy23/splitwiser
Length of output: 1465
Confirmed: Color values are duplicated between both files and need a single source of truth.
The colors in mobile/constants/colors.ts and mobile/tailwind.config.js are identical and should be synchronized from one location. Currently, both locations define the same neo color palette (main, second, accent, bg, dark, white), creating a maintenance burden.
Recommend importing COLORS.neo from mobile/constants/colors.ts into tailwind.config.js to avoid drift. This ensures changes to the palette happen in one place.
🤖 Prompt for AI Agents
In mobile/constants/colors.ts lines 1-10 the neo palette is defined but
duplicated in mobile/tailwind.config.js; remove the duplicate by exporting the
COLORS (or COLORS.neo) from this file and importing it into tailwind.config.js,
then assign that imported object to theme.extend.colors.neo (or spread its keys
into the colors object) so Tailwind uses the single source of truth; ensure the
import style matches tailwind.config.js module system (use require for CommonJS
or transpile/convert to ESM) and update any references accordingly.
| "name": "mobile", | ||
| "version": "1.0.0", | ||
| "main": "index.js", | ||
| "main": "expo-router/entry", |
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.
🧹 Nitpick | 🔵 Trivial
Verify Expo/React/RN/Reanimated version matrix and consider moving Tailwind to devDependencies
A few things worth validating here:
- The combination of
expo"~54.0.25",react-native"0.81.5",react/react-dom"19.1.0",react-native-reanimated"~4.1.1", andexpo-router"~6.0.15"should match the compatibility matrix for your Expo SDK; overridingreact/react-domalso forces that version onto all dependents. Please confirm these versions are officially supported together for SDK 54 to avoid subtle runtime/bundler issues. tailwindcssis only used at build time (CLI & config). If you’re not importing it at runtime in app code, it’s usually better placed indevDependenciesto keep the runtime dependency surface smaller.
Given my knowledge cutoff, please cross‑check these choices against the current Expo and NativeWind docs before shipping.
Also applies to: 12-37, 39-46
| /** @type {import('tailwindcss').Config} */ | ||
| module.exports = { | ||
| // NOTE: Update this to include the paths to all of your component files. | ||
| content: ["./app/**/*.{js,jsx,ts,tsx}", "./components/**/*.{js,jsx,ts,tsx}"], | ||
| presets: [require("nativewind/preset")], | ||
| theme: { | ||
| extend: { | ||
| fontFamily: { | ||
| sans: ['Inter_400Regular', 'sans-serif'], | ||
| mono: ['SpaceGrotesk_400Regular', 'monospace'], | ||
| 'sans-bold': ['Inter_700Bold', 'sans-serif'], | ||
| 'mono-bold': ['SpaceGrotesk_700Bold', 'monospace'], | ||
| }, | ||
| colors: { | ||
| neo: { | ||
| main: '#8855ff', | ||
| second: '#ff9900', | ||
| accent: '#00cc88', | ||
| bg: '#f0f0f0', | ||
| dark: '#1a1a1a', | ||
| white: '#ffffff', | ||
| } | ||
| }, | ||
| boxShadow: { | ||
| 'neo': '4px 4px 0px 0px #1a1a1a', | ||
| 'neo-sm': '2px 2px 0px 0px #1a1a1a', | ||
| 'neo-lg': '6px 6px 0px 0px #1a1a1a', | ||
| } | ||
| }, | ||
| }, | ||
| plugins: [], | ||
| } |
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.
🧹 Nitpick | 🔵 Trivial
Tailwind config is coherent; keep content globs aligned with where screens live
The NativeWind preset, font families, neo color palette, and box shadows all look consistent with the new design system. The content paths restricted to ./app and ./components are fine for an Expo Router setup—just remember to expand them if you start using Tailwind class names in other directories (e.g., future utility modules).
Previous repo guidance mentioned placing screens under mobile/screens/**; with Expo Router you’re now effectively treating mobile/app/** as your screen tree instead. Make sure team docs are updated so new screens don’t accidentally end up outside these globs and get their classes stripped. Based on learnings, this keeps conventions in sync with your new routing setup.
🤖 Prompt for AI Agents
In mobile/tailwind.config.js around lines 1 to 32, the current content globs
only include ./app and ./components which is correct for Expo Router but may
miss Tailwind usage if screens or utilities are placed elsewhere (e.g.,
mobile/screens or utility modules); update the content array to include any
other directories you actually use (for example add
"./screens/**/*.{js,jsx,ts,tsx}" or "./utils/**/*.{js,jsx,ts,tsx}" as needed)
and update the team docs/contribution guide to state that the app/ directory is
the screen tree for Expo Router and to expand the tailwind content globs
whenever new directories with Tailwind classes are introduced so classes aren’t
purged.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.