Skip to content

Fix SwipeView Threshold changes width and offset of the side menu (when visible)#34923

Open
KarthikRajaKalaimani wants to merge 9 commits intodotnet:mainfrom
KarthikRajaKalaimani:fix-6016
Open

Fix SwipeView Threshold changes width and offset of the side menu (when visible)#34923
KarthikRajaKalaimani wants to merge 9 commits intodotnet:mainfrom
KarthikRajaKalaimani:fix-6016

Conversation

@KarthikRajaKalaimani
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!

Issue Details:

SwipeView Threshold changes width and offset of the side menu (when visible) Opened.

Root Cause:

The SwipeView.Threshold property is designed for a single purpose: setting the minimum drag distance a user must swipe before the menu snaps open. It has nothing to do with how wide the menu
items should appear or where the content settles after the swipe. However, the platform code on both iOS and Android was incorrectly using this value in two unrelated calculations.

The first misuse was in GetSwipeItemSize(), where the code checked if Threshold > 0 and, if so, used the threshold value as the item's width. This meant that setting Threshold=200 would inflate
every swipe item to 200pt wide, far larger than the intended ~100pt. The second misuse was in GetSwipeThreshold(ISwipeItems), which had an early return that directly handed back the Threshold
value as the snap-open distance. So the content would snap to 200pt offset instead of snapping to the actual menu width. Together, these two bugs caused both the menu to look bloated and the
content displacement to differ depending on whether Threshold was set.

Description of Change:

The fix removes Threshold from both of those calculations entirely. Item sizing now uses only the item's configured WidthRequest or the default SwipeItemWidth, as it should. The snap distance is now computed as Math.Min(Threshold, menuWidth) — meaning if Threshold is set, it acts as a cap on how far the user needs to drag, but the content still snaps to the true menu width. If Threshold is not set, the default behaviour of 60% of menu width is preserved. This was applied consistently across iOS (MauiSwipeView.cs and SwipeViewExtensions.cs) and Android (MauiSwipeView.cs).

Tested the behavior in the following platforms:

  • Android
  • Windows
  • iOS
  • Mac

Reference:

N/A

Issues Fixed:

Fixes #6016

Screenshots

Before After
Before_fix_6016.mov
After_fix_6016.mov

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 13, 2026

🚀 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://github.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34923

Or

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

@dotnet-policy-service dotnet-policy-service Bot added the community ✨ Community Contribution label Apr 13, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Hey there @@KarthikRajaKalaimani! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service dotnet-policy-service Bot added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Apr 13, 2026
@sheiksyedm sheiksyedm marked this pull request as ready for review April 13, 2026 10:11
Copilot AI review requested due to automatic review settings April 13, 2026 10:11
@sheiksyedm
Copy link
Copy Markdown
Contributor

/azp run maui-pr-uitests , maui-pr-devicetests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes incorrect use of SwipeView.Threshold in the iOS and Android platform implementations so that Threshold affects only the trigger distance for opening, not the rendered swipe menu width or the final open offset.

Changes:

  • iOS/Android: stop using Threshold to size swipe items and to determine the open snap distance; compute open distance from actual menu size and treat Threshold as a cap for the trigger distance.
  • iOS/Android: rename internal “threshold” helper to reflect “open distance” semantics and update call sites.
  • Add a new UI test page + Appium test for Issue 6016 to validate menu width is unaffected by Threshold and that Execute mode still triggers.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Core/src/Platform/iOS/SwipeViewExtensions.cs Removes Threshold from swipe-item sizing calculations on iOS.
src/Core/src/Platform/iOS/MauiSwipeView.cs Uses menu open distance for snapping; uses Threshold only for trigger distance on iOS.
src/Core/src/Platform/Android/MauiSwipeView.cs Mirrors iOS behavior changes on Android; removes old reveal-threshold helper and stops sizing by Threshold.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue6016.cs Adds Appium UI coverage for the regression (menu width/offset + Execute mode).
src/Controls/tests/TestCases.HostApp/Issues/Issue6016.cs Adds HostApp reproduction page used by the new UI test.

Comment thread src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue6016.cs
Comment thread src/Core/src/Platform/iOS/MauiSwipeView.cs
Comment thread src/Core/src/Platform/Android/MauiSwipeView.cs
@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 Apr 13, 2026
@KarthikRajaKalaimani
Copy link
Copy Markdown
Contributor Author

🤖 AI Summary

