/rebase

/azp run

Independent Code Review

Summary: Adds UserLocationChanged event and LastUserLocation property to Map. Android uses GoogleMap.MyLocationChange, iOS uses MKMapView.DidUpdateUserLocation.

✅ What Looks Good

• Proper de-duplication: UserLocationUpdated checks Equals(_lastUserLocation, location) before raising
• Correct OnPropertyChanging/OnPropertyChanged notifications for binding support
• Both Android and iOS implementations properly subscribe/unsubscribe
• Good test coverage (7 tests)

🔴 Issues

1. LastUserLocation is not a BindableProperty
It's a plain CLR property backed by a _lastUserLocation field. While OnPropertyChanged is called, it can't be used as a binding target in XAML (e.g. {Binding LastUserLocation} won't work on the Map directly since it's not a BP). This is inconsistent with other Map properties.

2. Android MyLocationChange is deprecated
GoogleMap.MyLocationChangeEventArgs is deprecated in favor of FusedLocationProviderClient. This will generate warnings and may be removed in future Google Play Services updates.

3. No Windows implementation
Location updates won't fire on Windows. No unsupported platform attribute or documentation noting this.

4. High-frequency event with no throttling
GPS updates can fire very frequently. The Equals check helps only for identical coordinates. Consider documenting that consumers should throttle/debounce if using for UI updates.

🟡 Nits

• The Equals comparison on Location — does Location.Equals compare all properties (lat, lon, altitude, accuracy, etc.) or just lat/lon? If it compares everything, minor accuracy changes will fire events. If it only compares lat/lon, it might miss meaningful updates.

Thanks for the detailed review @kubaflo!

1. LastUserLocation is not a BindableProperty
This is intentional and consistent with the \VisibleRegion\ pattern already in Map. \VisibleRegion\ is also a plain CLR property with \OnPropertyChanged, not a BindableProperty. Both represent values that are set by the platform handler, not by the user — making them BindableProperties would imply you can bind to them (set them), which would be misleading. The \OnPropertyChanging/\OnPropertyChanged\ calls enable data binding from them in MVVM scenarios (e.g., {Binding LastUserLocation, Source={x:Reference myMap}}).

2. Android MyLocationChange is deprecated
Good catch. The \FusedLocationProviderClient\ is the recommended replacement, but it requires Google Play Services dependency and significantly more complex lifecycle management (separate location permission handling, client connection, etc.). For the initial implementation, \GoogleMap.MyLocationChange\ is simpler and still functional. A follow-up PR can migrate to \FusedLocationProviderClient\ as a separate improvement.

3. No Windows implementation
Same as other Maps PRs — Windows Maps (\MapControl) doesn't expose user location change events natively. This would require polling or integrating with \Geolocator, which is a separate API concern.

4. High-frequency event with no throttling
The \Equals\ check already deduplicates identical coordinates. The GPS update frequency is controlled by the platform handler's subscription, not the event itself. Consumers who need throttling can use \System.Reactive\ or a simple timer-based debounce. Adding throttling at the control level would make the behavior opaque and harder to reason about. I'll add a remark in the XML docs suggesting consumers debounce if needed.

5. Location.Equals comparison
\Location.Equals\ compares Latitude and Longitude only (the two properties that identify a geographic position). Altitude, accuracy, speed, etc. are not part of the equality comparison, which is the correct behavior for this use case.

🔍 Round 2 Review — PR #33799 (UserLocationChanged)

Updated with author response analysis

✅ What Looks Good

• Dedicated UserLocationChangedEventArgs type — good design choice.
• Proper null-check on Android: if (e.Location != null && VirtualView != null).
• iOS implementation through MKMapView.DidUpdateUserLocation is the correct iOS API.
• PropertyChanged/PropertyChanging notifications for LastUserLocation enable data binding scenarios.

⚠️ Issues & Author Responses

1. LastUserLocation is not a BindableProperty → Retracted
Author correctly points out this is consistent with the existing VisibleRegion pattern — also a plain CLR property with OnPropertyChanged, not a BindableProperty. Both represent values set by the platform handler, not by the user. Making them BindableProperty would imply they can be bound to (set), which is misleading. OnPropertyChanged calls still enable data binding from them (e.g., {Binding LastUserLocation, Source={x:Reference myMap}}). This is the right pattern. ✅

2. Android uses deprecated GoogleMap.MyLocationChange event → Acknowledged, follow-up planned
Author agrees this should move to FusedLocationProviderClient but notes it requires significantly more complex lifecycle management (separate location permission handling, client connection). Using the simpler deprecated API for the initial implementation is pragmatic. Follow-up PR planned. ✅

3. No Windows implementation → Acknowledged as platform limitation
Windows MapControl doesn't expose user location change events natively. Would require separate Geolocator API integration. ✅

4. High-frequency event with no throttling → Retracted
Author makes a strong case: GPS update frequency is controlled by the platform handler's subscription, not the event itself. Adding throttling at the control level would make behavior opaque and harder to reason about. Consumers can use System.Reactive or timer-based debounce. This is the correct separation of concerns. ✅

5. Location equality dedup → Partially retracted
Author states Location.Equals compares Latitude and Longitude only, so identical coordinates won't trigger redundant UI updates through the binding system. However, note that the UserLocationChanged event itself still fires for every platform callback, even with identical coordinates — there's no dedup before Invoke. This is consistent with how MapClicked works (no dedup there either), so it's acceptable.

6. Sample uses Debug.WriteLine → Still noted (minor)
The sample page uses Debug.WriteLine which is stripped in Release. Consider displaying info in UI only or using ILogger. Minor — this is sample code, not library code.

Summary

The author's VisibleRegion precedent argument was convincing — my most substantive concern was based on an incorrect assumption about the codebase's patterns. The remaining items are informational/minor. This PR is in good shape.

/azp run maui-pr

📋 Round 3 — PR #33799 (UserLocationChanged) — Final Recommendation

No outstanding issues. All Round 2 concerns were satisfactorily addressed.

Note on CI: This PR has a maui-pr-devicetests failure specifically in CoreCLR iOS Helix Tests. All other device tests pass (Android, MacCatalyst, Windows, Mono iOS). This looks like a flaky iOS CoreCLR Helix test — worth re-running to confirm it's not related to this PR's iOS MKMapView.DidUpdateUserLocation changes.

Recommendation: Re-run maui-pr-devicetests once to rule out flakiness, then merge. If iOS CoreCLR fails again, investigate whether the UserLocationChanged event handler on iOS has any CoreCLR-specific issue.

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 33799

Or

• Run remotely in PowerShell:

iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 33799"