feat: use MRMapView method to track user location #12
Conversation
WalkthroughThe update adds logic to synchronize the display of the user's location on the map with the Changes
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
react-native-meridian-maps/example/ios/Podfile.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
react-native-meridian-maps/ios/MeridianMapViewManager.m(4 hunks)
|
|
||
| // Also disable map view user location display | ||
| if (self.mapViewController.mapView) { | ||
| self.mapViewController.mapView.showsUserLocation = NO; | ||
| NSLog(@"[MeridianMapView] Disabled showsUserLocation"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wrap disabling of showsUserLocation on main thread
Disabling the user location display must also be performed on the main queue. Enclose this in dispatch_async(dispatch_get_main_queue(), ...) to maintain UI thread safety:
dispatch_async(dispatch_get_main_queue(), ^{
self.mapViewController.mapView.showsUserLocation = NO;
NSLog(@"[MeridianMapView] Disabled showsUserLocation");
});🤖 Prompt for AI Agents
In react-native-meridian-maps/ios/MeridianMapViewManager.m around lines 363 to
368, the code disables showsUserLocation directly, which must be done on the
main thread for UI safety. Wrap the disabling of showsUserLocation and the NSLog
call inside a dispatch_async block targeting dispatch_get_main_queue() to ensure
these UI updates happen on the main thread.
| // Enable user location display if needed | ||
| if (self.showLocationUpdates) { | ||
| mapViewController.mapView.showsUserLocation = YES; | ||
| NSLog(@"[MeridianMapView] Enabled showsUserLocation on mapView (setMapViewController)"); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate user-location logic in setMapViewController
This block duplicates the same enabling logic found elsewhere. Consider removing it and invoking updateLocationUpdates (or a shared handler) after setting the view controller to keep a single source of truth.
🤖 Prompt for AI Agents
In react-native-meridian-maps/ios/MeridianMapViewManager.m around lines 215 to
220, the code enabling user location display duplicates logic found elsewhere.
Remove this block and instead call the existing updateLocationUpdates method (or
a shared handler) after setting the mapViewController to centralize and
consolidate the user-location enabling logic in one place.
|
|
||
| // Also enable map view user location display | ||
| if (self.mapViewController.mapView) { | ||
| self.mapViewController.mapView.showsUserLocation = YES; | ||
| NSLog(@"[MeridianMapView] Enabled showsUserLocation (permission granted)"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure thread-safe UI updates
Toggling mapView.showsUserLocation should run on the main thread to avoid race conditions. Wrap these calls in:
dispatch_async(dispatch_get_main_queue(), ^{
self.mapViewController.mapView.showsUserLocation = YES;
NSLog(@"[MeridianMapView] Enabled showsUserLocation (permission granted)");
});🤖 Prompt for AI Agents
In react-native-meridian-maps/ios/MeridianMapViewManager.m around lines 342 to
347, the code sets showsUserLocation on the mapView which updates the UI and
must be done on the main thread to avoid race conditions. Wrap the block that
sets showsUserLocation and logs the message inside a dispatch_async call to
dispatch_get_main_queue() to ensure these UI updates happen on the main thread
safely.
| // Enable user location display on the map view | ||
| if (self.showLocationUpdates) { | ||
| self.mapViewController.mapView.showsUserLocation = YES; | ||
| NSLog(@"[MeridianMapView] Enabled showsUserLocation on mapView"); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid duplicating showsUserLocation toggles
The map’s showsUserLocation is being enabled here and also in setMapViewController and updateLocationUpdates. Centralize this logic—e.g., call a single helper or rely solely on updateLocationUpdates—to reduce code duplication and ensure consistency.
🤖 Prompt for AI Agents
In react-native-meridian-maps/ios/MeridianMapViewManager.m around lines 157 to
162, the showsUserLocation property is being set in multiple places causing
duplication. Refactor by removing the direct setting of showsUserLocation here
and instead centralize this logic in a single method like updateLocationUpdates.
Ensure all toggling of showsUserLocation is done through that helper method to
maintain consistency and avoid redundant code.
https://files.meridianapps.com/meridian-ios-sdk/docs-6.0.0/Classes/MRMapView.html#//api/name/showsUserLocation
Summary by CodeRabbit