👋 @KarthikRajaKalaimani — new AI review results are available. Please review the latest session below.

📊 Review Session2cf3f8e · Test case removed for windows platform · 2026-04-13 13:25 UTC
🔍 Pre-Flight — Context & Validation
Issue: #6016 - SwipeView Threshold changes width and offset of the side menu (when visible) PR: #34923 - Fix SwipeView Threshold changes width and offset of the side menu (when visible) Platforms Affected: Android (primary), iOS, macCatalyst (fix applied to both) Files Changed: 3 implementation, 2 test

Key Findings

  • Bug: GetSwipeThreshold() incorrectly used Element.Threshold as both item width AND snap distance, so setting Threshold=200 caused 200dp-wide menu items and 200dp content displacement instead of the intended ~100dp defaults.
  • Root cause (Android): GetSwipeItemSize() used threshold > 0 ? threshold : SwipeItemWidth for item width; GetSwipeThreshold(ISwipeItems) had early return if (threshold > 0) return threshold for snap distance.
  • Root cause (iOS): Same dual misuse in GetSwipeItemSize() (SwipeViewExtensions.cs) and GetSwipeThreshold(ISwipeItems) (MauiSwipeView.cs).
  • PR's fix: Renames GetSwipeThresholdGetSwipeOpenDistance (computes menu width only); ValidateSwipeThreshold now uses Min(Threshold, menuWidth) as trigger threshold if Threshold > 0, else 60% * menuWidth.
  • Gate: Without fix test FAILED correctly (proves bug exists). With fix got ENV ERROR — PowerShell regex match on null input in gate script, not a test failure; timing/infrastructure issue, not a code correctness issue.
  • Review comments (prior agent): WaitForXPosition doesn't assert on timeout (silent wrong X); _swipeThreshold field name misleading after rename; same on Android.
  • Test file wraps entire file in #if TEST_FAILS_ON_WINDOWS but [Issue] specifies PlatformAffected.Android — inconsistency; test should run on Android and iOS, not just non-Windows.
  • WaitForTextToBePresentInElement used in SwipeViewExecuteModeTriggers — need to confirm this API exists.

Fix Candidates

Source Approach Test Result Files Changed Notes

PR PR #34923 Remove Threshold from item sizing; rename to GetSwipeOpenDistance; trigger = Min(Threshold, menuWidth) ⚠️ ENV ERROR (Gate infra issue, not fix correctness) Android/MauiSwipeView.cs, iOS/MauiSwipeView.cs, iOS/SwipeViewExtensions.cs Original PR
🔧 Fix — Analysis & Comparison

Fix Candidates

Source Approach Test Result Files Changed Notes

1 try-fix (opus-4.6) Add GetMenuOpenDistance(ISwipeItems) helper; keeps existing method names; Android only ✅ PASS Android/MauiSwipeView.cs Clean separation, minimal call-site changes
2 try-fix (sonnet-4.6) Inline removal — no new helpers/renames, remove early-return + threshold from sizing, Min(Threshold,menuWidth) trigger; Android only ✅ PASS Android/MauiSwipeView.cs Smallest possible change
3 try-fix (codex) Add GetSnapOpenDistance cache field separate from trigger; iOS+Android ✅ PASS (Android; iOS unverified) iOS/MauiSwipeView.cs, iOS/SwipeViewExtensions.cs, Android/MauiSwipeView.cs Cross-platform but iOS not tested
4 try-fix (gpt-5.4) Harden test: TryWaitForXPosition returns (bool satisfied, float x), callers assert satisfaction; test-only fix ✅ PASS TestCases.Shared.Tests/Tests/Issues/Issue6016.cs Eliminates silent false-positive; Execute-mode API valid
PR PR #34923 Rename GetSwipeThreshold to GetSwipeOpenDistance; remove Threshold from sizing; Min(Threshold,menuWidth) trigger; iOS+Android ⚠️ ENV ERROR (gate infra, not fix correctness) iOS/MauiSwipeView.cs, iOS/SwipeViewExtensions.cs, Android/MauiSwipeView.cs Original PR; cross-platform; semantically clearest rename

Cross-Pollination

