[Android] SwipeItem Text and IconImageSource are vertically misaligned when SwipeView wraps CollectionView - #36820
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 36820Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 36820" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
|
Hey there @@HarishwaranVijayakumar! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
/azp run maui-pr-uitests , maui-pr-devicetests |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
kubaflo
left a comment
There was a problem hiding this comment.
Is this one ready for review?
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) |
|---|---|---|
📱 SwipeViewTests (SwipeItemIconAndTextVerticallyAligned) Category=SwipeView |
🔴 Without fix — 📱 SwipeViewTests (SwipeItemIconAndTextVerticallyAligned): ⚠️ ENV ERROR · 1745s
(no coded error found; showing last 1200 chars)
Waiting for command timed out: execution may be compromised
fail: Error: Exit code: -2
Std out:
info: Attempting to install /home/vsts/work/1/s/artifacts/bin/Controls.DeviceTests/Debug/net10.0-android/com.microsoft.maui.controls.devicetests-Signed.apk
fail: Error:
Exit code: 1
Std out:
Serving...
Performing Incremental Install
cmd: Failure calling service package: Broken pipe (32)
Performing Streamed Install
Std err:
All files should be loaded. Notifying the device.
adb: failed to install /home/vsts/work/1/s/artifacts/bin/Controls.DeviceTests/Debug/net10.0-android/com.microsoft.maui.controls.devicetests-Signed.apk: cmd: Can't find service: package
crit: Install failure: Test command cannot continue
info: Attempting to remove apk 'com.microsoft.maui.controls.devicetests'..
fail: Error: Exit code: 20
Std out:
Std err:
cmd: Can't find service: package
info: Attempting to remove apk 'com.microsoft.maui.controls.devicetests'..
fail: Error: Exit code: 20
Std out:
Std err:
cmd: Can't find service: package
XHarness exit code: 78 (PACKAGE_INSTALLATION_FAILURE)
Tests completed with exit code: 78
🟢 With fix — 📱 SwipeViewTests (SwipeItemIconAndTextVerticallyAligned): ⚠️ ENV ERROR · 756s
(no coded error found; showing last 1200 chars)
info: Wrote full ADB log (207 lines) to /home/vsts/work/1/s/artifacts/log/adb-logcat-com.microsoft.maui.controls.devicetests-default.log
info: ADB log contained no DOTNET-tagged entries (see full log file for details)
info: Wrote ADB bugreport to /home/vsts/work/1/s/artifacts/log/adb-bugreport-com.microsoft.maui.controls.devicetests.zip
info: <<XHARNESS_RESULT_START>>
{
"version": 1,
"machineName": "runnervmtroe5",
"exitCode": 80,
"exitCodeName": "APP_CRASH",
"platform": "android",
"device": "emulator-5554",
"deviceOsVersion": "API 30",
"architecture": "x86_64",
"files": [
{
"name": "adb-logcat-com.microsoft.maui.controls.devicetests-default.log",
"type": "logcat"
},
{
"name": "adb-bugreport-com.microsoft.maui.controls.devicetests.zip",
"type": "bugreport"
}
]
}
<<XHARNESS_RESULT_END>>
info: Attempting to remove apk 'com.microsoft.maui.controls.devicetests'..
info: Successfully uninstalled com.microsoft.maui.controls.devicetests
XHarness exit code: 80 (APP_CRASH)
Tests completed with exit code: 80
⚠️ Failure Details
⚠️ SwipeViewTests (SwipeItemIconAndTextVerticallyAligned) without fix:App crashed during test run (XHarness exit 80 APP_CRASH)⚠️ SwipeViewTests (SwipeItemIconAndTextVerticallyAligned) with fix:App crashed during test run (XHarness exit 80 APP_CRASH)
📁 Fix files reverted (1 files)
src/Core/src/Platform/Android/MauiSwipeView.cs
📱 UI Tests — ViewBaseTests
Detected UI test categories: ViewBaseTests
✅ Deep UI tests — 118 passed, 0 failed across 1 category on platform-pool agent (replaces in-process counts above).
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
ViewBaseTests |
118/119 ✓ | — |
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs) |
🔗 Regression Cross-Reference
🔍 Regression Cross-Reference
⚠ Overlaps with prior bug-fix PRs — same files modified, but no exact line revert detected.
| File | Fix PR | Fixed issue(s) |
|---|---|---|
src/Core/src/Platform/Android/MauiSwipeView.cs |
#36499 | #36154 |
🧪 Regression Tests to Verify
These tests were added by the overlapping fix PRs. Running them to verify no side-effect regressions:
| Fix PR | Type | Test | Filter |
|---|---|---|---|
| #36499 | UITest | Issue36154 | Issue36154 |
🧪 Regression Test Results
❌ FAILED — 0 passed, 1 failed, 0 skipped
| Fix PR | Test | Type | Result |
|---|---|---|---|
| #36499 | Issue36154 | UITest | ❌ FAILED |
📋 Pre-Flight — Context & Validation
Issue: #36736 - [Android][Regression] SwipeItem Text and IconImageSource are vertically misaligned when SwipeView wraps CollectionView
PR: #36820 - [Android] SwipeItem Text and IconImageSource are vertically misaligned when SwipeView wraps CollectionView
Platforms Affected: Android
Files Changed: 1 implementation, 1 test
Key Findings
- The PR changes
src/Core/src/Platform/Android/MauiSwipeView.csso defaultSwipeItembuttons keep exact width measurement but useAtMostheight measurement; this lets the native button self-size its icon/text content while preserving the full layout bounds. - The PR adds an Android regression test,
SwipeItemIconAndTextVerticallyAligned, insrc/Controls/tests/DeviceTests/Elements/SwipeView/SwipeViewTests.Android.cs. - The linked issue is [Android][Regression] SwipeItem Text and IconImageSource are vertically misaligned when SwipeView wraps CollectionView #36736, a regression from 10.0.60 to 10.0.70 caused by the earlier exact-height measurement change from PR [Android] SwipeView: Use MeasureSpecMode.Exactly for SwipeItem layout to fix text visibility #27399.
- Mandatory regression cross-reference: Issue36154 (
WebViewinsideSwipeView) must be run for every passing candidate.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 0
Key code review findings:
- No actionable code defects were found.
- CI/gate state remains inconclusive/undetermined, so the code-review verdict cannot be
LGTMunder the skill rules. - Blast radius is Android platform UI plumbing for
SwipeView; no startup path, no static/shared state, and no new subscriptions. - Failure-mode probe: width remains
Exactly, preserving the [Android] SwipeView: Use MeasureSpecMode.Exactly for SwipeItem layout to fix text visibility #27399 text-visibility behavior; hit testing uses layout bounds, soMeasuredHeightdoes not shrink the action target.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #36820 | Default SwipeItem width remains Exactly; height is always measured AtMost so icon+text self-size together. |
Gate inconclusive; local primary probe passed | src/Core/src/Platform/Android/MauiSwipeView.cs, src/Controls/tests/DeviceTests/Elements/SwipeView/SwipeViewTests.Android.cs |
Original PR fix |
🔬 Code Review — Deep Analysis
Code Review - PR #36820
Independent Assessment
What this changes: Android MauiSwipeView now measures default SwipeItem width exactly but height with AtMost; adds an Android device regression test for tall content alignment.
Inferred motivation: Fix icon/text vertical separation when SwipeView wraps tall content.
External Output Contract
| Consumer token/pattern | Producer location | Producer emission condition | Consumer assumption | Ordinary negative case | Downstream effect |
|---|---|---|---|---|---|
| N/A | N/A | No changed external-output regex/string classifier | N/A | N/A | N/A |
Reconciliation with PR Narrative
Author claims: PR reverts the unintended height part of #27399 while preserving exact width.
Agreement/disagreement: Matches the code. The fix is scoped to Android SwipeView layout and keeps tap/layout bounds full-sized.
Prior Review Reconciliation
No prior Error findings found. Prior MauiBot output had an inconclusive gate and coverage warning, not an unresolved code error.
Blast Radius Assessment
- Runs for all instances: No; only Android SwipeViews when swipe items are created/launched.
- Startup impact: No.
- Static/shared state: No.
- Scope: Android platform UI plumbing for frequently-regressed SwipeView, so confidence max is medium before CI caps.
CI Status
- Required-check result: fail (
ghunauthenticated; public check-run/AzDO fallback used). - Classification: undetermined / likely unrelated. Failures observed in macOS MultiProject integration tests, UI visual baselines/timeouts/upload issues, not the changed Android SwipeView device-test path; baseline confirmation was not completed.
- Action taken: invoked
azdo-build-investigator;ci-analysisunavailable; manually inspected public timelines/logs; confidence capped low.
Findings
No actionable Error, Warning, or Suggestion findings.
Failure-Mode Probing
- Default left/right SwipeItems: width remains
Exactly, preserving [Android] SwipeView: Use MeasureSpecMode.Exactly for SwipeItem layout to fix text visibility #27399 text-visibility behavior; height layout bounds remain full-sized. - Tap target shrinkage: hit testing uses
Left/Top/Width/Height, soMeasuredHeightdoes not shrink action area. - Vertical Top/Bottom items: also get
AtMostheight; no concrete break found, but adjacent coverage is limited. - Handler reconnect/disconnect/static state: no new subscriptions, latches, or shared state.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: Code review found no concrete defect. However CI is red/undetermined and the change touches Android SwipeView platform layout, so per workflow rules this cannot be LGTM until required checks are clean or failures are conclusively unrelated.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix-1 | Keep default SwipeItem exact measurement and fix native button gravity/padding. |
Fail | 2 files | Build failed: TextView.SetCompoundDrawablePadding is unavailable in the Android binding. |
| 2 | try-fix-2 | Use AtMost height only for horizontal default menu items that have both text and icon; keep other default items exact-height. |
Pass | 1 file | Primary test and mandatory Issue36154 regression passed. More surgical than PR, but narrower. |
| PR | PR #36820 | Default SwipeItem width remains Exactly; height is always AtMost. |
Gate inconclusive; local primary probe passed | 2 files | Simpler and broader original PR fix. |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| maui-expert-reviewer | 2 | No | Meaningful fix families appear exhausted: measurement-policy changes are represented by PR and try-fix-2; native button alignment/padding was represented by try-fix-1. Wrapping/reparenting button content would be more invasive and likely risk background, hit-target, or command behavior regressions. |
Exhausted: Yes
Selected Fix: Candidate #2 for alternative exploration - it passed the primary and mandatory regression tests and is more surgical than the PR's broad height policy. However, the PR's current fix remains simpler and may cover adjacent icon-only/text-only/vertical cases that Candidate #2 intentionally leaves unchanged.
🏁 Report — Final Recommendation
Comparative Fix Report — PR #36820
Candidates Reviewed
| Candidate | Approach | Regression/Test Evidence | Assessment |
|---|---|---|---|
pr |
Default Android SwipeItem width remains Exactly; height is always measured with AtMost. Adds SwipeItemIconAndTextVerticallyAligned. |
Gate inconclusive from environment errors; expert reviewer found a major test-coverage issue. | Production fix is simple and directly addresses the measured-height root cause. The regression test is weaker than needed because it does not reproduce the original CollectionView + Execute + Threshold=80 scenario and asserts an implementation detail rather than actual icon/text alignment. |
pr-plus-reviewer |
Same production fix as pr, plus test-only reviewer feedback: update the regression test to use a CollectionView repro with SwipeMode.Execute, Threshold=80, item-count mutation, and an alignment-focused assertion. |
Not rerun per instruction; expected to preserve the same production behavior while strengthening coverage. | Best PR-fix candidate because it keeps the sound production fix and addresses the only expert-review finding. |
try-fix-1 |
Restore exact width/exact height for default items, then center/adjust native button content in SwipeItemMenuItemHandler.Android.cs. |
Failed: build error CS1061 TextView.SetCompoundDrawablePadding not available; mandatory regression was not run. |
Ranked below all non-failing candidates. It also changes lower-level button content layout instead of the measurement policy that caused the regression. |
try-fix-2 |
Narrow AtMost height to horizontal default menu items that have both text and icon; custom SwipeItemView remains AtMost; other default items keep exact height. |
Passed primary SwipeItemIconAndTextVerticallyAligned and mandatory Issue36154 regression. |
More surgical and empirically verified. Remaining risk is under-coverage: if the same exact-height issue affects icon-only, text-only, or vertical default items, this candidate intentionally leaves them on exact height. |
Ranking
pr-plus-reviewer— Best balance of correctness, simplicity, and coverage. It keeps the production fix and applies the expert reviewer's test-only improvement so the regression test matches the reportedCollectionView/execute/threshold scenario.pr— Production code is still the strongest fix, but the submitted test has a major regression-prevention gap.try-fix-2— Strong alternative with passing targeted evidence, but it is narrower than the root-cause fix and may miss adjacent defaultSwipeItemshapes affected by the same exact-height behavior.try-fix-1— Failed to build and therefore must rank below candidates without failed regression/build evidence.
Winning Candidate
Winner: pr-plus-reviewer
The recommended winner is the PR production fix plus the expert reviewer's test-only feedback. The production code directly addresses the root cause by changing only the unintended exact-height measurement while retaining the exact-width behavior from PR #27399. try-fix-2 has stronger executed evidence but is narrower and may leave adjacent default SwipeItem shapes unfixed; try-fix-1 failed to build.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Fixes an Android regression where SwipeItem icon and text become vertically misaligned when a SwipeView wraps tall content (e.g., a CollectionView), by adjusting how default SwipeItem buttons are measured during layout.
Changes:
- Android: measure default SwipeItem width with
Exactly(to preserve text visibility) but measure height withAtMostto avoid forcing a full-height measurement. - Tests: add an Android device regression test covering the tall-content scenario.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Core/src/Platform/Android/MauiSwipeView.cs | Adjusts SwipeItem child measurement so height uses AtMost while preserving Exactly width for default items. |
| src/Controls/tests/DeviceTests/Elements/SwipeView/SwipeViewTests.Android.cs | Adds an Android device test intended to prevent regression in SwipeItem icon/text vertical alignment under tall content. |
| [Fact] | ||
| [Description("SwipeItem icon and text should be vertically aligned when SwipeView has large content")] | ||
| public async Task SwipeItemIconAndTextVerticallyAligned() | ||
| { |
…d when SwipeView wraps CollectionView (#36820) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details: - SwipeItem Icon and Text Vertically Misaligned ### Root Cause of the issue - PR #[27399](#27399) changed the Android measurement mode to "Exactly" for both width and height to fix text visibility. Only width needed this change — height was unintentional. - With "Exactly" height, the button is told it's as tall as the entire content area (e.g., 1489px). Since the icon and text live inside the same button, Android spreads them across that space, causing them to separate. With "AtMost", the button self-sizes to just fit icon+text (~126px), keeping them together. ### Description of Change **Bug fix:** - Ensured that the height of default `SwipeItem` buttons is always measured with `AtMost` mode, allowing the button to self-size and keep the icon and text vertically centered, regardless of the content's height. The width remains `Exactly` for default items and `AtMost` for custom items. **Testing:** - Added a regression test `SwipeItemIconAndTextVerticallyAligned` to verify that the `SwipeItem` button is self-sizing its height and that the icon and text remain vertically aligned when the `SwipeView` contains large content. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #36736 ### Tested the behavior in the following platforms - [ ] Windows - [x] Android - [ ] iOS - [ ] Mac ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/22a4fb48-4b5c-4a5f-8b17-a9e7bcb18b29"> | <video src="https://github.com/user-attachments/assets/be8dbee4-af87-49df-aa8b-96164b5c80f6"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
…d when SwipeView wraps CollectionView (#36820) <!-- 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! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details: - SwipeItem Icon and Text Vertically Misaligned ### Root Cause of the issue - PR #[27399](#27399) changed the Android measurement mode to "Exactly" for both width and height to fix text visibility. Only width needed this change — height was unintentional. - With "Exactly" height, the button is told it's as tall as the entire content area (e.g., 1489px). Since the icon and text live inside the same button, Android spreads them across that space, causing them to separate. With "AtMost", the button self-sizes to just fit icon+text (~126px), keeping them together. ### Description of Change **Bug fix:** - Ensured that the height of default `SwipeItem` buttons is always measured with `AtMost` mode, allowing the button to self-size and keep the icon and text vertically centered, regardless of the content's height. The width remains `Exactly` for default items and `AtMost` for custom items. **Testing:** - Added a regression test `SwipeItemIconAndTextVerticallyAligned` to verify that the `SwipeItem` button is self-sizing its height and that the icon and text remain vertically aligned when the `SwipeView` contains large content. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #36736 ### Tested the behavior in the following platforms - [ ] Windows - [x] Android - [ ] iOS - [ ] Mac ### Output | Before | After | |----------|----------| | <video src="https://github.com/user-attachments/assets/22a4fb48-4b5c-4a5f-8b17-a9e7bcb18b29"> | <video src="https://github.com/user-attachments/assets/be8dbee4-af87-49df-aa8b-96164b5c80f6"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. -->
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!
Issue Details:
Root Cause of the issue
Description of Change
Bug fix:
SwipeItembuttons is always measured withAtMostmode, allowing the button to self-size and keep the icon and text vertically centered, regardless of the content's height. The width remainsExactlyfor default items andAtMostfor custom items.Testing:
SwipeItemIconAndTextVerticallyAlignedto verify that theSwipeItembutton is self-sizing its height and that the icon and text remain vertically aligned when theSwipeViewcontains large content.Issues Fixed
Fixes #36736
Tested the behavior in the following platforms
Output
Before-36736.mov
After-36736.mov