Skip to content

[NET 11] SwipeItem: Add explicit IconColor and TextColor - #36884

Open
kubaflo wants to merge 30 commits into
net11.0from
swipeitem-icon-tint-api
Open

[NET 11] SwipeItem: Add explicit IconColor and TextColor#36884
kubaflo wants to merge 30 commits into
net11.0from
swipeitem-icon-tint-api

Conversation

@kubaflo

@kubaflo kubaflo commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description of Change

This is the .NET 11 half of a two-part change. #36883 reverts #35632 on the servicing line; this PR keeps the corrected .NET 11 behavior and adds explicit color APIs so applications no longer have to rely on an implicit tinting guess.

Background

Before #35632, a SwipeItem icon was always recolored from the item's background: white on a dark background and black on a light one. That prevented SVG and PNG icons from rendering with their authored colors, which #23074 asked to fix. #35632 corrected the default by only applying implicit tinting to font icons.

That default is appropriate for .NET 11, but #36766 exposed the missing capability: when an authored image color no longer contrasts after an AppThemeBinding changes the background, there was no way to set an explicit icon tint. The framework had to guess because SwipeItem exposed neither icon nor label colors.

Breaking behavior and migration

This is an intentional .NET 11 behavior change from .NET 10 and Xamarin.Forms. Existing PNG, SVG, and other non-font SwipeItem icons are no longer automatically recolored white or black to contrast with BackgroundColor when IconColor is unset; they render with their authored colors. Apps that relied on the implicit contrast tint must set IconColor explicitly, typically with AppThemeBinding, while apps that want authored image colors need no change. Font icons retain their color/fallback contrast behavior. The servicing line keeps the legacy implicit-tint default.

The t/breaking 💥 and needs-breaking-change-doc-created labels track this for the .NET 11 breaking-change documentation and release notes.

New APIs

SwipeItem now exposes two bindable properties:

  • IconColor explicitly tints the icon.
  • TextColor explicitly colors the label.

When IconColor is unset:

Source Behavior
Font icon Uses FontImageSource.Color, then TextColor, then a color contrasting the background
PNG/SVG/other image Renders with its original colors

When IconColor is set, Android, iOS, and MacCatalyst tint the resolved platform image. Windows supports font and packaged-file icons; packaged files use monochrome-mask semantics. URI, rooted, and stream-backed images on Windows, and icons on Tizen, retain their original colors.

When TextColor is unset, the label uses a color contrasting the background. It keeps the platform default when there is no background or when a font icon already supplies its own color.

Both properties support AppThemeBinding:

<SwipeItem Text="Delete"
           IconImageSource="delete.svg"
           BackgroundColor="{AppThemeBinding Light=White, Dark=Black}"
           IconColor="{AppThemeBinding Light=Black, Dark=White}"
           TextColor="{AppThemeBinding Light=Black, Dark=White}" />

Implementation

  • API compatibility - IconColor is surfaced to handlers through the optional, IntelliSense-hidden ISwipeItemMenuItemIconColor companion interface. Existing ISwipeItemMenuItem implementers do not acquire a new abstract member, including on netstandard2.0. TextColor uses the inherited ITextStyle.TextColor contract rather than introducing a second interface slot.
  • Mapper lifecycle - Initial and reconnect mapping load each icon source once. Runtime IconColor, TextColor, and BackgroundColor changes flow through their normal mapper keys so custom AppendToMapping/PrependToMapping logic is preserved. Android and iOS reapply the attached native image for color-only changes instead of decoding or downloading the source again.
  • Android - Reuses untinted drawables without cloning shared state, tracks the exact drawable it tinted because Drawable.ColorFilter is not authoritative, and mutates before applying or clearing a SrcAtop filter. Bounds are assigned to the actual drawable instance that will be attached, including custom drawables whose Mutate() returns a distinct instance. Clearing text color restores the cached themed default, and a missing attached image falls back through the normal Source mapper.
  • iOS/MacCatalyst - Font images always use template rendering, including colorless glyphs with no background. Clearing text color restores the native state-specific title color.
  • Windows - Packaged files with an explicit IconColor use an explicitly monochrome BitmapIconSource; ms-appx resource qualification selects the display-scale asset. URI, rooted, stream-backed, and custom-service images remain on their registered image-service pipeline and retain their original colors. Interface-level font image services receive a wrapper only when the effective IconColor or fallback TextColor differs from the font source’s own color; equal colors pass the original source through without another allocation. Concrete custom font-source registrations receive the original source and retain control of rendering. Per-handler generation and resolved-tint state prevent stale async results and duplicate font renders; custom handlers use a ConditionalWeakTable fallback whose read path does not allocate state. Clearing text color removes the local Foreground value.
  • Tizen - Reads the unified ITextStyle.TextColor value through the shared resolver and restores the platform default when cleared.

Issues Fixed

Contributes to #23074
Related to #36573, #36766, #36883

Testing

  • Controls.Core.UnitTests, focused SwipeViewTests: 52 passed, 0 failed.
  • iOS Core device tests, Category=SwipeView: 59 passed, 0 failed, 1 ignored.
  • Android Core device tests, Category=SwipeView: 67 passed, 0 failed, 2 ignored.
  • Added direct handler regressions for:
    • iOS text-color reset, colorless-font template rendering, runtime tint, and color-only updates without source reloads.
    • Android tint clearing without shared-drawable bleed, including drawables that do not report applied filters, mutate-before-bounds ordering, and color-only updates without source reloads.
    • Windows text-color reset, packaged/rooted file handling, URI/custom service preservation, service-rendered tinted fonts, equal-color source pass-through, and concrete custom font-source service preservation.
    • Initial mapper load suppression and stale same-source generation races.
    • Missing-native-image fallback on Android and iOS, conditional Android drawable mutation, and duplicate Windows font-render suppression.
  • Built the MAUI build tasks and the iOS/Android Core device-test targets locally. Windows-specific tests are included for CI; native Windows SDK tools cannot execute on the macOS host.

Notes for Reviewers

  • This adds public API and requires API review.
  • IconColor is a Color so it composes with AppThemeBinding and matches the existing SwipeItem.BackgroundColor model.
  • The default icon policy remains opt-in for image tinting: authored image colors are preserved unless IconColor is explicitly set.

.NET 10 SR9 shipped #35632, which stopped tinting SwipeItem icons with a
color derived from the item background and instead let image icons render
with their own colors. That fixed #23074, but it also silently changed how
existing apps look: an icon that used to be recolored to contrast the
background now keeps its authored color, which is how #36766 ended up with a
dark icon on a dark background in dark mode.

The behavior itself is right — it matches the authored asset and it is what
users asked for — but it is not something that should arrive unannounced in a
servicing release, so #36883 reverts it on the servicing line. This change
keeps the new behavior for .NET 11 and closes the gap that made the old
implicit tint necessary in the first place, by making the tint configurable
instead of inferred.

SwipeItem.IconColor is a new bindable property:

- unset (default): a FontImageSource uses its own Color, falling back to a
  color contrasting the item background, and image icons such as PNG and SVG
  render with their original colors. This is exactly the .NET 11 behavior
  today, so nothing changes for apps that do not opt in.