Model Round New Ideas? Details
claude-opus-4.6 2 NO NEW IDEAS Attempt 1 helper approach already complete for Android
claude-sonnet-4.6 2 YES Combine minimal platform fix (Attempt 2) + Attempt 4 test hardening
gpt-5.3-codex 2 NO NEW IDEAS Attempt 3 cross-platform already covered
gpt-5.4 2 YES Apply PR rename (most descriptive) + Attempt 4 test hardening together
Exhausted: Yes — all 4 models queried, 2 rounds completed Selected Fix: PR's fix + Attempt 4 test hardening — PR rename is most semantically clear (GetSwipeOpenDistance vs GetSwipeThreshold), covers iOS+Android, and the Gate's "without fix" phase confirmed the bug is real. Attempt 4 test hardening eliminates silent false-positives and should be incorporated.

📋 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase Status Notes
Pre-Flight ✅ COMPLETE Issue #6016, SwipeView Threshold bug on Android/iOS
Gate ❌ FAILED Without fix: ✅ FAIL (bug confirmed). With fix: ⚠️ ENV ERROR (gate infra null-input regex — not fix correctness)
Try-Fix ✅ COMPLETE 4 attempts, 4 passing; gemini unavailable (gpt-5.4 used)
Report ✅ COMPLETE

Summary

PR #34923 fixes a real bug (confirmed by Gate's "without fix" run) where SwipeView.Threshold incorrectly controlled both menu item width and content snap distance. The platform logic fix is sound and all 4 independent Try-Fix explorations confirmed the root cause and a correct fix strategy. However, the PR's test has a silent-failure bug (WaitForXPosition doesn't assert predicate satisfaction), and several issues found by the prior code review remain unaddressed. The Gate ENV ERROR is an infrastructure issue, not a correctness failure in the fix itself.

Root Cause

Two misuses of Element.Threshold in both Android and iOS MauiSwipeView:

  1. GetSwipeItemSize() — used threshold > 0 ? threshold : SwipeItemWidth as item width, inflating menu items to Threshold dp wide.
  2. GetSwipeThreshold(ISwipeItems) — had an early return if (threshold > 0) return threshold, so content snapped to Threshold dp offset instead of actual menu width.

Fix Quality

Platform fix (Android/iOS MauiSwipeView.cs + SwipeViewExtensions.cs): Correct. Removes Threshold from item sizing (always uses SwipeItemWidth), removes early-return from menu-width computation, applies Threshold only as a trigger cap: triggerThreshold = Min(Threshold, menuWidth) (or 60%*menuWidth if unset). All 4 independent Try-Fix models confirmed this approach.

Test (Issue6016.cs): Has actionable defects that should be fixed before merge:

  1. WaitForXPosition silent failure — Returns last observed X regardless of whether the predicate was satisfied. If a swipe gesture silently fails (content didn't move), defaultMenuWidth and thresholdMenuWidth both equal 0, and Assert.That(0, Is.EqualTo(0).Within(5)) passes falsely. Try-Fix Attempt 4 provides a tested fix: rename to TryWaitForXPosition, return bool satisfied, callers assert Is.True.
  2. _swipeThreshold field name misleading — The field now caches computed open distance (menu width/height), not the trigger threshold. Identified in prior code review. Should be renamed _swipeOpenDistance in both Android and iOS MauiSwipeView.cs to match GetSwipeOpenDistance.
  3. Missing newline at EOFIssue6016.cs (tests) is missing a final newline.
  4. #if TEST_FAILS_ON_WINDOWS scope vs PlatformAffected.Android — HostApp [Issue] declares PlatformAffected.Android but the entire test file is wrapped in #if TEST_FAILS_ON_WINDOWS, which would also run on iOS/macCatalyst. Either align the [Issue] attribute to PlatformAffected.All (minus Windows) or add an explicit [Platform] filter in the test.

Required Changes Before Merge

  • Fix WaitForXPositionTryWaitForXPosition (assert predicate satisfaction) per Try-Fix Attempt 4 diff
  • Rename _swipeThreshold_swipeOpenDistance in both Android/MauiSwipeView.cs and iOS/MauiSwipeView.cs
  • Add newline at end of TestCases.Shared.Tests/Tests/Issues/Issue6016.cs
  • Align PlatformAffected attribute with actual test platform scope

addressed AI summary concerns

@sheiksyedm
Copy link
Copy Markdown
Contributor

/azp run maui-pr-uitests , maui-pr-devicetests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented Apr 17, 2026

/azp run maui-pr-uitests , maui-pr-devicetests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Copy Markdown
Contributor

@kubaflo kubaflo left a comment

Choose a reason for hiding this comment

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

Looks like some tests are failing - could you please verify?

Copy link
Copy Markdown
Contributor

@kubaflo kubaflo left a comment

Choose a reason for hiding this comment

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

Could you please resolve conflicts?

@KarthikRajaKalaimani
Copy link
Copy Markdown
Contributor Author

Could you please resolve conflicts?

resolved the conflicts

@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented Apr 28, 2026

/azp run maui-pr-uitests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

PureWeen pushed a commit that referenced this pull request Apr 30, 2026
…ability (#35133)

<!-- 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!

> **Depends on #35136** (pipeline category detection — should merge
first)

## What this does

Two things:

### 1. UI test category detection in PR review

During the PR review workflow, Step 0.5 detects which UI test categories
the PR impacts and writes the result to the AI summary comment. This
gives reviewers visibility into which UI tests are relevant.

**Detection** reuses the 3-tier script from #35136 (test attributes →
source paths → AI reasoning).

