[iOS] Map: Update pin icon when Pin.ImageSource changes at runtime - #36293
Conversation
Add "Toggle Icon" (swaps ImageSource between two bundled images) and "Move & Rename" (updates Location + Label) buttons to CustomPinIconGallery, to exercise runtime IMapPin property changes and compare platform behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
On iOS/Mac Catalyst, MapPinHandler.MapImageSource was a no-op: a pin's custom image was only applied when MKMapView created the annotation view (GetViewForAnnotation). Changing Pin.ImageSource on an already-displayed pin therefore had no visible effect until the view was recreated (e.g. the pin scrolled off-screen and back), unlike Location/Label/Address which have real mappers. MapImageSource now refreshes the live annotation view in place via MauiMKMapView.UpdatePinImage: it looks up the current MKAnnotationView for the pin and re-applies (or clears) the image. Pins with no individual view (off-screen, or collapsed into a cluster) are left untouched — GetViewForAnnotation already applies the current ImageSource when they next appear. Updating in place avoids removing/re-adding the annotation, so selection, open callouts and cluster membership are preserved. ApplyCustomImageAsync now also captures the requested ImageSource and drops the result if the pin's ImageSource changed while the load was in flight, so rapid successive changes cannot land a stale icon. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36293Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36293" |
|
Hey there @@kevin68! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
|
Hey there @kevin68! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
added
the
s/agent-review-in-progress
MauiBot
removed
the
s/agent-review-in-progress
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
added
the
s/agent-review-in-progress
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
MauiBot
added
s/agent-fix-pr-picked
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
MauiBot
removed
the
s/agent-review-in-progress
kubaflo
left a comment
kubaflo
left a comment
There was a problem hiding this comment.
Could you please check the ai's suggestions?
… boundary Review feedback: custom-image pins use a plain MKAnnotationView while default pins use MKMarkerAnnotationView/MKPinAnnotationView, so updating the image in place is only valid while the view type still matches the new ImageSource. Clearing a custom image left a blank MKAnnotationView instead of restoring the default marker, and setting an image on a default pin loaded it into a marker view. UpdatePinImage now refreshes in place only when the current view type matches; when ImageSource crosses the null/non-null boundary it removes and re-adds the annotation so GetViewForAnnotation recreates the view through the standard path. A previously selected pin is re-selected, with an annotation-matched one-shot guard so the programmatic selection does not raise a synthetic PinClicked. The sample gallery Toggle Icon button now cycles custom A -> custom B -> default to exercise both in-place swaps and boundary transitions. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
added
the
s/agent-review-in-progress
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
MauiBot
removed
the
s/agent-review-in-progress
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
Tests Failure Analysis
Test Failure Review: Not ready - click to expandOverall verdict: Not ready. 22 legs/failures are red on this PR but green across all 5 recent
Coverage: 141 checks · 125 passing · 14 failing · 2 pending · 0 inaccessible · 1 unmapped · 23 unexplained build legs · 0 unaccounted failing checks · 3 aborted failing checks · 0 canceled-build checks · 7 device-test unverified · 30 unattributed · 22 regressed-vs-base. Deterministic ceiling: Not ready — 22 leg/failures are a deterministic regression vs base; 2 checks still pending; 3 checks did not finish cleanly. Builds (this PR): maui-pr 1511226, maui-pr-devicetests 1511228, maui-pr-uitests 1511227. Base sampling (net11.0, 5 recent builds per definition): maui-pr 1501450, maui-pr-devicetests 1501451, maui-pr-uitests 1501538. Recommended actionInvestigate the BoxView and MacCatalyst Shell/Navigation fixture timeouts — they regress cleanly vs base and are the strongest PR-caused signal; then have a human read the 23 unexplained build legs and confirm the 7 unverified device-test checks before merging. |
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
added
the
s/agent-review-in-progress
MauiBot
added
s/agent-changes-requested
AI Review Summary
🗂️ Review Sessions — click to expand🚦 Gate — Test Before & After FixGate Result: ✅ PASSEDPlatform: IOS · Base: net11.0 · Merge base:
🔴 Without fix — 📱 MapTests (PinImageSourceRuntimeChangeUpdatesAnnotationView): FAIL ✅ · 203s(no coded error found; showing last 1200 chars) 🟢 With fix — 📱 MapTests (PinImageSourceRuntimeChangeUpdatesAnnotationView): PASS ✅ · 47s(no coded error found; showing last 1200 chars) 📁 Fix files reverted (4 files)
📱 UI Tests — Button,Label,LayoutDetected UI test categories: ✅ Deep UI tests — 354 passed, 0 failed across 3 categories on platform-pool agent (replaces in-process counts above). 🧪 UI Test Execution Results (deep, platform pool)
📋 Pre-Flight — Context & ValidationIssue: #36293 - Runtime Key Findings
Code Review SummaryVerdict: NEEDS_DISCUSSION Key code review findings:
Fix Candidates
🔬 Code Review — Deep AnalysisCode Review — PR #36293Independent AssessmentWhat this changes: iOS/MacCatalyst map pins now refresh visible annotation views when Reconciliation with PR NarrativeAuthor claims: Runtime Prior Review Reconciliation
External Output Contract
Blast Radius Assessment
CI Status
Findings
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Defer selected custom/default boundary rebuild until deselection to avoid programmatic reselection and suppressor state. | 1 file | Runner failed before tests; not demonstrably better because selected pin can remain visually stale until deselection. | |
| 2 | try-fix | Replace default/custom view split with one MAUI-owned image-backed annotation view. | ❌ Failed self-review | 0 files | Rejected before implementation due native default marker fidelity/accessibility/clustering risk. |
| 3 | try-fix | Keep native boundary rebuild but make selection suppressor self-clear on next main-loop tick. | 1 file | Runner failed before tests; localized but still has MapKit callback timing risk. | |
| PR | PR #36293 | Refresh custom images in place; remove/re-add on custom/default boundary; restore selection with annotation-matched suppressor; guard stale async image loads. | ✅ PASSED (Gate) | 5 files | Original PR. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| maui-expert-reviewer | 1 | Yes | Selection-stable deferred boundary rebuild. Avoids suppressor but defers selected visual boundary updates until deselection. |
| maui-expert-reviewer | 2 | Yes | Unified in-place annotation view. Avoids boundary and suppressor but fails native defaults preservation review. |
| maui-expert-reviewer | 3 | Yes | Transaction-scoped selection restore. Bounds suppressor lifetime but depends on MapKit callback timing. |
| maui-expert-reviewer | 4 | No | Meaningfully different localized approaches are exhausted: remaining variants either repeat PR remove/re-add logic, replace native marker semantics, defer visual updates, or use arbitrary timing. |
Exhausted: Yes
Selected Fix: PR #36293 — Gate already passed, and no alternative candidate both passed tests and proved safer. Candidate 3 is the closest localized improvement idea, but it needs selected-pin timing tests before it can be considered better than the PR.
Candidate Details
try-fix-1 — Selection-Stable Deferred Boundary Rebuild
Approach: Defer custom/default annotation-view boundary rebuilds while the pin is selected, then rebuild after DidDeselectAnnotationView. Same-type updates still refresh in place. This avoids the PR's programmatic SelectAnnotation suppression path entirely.
Diff: See CustomAgentLogsTmp/PRState/36293/PRAgent/try-fix/attempt-1/fix.diff.
Test Results:
pwsh -NoProfile -File .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Controls -Platform ios -TestFilter 'Category=Map' -OutputDirectory 'artifacts/log/pr36293-try-fix-1'Both attempts failed before tests ran with:
Run-DeviceTests.ps1: A positional parameter cannot be found that accepts argument 'Assets'.
Failure Analysis: The local runner/configuration failed before build/deploy/test execution, so candidate behavior is unverified. Expert review identified a product tradeoff: selected pins crossing the custom/default boundary retain the old native view type until deselection, so this is not demonstrably better than the PR without additional selected-pin tests.
try-fix-1 diff
diff --git a/src/Core/maps/src/Platform/iOS/MauiMKMapView.cs b/src/Core/maps/src/Platform/iOS/MauiMKMapView.cs
index 756b78fe3b..d8ae5271f0 100644
--- a/src/Core/maps/src/Platform/iOS/MauiMKMapView.cs
+++ b/src/Core/maps/src/Platform/iOS/MauiMKMapView.cs
@@ -20,6 +20,7 @@ namespace Microsoft.Maui.Maps.Platform
UITapGestureRecognizer? _mapClickedGestureRecognizer;
bool _isClusteringEnabled;
IMKAnnotation? _suppressClickForAnnotation;
+ IMKAnnotation? _pendingImageRebuildForAnnotation;
UILongPressGestureRecognizer? _mapLongClickedGestureRecognizer;
List<IMapElement>? _trackedMapElements;
@@ -277,17 +278,14 @@ namespace Microsoft.Maui.Maps.Platform
// recreate the view through the standard path. Restore selection without raising a
// synthetic PinClicked.
bool wasSelected = SelectedAnnotations?.Any(a => ReferenceEquals(a, annotation) || a.Handle == annotation.Handle) == true;
- RemoveAnnotation(annotation);
- AddAnnotation(annotation);
-
if (wasSelected)
{
- // DidSelectAnnotationView may fire after the view is (re)created rather than inside
- // SelectAnnotation, so suppression is annotation-matched rather than a flag scoped to
- // this call; it's consumed by the matching event, or dropped in Cleanup.
- _suppressClickForAnnotation = annotation;
- SelectAnnotation(annotation, false);
+ _pendingImageRebuildForAnnotation = annotation;
+ return;
}
+
+ RemoveAnnotation(annotation);
+ AddAnnotation(annotation);
}
async System.Threading.Tasks.Task ApplyCustomImageAsync(MKAnnotationView annotationView, IMapPin pin)
@@ -443,6 +441,7 @@ namespace Microsoft.Maui.Maps.Platform
{
RegionChanged += MkMapViewOnRegionChanged;
DidSelectAnnotationView += MkMapViewOnAnnotationViewSelected;
+ DidDeselectAnnotationView += MkMapViewOnAnnotationViewDeselected;
DidUpdateUserLocation += MkMapViewOnUserLocationUpdated;
AddGestureRecognizer(_mapClickedGestureRecognizer = new UITapGestureRecognizer(OnMapClicked)
@@ -472,11 +471,27 @@ namespace Microsoft.Maui.Maps.Platform
}
RegionChanged -= MkMapViewOnRegionChanged;
DidSelectAnnotationView -= MkMapViewOnAnnotationViewSelected;
+ DidDeselectAnnotationView -= MkMapViewOnAnnotationViewDeselected;
DidUpdateUserLocation -= MkMapViewOnUserLocationUpdated;
// Annotations survive detach/reattach, so drop any pending click suppression to prevent it
// from swallowing the next real tap after navigation or a Shell tab switch.
_suppressClickForAnnotation = null;
+ _pendingImageRebuildForAnnotation = null;
+ }
+
+ void MkMapViewOnAnnotationViewDeselected(object? sender, MKAnnotationViewEventArgs e)
+ {
+ var annotation = e.View.Annotation;
+ if (annotation is null || _pendingImageRebuildForAnnotation is null ||
+ !(ReferenceEquals(annotation, _pendingImageRebuildForAnnotation) || annotation.Handle == _pendingImageRebuildForAnnotation.Handle))
+ {
+ return;
+ }
+
+ _pendingImageRebuildForAnnotation = null;
+ RemoveAnnotation(annotation);
+ AddAnnotation(annotation);
}
void MkMapViewOnAnnotationViewSelected(object? sender, MKAnnotationViewEventArgs e)try-fix-2 — Unified In-Place iOS Pin Annotation View
Approach: Use a single MAUI-owned MKAnnotationView representation for both default and custom-image pins so all ImageSource changes can update the same view in place, with no annotation remove/re-add and no selection suppressor.
Diff: Empty. The candidate was rejected before implementation.
Test Results: ❌ Failed at expert self-review; tests were not run.
Failure Analysis: The approach is materially different but not safer: replacing MKMarkerAnnotationView/MKPinAnnotationView with an image-backed custom view risks native-default fidelity, accessibility, clustering/collision behavior, and future MapKit styling. It would need a broader design and test matrix, so it is not demonstrably better than PR #36293.
try-fix-2 diff
try-fix-3 — Transaction-Scoped Selection Restore
Approach: Keep the PR's remove/re-add boundary transition and native default marker behavior, but make _suppressClickForAnnotation self-clear on the next main-loop tick if MapKit does not deliver the matching programmatic DidSelectAnnotationView.
Diff: See CustomAgentLogsTmp/PRState/36293/PRAgent/try-fix/attempt-3/fix.diff.
Test Results:
pwsh -NoProfile -File .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Controls -Platform ios -TestFilter 'Category=Map' -OutputDirectory 'artifacts/log/pr36293-try-fix-3'It failed before tests ran with:
Run-DeviceTests.ps1: A positional parameter cannot be found that accepts argument 'Assets'.
Failure Analysis: The local runner/configuration prevented empirical evaluation. The candidate is localized and avoids an indefinitely armed suppressor, but self-review found a remaining timing risk: if MapKit delivers the programmatic selection callback after the next main-loop tick, the synthetic callback may no longer be suppressed. This is not demonstrably better than the PR without targeted selected-pin timing tests.
try-fix-3 diff
diff --git a/src/Core/maps/src/Platform/iOS/MauiMKMapView.cs b/src/Core/maps/src/Platform/iOS/MauiMKMapView.cs
index 756b78fe3b..6043741012 100644
--- a/src/Core/maps/src/Platform/iOS/MauiMKMapView.cs
+++ b/src/Core/maps/src/Platform/iOS/MauiMKMapView.cs
@@ -2,6 +2,7 @@
using System.Collections;
using System.Collections.Generic;
using System.Linq;
+using CoreFoundation;
using CoreLocation;
using MapKit;
using Microsoft.Extensions.DependencyInjection;
@@ -287,6 +288,14 @@ namespace Microsoft.Maui.Maps.Platform
// this call; it's consumed by the matching event, or dropped in Cleanup.
_suppressClickForAnnotation = annotation;
SelectAnnotation(annotation, false);
+ DispatchQueue.MainQueue.DispatchAsync(() =>
+ {
+ if (_suppressClickForAnnotation is not null &&
+ (ReferenceEquals(annotation, _suppressClickForAnnotation) || annotation.Handle == _suppressClickForAnnotation.Handle))
+ {
+ _suppressClickForAnnotation = null;
+ }
+ });
}
}
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the title is accurate, but the description's Testing section is stale because the PR now adds an automated iOS device regression test.
Recommended title
[iOS] Map: Update pin icon when Pin.ImageSource changes at runtime
Recommended description
### Description of Change
On iOS/Mac Catalyst, changing a `Pin.ImageSource` at runtime did not update the icon of a pin already displayed on the map — the new image only appeared once MapKit recreated the annotation view (e.g. the pin scrolled off-screen and back). `MapPinHandler.MapImageSource` was a no-op; the image is applied in `MauiMKMapView.GetViewForAnnotation`, which only runs on view (re)creation. `Location`/`Label`/`Address` update live, and Android already updates the marker icon in place, so iOS was inconsistent.
**Fix:** `MapImageSource` now refreshes the live annotation view via `MauiMKMapView.UpdatePinImage`:
- Looks up the current `MKAnnotationView` for the pin (`ViewForAnnotation`) and re-applies the image in place when the current native view type still matches the requested state. Pins with no individual view — off-screen, or collapsed into a cluster — are left untouched; `GetViewForAnnotation` already applies the current `ImageSource` when they next appear.
- When `ImageSource` crosses the custom/default boundary, removes and re-adds the annotation so MapKit recreates the correct native view type (`MKAnnotationView` for custom images, `MKMarkerAnnotationView`/`MKPinAnnotationView` for default markers).
- Restores selection for selected pins after a custom/default boundary rebuild while suppressing the synthetic `PinClicked` event.
- `ApplyCustomImageAsync` now captures the requested `ImageSource` and drops the result if the annotation view was reused/released or the pin's `ImageSource` changed while the load was in flight, so rapid successive changes can't land a stale icon.
A **"Toggle Icon"** and **"Move & Rename"** button pair is added to the `CustomPinIconGallery` sample to exercise runtime `IMapPin` property changes.
#### Testing
Added `MapTests.PinImageSourceRuntimeChangeUpdatesAnnotationView` for iOS to cover:
- Default marker -> custom image (`null` -> `ImageSource`)
- Custom image -> custom image
- Custom image -> default marker (`ImageSource` -> `null`)
Also verified manually on a physical device (iPad A16, iOS 26.5):
- Non-clustered: changing `ImageSource` at runtime updates the icon in place, with no flicker and without deselecting a pin whose callout is open.
- Clustered: changing `ImageSource` while pins are collapsed into a cluster is deferred; zooming in shows the pins with the updated icon.
- `Location`/`Label` continue to update live (control case).
### Issues Fixed
Fixes #36292
🏁 Report — Final Recommendation
Comparative Report — PR #36293
Candidates compared
| Rank | Candidate | Test result | Assessment |
|---|---|---|---|
| 1 | pr |
✅ Passed gate | Best candidate. It fixes the runtime iOS/MacCatalyst Pin.ImageSource refresh bug, preserves native default marker behavior, handles custom/default view-type boundaries, and has regression evidence. |
| 1 | pr-plus-reviewer |
✅ Same as PR | The expert reviewer produced no actionable inline findings, so this candidate is identical to pr. It is not ranked above pr because it contains no additional change. |
| 3 | try-fix-3 |
Localized alternative that self-clears the selection suppressor on the next main-loop tick, but tests did not run and the approach introduces timing risk if MapKit delivers the synthetic selection after the tick. | |
| 4 | try-fix-1 |
Avoids the suppressor by deferring selected custom/default rebuilds until deselection, but this can leave a selected pin visually stale until the user deselects it. Tests did not run. | |
| 5 | try-fix-2 |
❌ Failed self-review | Rejected before implementation. Replacing native default marker views with a unified image-backed custom view risks marker fidelity, accessibility, clustering/collision behavior, and future MapKit styling. |
Analysis
The submitted PR is the only candidate with successful regression evidence. It addresses the core bug while preserving MapKit's native default annotation views and existing clustering/offscreen behavior. Its async image loading guards also cover rapid successive image changes and annotation view reuse.
pr-plus-reviewer is equivalent to pr because the expert reviewer wrote no actionable inline findings (inline-findings.json is []). There is therefore no reviewer-derived sandbox improvement to prefer over the raw PR.
The try-fix candidates explored useful alternatives but do not overtake the PR. try-fix-1 removes the suppressor path by deferring selected rebuilds, but trades that for stale selected-pin visuals. try-fix-3 bounds the suppressor lifetime, but relies on an arbitrary main-loop timing assumption. try-fix-2 is broader and riskier than the bug requires.
Winner
Winner: pr
The raw PR fix wins because it is the only non-failed, regression-passing implementation and the expert reviewer found no actionable changes to apply. The residual selected-pin suppressor concern is a discussion risk, but every alternative either failed self-review, was blocked without test evidence, or introduced a different product/timing tradeoff.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
MauiBot
removed
the
s/agent-review-in-progress
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81d69925-4844-4957-ba6e-9164b2736888
Resolve the MauiMKMapView conflict by preserving runtime pin image staleness guards alongside the newer cluster icon lifecycle and selection handling. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81d69925-4844-4957-ba6e-9164b2736888
Description of Change
On iOS/Mac Catalyst, changing a
Pin.ImageSourceat runtime did not update the icon of a pin already displayed on the map — the new image only appeared once MapKit recreated the annotation view (e.g. the pin scrolled off-screen and back).MapPinHandler.MapImageSourcewas a no-op; the image is applied inMauiMKMapView.GetViewForAnnotation, which only runs on view (re)creation.Location/Label/Addressupdate live, and Android already updates the marker icon in place, so iOS was inconsistent.Fix:
MapImageSourcenow refreshes the live annotation view in place viaMauiMKMapView.UpdatePinImage:MKAnnotationViewfor the pin (ViewForAnnotation) and re-applies (or clears) its image. Pins with no individual view — off-screen, or collapsed into a cluster — are left untouched;GetViewForAnnotationalready applies the currentImageSourcewhen they next appear.ApplyCustomImageAsyncnow captures the requestedImageSourceand drops the result if the pin'sImageSourcechanged while the load was in flight, so rapid successive changes can't land a stale icon.A "Toggle Icon" and "Move & Rename" button pair is added to the
CustomPinIconGallerysample to exercise runtimeIMapPinproperty changes.Testing
The iOS
Maphandler is not covered by automated tests (device tests forMapare gated to iOS/MacCatalyst but require a Google/Apple map environment). Verified manually on a physical device (iPad A16, iOS 26.5):ImageSourceat runtime updates the icon in place, with no flicker and without deselecting a pin whose callout is open.ImageSourcewhile pins are collapsed into a cluster is deferred; zooming in shows the pins with the updated icon.Location/Labelcontinue to update live (control case).Issues Fixed
Fixes #36292