/azp run

Independent Code Review

Summary: Adds a MapStyle string property to IMap/Map for applying Google Maps JSON styling on Android, with no-op implementations on iOS/MacCatalyst/Windows/Tizen. Includes a sample gallery page and 3 unit tests. Closes #10706.

✅ What Looks Good

• Correct handler plumbing — Property mapper registration in MapHandler.cs, platform-specific handler methods, and OnMapReady initialization are all wired up properly
• Good use of [UnsupportedOSPlatform] with #if !NETSTANDARD guards to give compile-time CA1416 warnings
• Android implementation is solid — Null-check on googleMap, SetMapStyle(null) to clear, and checking the return value of SetMapStyle
• PublicAPI files all updated consistently

🔴 Issues

1. Error handling uses Debug.WriteLine — silent failure in production

if (!result)
{
    System.Diagnostics.Debug.WriteLine("Failed to apply map style...");
}

Debug.WriteLine is stripped in Release builds. Invalid JSON will silently fail with no feedback. This should use ILogger (already available via MauiContext) or at minimum throw so developers know their style JSON is malformed.

2. MapStyleOptions is never disposed
MapStyleOptions inherits from Java.Lang.Object which implements IDisposable. This should be wrapped in a using statement to avoid leaking a JNI reference:

using var styleOptions = new MapStyleOptions(map.MapStyle);

3. Non-Android platform handlers throw NotImplementedException
Standard/Tizen/Windows all throw new NotImplementedException() for MapMapStyle. If a developer writes cross-platform code that sets MapStyle, the iOS handler correctly no-ops, but Windows will crash at runtime. The iOS approach (empty method body) should be used consistently.

🟡 Suggestions

