[Android] Fix VerifyFlowDirectionRTLCanReorderItemsTrueWithCanMixGroups test failure regression#35000
Merged
Conversation
dotnet-policy-service
Bot
added
the
partner/syncfusion
SuthiYuvaraj
changed the title
sheiksyedm
added
area-testing
Contributor
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This was referenced Apr 17, 2026
Open
kubaflo
approved these changes
Apr 17, 2026
Ahamed-Ali
pushed a commit
that referenced
this pull request
Apr 22, 2026
…ps test failure regression (#35000) <!-- 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! ### Issue Description Fixes a regression introduced by PR #31867, where all grouped CollectionView drag-and-drop reorder tests fail on Android — most visibly `VerifyFlowDirectionRTLCanReorderItemsTrueWithCanMixGroups`. ### Root cause PR #31867 changed SimpleItemTouchHelperCallback.OnMove from checking the target view type to checking the source view type, so that items can be dropped onto empty group headers. The side effect: OnItemMove can now be called with a non-empty group's GroupHeader as toPosition. When dragging an item forward past the next group's header (e.g., "Banana" toward "Potato"), OnItemMove fires with the GroupHeader as target and prematurely inserts the item mid-gesture, before the drop is intentional. LTR tests pass accidentally — the premature insertion moves the item downward, so the newY > initialY assertion coincidentally passes. RTL test fails — the mid-gesture NotifyItemMoved causes a layout pass that makes ItemTouchHelper's internal gesture offset stale in RTL coordinate space, aborting the drag and leaving newY == initialY. ### Description of Change Added a guard in `ReorderableItemsViewAdapter.OnItemMove:` when the drag targets a non-empty group's header (toIndex == 0, HasHeader == true, Count > 0), return false immediately — no data mutation, drag continues cleanly. Empty group headers are still allowed as drop targets, preserving the original PR #31867 intent. ### Issues Fixed - Regression introduced by PR #31867 - **Resolved test case:** VerifyFlowDirectionRTLCanReorderItemsTrueWithCanMixGroups ### Output | Before | After | |----------|----------| | <img src="https://github.com/user-attachments/assets/e61cb83d-d631-4a75-9715-85366cad9593"> | <img src="https://github.com/user-attachments/assets/9f10fd8b-f173-426d-ae54-79daa82beb56">|
github-actions
Bot
added
the
area-controls-collectionview
PureWeen
added a commit
that referenced
this pull request
May 18, 2026
Multi-model review (3 independent reviewers w/ gh-aw-guide context) found: 1. (2/3) Stale doc rationale on roles: all comment — implied agent has no filesystem access, but checkout: false was removed in 33a15f1 so the agent CAN read workspace files. Real protection is the gh-aw restore_base_github_folders.sh step that restores .github/ from the base branch AFTER the PR-branch checkout. Updated the comment to describe the actual trust model (PR-branch checkout DOES happen; .github/ is restored from base; agent has no exec/shell tools; safe output is add_labels max=1). 2. (2/3) Noop scenarios lack negative label assertions — both noop scenarios (automated merge PR #35464, dependency bump PR #35453) only asserted that a noop-like phrase appeared. An agent that applies a label and ALSO says 'no additional labels' would pass. Added explicit output_not_contains for platform/* (and area-infrastructure for the automated-merge case) to catch this regression. 3. (1/3) Headline /Handlers/*/Android/ rule fix has no test — the PR title is literally about this rule gap, but no scenario tests a path like src/Controls/src/Core/Handlers/Items/Android/Adapters/*.cs (no .android.cs extension). Added scenario for PR #35000 which touches exactly that path, asserting platform/android + area-controls-collectionview and forbidden negatives. 4. (1/3) SKILL.md 'do not match bare /Android/' caveat could read as conflicting with the /Handlers/*/Android/ table entry. Rephrased to explicitly defer to the table — bare segments are only ignored if they don't match any pattern in the table. Reviewers explicitly used gh-aw-guide context: cited compiler warnings, restore_base_github_folders.sh, --add-dir GITHUB_WORKSPACE, lock.yml internals, safe-outputs max enforcement. Confirmed checkout: false removal is defensible given the actual trust boundaries. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced May 18, 2026
This was referenced May 22, 2026
Draft
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 Description
Fixes a regression introduced by PR #31867, where all grouped CollectionView drag-and-drop reorder tests fail on Android — most visibly
VerifyFlowDirectionRTLCanReorderItemsTrueWithCanMixGroups.Root cause
PR #31867 changed SimpleItemTouchHelperCallback.OnMove from checking the target view type to checking the source view type, so that items can be dropped onto empty group headers. The side effect: OnItemMove can now be called with a non-empty group's GroupHeader as toPosition.
When dragging an item forward past the next group's header (e.g., "Banana" toward "Potato"), OnItemMove fires with the GroupHeader as target and prematurely inserts the item mid-gesture, before the drop is intentional.
LTR tests pass accidentally — the premature insertion moves the item downward, so the newY > initialY assertion coincidentally passes.
RTL test fails — the mid-gesture NotifyItemMoved causes a layout pass that makes ItemTouchHelper's internal gesture offset stale in RTL coordinate space, aborting the drag and leaving newY == initialY.
Description of Change
Added a guard in
ReorderableItemsViewAdapter.OnItemMove:when the drag targets a non-empty group's header (toIndex == 0, HasHeader == true, Count > 0), return false immediately — no data mutation, drag continues cleanly. Empty group headers are still allowed as drop targets, preserving the original PR #31867 intent.Issues Fixed
Output