feat: Add logo, update navigation#53
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 45 minutes and 48 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes update the Expo app configuration to consolidate icon assets into a single favicon, adjust frontend dependency versions, and refactor the navigation header from a bottom-tab configuration to a custom island-style header component with interactive logo functionality. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/app.json (1)
8-21:⚠️ Potential issue | 🟡 MinorAsset consolidation is unnecessary; dedicated icon/splash assets already exist.
The code consolidates all three assets (app icon, splash, adaptive icon foreground) onto
favicon.png(1024×1024). While this size meets Expo's requirements for these fields, it is suboptimal:
- Separate, purpose-specific assets already exist:
icon.png,splash-icon.png, andadaptive-icon.png(all 1024×1024)- The naming
favicon.pngconventionally refers to web favicons, making the consolidation confusing- Using dedicated assets per purpose improves maintainability and allows different designs for each context
Consider using the purpose-specific assets instead:
icon.pngfor app icon,splash-icon.pngfor splash, andadaptive-icon.pngfor Android adaptive icon foreground.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app.json` around lines 8 - 21, Replace the consolidated use of favicon.png with the dedicated purpose-specific assets: set the top-level "icon" property to use "icon.png", change "splash.image" to "splash-icon.png", and change "android.adaptiveIcon.foregroundImage" to "adaptive-icon.png" so each config key ("icon", "splash.image", "android.adaptiveIcon.foregroundImage") points to its respective asset instead of "./assets/favicon.png".
🧹 Nitpick comments (1)
frontend/package.json (1)
19-28: Version realignment aligns with Expo SDK 54.
expo-blur ~15.0.8,react-native-screens ~4.16.0,react-native 0.81.5, andreact/react-dom ^19.1.0match the versions bundled with Expo SDK 54. Note: Expo recommends tilde ranges forreact/react-dom(matching the version paired with the RN runtime) rather than caret, to prevent accidental minor version drift fromreact-native's peer dependency. Runnpx expo install --checkto confirm resolution.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/package.json` around lines 19 - 28, Update the dependency version ranges in package.json to use tilde for react and react-dom instead of caret so they stay aligned with the React version bundled with the RN runtime; specifically change the "react" and "react-dom" entries (alongside verifying "react-native", "expo-blur", and "react-native-screens") to the exact tilde ranges recommended by Expo SDK 54 and then run npx expo install --check to validate and resolve any peer dependency mismatches.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/navigation/MainTabs.tsx`:
- Around line 33-58: IslandHeader is currently absolutely positioned and
overlaps HomeStack; change the layout so the header occupies layout space
instead of overlaying content: render <IslandHeader/> before <HomeStack/> in
MainContent (i.e., make MainContent use column flow with IslandHeader first) and
update createIslandHeaderStyles / IslandHeader to remove position: "absolute" /
zIndex styling (or make it non-absolute and height-stable using insets.top), so
screens in HomeStack no longer start at y=0 and don't get obscured;
alternatively, if you prefer keeping absolute positioning, add top padding to
HomeStack screens equal to insets.top + islandHeight (measure via onLayout if
dynamic) so HomeStack content is pushed below the island.
- Around line 16-31: HeaderLogo currently calls CommonActions.reset in
handlePress and uses untyped useNavigation and an undersized/unenhanced
Pressable; change handlePress to call navigation.navigate("HomeStack", { screen:
"SelectService" }) instead of CommonActions.reset, type useNavigation as
useNavigation<NavigationProp<MainTabParamList>>() to get route-safe navigation,
and update the Pressable/Image to meet touch and accessibility requirements by
ensuring a minimum 44x44 hit area (e.g., increase container padding or use
hitSlop), add accessibilityRole="button" and an appropriate accessibilityLabel,
and include hitSlop to expand the tappable area.
---
Outside diff comments:
In `@frontend/app.json`:
- Around line 8-21: Replace the consolidated use of favicon.png with the
dedicated purpose-specific assets: set the top-level "icon" property to use
"icon.png", change "splash.image" to "splash-icon.png", and change
"android.adaptiveIcon.foregroundImage" to "adaptive-icon.png" so each config key
("icon", "splash.image", "android.adaptiveIcon.foregroundImage") points to its
respective asset instead of "./assets/favicon.png".
---
Nitpick comments:
In `@frontend/package.json`:
- Around line 19-28: Update the dependency version ranges in package.json to use
tilde for react and react-dom instead of caret so they stay aligned with the
React version bundled with the RN runtime; specifically change the "react" and
"react-dom" entries (alongside verifying "react-native", "expo-blur", and
"react-native-screens") to the exact tilde ranges recommended by Expo SDK 54 and
then run npx expo install --check to validate and resolve any peer dependency
mismatches.
🪄 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: CHILL
Plan: Pro
Run ID: 8da41a84-9d38-4fa8-b020-0f8e8de00eca
⛔ Files ignored due to path filters (2)
frontend/assets/favicon.pngis excluded by!**/*.pngfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
frontend/app.jsonfrontend/package.jsonfrontend/src/navigation/MainTabs.tsxfrontend/src/navigation/navigation.styles.ts
|
1 participant
Summary by CodeRabbit
New Features
Chores