Skip to content

feat: use MRMapView method to track user location #12

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions react-native-meridian-maps/example/ios/Podfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ PODS:
- hermes-engine (0.79.2):
- hermes-engine/Pre-built (= 0.79.2)
- hermes-engine/Pre-built (0.79.2)
- MeridianMaps (0.1.19):
- MeridianMaps (0.1.21):
- DoubleConversion
- glog
- hermes-engine
Expand Down Expand Up @@ -1913,7 +1913,7 @@ SPEC CHECKSUMS:
fmt: a40bb5bd0294ea969aaaba240a927bd33d878cdd
glog: 5683914934d5b6e4240e497e0f4a3b42d1854183
hermes-engine: 314be5250afa5692b57b4dd1705959e1973a8ebe
MeridianMaps: 3d2e11d0738f596a3fbc1d36b722679f1dcd9100
MeridianMaps: b02395f5b0c29548d7d452446a55ce8b7e321580
RCT-Folly: e78785aa9ba2ed998ea4151e314036f6c49e6d82
RCTDeprecation: 83ffb90c23ee5cea353bd32008a7bca100908f8c
RCTRequired: eb7c0aba998009f47a540bec9e9d69a54f68136e
Expand Down
24 changes: 24 additions & 0 deletions react-native-meridian-maps/ios/MeridianMapViewManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,12 @@ - (void)setupMap {

self.mapViewController = mapViewController;

// Enable user location display on the map view
if (self.showLocationUpdates) {
self.mapViewController.mapView.showsUserLocation = YES;
NSLog(@"[MeridianMapView] Enabled showsUserLocation on mapView");
}

Comment on lines +157 to +162
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

// Set up location manager
self.appKey = [MREditorKey keyWithIdentifier:self.appId];
self.locationManager = [[MRLocationManager alloc] initWithApp:self.appKey];
Expand Down Expand Up @@ -206,6 +212,12 @@ - (void)setMapViewController:(CustomMapViewController *)mapViewController {
UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight;
[self addSubview:mapViewController.view];

// Enable user location display if needed
if (self.showLocationUpdates) {
mapViewController.mapView.showsUserLocation = YES;
NSLog(@"[MeridianMapView] Enabled showsUserLocation on mapView (setMapViewController)");
}

Comment on lines +215 to +220
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

// Update location updates based on current setting
[self updateLocationUpdates];

Expand Down Expand Up @@ -327,6 +339,12 @@ - (void)updateLocationUpdates {
} else if (status == kCLAuthorizationStatusAuthorizedWhenInUse || status == kCLAuthorizationStatusAuthorizedAlways) {
NSLog(@"[MeridianMapView] Starting location updates");
[self.locationManager startUpdatingLocation];

// Also enable map view user location display
if (self.mapViewController.mapView) {
self.mapViewController.mapView.showsUserLocation = YES;
NSLog(@"[MeridianMapView] Enabled showsUserLocation (permission granted)");
}
Comment on lines +342 to +347
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

} else {
NSLog(@"[MeridianMapView] Location permission is denied or restricted. Status: %d", status);
MMEventEmitter *emitter = [self.bridge moduleForClass:[MMEventEmitter class]];
Expand All @@ -342,6 +360,12 @@ - (void)updateLocationUpdates {
} else {
NSLog(@"[MeridianMapView] Stopping location updates");
[self.locationManager stopUpdatingLocation];

// Also disable map view user location display
if (self.mapViewController.mapView) {
self.mapViewController.mapView.showsUserLocation = NO;
NSLog(@"[MeridianMapView] Disabled showsUserLocation");
}
Comment on lines +363 to +368
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.

}
}

Expand Down