- set: the color tints every icon type, and because it is a bindable property
  it can be driven by an AppThemeBinding to follow the current theme — the
  scenario #36766 was really after.

All three platforms now resolve the tint through a single shared helper,
ISwipeItemMenuItem.GetIconTintColor(), so the rule is defined once instead of
being reimplemented per handler:

- Android applies it as a SrcAtop color filter, and now clears the filter when
  there is no tint. Drawables can be cached and reused, so an earlier tint
  would otherwise be left stale.
- iOS renders the image as a template only when there is a tint, and keeps
  AlwaysOriginal otherwise.
- Windows routes tinted icons through ToIconSource(), which produces a
  BitmapIconSource/FontIconSource that honors Foreground. ImageIconSource
  ignores Foreground, so it is still used for the untinted path.

IconColor changes are mapped by reloading the source, which is what
re-evaluates the tint and guarantees a previous tint is cleared rather than
left behind.

ISwipeItemMenuItem.IconColor is a default interface member so existing
implementers keep compiling, guarded with #if NETSTANDARD2_0 like
IRefreshView.IsRefreshEnabled, since netstandard2.0 cannot express one.
Copilot AI lite review requested due to automatic review settings July 28, 2026 15:40
@kubaflo
kubaflo temporarily deployed to copilot-pat-pool July 28, 2026 15:40 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown
Contributor

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

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36884"

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a new IconColor API to make SwipeItem icon tinting explicit and consistent across platforms, while preserving the current default behavior (only font icons get an implicit contrast tint; image icons keep authored colors unless IconColor is set).

Changes:

  • Introduces ISwipeItemMenuItem.IconColor and SwipeItem.IconColor (bindable) plus PublicAPI updates.
  • Centralizes tint resolution in ISwipeItemMenuItem.GetIconTintColor() and applies it in Android/iOS/Windows handlers (including Android stale-tint clearing).
  • Adds unit tests covering the tint-resolution matrix.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Core/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt Public API entries for ISwipeItemMenuItem.IconColor + handler mapper.
src/Core/src/PublicAPI/netstandard/PublicAPI.Unshipped.txt Public API entries for ISwipeItemMenuItem.IconColor + handler mapper.
src/Core/src/PublicAPI/net/PublicAPI.Unshipped.txt Public API entries for ISwipeItemMenuItem.IconColor + handler mapper.
src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt Public API entries for ISwipeItemMenuItem.IconColor + handler mapper.
src/Core/src/PublicAPI/net-tizen/PublicAPI.Unshipped.txt Public API entries for ISwipeItemMenuItem.IconColor + handler mapper.
src/Core/src/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt Public API entries for ISwipeItemMenuItem.IconColor + handler mapper.
src/Core/src/PublicAPI/net-ios/PublicAPI.Unshipped.txt Public API entries for ISwipeItemMenuItem.IconColor + handler mapper.
src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt Public API entries for ISwipeItemMenuItem.IconColor + handler mapper.
src/Core/src/Platform/SwipeViewExtensions.cs Adds shared GetIconTintColor() helper for consistent tint rules.
src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Windows.cs Applies tint via ToIconSource() when tint is requested; otherwise preserves existing image loading.
src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.iOS.cs Applies template/original rendering based on resolved tint color.
src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.cs Adds IconColor mapper hook (reloads icon on change).
src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Android.cs Applies or clears drawable color filters based on resolved tint.
src/Core/src/Core/ISwipeItemMenuItem.cs Adds IconColor to the public interface (DIM except on netstandard2.0).
src/Controls/tests/Core.UnitTests/SwipeViewTests.cs Unit tests for icon tint resolution and precedence rules.
src/Controls/src/Core/SwipeView/SwipeItem.cs Adds SwipeItem.IconColor bindable property and maps it to the handler.
src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt Public API entries for new SwipeItem.IconColor and IconColorProperty.
src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt Public API entries for new SwipeItem.IconColor and IconColorProperty.
src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt Public API entries for new SwipeItem.IconColor and IconColorProperty.
src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt Public API entries for new SwipeItem.IconColor and IconColorProperty.
src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt Public API entries for new SwipeItem.IconColor and IconColorProperty.
src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt Public API entries for new SwipeItem.IconColor and IconColorProperty.
src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt Public API entries for new SwipeItem.IconColor and IconColorProperty.

Comment thread src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.cs Outdated
@kubaflo

This comment has been minimized.

