[iOS] Add legacy Done accessory compatibility switch#36776
Conversation
Lift the Liquid Glass button 4 points while retaining the 44-point accessory height, and update focused device and visual coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c80a2e76-7ce5-417a-b4fe-0e24bb5d39f5
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c80a2e76-7ce5-417a-b4fe-0e24bb5d39f5
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c80a2e76-7ce5-417a-b4fe-0e24bb5d39f5 # Conflicts: # src/Core/src/Platform/iOS/MauiDoneAccessoryView.cs
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
… and no deep results Fixes a two-sided always-finish gap exposed by the split PostReview job: when a PR's BASE branch fails to build (e.g. a broken inflight/* base), every stage produces nothing. RunPost defers with aiSummaryReviewId= 'DEFERRED' (non-empty), so the PostReview job's review-incomplete notice is SKIPPED — while Stage 3's deferred path found no PRAgent content and no deep results and merely LOGGED 'leaving the notice as the honest outcome', deferring to a notice that was never posted. Result: the PR got zero feedback (observed on #36776, base inflight/current, build 14768589). Stage 3 now posts the review-incomplete notice itself in that branch (adds BUILD_BUILDID to its env), so the PR is always finished with a notice. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 15d2af20-e4ab-4e88-9011-cfbd83513bc0
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
AI Review Summary
🗂️ Review Sessions — click to expand🚦 Gate — Test Before & After FixGate Result:
|
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
📱 MauiDoneAccessoryViewAppContextSwitchCollection (LegacyDoneAccessorySwitchSelectsExpectedImplementation) Category=Entry |
🛠️ BUILD ERROR | 🛠️ BUILD ERROR |
🔴 Without fix — 📱 MauiDoneAccessoryViewAppContextSwitchCollection (LegacyDoneAccessorySwitchSelectsExpectedImplementation): 🛠️ BUILD ERROR · 35s
Error-relevant lines (filtered from the build log):
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : We have detected that the required MSBuild tasks are not yet built or they are out of date. (Expand to see more info) [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : You will need to exit Visual Studio and wait for it to completely close. [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : After the process has ended, run the build command on the CLI with the .NET MAUI repository as the working directory: [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : dotnet build Microsoft.Maui.BuildTasks.slnf [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : You can also start Visual Studio using the cake script: [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : dotnet cake --target VS --workloads global [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : For best results, each time you rebase, merge from main or checkout a different branch, rebuild the build tasks. [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
Build FAILED.
🟢 With fix — 📱 MauiDoneAccessoryViewAppContextSwitchCollection (LegacyDoneAccessorySwitchSelectsExpectedImplementation): 🛠️ BUILD ERROR · 8s
Error-relevant lines (filtered from the build log):
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : We have detected that the required MSBuild tasks are not yet built or they are out of date. (Expand to see more info) [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : You will need to exit Visual Studio and wait for it to completely close. [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : After the process has ended, run the build command on the CLI with the .NET MAUI repository as the working directory: [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : dotnet build Microsoft.Maui.BuildTasks.slnf [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : You can also start Visual Studio using the cake script: [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : dotnet cake --target VS --workloads global [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
/Users/cloudtest/vss/_work/1/s/src/Maui.InTree.targets(34,5): error : For best results, each time you rebase, merge from main or checkout a different branch, rebuild the build tasks. [/Users/cloudtest/vss/_work/1/s/src/Core/tests/DeviceTests/Core.DeviceTests.csproj::TargetFramework=net10.0-ios]
Build FAILED.
⚠️ Failure Details
- 🛠️ MauiDoneAccessoryViewAppContextSwitchCollection (LegacyDoneAccessorySwitchSelectsExpectedImplementation) without fix: build failed before tests could run
- 🛠️ MauiDoneAccessoryViewAppContextSwitchCollection (LegacyDoneAccessorySwitchSelectsExpectedImplementation) with fix: build failed (fix does not compile)
📁 Fix files reverted (1 files)
src/Core/src/Platform/iOS/MauiDoneAccessoryView.cs
📱 UI Tests — Entry,ViewBaseTests
Detected UI test categories: Entry,ViewBaseTests
[!WARNING]
No UI test results were produced for the detected categories. The PR build itself was
fine — the deep UI stage was skipped or interrupted on infrastructure (the
merge-for-testing step, emulator/simulator boot, or an Appium hang), not by this PR's code.
This is usually transient; comment/review rerunto try again.
📋 Pre-Flight — Context & Validation
Issue: #36412 - [iOS] Done keyboard accessory (MauiDoneAccessoryView) is full-width and blocks taps on the Entry above the keyboard
PR: #36776 - [iOS] Add legacy Done accessory compatibility switch
Platforms Affected: iOS
Files Changed: 1 implementation, 1 test
Key Findings
- PR [iOS] Add legacy Done accessory compatibility switch #36776 is a follow-up compatibility escape hatch for [iOS] Done keyboard accessory (MauiDoneAccessoryView) is full-width and blocks taps on the Entry above the keyboard #36412, [iOS] Entry: Fix Done accessory hit testing #36413, and [iOS] Add spacing below Done accessory button #36756. It keeps the iOS 26+ Liquid Glass pass-through accessory as the default while allowing apps to opt back into the legacy full-width touch-blocking toolbar.
- The current PR fix adds an internal
AppContextswitch and snapshots the selected implementation perMauiDoneAccessoryViewinstance, so later process-wide switch changes do not mix rendering, hit testing, and action dispatch paths. - Device coverage added in
MauiDoneAccessoryViewTests.iOS.csverifies unset/default, explicit modern, explicit legacy, construction-time immutability, legacy Done dispatch, and hit-test behavior. - Linked issue [iOS] Done keyboard accessory (MauiDoneAccessoryView) is full-width and blocks taps on the Entry above the keyboard #36412 describes the original regression: the full-width transparent accessory captures taps across its 44pt height, preventing visible fields behind it from being tapped. PR [iOS] Entry: Fix Done accessory hit testing #36413 introduced the iOS 26+ floating pass-through button; PR [iOS] Add spacing below Done accessory button #36756 added the 4pt bottom spacing.
- No prior public inline review comments or review-body ❌ findings were present for [iOS] Add legacy Done accessory compatibility switch #36776. The top-level issue comments contain Azure Pipelines and MauiBot review-incomplete messages only.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 0
Key code review findings:
- ℹ No actionable code findings. The review found the implementation consistent with the PR narrative and the expected compatibility behavior.
- ℹ CI confidence is capped low because
gh pr checks --requiredwas unavailable in this environment and public CI is red, though observed failures appear to match base-branch failures rather than this PR. - ℹ Blast radius is platform-specific iOS input accessory plumbing. The new process-wide switch is read for every
MauiDoneAccessoryView, but the behavior is snapshotted into instance state.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #36776 | Add internal AppContext switch Microsoft.Maui.Platform.iOS.UseLegacyDoneAccessory; snapshot _useGlassButton at construction and route initialization, hit testing, and Done dispatch through that instance value. |
src/Core/src/Platform/iOS/MauiDoneAccessoryView.cs, src/Core/tests/DeviceTests/Platform/iOS/MauiDoneAccessoryViewTests.iOS.cs |
Original PR fix; gate result must not be treated as a failing fix. |
🔬 Code Review — Deep Analysis
Code Review — PR #36776
Independent Assessment
What this changes: Adds an internal iOS AppContext switch (Microsoft.Maui.Platform.iOS.UseLegacyDoneAccessory) that lets apps opt out of the iOS 26+ floating/pass-through Done accessory and restore the legacy full-width UIToolbar. The implementation snapshots the selected path per MauiDoneAccessoryView instance and uses that snapshot for initialization, hit testing, and programmatic Done dispatch.
Inferred motivation: Provide a temporary compatibility escape hatch for apps that depend on the old full-width toolbar/touch-blocking behavior, without changing the iOS 26+ default fix.
Reconciliation with PR Narrative
Author claims: The PR adds a default-off compatibility switch, preserves iOS 26+ glass behavior by default, restores legacy toolbar when explicitly enabled before control creation, snapshots behavior at construction time, and adds device coverage for default/modern/legacy paths.
Agreement/disagreement: The code matches the narrative. MauiDoneAccessoryView.cs:26-31,42-50,72-83,104-107 implements construction-time selection consistently. The device test covers unset/false/true switch cases and verifies existing instances do not change after later switch changes (MauiDoneAccessoryViewTests.iOS.cs:27-68).
Prior Review Reconciliation
| Prior ❌ Error Finding | Source | Status | Evidence |
|---|---|---|---|
| No prior ❌ Error findings found. | Reviews, inline comments, issue comments | n/a | Public review and inline comment surfaces are empty; issue comments only contain Azure Pipelines start and two MauiBot “review incomplete” warnings. |
Blast Radius Assessment
- Runs for all instances: Yes, every iOS
MauiDoneAccessoryViewconstruction now reads the switch once; behavior remains default-equivalent unless the switch is explicitly enabled. - Startup impact: No direct startup path; runs when iOS input accessory views are created for Entry/Editor/Picker/DatePicker/TimePicker.
- Static/shared state: Uses process-wide
AppContextstate, but snapshots into instance state to avoid later mismatches.
CI Status
- Required-check result:
gh pr checks --requiredunavailable becauseghis unauthenticated. - Classification: Public check-runs show
maui-prfailed. AzDO inspection found failures matching recentinflight/currentbase builds, notablySimpleTemplateTest.csAssert.IsTruecompile errors and repeated Helix unit-test failures. - Action taken: Invoked
azdo-build-investigator;ci-analysiswas unavailable. Confidence capped low due unavailable required-check query and red CI.
Findings
No ❌ Error,
Failure-Mode Probing
- Switch changes after construction: Existing views keep
_useGlassButton, so rendering, hit testing, andSendDoneClickedstay consistent. - Legacy path on iOS 26+: Uses the existing toolbar path and
_proxy.Invoke(), avoiding attempts to find/send aUIButtonthat does not exist. - Null/missing Done button: Glass hit testing and dispatch handle
DoneButton is nullwithout throwing. - Test isolation: The switch test disables parallelization for this collection and restores behavior-equivalent false/default state.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: Code review found no actionable code issues, and the implementation matches the stated compatibility behavior. However, required CI could not be queried with gh, and public CI is red; although observed failures appear pre-existing on the base branch, the CI state prevents an LGTM verdict under the skill rules.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Shape-derived glass mode: infer glass/legacy from direct native subviews instead of storing _useGlassButton. |
1 implementation, 1 test | Reduces one field but couples behavior to UIKit subview shape; not demonstrably better. | |
| 2 | try-fix | Explicit call-site policy injection: move switch evaluation to accessory creation call sites and pass useGlassButton to constructors. |
5 implementation, 1 test | Preserves explicit mode but expands blast radius across Entry/Editor/Picker/DatePicker/TimePicker call sites. | |
| 3 | try-fix | Process-level cached accessory mode: resolve AppContext switch once on first type use. | 1 implementation, 1 test | Worse startup-order contract and poor test isolation due static cached state. | |
| 4 | try-fix | Dynamic behavior-time AppContext evaluation: re-read switch in HitTest/SendDoneClicked. |
1 implementation | Fundamentally flawed because behavior can diverge from initialized native subviews. | |
| PR | PR #36776 | Per-instance AppContext snapshot in _useGlassButton; initialization, hit testing, and dispatch all use the same construction-time mode. |
1 implementation, 1 test | Original PR. Gate was inconclusive due build/environment error, not a code failure. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| maui-expert-reviewer | 1 | Yes | Candidate 1: infer mode from direct subview shape. |
| maui-expert-reviewer | 2 | Yes | Candidate 2: move policy to call sites and inject explicit mode. |
| maui-expert-reviewer | 3 | Yes | Candidate 3: process-level cached AppContext decision. |
| maui-expert-reviewer | 4 | Yes | Candidate 4: dynamic behavior-time AppContext reads. |
| maui-expert-reviewer | Final | No | Remaining meaningful variants either repeat these root approaches or require a larger lifecycle feature to rebuild accessories when AppContext changes. |
Test Blocker: All candidate testing was blocked before application because EstablishBrokenBaseline.ps1 -DryRun failed on unrelated uncommitted worktree changes outside the PR target files. Per autonomous execution instructions, the blocked test phase was recorded and the loop continued without reverting environment/user changes.
Exhausted: Yes
Selected Fix: PR #36776 — It is the best candidate on code merits. It keeps the change localized, preserves the intended "set before creating controls" timing, and maintains a single invariant: rendering, hit testing, and Done dispatch all use the same construction-time mode. None of the alternatives is demonstrably better, and two introduce major correctness/regression risks.
📝 Recommended PR Title & Description
Assessment: ✏️ Recommend updating — the current title and description accurately describe the compatibility switch, but the winning fix also includes the reviewer-required hit-testing correction for the glass button's top overhang.
Recommended title
[iOS] Done accessory: Add legacy compatibility switch and preserve glass hit testing
Recommended description
### Description of Change
Adds a temporary, default-off `AppContext` compatibility switch for iOS applications that specifically depend on the legacy full-width Done toolbar.
The pass-through Liquid Glass close button remains the default on iOS 26+. To explicitly restore the legacy toolbar, set the switch before creating MAUI controls:
```csharp
AppContext.SetSwitch("Microsoft.Maui.Platform.iOS.UseLegacyDoneAccessory", true);
```
This opt-out intentionally restores the complete legacy behavior: the blue system Done/checkmark affordance and the full-width toolbar that intercepts touches. Applications that do not depend on that behavior should leave the switch unset.
The switch remains internal implementation rather than adding permanent public MAUI API. Each accessory records the selected path when it is constructed, preventing later process-wide switch changes from creating mismatched rendering, action dispatch, and hit-testing behavior.
The iOS 26+ glass path also preserves hit testing for the visible top edge of the floating Done button, which is shifted above the 44pt accessory bounds for spacing. The transparent accessory area still passes touches through except where the Done button is actually visible.
Device coverage verifies the unset default, explicit modern, and explicit legacy paths; construction-time behavior; Done dispatch; full-width legacy hit interception; glass-path pass-through hit testing; and glass-button top-overhang hit testing. Earlier iOS versions continue using the legacy toolbar regardless of the switch value.
### Issues Fixed
Follow-up compatibility escape hatch for #36412, #36413, and #36756.
### Testing
- Core iOS Entry device tests on iOS 26.5: 243 run, 242 passed, 1 skipped, 0 failed
- Core iOS Entry device tests on iOS 18.6: 243 run, 242 passed, 1 skipped, 0 failed
- New compatibility-switch test passed on both exact simulator runtimes
🏁 Report — Final Recommendation
Comparative Report — PR #36776
Candidates compared
| Candidate | Approach | Test/regression result | Ranking notes |
|---|---|---|---|
pr |
Localized MauiDoneAccessoryView change with internal AppContext switch and per-instance _useGlassButton snapshot. |
Strong base design, but expert review found an actionable glass-button hit-test gap for the top 4pt overhang. | |
pr-plus-reviewer |
pr plus sandboxed reviewer fix: override PointInside so the out-of-bounds top edge of the glass Done button remains hittable, with a matching device-test assertion. |
Best balance: keeps the PR's low blast radius and correct construction-time invariant while fixing the only actionable reviewer finding. | |
try-fix-1 |
Shape-derived glass mode: infer mode from direct native subviews instead of storing _useGlassButton. |
Not demonstrably better than pr; couples behavior to UIKit subview shape and risks future misclassification if another direct UIButton appears. |
|
try-fix-2 |
Explicit call-site policy injection: evaluate switch outside the view and pass useGlassButton through Entry/Editor/Picker/DatePicker/TimePicker call sites. |
Preserves explicit state, but expands blast radius across multiple iOS accessory creation paths without improving behavior. | |
try-fix-3 |
Process-level cached accessory mode via static/Lazy switch resolution. | Worse than pr: changes the contract from "set before creating controls" to "set before first type use" and harms test isolation. |
|
try-fix-4 |
Dynamic behavior-time AppContext evaluation in initialization, hit testing, and Done dispatch. | Not viable: late switch changes can desynchronize the one-time native subview tree from hit testing and dispatch behavior. |
Analysis
No candidate has a passing regression-test result, and none has a recorded regression-test failure. The gate for pr was inconclusive due build/environment issues, and all try-fix candidates were blocked before safe application/testing because the shared worktree contained unrelated uncommitted changes.
On code merits, the PR's construction-time snapshot is the correct compatibility-switch model. It keeps the change inside MauiDoneAccessoryView, avoids updating every call site, and prevents later process-wide switch changes from creating mismatched visual, hit-testing, and dispatch behavior.
The expert reviewer identified a concrete issue in the raw PR fix: the glass button is shifted 4pt upward, so part of the visible button sits outside the accessory view's bounds and may be excluded by UIKit hit testing. pr-plus-reviewer addresses that without changing the switch semantics or broadening the implementation surface.
Ranking
pr-plus-reviewer— fixes the only actionable expert-review finding while preserving the PR's design.pr— good localized design, but leaves the top-overhang hit-test gap.try-fix-1— plausible but more brittle than the explicit snapshot.try-fix-2— plausible but unnecessarily broad blast radius.try-fix-3— worse startup-ordering and test-isolation contract.try-fix-4— flawed because behavior-time switch reads can diverge from constructed native UI.
Winning candidate
Winner: pr-plus-reviewer
pr-plus-reviewer is the single best candidate because it keeps the raw PR's localized, construction-time compatibility-switch architecture and adds the minimal hit-testing correction needed for the iOS 26+ glass button's out-of-bounds top edge.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
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
Adds a temporary, default-off
AppContextcompatibility switch for iOS applications that specifically depend on the legacy full-width Done toolbar.The pass-through Liquid Glass close button remains the default on iOS 26+. To explicitly restore the legacy toolbar, set the switch before creating MAUI controls:
This opt-out intentionally restores the complete legacy behavior: the blue system Done/checkmark affordance and the full-width toolbar that intercepts touches. Applications that do not depend on that behavior should leave the switch unset.
The switch remains internal implementation rather than adding permanent public MAUI API. Each accessory records the selected path when it is constructed, preventing later process-wide switch changes from creating mismatched rendering, action dispatch, and hit-testing behavior.
Device coverage verifies the unset default, explicit modern, and explicit legacy paths; construction-time behavior; Done dispatch; full-width legacy hit interception; and glass-path pass-through hit testing. Earlier iOS versions continue using the legacy toolbar regardless of the switch value.
Issues Fixed
Follow-up compatibility escape hatch for #36412, #36413, and #36756.
Testing