• Sample code has verbose #pragma warning disable CA1416 in every button handler — consider suppressing once at file level or using OperatingSystem.IsAndroid() checks instead (better example for users)
• No JSON validation on set — consider basic validation (e.g., checking the string starts with [)
• Missing test for PropertyChanged notification when MapStyle changes

🚀 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 -- 33991

Or

• Run remotely in PowerShell:

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

Review Response

Thanks for the thorough review @kubaflo! All three red issues have been addressed in the latest commit:

Issues - All Fixed

1. Debug.WriteLine to ILogger Fixed.
Replaced Debug.WriteLine with ILogger resolved via MauiContext.Services. The UpdateMapStyle signature now accepts an optional IMauiContext? parameter (following the same pattern as UpdateIsShowingUser). Both call sites in MapHandler.Android.cs now pass MauiContext.

2. MapStyleOptions disposal Fixed.
Added using var styleOptions = new MapStyleOptions(map.MapStyle); to properly dispose the JNI reference after SetMapStyle returns.

3. NotImplementedException to no-op Fixed.
Changed Standard, Tizen, and Windows handlers from throw new NotImplementedException() to empty method bodies { }, consistent with the iOS handler approach. Cross-platform code that sets MapStyle will now silently no-op on unsupported platforms instead of crashing.

Suggestions - Responses

• Sample pragma warnings: Good point - the per-method pragma warning disable CA1416 is verbose. However, file-level suppression would suppress the warning for the entire file which could mask legitimate issues. The current approach, while verbose, is explicit. OperatingSystem.IsAndroid() checks would be cleaner for a sample but add more code. Happy to refactor if preferred.

• JSON validation on set: Intentionally omitted - SetMapStyle already validates and returns false for invalid JSON. Adding our own validation would duplicate Google's parser and could reject valid styles that use features we don't anticipate. The ILogger warning now surfaces failures to developers.

• PropertyChanged test: Good catch. The existing MapStyleIsSetCorrectly test verifies the property value, and MapStyle is a standard BindableProperty which inherits PropertyChanged notification from the framework. Adding an explicit test is possible but would be testing BindableProperty infrastructure rather than MapStyle-specific logic.

All changes verified: Build passes, 35 map tests pass.

🔍 Round 2 Review — PR #33991 (MapStyle)

Updated with author response analysis

✅ Fixed Since Round 1

• MapStyleOptions disposal: using var styleOptions = new MapStyleOptions(map.MapStyle) — JNI object leak fixed. ✅
• Error logging: Uses ILogger.LogWarning instead of Debug.WriteLine — works in Release builds. ✅
• Non-Android handlers: Standard, Tizen, Windows, and iOS all use empty method bodies { }. ✅
• [UnsupportedOSPlatform] attributes: Compile-time warnings for iOS, macCatalyst, Windows. ✅
• MapStyle applied on OnMapReady: Style applied during initial map setup. ✅

⚠️ Issues & Author Responses

1. UpdateMapStyle is a public extension method → Minor, not a blocker
This follows the existing MAUI Maps pattern (UpdateIsTrafficEnabled, UpdateIsScrollEnabled are also public extension methods). Consistent with the codebase.

2. MapStyle property has no validation → Retracted
Author correctly explains: GoogleMap.SetMapStyle already validates the JSON and returns false for invalid styles. Adding our own validation would duplicate Google's parser and could reject valid styles using features we don't anticipate. The ILogger warning now surfaces failures. This is the right approach — validate at the platform layer, not the abstraction layer. ✅

3. iOS silent no-op → Retracted
The [UnsupportedOSPlatform] attributes provide compile-time warnings, which is the .NET standard mechanism for platform-specific APIs. Consumers get warned at build time. The empty handler body is the correct behavior. ✅

4. Sample pragma warnings → Acknowledged as intentional
Author explains per-method #pragma warning disable CA1416 is verbose but explicit — file-level suppression could mask legitimate issues. Valid reasoning. ✅

👍 What Looks Good

• UpdateMapStyle follows existing MAUI extension method patterns.
• Clean separation: handler delegates to extension method for testability.
• Setting MapStyle = null correctly reverts to default.
• BindableProperty enables XAML binding and runtime style switching.
• Comprehensive tests including null, valid JSON, invalid JSON, and PropertyChanged.

Summary

All Round 1 critical issues were fixed, and all Round 2 concerns were satisfactorily addressed. The author's point about delegating JSON validation to the platform layer (Google's parser) is particularly well-reasoned. This is the most merge-ready PR of the 8.

/azp run maui-pr

📋 Round 3 — PR #33991 (MapStyle) — Final Recommendation

No outstanding issues. This was the most polished PR throughout the review process.

Recommendation: Merge when CI passes. This is the simplest and cleanest of the 8 PRs — good candidate to merge first (least conflict risk).

🗺️ Suggested Merge Order for All 8 PRs

To minimize rebase conflicts (all 8 PRs modify MapsGallery.cs, IMap.cs, and PublicAPI files):

1. Add MapStyle property for custom JSON map styling (Android) #33991 (MapStyle) — Smallest footprint, fewest file overlaps
2. Add MapLongClicked event to Map control #33792 (MapLongClicked) — Small, self-contained
3. Add UserLocationChanged event and LastUserLocation property to Map #33799 (UserLocationChanged) — Re-run iOS CoreCLR device tests first
4. Add MoveToRegion animation control, MapSpan.FromLocations, and fix initial map animation #33982 (MoveToRegion animated) — Introduces MoveToRegionRequest other PRs don't depend on
5. Add Pin.ShowInfoWindow() and Pin.HideInfoWindow() methods #33985 (ShowInfoWindow) — Adds Pin.Parent assignment needed by clustering
6. Add Custom Pin Icons support for Maps #33950 (Custom Pin Icons) — Modifies AddPins async flow
7. Add Pin Clustering support for Maps #33831 (Pin Clustering) — Largest map PR, most interface changes
8. Add LongPressGestureRecognizer for .NET MAUI 11 #33432 (LongPressGesture) — Separate from Maps, can merge independently at any point

🚨 API change(s) detected @davidortinau FYI