@github-actions github-actions Bot added the s/agent-review-in-progress AI review is currently running for this PR label Jul 28, 2026
kubaflo added a commit that referenced this pull request Jul 28, 2026
…ross platforms (#35632)" (#36887)

<!-- Please let the below note in for people that find this PR -->
> [!NOTE]
> Are you waiting for the changes in this PR to be merged?
> It would be very helpful if you could [test the resulting
artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from
this PR and let us know in a comment if this change resolves your issue.
Thank you!

> Supersedes #36883, which was branched off `main` while targeting
`inflight/candidate`. That mismatch dragged in 42 unrelated `main`-only
commits and produced four spurious conflicts in files that have nothing
to do with `SwipeItem`. This branch is built directly on
`inflight/candidate`, so it is a clean single-commit diff.

### Issue Details

This reverts #35632 ("Fix SwipeItem IconImageSource color handling and
rendering across platforms") from the .NET 10 servicing line.

#35632 stopped auto-tinting `SwipeItem.IconImageSource` for PNG/SVG
sources, so those icons now render in their own colors instead of a
contrast color derived from the item background. It shipped in 10.0.90
(SR9) and is causing #36766 — an SVG icon whose fill is `#212121` now
renders black on a black swipe item in dark mode, i.e. effectively
invisible.

The change is a reasonable direction, but it is too breaking for a
servicing release:

- **It removes a legibility guarantee with no replacement.**
`GetTextColor()` picks white or black from background luminosity. #35632
removed that for PNG/SVG but left it in place for the item's label text,
so the text is still contrast-corrected while the icon is not.
- **There is no opt-in.** #23074 was labelled `proposal/open` and asked
for *more configuration*; what shipped was a behavior change with none.
An affected app's only options today are pinning to 10.0.80 or
re-authoring every SVG per theme.
- **It compounds with #36271** (also SR9), which makes `BackgroundColor`
correctly follow `AppThemeBinding`. Before SR9 the stale light
background accidentally preserved contrast; together the two changes
produce black-on-black.
- **The prior behavior was the Xamarin.Forms behavior**, not an
inconsistency — Xamarin tinted every icon unconditionally on both
Android
([`SwipeViewRenderer.cs#L858`](https://github.com/xamarin/Xamarin.Forms/blob/main/Xamarin.Forms.Platform.Android/Renderers/SwipeViewRenderer.cs#L858))
and iOS
([`SwipeViewRenderer.cs#L713`](https://github.com/xamarin/Xamarin.Forms/blob/main/Xamarin.Forms.Platform.iOS/Renderers/SwipeViewRenderer.cs#L713)).

### Description of Change

`git revert` of c78acfe, restoring the previous behavior on all
platforms:

- **Android** — `SetColorFilter(GetTextColor(), SrcAtop)` applied to
every drawable again
- **iOS/Mac** — `AlwaysTemplate` rendering mode on every image again,
with `TintColor = fontImageSource.Color ?? GetTextColor()`
- **Windows** — `MapSourceAsync` back to `ToIconSource()`
(`BitmapIconSource`, whose `ShowAsMonochrome` defaults to `true`); the
`LoadFileIconAsync` helper is removed

The `Issue23074` host-app page, shared test, `cancel_red.svg` and the
`SwipeItemFontAndSvgIconsRenderCorrectly` snapshots are removed with it,
and the SwipeView snapshots are restored to their pre-#35632 baselines.

The revert is scoped strictly to #35632. `MapVisibility` on the Windows
handler — added on `inflight/candidate` after #35632 — is untouched.

**Conflict resolution note:**
`TestCases.iOS.Tests/snapshots/ios/VerifyCollectionViewContentWithIconImageSwipeItem.png`
conflicted because #36202 re-saved it for iOS 18 after #35632 landed.
That test's swipe item has `BackgroundColor = #6A5ACD` (luminosity ≈
0.40 → white), so with this revert `groceries.png` is tinted solid white
again and the pre-#35632 baseline is the correct content. If iOS 18 CI
shows drift unrelated to the tint, this one snapshot may need a re-save
from the CI artifact.

### Follow-up

The behavior change itself is good and should ship — just in .NET 11
rather than servicing, paired with an explicit opt-in API so users get a
real migration path instead of a silent rendering change in a patch
release. That is #36884, which keeps the #35632 behavior and adds
`SwipeItem.IconColor`.

### Issues Fixed

Fixes #36766

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: cb2a11ab-30ba-4020-836a-3acccb5f58cc
@MauiBot MauiBot added s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) labels Jul 28, 2026
@MauiBot

This comment has been minimized.

@MauiBot MauiBot removed the s/agent-review-in-progress AI review is currently running for this PR label Jul 28, 2026
Treat renderer output within one physical pixel of the target bounds as already resized, and cover the color-only update with an oversized renderer-backed image identity assertion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Windows.cs:83

  • This method is named LoadFileIconAsync, but it now loads icons for all IImageSource types (font, file, URI, etc.) and also contains tint-specific branching. Renaming it (e.g., LoadIconAsync/LoadSwipeItemIconAsync) would make the Windows source-mapping flow much clearer when debugging generation/tint behavior.
			int generation = BeginIconLoad(handler);

			var source = item.Source;
			var fontSource = source as IFontImageSource;
			var resolvedFontColor = fontSource is null ? null : item.GetIconTintColor();

@kubaflo

kubaflo commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Reviewed the suppressed naming suggestion in Copilot review 4898865126. LoadFileIconAsync and its generic IImageSourceService path already existed on net11.0; this PR did not broaden that helper from file-only to all source types. Renaming the internal single-caller helper would therefore be unrelated, non-functional churn, so I am leaving it unchanged.

@PureWeen PureWeen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Round 7 adversarial review found one new iOS image-sizing warning in the final retint fix. The prior redraw and regression-test findings are otherwise resolved, and no additional API, handler-lifecycle, or cross-platform defects survived consensus.\n\nTest coverage: The new oversized-image handle test discriminates the renderer-redraw regression, but it does not cover a scale-1 source just above the size limit.\n\nPrior review status: Earlier findings remain addressed; this warning is introduced by the round-7 tolerance.\n\nVerdict: One warning remains in the iOS resize boundary.\n\n_Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist._

Comment thread src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.iOS.cs Outdated
@kubaflo

This comment has been minimized.

@MauiBot

This comment has been minimized.

Copilot CI added 2 commits August 12, 2026 11:43
Resolve handler and PublicAPI conflicts while preserving IconColor and TextColor behavior. Use the actual renderer scale for iOS resize tolerance and cover scale-1 oversized images.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Windows.cs:113

  • LoadFileIconAsync now loads/tints icons for multiple IImageSource types (font, URI, custom services), not just files. The name is misleading and makes the Windows handler harder to reason about when tracing icon update flows.
		internal static async Task LoadFileIconAsync(ISwipeItemMenuItemHandler handler, ISwipeItemMenuItem item)

@kubaflo

This comment has been minimized.

@PureWeen PureWeen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Findings

  • ⚠️ Windows image loads do not retain or dispose IImageSourceServiceResult, leaking custom-service resource leases on replacement, stale completion, and disconnect. 3/3 reviewers after dispute.
  • 💡 The TextColor dependency calls the icon-color implementation directly, bypassing user AppendToMapping/PrependToMapping customizations. 3/3 reviewers after dispute.
  • 💡 The new iOS boundary test assumes a display scale greater than 1 and can fail on a valid 1× Mac Catalyst display. 2/3 reviewers.

Resolved prior feedback

The prior scale-1 source-image tolerance defect is resolved: production code now derives tolerance from the display scale, and the new test exercises a scale-1 source just over the limit.

Test coverage

Coverage is broad across the public API and platform handlers, but it does not yet exercise custom image-service disposal or mapper customization on dependency-driven updates. The new iOS boundary test should also derive its input from the runtime tolerance.

Existing review status: the previous iOS tolerance warning is addressed; the findings above are newly validated at head 588e554.

Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist.

Comment thread src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Windows.cs Outdated
Comment thread src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.cs Outdated

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Expert Review — 4 findings

See inline comments for details.

if (platformView is SwipeItemButton swipeItemButton)
_proxy.Disconnect(swipeItemButton);

_defaultTitleColor = null;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[moderate] Handler Mapper and Property Patterns / Native Platform Defaults Preservation_defaultTitleColor is nulled in DisconnectHandler, but it is only captured in ConnectHandler (line 43), which ElementHandler runs once, when the platform view is first created. If this handler instance is later reconnected to a new virtual view while keeping the same UIButton (handler reuse / view removed and re-added), ConnectHandler does not run again, so the cached default stays null and MapTextColor then executes SetTitleColor(null, UIControlState.Normal) instead of restoring the captured native default. Concrete scenario: a SwipeItem with BackgroundColor set (resolved contrast title color applied) is disconnected and the handler is reused for an item with neither TextColor nor BackgroundColor — the title color is reset to UIKit's implicit value rather than the button's original default. Either leave _defaultTitleColor populated across disconnect (the reviewer guidance is not to null handler state eagerly in DisconnectHandler, since the view can be removed and re-added), or re-capture it lazily in MapTextColor before the first SetTitleColor call.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I do not think the described same-button reconnect state is reachable through the supported handler lifecycle. IElementHandler.DisconnectHandler() sets PlatformView to null before invoking the platform disconnect hook. A subsequent SetVirtualView therefore creates a new UIButton, treats it as initial setup, and invokes ConnectHandler, which captures that new button default again. Keeping the old cached color across disconnect could instead carry state from a discarded native view, so I am leaving the current reset in place.

Comment thread src/Core/src/Core/ISwipeItemMenuItem.cs
return null;
}

return new BitmapIconSource

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔍 AI-Generated Review (multi-model)

[moderate] Cross-Platform Behavioral Consistency / Null Safety — When IconColor is set, this path abandons the registered IImageSourceService and hands a hand-built ms-appx:/// URI to BitmapIconSource. The URI construction correctly mirrors FileImageSourceService.GetAppPackage (Path.GetFileName + flattened package root), and rooted paths are excluded by CanCreateTintedIconSource, so resolution is consistent. What diverges is the failure path: BitmapIconSource resolves its UriSource asynchronously inside WinUI and swallows failures, so a packaged file that cannot be resolved now renders as an empty icon with no diagnostic, whereas the untinted path throws InvalidOperationException("Unable to load image file.") and logs "Cannot load SwipeItem Icon" via the handler's catch block. Concrete scenario: a SwipeItem whose IconImageSource filename is wrong renders (and logs) differently depending only on whether IconColor is set, which makes the Windows-only tint path hard to diagnose. Consider subscribing to BitmapIconSource.ImageFailed/ImageOpened (or probing via the service first) so the same warning is logged in both paths.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I do not see the claimed diagnostic divergence in the existing service path. FileImageSourceService.GetAppPackage also constructs and returns a BitmapImage from the same ms-appx:/// URI without synchronously awaiting or validating the packaged resource; its InvalidOperationException branch is not reached for a missing packaged filename because GetAppPackage is non-null. The later WinUI URI/decode failure is asynchronous in both paths, so probing through that service would not add the stated warning parity. I am leaving the direct BitmapIconSource path unchanged.

Comment thread src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.cs Outdated
MauiBot

This comment was marked as outdated.

Copilot CI added 2 commits August 12, 2026 23:33
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
@kubaflo

kubaflo commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

@PureWeen @MauiBot @copilot latest feedback is addressed in f0aabd2ba2, and the branch now includes a normal merge of the current net11.0 tip (15974ee356) to resolve the conflicts. This includes Windows image-result ownership/cancellation/disposal, mapper-chain routing, the scale-independent iOS regression fixture, API/loader rationale, and the broader LoadIconAsync name. Validation: 52 SwipeView unit tests passed; iOS 60 passed/1 ignored; Android 67 passed/2 ignored. Two MauiBot findings were answered with lifecycle/service evidence and intentionally left open for reviewer follow-up. Ready for re-review — thanks!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated 4 comments.

Comment thread src/Core/src/PublicAPI/net-ios/PublicAPI.Unshipped.txt Outdated
Comment thread src/Core/src/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt Outdated
Comment thread src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt Outdated
Comment thread src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
@kubaflo

kubaflo commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@copilot the four Apple PublicAPI findings are fixed in de335c6c7c, which also merges the latest net11.0 tip normally. Both net11.0-ios26.5 and net11.0-maccatalyst26.5 Controls/Core builds pass PublicAPI validation. Ready for re-review — thanks!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 30 out of 30 changed files in this pull request and generated no new comments.

@kubaflo

This comment has been minimized.

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Expert Review — 1 findings

See inline comments for details.

Comment thread src/Core/src/Platform/SwipeViewExtensions.cs

@MauiBot MauiBot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI Review Summary

@kubaflo — new AI review results are available based on commit de335c6.

Gate Passed Confidence Unknown Platform Android


🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix

Gate Result: ✅ PASSED

Platform: ANDROID · Base: net11.0 · Merge base: 342bf0b1

Verified (new API / feature) — this PR adds new API and a test that references it in the same project, so reverting the fix un-compiles the test: there is no valid "fails without the fix" baseline to establish (a compile-coupled baseline). The gate instead verified the fix by a clean build + pass with the fix, so this is a real PASS rather than a non-committal INCONCLUSIVE.

Test Without Fix (expect FAIL) With Fix (expect PASS)
🧪 SwipeViewTests SwipeViewTests 🛠️ BUILD ERROR ✅ PASS — 82s
📱 SwipeItemMenuItemHandlerTests (IconTintCanBeClearedWithoutMutatingSharedDrawable, IconTintCanBeClearedWhenDrawableDoesNotReportColorFilter, IconColorLoadsSourceWhenPlatformImageIsMissing, IconColorChangeMutatesAttachedDrawableWithoutReloading) Category=SwipeView 🛠️ BUILD ERROR ✅ PASS — 755s
🔴 Without fix — 🧪 SwipeViewTests: 🛠️ BUILD ERROR · 80s

Error-relevant lines (filtered from the build log):

/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(912,47): error CS0103: The name 'ISwipeItemMenuItemIconColor' does not exist in the current context [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(928,14): error CS1061: 'SwipeItem' does not contain a definition for 'TextColor' and no accessible extension method 'TextColor' accepting a first argument of type 'SwipeItem' could be found (are you missing a using directive or an assembly reference?) [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(929,29): error CS0117: 'SwipeItemMenuItemHandler' does not contain a definition for 'UpdateTextColorIconDependency' [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(931,47): error CS0103: The name 'ISwipeItemMenuItemIconColor' does not exist in the current context [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(945,14): error CS1061: 'SwipeItem' does not contain a definition for 'IconColor' and no accessible extension method 'IconColor' accepting a first argument of type 'SwipeItem' could be found (are you missing a using directive or an assembly reference?) [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(947,43): error CS0103: The name 'ISwipeItemMenuItemIconColor' does not exist in the current context [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(968,29): error CS0117: 'SwipeItemMenuItemHandler' does not contain a definition for 'UpdateBackgroundColorDependencies' [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(982,29): error CS0117: 'SwipeItemMenuItemHandler' does not contain a definition for 'MapIconColor' [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(986,29): error CS0117: 'SwipeItemMenuItemHandler' does not contain a definition for 'MapIconColor' [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(1001,29): error CS0117: 'SwipeItemMenuItemHandler' does not contain a definition for 'UpdateTextColorIconDependency' [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(1005,29): error CS0117: 'SwipeItemMenuItemHandler' does not contain a definition for 'UpdateTextColorIconDependency' [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(1006,32): error CS0103: The name 'ISwipeItemMenuItemIconColor' does not exist in the current context [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(1017,14): error CS1061: 'SwipeItem' does not contain a definition for 'TextColor' and no accessible extension method 'TextColor' accepting a first argument of type 'SwipeItem' could be found (are you missing a using directive or an assembly reference?) [/home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj]
🟢 With fix — 🧪 SwipeViewTests: PASS ✅ · 82s

(no coded error found; showing last 1200 chars)

sTextColorDependency [< 1 ms]
  Passed TestLeftItems [1 ms]
  Passed TestContentBindingContextPropagatesToNewSwipeItems [< 1 ms]
  Passed ColorlessFontIconContrastsWithTheBackground(darkBackground: False, expected: "#000000") [< 1 ms]
  Passed ColorlessFontIconContrastsWithTheBackground(darkBackground: True, expected: "#FFFFFF") [< 1 ms]
  Passed TextColorFallsBackToNullWhenBackgroundIsUnset [< 1 ms]
  Passed InitialTextColorMappingDoesNotReloadSource [2 ms]
  Passed SwipeItemTextColorDefaultsToNull [< 1 ms]
  Passed InitialIconColorMappingDoesNotReloadSource [< 1 ms]
  Passed TestSwipeItemsExecuteMode [< 1 ms]
[xUnit.net 00:00:03.49]   Finished:    Microsoft.Maui.Controls.Core.UnitTests
  Passed SettingIconColorInvokesIconMapperOnce [< 1 ms]
  Passed TestTopItems [< 1 ms]
  Passed FontIconUsesItsOwnColorWhenIconColorIsNotSet [< 1 ms]
  Passed ChangingTextColorRefreshesIconForColorlessFontIcon [1 ms]
  Passed SwipeViewRediscoversScrollParentWhenTemplateRootIsReparented [< 1 ms]
  Passed SwipeViewFindsScrollParentAfterTemplateParentConnected [< 1 ms]
  Passed ImageIconIsTintedWhenIconColorIsSet [< 1 ms]
Test Run Successful.
Total tests: 52
     Passed: 52
 Total time: 4.6201 Seconds
🔴 Without fix — 📱 SwipeItemMenuItemHandlerTests (IconTintCanBeClearedWithoutMutatingSharedDrawable, IconTintCanBeClearedWhenDrawableDoesNotReportColorFilter, IconColorLoadsSourceWhenPlatformImageIsMissing, IconColorChangeMutatesAttachedDrawableWithoutReloading): 🛠️ BUILD ERROR · 289s

Error-relevant lines (filtered from the build log):

/home/vsts/work/1/s/src/Core/tests/DeviceTests/Stubs/SwipeItemMenuItemStub.cs(5,72): error CS0246: The type or namespace name 'ISwipeItemMenuItemIconColor' could not be found (are you missing a using directive or an assembly reference?) [/home/vsts/work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net11.0-android]
Build FAILED.
🟢 With fix — 📱 SwipeItemMenuItemHandlerTests (IconTintCanBeClearedWithoutMutatingSharedDrawable, IconTintCanBeClearedWhenDrawableDoesNotReportColorFilter, IconColorLoadsSourceWhenPlatformImageIsMissing, IconColorChangeMutatesAttachedDrawableWithoutReloading): PASS ✅ · 755s

(no coded error found; showing last 1200 chars)

tive.Android/pal_jni.c
      08-13 06:34:02.320 10452 10452 I DOTNET  : [Maui Copilot Gate] XHarness class filter: Microsoft.Maui.DeviceTests.SwipeItemMenuItemHandlerTests
      08-13 06:34:03.374 10452 10452 I DOTNET  : TestFilter: Category=SwipeView
info: <<XHARNESS_RESULT_START>>
      {
        "version": 1,
        "machineName": "runnervm2z4qq",
        "exitCode": 0,
        "exitCodeName": "SUCCESS",
        "platform": "android",
        "instrumentationExitCode": 0,
        "device": "emulator-5554",
        "deviceOsVersion": "API 30",
        "architecture": "x86_64",
        "files": [
          {
            "name": "testResults-77bc92c4dbce425686845839582614c9.xml",
            "type": "test-results"
          },
          {
            "name": "adb-logcat-com.microsoft.maui.core.devicetests-default.log",
            "type": "logcat"
          }
        ]
      }
      <<XHARNESS_RESULT_END>>
info: Attempting to remove apk 'com.microsoft.maui.core.devicetests'..
fail: Waiting for command timed out: execution may be compromised
fail: Error: Exit code: -2
      Std out:
XHarness exit code: 0
  Passed: 4
  Failed: 0
  Skipped: 0
  Total: 4
  Tests completed successfully

⚠️ Failure Details

  • 🛠️ SwipeViewTests without fix: build failed before tests could run
    • /home/vsts/work/1/s/src/Controls/tests/Core.UnitTests/SwipeViewTests.cs(691,32): error CS1061: 'SwipeItem' does not contain a definition for 'IconColor' and no accessible extension method 'IconColor' ...
  • 🛠️ SwipeItemMenuItemHandlerTests (IconTintCanBeClearedWithoutMutatingSharedDrawable, IconTintCanBeClearedWhenDrawableDoesNotReportColorFilter, IconColorLoadsSourceWhenPlatformImageIsMissing, IconColorChangeMutatesAttachedDrawableWithoutReloading) without fix: build failed before tests could run
    • /home/vsts/work/1/s/src/Core/tests/DeviceTests/Stubs/SwipeItemMenuItemStub.cs(5,72): error CS0246: The type or namespace name 'ISwipeItemMenuItemIconColor' could not be found (are you missing a using ...
📁 Fix files reverted (25 files)
  • src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt
  • src/Controls/src/Core/PublicAPI/net-ios/PublicAPI.Unshipped.txt
  • src/Controls/src/Core/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt
  • src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt
  • src/Controls/src/Core/PublicAPI/net-windows/PublicAPI.Unshipped.txt
  • src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt
  • src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt
  • src/Controls/src/Core/SwipeView/SwipeItem.cs
  • src/Core/src/Core/ISwipeItemMenuItem.cs
  • src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Android.cs
  • src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Tizen.cs
  • src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Windows.cs
  • src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.cs
  • src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.iOS.cs
  • src/Core/src/Platform/Android/TextViewExtensions.cs
  • src/Core/src/Platform/SwipeViewExtensions.cs
  • src/Core/src/Platform/Windows/SwipeViewExtensions.cs
  • src/Core/src/PublicAPI/net-android/PublicAPI.Unshipped.txt
  • src/Core/src/PublicAPI/net-ios/PublicAPI.Unshipped.txt
  • src/Core/src/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt
  • src/Core/src/PublicAPI/net-tizen/PublicAPI.Unshipped.txt
  • src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt
  • src/Core/src/PublicAPI/net/PublicAPI.Unshipped.txt
  • src/Core/src/PublicAPI/netstandard/PublicAPI.Unshipped.txt
  • src/Core/src/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt

📋 Pre-Flight — Context & Validation

PR #36884 Pre-Flight

Scope

  • PR: [NET 11] SwipeItem: Add explicit IconColor and TextColor
  • Base / merge base: net11.0 / 342bf0b19754f1bdfd5eb94c5adc50950e4a7d3e
  • Materialized PR commit: 35f40ad0d40d4f3b80005caacd3c3f0351c856c2
  • Platform for candidate validation: Android
  • STEP 5a deadline: 2026-08-13 13:09:39 UTC (90 minutes from the supplied start time)

Problem and intended behavior

Issue #23074 asks for control over Android SwipeItem icon tinting because implicit monochrome tinting destroys authored PNG/SVG colors. Regression #36766 shows the inverse problem after implicit tinting was removed: an authored black icon can become invisible against an AppThemeBinding dark background, with no API to supply a contrasting tint.

The PR adds nullable SwipeItem.IconColor and SwipeItem.TextColor bindable properties. An explicit IconColor must tint the icon; otherwise font icons use their own color, then TextColor, then a contrasting background-derived color, while non-font images retain authored colors. An explicit TextColor colors the label; otherwise the native/default or contrasting color is used as documented. Runtime theme/property changes must update native state without stale tint, shared-drawable bleed, or unnecessary image reloads.

Existing PR approach

The committed diff changes 30 files (+2122/-61), including public API baselines, shared contracts/resolvers, all platform handlers, and focused tests.

  • Adds an optional ISwipeItemMenuItemIconColor companion interface rather than adding a required member to ISwipeItemMenuItem, preserving existing and netstandard2.0 implementers.
  • Uses inherited ITextStyle.TextColor for label color.
  • Centralizes effective text/icon resolution in SwipeViewExtensions.GetTextColor() and GetIconTintColor().
  • Adds mapper keys for TextColor and IconColor; suppresses dependent work during initial property mapping.
  • For Android color-only changes, reuses the attached compound drawable instead of reloading the source. It tracks the exact drawable it tinted because Drawable.ColorFilter is not authoritative, calls Mutate() before filter changes, computes bounds on the post-mutation instance, and clears only the tracked tinted drawable. A missing native drawable falls back to source mapping.
  • The iOS/MacCatalyst implementation similarly reapplies the attached native image. Windows selects between packaged monochrome-mask handling and the registered image-service pipeline. Tizen only applies text color.

Any candidate must use a different root-cause/implementation strategy, not a cosmetic relocation of this same tracking-and-reapply design.

Relevant files

Primary implementation surfaces:

  • src/Controls/src/Core/SwipeView/SwipeItem.cs
  • src/Core/src/Core/ISwipeItemMenuItem.cs
  • src/Core/src/Platform/SwipeViewExtensions.cs
  • src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.cs
  • src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Android.cs
  • src/Core/src/Platform/Android/TextViewExtensions.cs
  • Corresponding platform handlers and PublicAPI.Unshipped.txt files required for compilation/API consistency

Focused tests:

  • src/Controls/tests/Core.UnitTests/SwipeViewTests.cs
  • src/Core/tests/DeviceTests/Handlers/SwipeView/SwipeItemMenuItemHandlerTests.Android.cs
  • src/Core/tests/DeviceTests/Stubs/SwipeItemMenuItemStub.cs

Gate evidence and bounded validation

The prior gate is authoritative and must not be rerun. It passed: the PR's API-coupled tests failed to compile without the source change and passed with it.

Run only these validations for each candidate:

  1. Primary unit test:

    dotnet test src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj --filter "FullyQualifiedName~SwipeViewTests"
  2. Mandatory Android regressions:

    pwsh .github/skills/run-device-tests/scripts/Run-DeviceTests.ps1 -Project Core -Platform android -TestFilter "Category=SwipeView" -IncludeClasses "Microsoft.Maui.DeviceTests.SwipeItemMenuItemHandlerTests" -IncludeMethods "IconTintCanBeClearedWithoutMutatingSharedDrawable,IconTintCanBeClearedWhenDrawableDoesNotReportColorFilter,IconColorLoadsSourceWhenPlatformImageIsMissing,IconColorChangeMutatesAttachedDrawableWithoutReloading"

Gate results with the PR fix were 52/52 unit tests and 4/4 Android device tests. Do not run a full suite.

Workspace constraints

The branch is pr-review-36884. The worktree contains extensive pre-existing, unrelated .github/eng script changes used by the review environment. Preserve them exactly; do not clean, reset, restore, or include them in candidate diffs. Use only EstablishBrokenBaseline.ps1 and its -Restore mode for PR fix files as required by the try-fix skill. Do not create or overwrite gate/content.md.

Candidate contract

Each candidate gets one implementation/validation pass and at most one focused correction plus retest. It must perform the try-fix skill's inline expert self-review, save its normal attempt artifacts, write the requested narrative to try-fix-N/content.md, restore the baseline, and report Pass, Fail, or Blocked honestly.


🔬 Code Review — Deep Analysis

Expert Evaluation — PR #36884

Verdict

APPROVE with one non-blocking compatibility call-out. Confidence is high in the code assessment; the authoritative Gate passed, and the submitted implementation has focused unit and platform regression coverage.

Independent Assessment

The PR adds nullable SwipeItem.IconColor and SwipeItem.TextColor bindable properties, exposes icon color through the optional ISwipeItemMenuItemIconColor companion interface, and centralizes effective text/icon color resolution. Platform handlers apply explicit colors while preserving authored colors for non-font images when no icon tint is set.

The approach is sound:

  • Existing ISwipeItemMenuItem implementations remain source- and binary-compatible because the icon API is carried by a separate optional interface, including for netstandard2.0.
  • Initial mapper ordering is guarded by IsMappingProperties(), while runtime BackgroundColor, TextColor, and IconColor changes flow through their normal mapper keys.
  • Android mutates a drawable before changing its filter, tracks the exact tinted instance without trusting Drawable.ColorFilter, reapplies attached images for color-only updates, and falls back to source loading when no native image is attached.
  • iOS/MacCatalyst restore state-specific title colors and use the appropriate original/template rendering mode.
  • Windows rejects stale asynchronous image results and distinguishes packaged monochrome-mask icons from image-service-backed sources.
  • No new event-subscription, static-state, threading, trim, or AOT hazard was found.

Actionable Finding

One moderate, non-blocking finding was written to inline-findings.json:

  • src/Core/src/Platform/SwipeViewExtensions.cs:41 intentionally stops applying an implicit contrast tint to non-font images. This fixes authored PNG/SVG color loss, but it is a visible behavior change for existing apps that relied on automatic tinting of monochrome bitmap icons. The new behavior is covered by tests and is consistent with the PR's purpose; it needs explicit release-note/breaking-change sign-off rather than a code repair in this PR.

Blast Radius and Failure Modes

  • Runs for all instances: Color resolution runs for every SwipeItem, but unset properties preserve the intended defaults; non-font images intentionally switch to authored colors.
  • Startup impact: None. The change is handler/property mapping work, not application startup infrastructure.
  • Static/shared state: No new static mutable state. Android shared drawables are isolated with Mutate() before filter changes.
  • Null/default values: Both properties are nullable and their unset paths are explicitly handled on all platforms.
  • Reconnect/disconnect: Android tint tracking and iOS cached native title color are reset; source mapping re-establishes native images on reconnect.
  • Missing native image: Runtime icon-color updates fall back to the source mapper rather than silently doing nothing.
  • Theme changes: Bindable-property propagation invokes the matching mapper keys, and color-only updates avoid unnecessary source reloads.

Validation Evidence

The trusted Gate result is authoritative: tests fail without the fix and pass with it. Its submitted-PR run passed 52/52 focused SwipeViewTests and 4/4 mandatory Android device regressions. Gate verification was not rerun.

GitHub review and required-check surfaces were unavailable because gh was unauthenticated; no contradictory prior-review or CI claim is made from that unavailable surface.


🛠️ Try-Fix — Analysis & Comparison

PR #36884 — Try-Fix Aggregate

Candidate 1 — Stateless in-place retint (claude-opus-5)

Result: Pass — first implementation/validation pass; no correction round used.

Approach

Candidate 1 separates loading/attaching from runtime retinting and removes the PR's Android _appliedIconTintDrawable state:

  • A newly loaded drawable is mutated only when an explicit/effective tint will be applied, then bounded and attached.
  • A runtime color change operates on the attached compound drawable and unconditionally performs Mutate() followed by SetColorFilter(...) or ClearColorFilter(). Because clearing is unconditional and idempotent, the implementation does not consult the non-authoritative Drawable.ColorFilter getter and does not need to remember which drawable was tinted.
  • If Mutate() returns a distinct drawable, bounds are applied to that returned instance and it is reattached.
  • If no native drawable is attached, the shared mapper retains its source-reload fallback.
  • Bounds calculation is extracted and guards zero/negative intrinsic dimensions.

This differs materially from the PR's tracked-instance/reference-equality approach: it removes mutable handler tint state, the disconnect reset, the conditional clear path, and color-only re-entry through the image-source setter.

Candidate delta

Only src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Android.cs differs from the PR implementation. The complete tested candidate diff is in:

  • CustomAgentLogsTmp/PRState/36884/PRAgent/try-fix-1/android-candidate.diff (Android delta versus PR)
  • CustomAgentLogsTmp/PRState/36884/PRAgent/try-fix/attempt-1/fix.diff (complete alternative versus broken baseline)

Validation

Validation Result
dotnet test src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj --filter "FullyQualifiedName~SwipeViewTests" 52 passed, 0 failed
Android Run-DeviceTests.ps1, SwipeItemMenuItemHandlerTests, four mandated methods 4 passed, 0 failed; XHarness exit 0, emulator API 30

Inline expert self-review

1 moderate finding; 0 critical/major. The runtime retint path calls Mutate() even when the resolved tint is null and the drawable was never tinted. A theme switch that resolves IconColor to null can therefore create an avoidable private drawable copy per swipe item. Correctness is preserved; avoiding the copy would require reintroducing state that this candidate intentionally removes.

Analysis

The tests demonstrate that tracking the exact previously tinted drawable is not required for the mandated behavior. The stateless operation is simpler and remains correct when Android's ColorFilter getter does not report an applied filter. Its tradeoff is potentially unnecessary copy-on-write work during null-tint updates.

Full narrative and self-review:

  • CustomAgentLogsTmp/PRState/36884/PRAgent/try-fix-1/content.md
  • CustomAgentLogsTmp/PRState/36884/PRAgent/try-fix/attempt-1/reviewer-findings.json

Candidate 2 — Semantic drawable ownership state (gpt-5.6-sol)

Result: Pass — first implementation/validation pass; no correction round used.

Approach

Candidate 2 replaces exact drawable-reference tracking with two semantic lifecycle flags: whether the attached icon has become private and whether this handler currently has a tint applied.

  • On the first untinted/shared-to-tinted transition, it calls Mutate(), copies bounds and reattaches if Android returns a distinct drawable, then applies the filter.
  • Later tint changes reuse the known-private drawable.
  • Clearing occurs only when semantic state says this handler applied a tint.
  • A null-tint update with no active handler tint is a no-op, avoiding candidate 1's unconditional null-tint copy-on-write.
  • Loading a new source resets both flags and makes the new drawable private before applying an initial tint.

This differs from the PR because it stores no drawable reference and makes runtime transitions directly rather than re-entering the setter with a ReferenceEquals clear authorization. It differs from candidate 1 because it is stateful, mutates only on the ownership transition, and skips null-tint no-op updates.

Candidate delta

Only src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Android.cs differs from the PR implementation. The complete tested candidate diff is in:

  • CustomAgentLogsTmp/PRState/36884/PRAgent/try-fix-2/android-candidate.diff (Android delta versus PR)
  • CustomAgentLogsTmp/PRState/36884/PRAgent/try-fix/attempt-2/fix.diff (complete alternative versus broken baseline)

Validation

Validation Result
dotnet test src/Controls/tests/Core.UnitTests/Controls.Core.UnitTests.csproj --filter "FullyQualifiedName~SwipeViewTests" 52 passed, 0 failed
Android Run-DeviceTests.ps1, SwipeItemMenuItemHandlerTests, four mandated methods 4 passed, 0 failed; XHarness exit 0

Inline expert self-review

1 moderate finding; 0 critical/major. UpdateSize() passes the already-attached icon through SetImageSource(), which resets ownership state. For drawables whose Mutate() returns a new instance, an active tint can therefore allocate another private drawable on each attach/resize. The runtime color path itself remains one-time and avoids candidate 1's null-tint allocation.

Analysis

Semantic state is sufficient for all mandated behaviors: it protects shared untinted drawables, clears non-reporting drawables without trusting ColorFilter, preserves source fallback when the native image is absent, and mutates/reattaches on the first runtime tint without reloading. Its tradeoff is that the state reset in the resize/setter path can cause repeated private copies.

Full narrative and self-review:

  • CustomAgentLogsTmp/PRState/36884/PRAgent/try-fix-2/content.md
  • CustomAgentLogsTmp/PRState/36884/PRAgent/try-fix/attempt-2/reviewer-findings.json

Aggregate status

Two candidates were attempted, matching the hard cap. Both passed the focused 52-test unit run and all four mandatory Android device regressions on their first implementation pass. Neither used its correction allowance. Each retained one moderate performance concern and no critical/major self-review findings.


🏁 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Winner: pr-plus-reviewer

pr-plus-reviewer is the strongest candidate because it preserves the submitted PR's expert-reviewed runtime implementation and comprehensive regression coverage while resolving the sole expert finding with explicit migration guidance in the SwipeItem.IconColor API remarks. Its focused validation passed 52/52 unit tests and 4/4 mandatory Android device regressions.

Comparative ranking

Rank Candidate Regression evidence Assessment
1 pr-plus-reviewer 52/52 unit; 4/4 Android Keeps the sound PR implementation and adds precise public migration guidance for the intentional non-font image behavior change. No runtime tradeoff was introduced.
2 pr Trusted Gate passed; 52/52 unit; 4/4 Android Runtime design is correct and received no blocking expert finding. It ranks below the winner only because its API remarks describe the new end state without explicitly warning existing users that implicit contrast tinting changed.
3 try-fix-1 52/52 unit; 4/4 Android The stateless Android retint path is simpler and correct under the mandatory regressions, but it unconditionally calls Mutate() for null-tint updates, creating avoidable private drawable copies during relevant theme changes. It also does not address the compatibility-documentation finding.
4 try-fix-2 52/52 unit; 4/4 Android Semantic ownership flags pass the mandatory behaviors and avoid candidate 1's null-tint copy, but UpdateSize() re-enters SetImageSource(), resets ownership state, and can repeatedly allocate private drawable copies on attach/resize. Its additional lifecycle state is less robust than the PR's exact-instance tracking and it does not address the documentation finding.

No candidate failed regression validation, so no failure-based demotion was required. Both try-fix alternatives retain a known moderate performance concern; neither provides a correctness or maintainability advantage sufficient to replace the submitted Android implementation.

Expert review

The single MAUI expert review found:

  • 0 critical or major findings
  • 1 moderate, non-blocking finding at src/Core/src/Platform/SwipeViewExtensions.cs:41: non-font images now retain authored colors rather than receiving implicit contrast tinting, which is an intentional but migration-relevant behavior change.

The winner addresses that feedback without changing the behavior or expanding the patch beyond the public API documentation. Because pr-plus-reviewer, rather than the raw submitted pr, is the winner, the required recommendation is REQUEST CHANGES.


📱 UI Tests — SwipeView,ViewBaseTests

Detected UI test categories: SwipeView,ViewBaseTests

Deep UI tests — 193 passed, 3 failed, 1 skipped across 2 categories on platform-pool agent (replaces in-process counts above).

🧪 UI Test Execution Results (deep, platform pool)

Category Tests Snapshot diffs
SwipeView 75/78 (3 ❌) 6 diff PNGs
ViewBaseTests 118/119 (1 skipped) ✓
🔍 AI analysis of failures — PR-related vs unrelated

🔍 AI-generated triage (GitHub Copilot CLI) — a heuristic judgement of whether each deep UI test failure is connected to this PR's changes. Verify before relying on it.

Likely PR-related: one or more failures appear connected to this PR's changes.

  • ✗ PR-related — Android SwipeView visual snapshots (3 tests): all failures exercise SwipeView rendering, while the PR changes shared SwipeItem color fallbacks and Android icon, text, and background mapping in the rendered path.

Strongest signal: the consistent 0.91–1.83% deltas across SwipeView snapshots match the kind of small rendering shift these changes can produce; inspect the new images and update Android baselines only if the appearance is intended.

📸 Snapshot differences — baseline vs actual vs diff (all 6)

For each failing VerifyScreenshot snapshot: the committed baseline, the actual render on this CI agent, and the computed diff. Ordered by likely PR-relevance — snapshots whose baseline/test file this PR changed are shown first. A large, uniform diff across many snapshots is usually a cross-machine baseline/environment mismatch (e.g. the macOS TitleBar / window chrome), not a code regression — compare against baseline history before concluding.

Bottom_SwipeItems — `android` · SwipeView
Baseline (committed)Actual (CI)Diff
Bottom_SwipeItems baselineBottom_SwipeItems actualBottom_SwipeItems diff
Left_SwipeItems — `android` · SwipeView
Baseline (committed)Actual (CI)Diff
Left_SwipeItems baselineLeft_SwipeItems actualLeft_SwipeItems diff
Right_SwipeItems — `android` · SwipeView
Baseline (committed)Actual (CI)Diff
Right_SwipeItems baselineRight_SwipeItems actualRight_SwipeItems diff
Top_SwipeItems — `android` · SwipeView
Baseline (committed)Actual (CI)Diff
Top_SwipeItems baselineTop_SwipeItems actualTop_SwipeItems diff
VerifyCollectionViewContentWithIconImageSwipeItem — `android` · SwipeView
Baseline (committed)Actual (CI)Diff
VerifyCollectionViewContentWithIconImageSwipeItem baselineVerifyCollectionViewContentWithIconImageSwipeItem actualVerifyCollectionViewContentWithIconImageSwipeItem diff
VerifySwipeViewApperance — `android` · SwipeView
Baseline (committed)Actual (CI)Diff
VerifySwipeViewApperance baselineVerifySwipeViewApperance actualVerifySwipeViewApperance diff
SwipeView — 3 failed tests
VerifySwipeViewApperance
VisualTestUtils.VisualTestFailedException : 
Snapshot different than baseline: VerifySwipeViewApperance.png (1.83% difference)
If the correct baseline has changed (this isn't a a bug), then update the baseline image.
See test attachment or download the build artifacts to get the new snapshot file.

More info: https://aka.ms/visual-test-workflow
at VisualTestUtils.VisualRegressionTester.Fail(String message) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 162
   at VisualTestUtils.VisualRegressionTester.VerifyMatchesSnapshot(String name, ImageSnapshot actualImage, String environmentName, ITestContext testContext) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 123
   at Microsoft.Maui.TestCases.Tests.UITest.<VerifyScreenshot>g__Verify|13_0(String name, <>c__DisplayClass13_0&) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 485
   at Microsoft.Maui.TestCases.Tests.UITest.VerifyScreenshot(String name, Nullable`1 retryDelay, Nulla
...
Issue10563OpenSwipeViewTest
VisualTestUtils.VisualTestFailedException : 
Snapshot different than baseline: Left_SwipeItems.png (0.91% difference)
If the correct baseline has changed (this isn't a a bug), then update the baseline image.
See test attachment or download the build artifacts to get the new snapshot file.

More info: https://aka.ms/visual-test-workflow
at Microsoft.Maui.TestCases.Tests.Issues.Issue10563.Issue10563OpenSwipeViewTest() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue10563.cs:line 66
   at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
VerifyCollectionViewContentWithIconImageSwipeItem
VisualTestUtils.VisualTestFailedException : 
Snapshot different than baseline: VerifyCollectionViewContentWithIconImageSwipeItem.png (0.95% difference)
If the correct baseline has changed (this isn't a a bug), then update the baseline image.
See test attachment or download the build artifacts to get the new snapshot file.

More info: https://aka.ms/visual-test-workflow
at Microsoft.Maui.TestCases.Tests.UITest.VerifyScreenshot(String name, Nullable`1 retryDelay, Nullable`1 retryTimeout, Int32 cropLeft, Int32 cropRight, Int32 cropTop, Int32 cropBottom, Double tolerance) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 296
   at Microsoft.Maui.TestCases.Tests.SwipeViewFeatureTests.VerifySwipeViewScreenshot() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/SwipeViewFeatureTests.cs:line 826
   at Microsoft.Maui.TestCases.Tests.SwipeViewFeatureTests.VerifyCollectionViewContentWithIconImageSwipeItem() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Feat
...

📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)


🧭 Next Steps — reviewer patch required (pr-plus-reviewer)

The reviewer-enhanced candidate won, so the submitted PR still needs those changes.

Why: The submitted runtime implementation is sound and all compared candidates passed the focused regressions. pr-plus-reviewer wins because it preserves that implementation while adding explicit API migration guidance for the expert review's sole compatibility concern, with 52/52 unit and 4/4 Android tests passing.

Apply PRAgent/pr-plus-reviewer/reviewer.patch from the CopilotLogs
artifact (or follow the report's Required submitted-PR change), push the update, and run
the review again.

@kubaflo

kubaflo commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

@MauiBot addressed the latest compatibility finding: the intended .NET 11 behavior and migration path are now explicit in the PR description, and the PR is tagged t/breaking 💥 plus needs-breaking-change-doc-created for release-note documentation. The thread is resolved and this is ready for re-review — thanks!

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

🚨 API change(s) detected @davidbritch FYI

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

🚨 API change(s) detected @davidortinau FYI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-controls-swipeview SwipeView needs-breaking-change-doc-created platform/android platform/ios platform/macos macOS / Mac Catalyst platform/windows s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-fix-win AI found a better alternative fix than the PR s/agent-gate-passed AI verified tests catch the bug (fail without fix, pass with fix) s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) t/breaking 💥

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants