[release/10.0.1xx-sr10] Preserve custom iOS button styling - #37726
Conversation
Manual backport of dotnet#36769 for .NET MAUI 10.0.101. Source head: 348f69b Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1e7f77e3-4862-4b92-a662-d12b5918bb59
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 37726Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37726" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Backports the iOS ButtonHandler regression fix to release/10.0.1xx-sr10 by updating the iOS UIButton background-mapping behavior to preserve native styling when Background is unset during initial handler connection, and adds a UI regression test covering the scenario.
Changes:
- iOS: Guard
UIButton.BackgroundColor = UIColor.Clearbehind aWindow != nullcheck whenpaintis null/empty, preserving constructor-set native styling on initial render. - Tests: Add HostApp repro page + custom handler and a corresponding Appium UI test for issue #36749.
- Register the custom test handler in the HostApp’s
MauiProgram.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Core/src/Platform/iOS/ButtonExtensions.cs | Updates iOS button background mapping to avoid clearing native styling during initial render when Window is null. |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36749.cs | Adds an Appium UI test asserting the regression is fixed on iOS/MacCatalyst. |
| src/Controls/tests/TestCases.HostApp/MauiProgram.cs | Registers the issue-specific custom button handler for the HostApp test page. |
| src/Controls/tests/TestCases.HostApp/Issues/Issue36749.cs | Adds the HostApp issue page and custom ButtonHandler/UIButton subclass used to reproduce and validate the fix. |
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
💡 Regression — src/Core/src/Platform/iOS/ButtonExtensions.cs:74: Window == null identifies both the initial map and a previously shown button that is temporarily detached. If a button with a MAUI solid background is detached (for example, during navigation or cell reuse) and its Background becomes null, the mapper removes its gradient layer but skips BackgroundColor = UIColor.Clear. Reattaching the existing native button can therefore show the stale solid background instead of restoring the null-background state.
Use a one-time initial-map/handler-lifecycle signal rather than current window attachment, so only the true first map preserves native styling while later null-background updates always clear prior MAUI state.
Flagged by: 3/3 reviewers, including independent reviewer validation.
The added UI test covers initial native styling preservation, but not the detached-view reset path above.
Methodology: 3 independent reviewers with adversarial consensus.
Guard the nullable UIButton background before reading its RGBA components so the HostApp reports a deterministic test failure instead of crashing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
|
@copilot addressed the latest feedback by making a missing native background color a deterministic UI-test failure. The iOS-simulator HostApp build succeeds cleanly. This is ready for re-review — thanks! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36749.cs:26
- The test reads the result label immediately after
WaitForElement(...), but the label is created with initial text "Checking..." and is only updated inOnAppearing()in the HostApp page. Since_IssuesUITestnavigation doesn’t wait forOnAppearing, this can flake by asserting against "Checking...". Prefer waiting for the label text to become "PASS" (or timing out) usingWaitForTextToBePresentInElement.
App.WaitForElement("Issue36749Result");
var resultText = App.FindElement("Issue36749Result").GetText();
Assert.That(resultText, Is.EqualTo("PASS"),
Use platform-view lifecycle state rather than window attachment to preserve native styling only on the initial map, while clearing later detached updates. Extend the regression coverage for the detached-view case.\n\nCo-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>\nCopilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
|
@PureWeen fixed in 6c898fa. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36749.cs:1
- Minor: the preprocessor line comment is missing a space after // and uses "Mac Catalyst" inconsistently with the rest of the repo's "MacCatalyst" naming.
#if IOS || MACCATALYST //This is an iOS-specific issue and can be reproduced by extending UIButton. Therefore, the test was added only for iOS and Mac Catalyst.
src/Controls/tests/TestCases.HostApp/Issues/Issue36749.cs:70
- This comment explains the detached-null scenario in terms of a Window-based guard, but the production fix is now initial-update state tracking. Rewording to a conditional rationale avoids implying the current code uses Window checks.
// A later null mapping must clear MAUI-applied state even while the native view is
// detached. Window-based initial-map detection incorrectly leaves this color red.
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
src/Core/src/Platform/iOS/ButtonExtensions.cs:63 records whether a UIButton has ever received a background mapping, rather than whether the current virtual button has applied a MAUI background. ElementHandler.SetVirtualView deliberately retains an existing platform view when it reconnects that handler to a different virtual view, then reruns all property mappers. A recycled custom button whose new virtual view still has the default null Background therefore takes the later-null path and sets UIColor.Clear, wiping the native constructor styling this change is intended to preserve.
Track the initialization state per virtual-view assignment, or clear only after MAUI has applied a non-null background, and add a reconnect/reuse regression case.
Flagged by 2/3 reviewers after adversarial consensus and independent reviewer source validation.
Automated assessment complete: verified current head 6c898fa576dbeda5dd933b8421ea4e068fb7e859; sender session 20939cfd-13dd-40eb-83ee-cdb524c95e95.
Only clear a null background after MAUI applied one, and cover reconnecting the same native button to a new virtual view. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
💡 Logic — src/Core/src/Platform/iOS/ButtonExtensions.cs:83 records HasMauiBackground for every non-empty paint even though ViewExtensions.UpdateBackground applies only SolidPaint and GradientPaint. A custom UIButton with native styling can receive a valid ImagePaint or PatternPaint (neither changes BackgroundColor), then receive Background = null; the recorded state makes the null path clear the native color even though MAUI never applied a background. Mark state only for paint types that the delegated updater applies, and add this transition to the regression coverage.
Flagged by: 3/3 reviewers after adversarial consensus, including independent reviewer validation.
The current tests cover initial native styling, a detached solid-background clear, and handler reuse, but not the image/pattern-paint transition above.
Methodology: 3 independent reviewers with adversarial consensus.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
|
@PureWeen addressed in 8ef8ece: weak state is now recorded only for |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (2)
Previously missed (2) — in code that hasn't changed since the last review.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36749.cs:21
- The test can race by reading the result label immediately after it appears; the label exists with initial text (e.g., "Checking...") before OnAppearing updates it to PASS/FAIL. This can make the test flaky.
App.WaitForElement("Issue36749Result");
var resultText = App.FindElement("Issue36749Result").GetText();
src/Core/src/Platform/iOS/ButtonExtensions.cs:67
UpdateBackgroundcallsRemoveBackgroundLayer()and then delegates toViewExtensions.UpdateBackground(...), which callsRemoveBackgroundLayer()again for non-null paints. This duplicates work on every non-null background update and makes the cleanup logic harder to follow.
// Remove previous background gradient layer if any.
// Safe to call when MAUI has not applied a gradient because this is a no-op.
// Running it before the paint guard ensures any previously-applied gradient is cleaned
// up regardless of the new paint value.
platformButton.RemoveBackgroundLayer();
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
ButtonExtensions.UpdateBackground records HasMauiBackground for every GradientPaint, but the delegated iOS updater creates a layer only for LinearGradientPaint and RadialGradientPaint. A custom non-empty GradientPaint subclass therefore leaves constructor/native styling intact while arming the state; clearing Background afterward resets that native color to UIColor.Clear even though MAUI never applied a background.
Track the state only for SolidPaint, LinearGradientPaint, and RadialGradientPaint, or set it only when the delegated updater actually applies a layer. Add the analogous custom-gradient-to-null regression case.
Flagged by: 3/3 reviewers after adversarial consensus, including independent reviewer validation.
Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist.
Track MAUI background ownership only for paint types the iOS updater actually applies, with device coverage for custom gradient subclasses. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
|
@PureWeen addressed the custom-gradient background ownership finding in |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue36749.cs:1
- The
#ifline comment uses inconsistent spacing and platform naming ("Mac Catalyst") compared to other issue tests (e.g., Issue35490.cs:1 uses "MacCatalyst"). Normalizing this makes the comment easier to scan and keeps terminology consistent.
#if IOS || MACCATALYST //This is an iOS-specific issue and can be reproduced by extending UIButton. Therefore, the test was added only for iOS and Mac Catalyst.
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review
An independent reviewer completed an exact-head review of c28674d9e9d2ffb9980cef0ff54b5c6da0869445.
No actionable findings. The iOS background ownership tracking and its UI/device regression coverage are internally consistent.
Methodology: 3 independent reviewers with adversarial consensus.
Test coverage: The PR adds targeted HostApp, Appium, and iOS device coverage; this was a review-only pass.
SR10.1 merge assessmentRecommendation: merge. Current head The remaining |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1e7f77e3-4862-4b92-a662-d12b5918bb59
…otnet#37726)" This reverts commit 470a24a, reversing changes made to a02bcc0.
…load (#37746) <!-- 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! <!-- release-readiness-agent: human-approval-required --> ## Summary Integrates the staged **.NET MAUI 10.0.101 (SR10.1)** payload from `inflight/10.0.101` into `release/10.0.1xx-sr10`, based on the shipped `10.0.100` release. The full release inventory and servicing decisions are tracked in #37723. The [authoritative regression query](https://github.com/dotnet/maui/issues?q=repo%3Adotnet%2Fmaui%20is%3Aissue%20is%3Aopen%20%28label%3A%22regressed-in-10.0.10%22%20OR%20label%3A%22regressed-in-10.0.20%22%20OR%20label%3A%22regressed-in-10.0.30%22%20OR%20label%3A%22regressed-in-10.0.40%22%20OR%20label%3A%22regressed-in-10.0.50%22%20OR%20label%3A%22regressed-in-10.0.60%22%20OR%20label%3A%22regressed-in-10.0.70%22%20OR%20label%3A%22regressed-in-10.0.80%22%20OR%20label%3A%22regressed-in-10.0.90%22%20OR%20label%3A%22regressed-in-10.0.100%22%29) remains the source for the servicing-regression scope. > [!IMPORTANT] > Checked items under **Staged in this PR** are merged into `inflight/10.0.101`, not yet into `release/10.0.1xx-sr10`. They reach the release branch only when this aggregate PR is merged. ## Status at a glance - [x] Stage the selected regression backports independently on `inflight/10.0.101`. - [x] Stage the SkiaSharp 4.150.1 payload and selective `SKPath.AddCircle` compatibility fix. - [x] Stage the isolated iOS 26 Liquid Glass TabBar payload from #37542. - [ ] Complete or explicitly defer the six unresolved servicing regressions listed below. - [ ] Complete the release gates and merge this PR into `release/10.0.1xx-sr10`. ## Staged in this PR ### Servicing regressions - [x] #35826 — Android MediaPicker completion from child activities. Source #35944; component backport #37724. - [x] #36298 — Windows dynamic `ContentPresenter` assignment. Source #36430; component backport #37725. - [x] #36749 — preserve custom iOS Button styling. Source #36769; component backport #37726. - [x] #36736 — align Android SwipeItem content. Source #36820; component backport #37727. - [x] #34563 — per-edge iOS safe-area handling. Source #37033; component backport #37728. - [x] #37706 — Android root-page back handling. Source #37709; component backport #37729. - [x] #37705 — Android Material 3 status-bar contrast. Source #37710; component backport #37730. - [x] #37423 — restore native iOS 26 Liquid Glass TabBar behavior through the isolated four-file payload from merged PR #37542. - [x] #37418 — Android off-screen `TranslationY` layout padding. Source fix #37772; focused backport commit `10799e67df`. - [x] #37361 — iOS RefreshView pull-to-refresh with an empty CollectionView. Source fix #37404; focused backport commit `d26faa8e93`. - [x] #37638 — Android `Screenshot.CaptureAsync` synchronous UI-thread deadlock. Source fix #37680; focused backport commit `778040a233`. ### Planned Skia payload - [x] Port the Skia/Svg.Skia update from #36255 through component backport #37731. - [x] Include only the required `SKPath.AddCircle` compatibility change from #36787; exclude its unrelated SourceGen changes. - [x] Confirm a matching SkiaSharp 4.150.1 native-assets/PDB build for `_SkiaSharpNativeAssetsVersion`, or record an explicit release-owner waiver. ### Integration follow-ups - [x] Apply the aggregate review fixes for safe-area cache invalidation, Activity-for-result cancellation ownership, and Android theme-test semantics. - [x] Keep the final branch payload as focused release commits without unrelated `inflight/current` history. - [x] Validate the #37418 and #37638 focused Android UI tests on the aggregate branch (2/2 passed). - [ ] Validate the #37361 focused iOS UI test on the aggregate branch. ## Already present on the SR10 base — no new merge needed - [x] #36852 — Android Shell hidden-fragment leak; fix #36903 is already in SR10. - [x] #36735 — Android ActivityIndicator visibility; fix #36748 is already in SR10. - [x] #36853 — Android Shell DI-singleton route blank page; fix #36903 is already in SR10. - [x] #36942 — Android UI event handling; fix #36988 is already in SR10. - [x] #37281 — Android scrolling inside shadowed containers; backport #37312 is already in SR10. ## Completed disposition — no backport required - [x] #37700 — closed as intended SR10 `SwipeView.Threshold` behavior from #36878; any SwipeItem sizing concern should be tracked separately. ## Not included yet — merge, defer, or explicitly disposition before payload freeze - [ ] #35301 — Windows CollectionView applies WinUI styling by default; no linked fix PR. - [ ] #34491 — Android CollectionView selection with `PointerGestureRecognizer`; replacement fix #37952 remains draft against `main`. - [ ] #35059 — iOS app becomes unresponsive when opening a ComboBox dropdown in landscape; no linked fix PR. - [ ] #37407 — iOS TimePicker default `t` format forces en-US; fix #37797 remains draft against `main`. - [ ] #36269 — Android `SafeAreaEdges` with Shell tab navigation and hidden TabBar; source fix #36474 remains open against `main` and has no SR10 backport. - [ ] #37657 — Android Shell/root-page `OnBackButtonPressed()` in Release builds; no linked fix PR. ## Release completion ### Before merging this PR - [ ] Refresh the authoritative query and reconcile #37723 with its current results. - [ ] Record an include/defer/close decision for every unresolved regression above. - [ ] Validate every included regression on its reported OS/version using the `10.0.101` candidate packages. - [ ] Complete current-head aggregate CI and resolve or explicitly waive every required build/device/UI-test failure. - [ ] Resolve or waive the Skia native-assets/PDB release gate. - [ ] Bump `eng/Versions.props` from patch `100` to `101` after the payload is frozen. - [ ] Obtain the required independent current-head maintainer approvals. - [ ] Merge #37746 into `release/10.0.1xx-sr10`. ### After merge - [ ] Confirm BAR promotion and the per-build validation feed used by the ship assessment. - [ ] Publish and tag `10.0.101`. - [ ] Complete the Visual Studio insertion/default-MAUI-version update. ## Notes - No automated `/backport` command was used for this release payload. - Supersedes #37745, which GitHub automatically closed when the integration branch was renamed from `vs/10.0.101` to `inflight/10.0.101`.
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!
Summary
Manual backport of #36769 to
release/10.0.1xx-sr10for .NET MAUI 10.0.101.348f69b8e04a72d83b563064404a7c4fd79aef75.inflight/currentand has not flowed tomain; this manual port intentionally excludes unrelated inflight branch history.Validation
f5eb0ffddfe5e87e84c64060fcb73927191d00e1.Core.csprojbuilds successfully fornet10.0-ios26.0in Release configuration with zero warnings and zero errors.Review gate
This agent-authored release PR must remain unmerged until two distinct non-bot MAUI maintainers with write access, other than the PR author, approve the current head SHA.