Implement Maplibre and patch dependency/font issues#36
Merged
Conversation
…wiftUI Previews - Created a patch file for expo-widgets@55.0.4 to disable Xcode 26's debug-dylib and SwiftUI Previews for the widget extension target. - Updated pnpm-lock.yaml to include the new patch and its hash. - Modified pnpm-workspace.yaml to register the patched dependency. Co-authored-by: Copilot <copilot@github.com>
Without these UIAppFonts entries, react-native-vector-icons fonts were copied into the bundle by the Pod but not registered with the system, so every icon rendered as a missing-glyph "?" box on a fresh prebuild. Covers the four font families actually imported in the app: Ionicons, MaterialCommunityIcons, MaterialIcons (used only by MapSettingsPill), and FontAwesome6 (solid variant). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe PR migrates the mobile app from react-native-maps to ChangesMap Library Migration from react-native-maps to MapLibre
Sequence Diagram(s)sequenceDiagram
participant User
participant MapHandlers
participant CameraRef
participant MapLibre
User->>MapHandlers: tap/select/fit request
MapHandlers->>CameraRef: easeTo(center, zoom) / fitBounds(bounds)
CameraRef->>MapLibre: update camera/view
MapLibre-->>MapHandlers: onRegionDidChange(viewState)
MapHandlers->>MapHandlers: update region state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/mobile/components/map/AnimatedRoute.tsx (2)
14-14: 💤 Low valueUnused
zoomOpacityprop.The
zoomOpacityprop is declared in the interface but never used in the component. Consider removing it if it's no longer needed after the MapLibre migration, or implementing the intended opacity behavior.🤖 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/map/AnimatedRoute.tsx` at line 14, The props interface for the AnimatedRoute component declares zoomOpacity but the component does not use it; either remove zoomOpacity from the props interface and any callers that pass it, or implement its intended behavior inside the AnimatedRoute component (e.g., apply zoomOpacity to the route layer or container style when rendering) so the prop has effect; locate the zoomOpacity declaration in the AnimatedRoute props interface and the AnimatedRoute functional component to update the signature and callers or to read the prop and apply it to the rendered map layer/container.
18-25: ⚡ Quick winMemoize the GeoJSON object to avoid unnecessary layer updates.
The
geoJSONobject is recreated on every render. Since MapLibre may diff/update the source when the object reference changes, consider memoizing it withuseMemoto only recompute whencoordinateschanges.♻️ Proposed fix
+import React, { useMemo } from 'react'; -import React from 'react'; import { GeoJSONSource, Layer } from '@maplibre/maplibre-react-native'; // ... interface definitions ... export const AnimatedRoute = React.memo(function AnimatedRoute({ id, coordinates, strokeColor, strokeWidth }: AnimatedRouteProps) { - const geoJSON: GeoJSON.Feature<GeoJSON.LineString> = { + const geoJSON = useMemo<GeoJSON.Feature<GeoJSON.LineString>>(() => ({ type: 'Feature', geometry: { type: 'LineString', coordinates: coordinates.map(c => [c.longitude, c.latitude]), }, properties: {}, - }; + }), [coordinates]);🤖 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/map/AnimatedRoute.tsx` around lines 18 - 25, The geoJSON object in AnimatedRoute is being recreated every render which can trigger unnecessary MapLibre source updates; wrap the geoJSON construction in React.useMemo so it only recomputes when coordinates changes (e.g., const geoJSON = useMemo(() => ({ type: 'Feature', geometry: { type: 'LineString', coordinates: coordinates.map(c => [c.longitude, c.latitude]) }, properties: {} }), [coordinates])); update references in the component to use this memoized geoJSON.apps/mobile/constants/map-styles.ts (1)
5-5: 💤 Low valueSatellite style uses the same base map as standard.
The
satellitekey usesvoyager-gl-style, which is identical tostandard. If satellite imagery isn't available from CARTO's free tier, consider adding a comment clarifying this is a fallback, or removing thesatelliteoption from the UI to avoid user confusion.🤖 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/constants/map-styles.ts` at line 5, The satellite entry in the map styles object (the "satellite" key) points to the same CARTO Voyager style as "standard", so update apps/mobile/constants/map-styles.ts to avoid confusion by adding a clear comment above the "satellite" key stating that true satellite imagery is not available on CARTO's free tier and that this entry is a visual fallback (or remove the "satellite" key entirely); if you remove it, also remove or disable the "satellite" option in the UI component (e.g., MapStylePicker) so the option no longer appears to users.
🤖 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/constants/map.ts`:
- Around line 59-61: latDeltaToZoom should guard against zero or negative
latitudeDelta to avoid Infinity/NaN from Math.log2; update the function
(latDeltaToZoom) to validate or clamp latitudeDelta (e.g., if latitudeDelta <= 0
then set to a small positive minimum like 1e-6 or return a sensible max zoom
value) before computing Math.log2(360 / latitudeDelta), and document the chosen
minimum/clamp behavior in a short comment.
---
Nitpick comments:
In `@apps/mobile/components/map/AnimatedRoute.tsx`:
- Line 14: The props interface for the AnimatedRoute component declares
zoomOpacity but the component does not use it; either remove zoomOpacity from
the props interface and any callers that pass it, or implement its intended
behavior inside the AnimatedRoute component (e.g., apply zoomOpacity to the
route layer or container style when rendering) so the prop has effect; locate
the zoomOpacity declaration in the AnimatedRoute props interface and the
AnimatedRoute functional component to update the signature and callers or to
read the prop and apply it to the rendered map layer/container.
- Around line 18-25: The geoJSON object in AnimatedRoute is being recreated
every render which can trigger unnecessary MapLibre source updates; wrap the
geoJSON construction in React.useMemo so it only recomputes when coordinates
changes (e.g., const geoJSON = useMemo(() => ({ type: 'Feature', geometry: {
type: 'LineString', coordinates: coordinates.map(c => [c.longitude, c.latitude])
}, properties: {} }), [coordinates])); update references in the component to use
this memoized geoJSON.
In `@apps/mobile/constants/map-styles.ts`:
- Line 5: The satellite entry in the map styles object (the "satellite" key)
points to the same CARTO Voyager style as "standard", so update
apps/mobile/constants/map-styles.ts to avoid confusion by adding a clear comment
above the "satellite" key stating that true satellite imagery is not available
on CARTO's free tier and that this entry is a visual fallback (or remove the
"satellite" key entirely); if you remove it, also remove or disable the
"satellite" option in the UI component (e.g., MapStylePicker) so the option no
longer appears to users.
🪄 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: 9d5b9248-5b6b-4717-be71-233281279065
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
apps/mobile/app.config.tsapps/mobile/components/map/AnimatedRoute.tsxapps/mobile/components/map/AnimatedStationMarker.tsxapps/mobile/components/map/LiveTrainMarker.tsxapps/mobile/components/map/RouteOverlay.tsxapps/mobile/constants/map-styles.tsapps/mobile/constants/map.tsapps/mobile/jest.setup.jsapps/mobile/package.jsonapps/mobile/screens/MapScreen.tsxpatches/expo-widgets@55.0.4.patchpnpm-workspace.yaml
💤 Files with no reviewable changes (1)
- apps/mobile/components/map/RouteOverlay.tsx
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
This pull request migrates the mobile app's map implementation from
react-native-maps(Google Maps) to MapLibre (@maplibre/maplibre-react-native), updating all related components, configuration, and dependencies. It also introduces MapLibre-compatible map styles, updates mocks for testing, and removes now-obsolete code and dependencies.Migration to MapLibre:
react-native-mapswith@maplibre/maplibre-react-nativein map-related components such asAnimatedRoute,AnimatedStationMarker,LiveTrainMarker, andMapScreen. This includes switching to MapLibre'sMap,Marker,GeoJSONSource,Layer, and related APIs. [1] [2] [3] [4]app.config.ts) to add the MapLibre plugin and remove the Google Maps plugin and API keys. [1] [2]Map Styles and Configuration:
map-styles.ts, supporting standard, dark, and satellite modes.map.ts.Dependency and Build Updates:
react-native-mapsfrom dependencies and added@maplibre/maplibre-react-native. [1] [2]Component and Code Cleanup:
tracksViewChangesand animated coordinate logic) from marker components, adapting them for MapLibre. [1] [2]RouteOverlaydependency onreact-native-maps. [1] [2]iOS Font Configuration:
Summary by CodeRabbit
New Features
Chores