**AI summary** shows a new 🧪 UI Tests section with detected categories
before the gate section.

### 2. Gate reliability fixes

Multiple fixes to make the gate (`verify-tests-fail.ps1`) more
deterministic:

| Fix | Problem it solves |
|-----|-------------------|
| **Absolute path resolution** | Gate scripts not found on Linux CI
agents (`Resolve-Path`, `GetFullPath`) |
| **File existence check** | Instant cryptic failure when verify script
is missing — now logs clear error |
| **3x retry on ENV ERROR** | Emulator timeouts, ADB failures, app
crashes — transient issues that pass on retry |
| **Strip bad report blocks** | Old verify script produces `Passed:
False` with empty counts — stripped instead of shown |
| **Gate log in fallback** | When report is missing, shows last 20 lines
of gate output instead of just `❌ FAILED / Platform: IOS` |

## Files

| File | Changes |
|------|---------|
| `.github/scripts/Review-PR.ps1` | Step 0.5 category detection + all 5
gate fixes |
| `.github/scripts/post-ai-summary-comment.ps1` | Add `uitests` phase to
render detected categories |
| `.github/pr-review/pr-preflight.md` | Step 7: AI identifies impacted
UI test categories |

## Validation — PR reviewer builds (Apr 26)

10 builds against real PRs — all succeeded ✅. Category detection shown
in AI summary comment.

| PR | Categories Detected | Build | AI Summary |
|----|-------------------|-------|------------|
| #35037 (WebView theme) | `ViewBaseTests,WebView` |
[13940071](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940071)
|
[comment](#35037 (comment))
|
| #35031 (Shell memory leak) | `Shell` |
[13940072](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940072)
|
[comment](#35031 (comment))
|
| #35020 (XAML Hot Reload) | _(none — XAML only)_ |
[13940073](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940073)
| ✅ Shows "No UI test categories" |
| #35008 (Shell SearchHandler) | `Shell` |
[13940074](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940074)
| ✅ |
| #34997 (RadioButton gradient) | `RadioButton,ViewBaseTests` |
[13940075](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940075)
| ✅ |
| #34980 (DatePicker rotation) | `ViewBaseTests` |
[13940076](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940076)
| ✅ |
| #34974 (Picker CharacterSpacing) | `ViewBaseTests` |
[13940077](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940077)
| ✅ |
| #34923 (SwipeView threshold) | `SwipeView,ViewBaseTests` |
[13940078](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940078)
| ✅ |
| #34907 (CollectionView ScrollTo) | `CollectionView` |
[13940079](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940079)
| ✅ |
| #34845 (RefreshView binding) | `RefreshView,ViewBaseTests` |
[13940080](https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=13940080)
| ✅ |

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@dotnet dotnet deleted a comment from MauiBot May 2, 2026
Copy link
Copy Markdown
Collaborator

@MauiBot MauiBot left a comment

Choose a reason for hiding this comment

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

Expert Review — 7 findings

See inline comments for details.

@dotnet dotnet deleted a comment from MauiBot May 3, 2026
@dotnet dotnet deleted a comment from MauiBot May 3, 2026
@dotnet dotnet deleted a comment from MauiBot May 3, 2026
@dotnet dotnet deleted a comment from MauiBot May 3, 2026
@dotnet dotnet deleted a comment from MauiBot May 3, 2026
@dotnet dotnet deleted a comment from MauiBot May 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration 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)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SwipeView Threshold changes width and offset of the side menu (when visible)

6 participants