Skip to content

[Windows] Fix Indicator View Causing Looping Issue with CarouselView#27880

Open
Tamilarasan-Paranthaman wants to merge 8 commits intodotnet:mainfrom
Tamilarasan-Paranthaman:fix-27563
Open

[Windows] Fix Indicator View Causing Looping Issue with CarouselView#27880
Tamilarasan-Paranthaman wants to merge 8 commits intodotnet:mainfrom
Tamilarasan-Paranthaman:fix-27563

Conversation

@Tamilarasan-Paranthaman
Copy link
Copy Markdown
Member

@Tamilarasan-Paranthaman Tamilarasan-Paranthaman commented Feb 18, 2025

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!

Root Cause of the issue

  • In the UpdateCurrentItem method, the view scrolls to the current item position. When the item position is updated from the IndicatorView, it triggers the Scrolled event, which then updates the position based on the current index. In the UpdatePosition method, the current item is continuously updated even while the view is still in a scrolling state, leading to an infinite scrolling loop and other position related issues.

Description of Change

  • Introduced a _lastScrolledToPosition tracking field to prevent re-entrant ScrollTo calls that cause the infinite loop.
  • Modified UpdateCurrentItem to only issue ScrollTo when the position actually differs AND has not already been scrolled to.
  • Added ScrollTo in UpdatePosition (guarded by IsDragging/IsScrolling checks) so IndicatorView-driven position changes scroll the carousel correctly.
  • Reset the tracker on user scrolling (CarouselScrolled) and collection source changes (OnCollectionItemsSourceChanged).
  • Removed the Loop centering code from UpdateInitialPosition (see code review note about potential regression).

Why tests are restricted on Windows and Mac:

  • The test added in this PR for Issue27563 is currently restricted on Windows and MacCatalyst because it relies on Appium-driven CarouselView swipe/position behavior, which is not reliable on these platforms.
  • For Windows, this aligns with the existing issue tracked in [Testing] Appium test does not perform the tap action with CarouselView. #29245, where CarouselView automation leads to timeouts and failures unrelated to the fix. Once this issue is resolved, we can remove the platform restriction.
  • Keeping the test behind platform guards helps avoid introducing known UI test flakiness into the PR.

Issues Fixed

Fixes #27563
Fixes #22468
Fixes #7149

Tested the behaviour in the following platforms

  • Windows
  • Android
  • iOS
  • Mac

Screenshot

Before Issue Fix After Issue Fix
Before-Fix.mp4
After-Fix.mp4

@dotnet-policy-service dotnet-policy-service bot added community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration labels Feb 18, 2025
@jsuarezruiz jsuarezruiz added the area-controls-collectionview CollectionView, CarouselView, IndicatorView label Feb 18, 2025
@Tamilarasan-Paranthaman Tamilarasan-Paranthaman marked this pull request as ready for review February 24, 2025 13:32
@Tamilarasan-Paranthaman Tamilarasan-Paranthaman requested a review from a team as a code owner February 24, 2025 13:32
@jsuarezruiz
Copy link
Copy Markdown
Contributor

/rebase

@jsuarezruiz
Copy link
Copy Markdown
Contributor

/rebase

@jsuarezruiz
Copy link
Copy Markdown
Contributor

/rebase

@jsuarezruiz
Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).


[Test]
[Category(UITestCategories.CarouselView)]
public void VerifyCarouselViewScrolling()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The test is failing on Mac and Windows:
image

   at UITest.Appium.HelperExtensions.Wait(Func`1 query, Func`2 satisfactory, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2367
   at UITest.Appium.HelperExtensions.WaitForAtLeastOne(Func`1 query, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 2384
   at UITest.Appium.HelperExtensions.WaitForElement(IApp app, String marked, String timeoutMessage, Nullable`1 timeout, Nullable`1 retryFrequency, Nullable`1 postTimeout) in /_/src/TestUtils/src/UITest.Appium/HelperExtensions.cs:line 665
   at Microsoft.Maui.TestCases.Tests.Issues.Issue27563.VerifyCarouselViewScrolling() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue27563.cs:line 21
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The test failed with a timeout exception on both Windows and Mac.

On Mac, this appears to be due to the lack of support for SwipeRightToLeft. We can modify the test case specifically for the Mac platform.

On Windows, the test ran for an extended period before failing with a timeout exception.

I conducted further testing on a local machine and found that the test passed when CarouselView.Loop was set to false. This suggests that the issue occurs only when CarouselView.Loop is set to true. Additionally, I identified other test cases where Loop is enabled, and they also include the TEST_FAILS_ON_WINDOWS condition referencing this issue (e.g., Script: 12574 - Sample - 12574).

However, in the CI environment, the failure when Loop is enabled appears to be due to a different issue.

@jsuarezruiz, could you please share your insights on this and suggest how we should proceed on Windows platforms?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jsuarezruiz @Tamilarasan-Paranthaman is there a way forward with this PR?

@bronteq
Copy link
Copy Markdown

bronteq commented Jul 22, 2025

@jsuarezruiz friendly ping

@jsuarezruiz
Copy link
Copy Markdown
Contributor

/rebase

Copilot AI review requested due to automatic review settings October 31, 2025 12:17
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@jsuarezruiz
Copy link
Copy Markdown
Contributor

Rebased and running a new build.


[Test]
[Category(UITestCategories.CarouselView)]
public void VerifyCarouselViewScrolling()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can extend the new test to cover loop initial centering and the reentrancy case (tap indicator repeatedly while swiping), and a test that toggles Position to the same value to ensure no scroll storm occurs.

@Tamilarasan-Paranthaman Tamilarasan-Paranthaman marked this pull request as draft December 3, 2025 12:33
@kubaflo kubaflo added the stale Indicates a stale issue/pr and will be closed soon label Mar 8, 2026
@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented Mar 8, 2026

Hi, is this still valid?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 10, 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://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 27880

Or

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

@Vignesh-SF3580
Copy link
Copy Markdown
Contributor

Hi, is this still valid?

@kubaflo I have verified this on my end, and it is still valid PR. The issue still reproduces on the latest main source and is resolved by the changes made in this PR.

Without Fix With Fix
WithoutFix27880.mp4
WithFix27880.mp4

@Tamilarasan-Paranthaman Tamilarasan-Paranthaman marked this pull request as ready for review March 11, 2026 11:21
@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented Mar 11, 2026

📋 PR Finalization Review

Title: ✅ Good

Current: [Windows] Fix Indicator View Causing Looping Issue with CarouselView

Description: ⚠️ Needs Update

The description has good technical content:

  • ✅ Clear root cause explanation (ScrollTo → Scrolled → UpdatePosition loop)

  • ✅ Description of change with rationale

  • ✅ Issues Fixed section (3 issues)

  • ✅ Platform testing matrix (all 4 platforms)

  • ✅ Before/after video comparisons

  • Missing required NOTE block at the top for testing PR artifacts

  • 🟡 Claim about 'aligning with Android' is imprecise — Android CarouselViewHandler delegates to MauiCarouselRecyclerView with a fundamentally different architecture; the behavioral outcome is similar but the implementation pattern is different
    Missing Elements:

  • Required NOTE block must be prepended at the top of the description (this is mandatory for all PRs)

  • Otherwise the existing description is solid and should be preserved

✨ Suggested PR Description

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

Root Cause of the issue

  • In the UpdateCurrentItem method, the view scrolls to the current item position. When the item position is updated from the IndicatorView, it triggers the Scrolled event, which then updates the position based on the current index. In the UpdatePosition method, the current item is continuously updated even while the view is still in a scrolling state, leading to an infinite scrolling loop and other position related issues.

Description of Change

  • Introduced a _lastScrolledToPosition tracking field to prevent re-entrant ScrollTo calls that cause the infinite loop.
  • Modified UpdateCurrentItem to only issue ScrollTo when the position actually differs AND has not already been scrolled to.
  • Added ScrollTo in UpdatePosition (guarded by IsDragging/IsScrolling checks) so IndicatorView-driven position changes scroll the carousel correctly.
  • Reset the tracker on user scrolling (CarouselScrolled) and collection source changes (OnCollectionItemsSourceChanged).
  • Removed the Loop centering code from UpdateInitialPosition (see code review note about potential regression).

Issues Fixed

Fixes #27563
Fixes #22468
Fixes #7149

Tested the behaviour in the following platforms

  • Windows
  • Android
  • iOS
  • Mac

Screenshot

Before Issue Fix After Issue Fix
Before-Fix.mp4
After-Fix.mp4
Code Review: ⚠️ Issues Found

🔴 Critical Issues

1. Removed Loop centering code in UpdateInitialPosition

  • File: src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs
  • Problem: The original code had a block that set _loopableCollectionView.CenterMode = true, called ListViewBase.ScrollIntoView(item), then reset CenterMode = false when Element.Loop was enabled. This entire block was removed. For Loop mode, the carousel needs to be centered in the virtualized list to allow bi-directional scrolling. Without this, Loop mode initial positioning may regress on Windows.
  • Recommendation: Verify that Loop mode still works correctly on initial load. If the centering was removed intentionally, add a code comment explaining why it is no longer needed.

2. UI test excluded from Windows — the target platform

  • File: src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue27563.cs
  • Problem: The test uses #if TEST_FAILS_ON_WINDOWS && TEST_FAILS_ON_CATALYST which excludes it from both Windows and MacCatalyst. Since this is a Windows-targeted fix, the test cannot verify the fix on the platform it targets. The comment references issue [Testing] Appium test does not perform the tap action with CarouselView. #29245 (TimeoutException with Loop), but the test page does not even use Loop = true.
  • Recommendation: Investigate whether the Windows exclusion is still necessary. If the Loop-related issue ([Testing] Appium test does not perform the tap action with CarouselView. #29245) does not affect this test scenario (no Loop in the test page), consider removing the Windows exclusion so CI validates the fix on Windows.

🟡 Suggestions

  1. Snapshot label shows stale position — Both Android and iOS reference snapshots show "CarouselView Position - 0" even though the carousel is visually at position 2 ("Percentage View"). The HostApp button handler reads carousel.Position synchronously after setting indicatorView.Position = 2, before the carousel finishes scrolling. Consider using a PropertyChanged handler on CarouselView.Position to update the label asynchronously for accurate snapshot verification.

  2. CarouselScrolled reset edge case — The tracker reset (_lastScrolledToPosition = -1) in CarouselScrolled only fires when position != _lastScrolledToPosition. If a user manually scrolls back to the same position the tracker already holds, it will not reset, which could prevent a subsequent programmatic scroll to that index.

  3. Missing newline at end of file — Both Issue27563.cs files (HostApp and Shared.Tests) are missing a trailing newline.

✅ Looks Good

  1. The _lastScrolledToPosition tracking mechanism is a clean, effective approach to break the infinite ScrollTo → Scrolled → UpdatePosition loop
  2. Good use of IsDragging and IsScrolling guards in UpdatePosition to prevent programmatic scrolls during user interaction
  3. Proper reset of _lastScrolledToPosition in OnCollectionItemsSourceChanged ensures stale state does not persist after data changes
  4. The HostApp test page uses C# only (preferred pattern), has correct AutomationId attributes, and appropriate [Issue] attribute
  5. Single [Category(UITestCategories.CarouselView)] on the test (correct per guidelines)
  6. Fixes 3 long-standing related issues ([Windows] Indicator view causes looping with CarouselView #27563, CarouselView behaves strangely when manipulating position in ViewModel #22468, [Windows] [Scenario Day] Scroll by Object not working the first time #7149) with a single cohesive approach

@Vignesh-SF3580
Copy link
Copy Markdown
Contributor

📋 PR Finalization Review

Title: ✅ Good

Current: [Windows] Fix Indicator View Causing Looping Issue with CarouselView

Description: ⚠️ Needs Update

The description has good technical content:

  • ✅ Clear root cause explanation (ScrollTo → Scrolled → UpdatePosition loop)
  • ✅ Description of change with rationale
  • ✅ Issues Fixed section (3 issues)
  • ✅ Platform testing matrix (all 4 platforms)
  • ✅ Before/after video comparisons
  • Missing required NOTE block at the top for testing PR artifacts
  • 🟡 Claim about 'aligning with Android' is imprecise — Android CarouselViewHandler delegates to MauiCarouselRecyclerView with a fundamentally different architecture; the behavioral outcome is similar but the implementation pattern is different
    Missing Elements:
  • Required NOTE block must be prepended at the top of the description (this is mandatory for all PRs)
  • Otherwise the existing description is solid and should be preserved

✨ Suggested PR Description

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

Root Cause of the issue

  • In the UpdateCurrentItem method, the view scrolls to the current item position. When the item position is updated from the IndicatorView, it triggers the Scrolled event, which then updates the position based on the current index. In the UpdatePosition method, the current item is continuously updated even while the view is still in a scrolling state, leading to an infinite scrolling loop and other position related issues.

Description of Change

  • Introduced a _lastScrolledToPosition tracking field to prevent re-entrant ScrollTo calls that cause the infinite loop.
  • Modified UpdateCurrentItem to only issue ScrollTo when the position actually differs AND has not already been scrolled to.
  • Added ScrollTo in UpdatePosition (guarded by IsDragging/IsScrolling checks) so IndicatorView-driven position changes scroll the carousel correctly.
  • Reset the tracker on user scrolling (CarouselScrolled) and collection source changes (OnCollectionItemsSourceChanged).
  • Removed the Loop centering code from UpdateInitialPosition (see code review note about potential regression).

Issues Fixed

Fixes #27563 Fixes #22468 Fixes #7149

Tested the behaviour in the following platforms

  • Windows
  • Android
  • iOS
  • Mac

Screenshot

Before Issue Fix After Issue Fix
Before-Fix.mp4
After-Fix.mp4
Code Review: ⚠️ Issues Found

@MauiBot
Copy link
Copy Markdown
Collaborator

MauiBot commented Mar 21, 2026

🤖 AI Summary

📊 Expand Full Reviewf26b850 · tests updated.
🔍 Pre-Flight — Context & Validation

Issue: #27563 - [Windows] Indicator view causes looping with CarouselView
PR: #27880 - [Windows] Fix Indicator View Causing Looping Issue with CarouselView
Platforms Affected: Windows (fix), Android/iOS (tests)
Files Changed: 1 implementation, 2 test files, 2 snapshots

Key Findings

  • Root cause: In UpdateCurrentItem, ScrollTo fires unconditionally. When IndicatorView updates position, it triggers ScrolledUpdatePositionScrollTo in a loop.
  • Fix adds _lastScrolledToPosition tracking field to guard against re-entrant ScrollTo calls in both UpdateCurrentItem and UpdatePosition.
  • Also removes Loop centering code from UpdateInitialPosition — PR author notes this as a potential regression for Loop feature.
  • Tests added (Issue27563.cs) but both tests are excluded from Windows using #if !WINDOWS:
  • The #if !WINDOWS guard is a blanket exclusion citing Loop-related timeouts, but VerifyCarouselViewIndicatorPositionWithoutLooping does NOT enable Loop — the exclusion may be unnecessarily broad.
  • Gate FAILED: No Windows tests exist to validate the Windows-specific fix.
  • Bug in CarouselScrolled reset logic: _lastScrolledToPosition is reset to -1 when position != _lastScrolledToPosition, but this means any user scroll to a different position resets tracking. This may cause issues in some scroll→indicator→scroll sequences.
  • Reviewer (jsuarezruiz) asked to extend the test to cover "loop initial centering and the reentrancy case (tap indicator repeatedly while swiping)" — not addressed.

Fix Candidates

# Source Approach Test Result Files Changed Notes
PR PR #27880 Track _lastScrolledToPosition to guard ScrollTo re-entry ❌ FAILED (Gate) CarouselViewHandler.Windows.cs Tests excluded from Windows

🔧 Fix — Analysis & Comparison

Fix Candidates

# Source Approach Test Result Files Changed Notes
1 try-fix (claude-opus-4.6) Remove #if !WINDOWS from VerifyCarouselViewIndicatorPositionWithoutLooping FAIL Issue27563.cs (test) App unresponsive on Windows; WinAppDriver proxy timeout (pre-existing #29245)
2 try-fix (claude-sonnet-4.6) Re-entrancy bool flag _isScrollingProgrammatically reset in OnScrollViewChanged PASS (build) CarouselViewHandler.Windows.cs Build 0 errors/0 warnings; cleaner semantics than int tracking; avoids subtle edge cases
3 try-fix (gpt-5.3-codex) Block CarouselScrolled sync-back during programmatic scroll FAIL (env) CarouselViewHandler.Windows.cs Build env TFM mismatch; approach conceptually valid but different risk profile
4 try-fix (gpt-5.4) Restore Loop centering + int tracking (like PR) BLOCKED (env) CarouselViewHandler.Windows.cs Build env issues; approach is PR's fix + restored Loop centering
PR PR #27880 Track _lastScrolledToPosition (int) to guard ScrollTo re-entry; removes Loop centering FAILED (Gate) CarouselViewHandler.Windows.cs Tests excluded from Windows (#if !WINDOWS)

Cross-Pollination

Model Round New Ideas? Details
claude-opus-4.6 2 Yes Defer ScrollTo via Dispatcher.Dispatch to break synchronous re-entrant call stack
claude-sonnet-4.6 2 Yes Position equality guard compare target position vs CarouselView.Position before ScrollTo
gpt-5.3-codex 2 Yes Idempotent check: bail if target index equals both Position and CurrentItem
gpt-5.4 2 Yes Use SetValueFromRenderer instead of direct property sets to avoid triggering MapPosition feedback loop
claude-opus-4.6 3 NO NEW IDEAS
claude-sonnet-4.6 3 NO NEW IDEAS
gpt-5.3-codex 3 NO NEW IDEAS
gpt-5.4 3 NO NEW IDEAS

Exhausted: Yes (Round 2 new ideas not actionable due to build environment constraints; 4 models confirm NO NEW IDEAS in round 3)
Selected Fix: Candidate #2 (bool re-entrancy flag) Only validated passing fix; cleaner semantics than PR's int tracking


📋 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase Status Notes
Pre-Flight ✅ COMPLETE Issue #27563 identified; Windows-only bug; 5 files changed
Gate ❌ FAILED Windows — both tests excluded via #if !WINDOWS
Try-Fix ✅ COMPLETE 4 attempts; 1 passing (bool re-entrancy flag, build validated)
Report ✅ COMPLETE

Summary

PR #27880 fixes a Windows CarouselView infinite scrolling loop caused by IndicatorView interactions, but the gate failed because both added tests are excluded from Windows with #if !WINDOWS guards. A PR that fixes a Windows-specific bug with no Windows test coverage is the core problem. Try-Fix found a cleaner alternative implementation (bool flag instead of int position tracking). The PR also removes Loop centering code from UpdateInitialPosition, which the author acknowledges may be a regression.

Root Cause

In CarouselViewHandler.Windows.cs, UpdateCurrentItem calls ItemsView.ScrollTo(...) unconditionally. This triggers the Scrolled event → CarouselScrolledSetCarouselViewPositionMapPositionUpdatePosition → another ScrollTo, creating an infinite re-entrancy loop when an IndicatorView is connected to the CarouselView.

Fix Quality

Issues with the PR's fix:

  1. No Windows test coverage (critical): Both tests use #if !WINDOWS, meaning the Windows fix has zero automated validation. The tests cite issue [Testing] Appium test does not perform the tap action with CarouselView. #29245 (WinAppDriver timeout with Loop), but VerifyCarouselViewIndicatorPositionWithoutLooping does not use Loop = true — the blanket exclusion is unnecessarily broad.

  2. Subtle reset logic in CarouselScrolled: The reset condition if (position != _lastScrolledToPosition) { _lastScrolledToPosition = -1; } is inverted — it resets the tracker only when the scrolled position is different from what was tracked. This means: if a user scrolls to the same position that was programmatically scrolled to, the tracker isn't reset. Subsequent programmatic scrolls to that same index will be silently blocked. A cleaner alternative (bool flag reset in OnScrollViewChanged when !e.IsIntermediate) avoids this edge case entirely.

  3. Removed Loop centering code (potential regression): The PR removes the Loop centering block from UpdateInitialPosition and states this is intentional, but acknowledges a "potential regression." This changes behavior for apps using Loop = true — initial centering no longer happens via CenterMode. The PR does not include a regression test for Loop behavior.

  4. UpdatePosition adds ScrollTo with incomplete guard: The new ScrollTo in UpdatePosition is guarded by !IsDragging && !IsScrolling, but IsDragging/IsScrolling are set in OnScrollViewChanging/OnScrollViewChanged (WinUI scroll viewer events). Between a programmatic ScrollTo call and the viewer's ViewChanging event firing, these flags may still be false — the guard may not prevent all re-entry paths.

Agent's alternative (Candidate #2):
A bool _isScrollingProgrammatically flag (set before ScrollTo, reset in OnScrollViewChanged when !e.IsIntermediate) is simpler, has no subtle edge cases with position comparison, and compiled with 0 errors/0 warnings. It directly expresses the invariant: "one programmatic scroll at a time."

Reviewer feedback not addressed: jsuarezruiz asked to extend the test to cover "loop initial centering and the reentrancy case (tap indicator repeatedly while swiping)". This is still unresolved.


@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 Mar 21, 2026
PureWeen pushed a commit that referenced this pull request Mar 23, 2026
#34575)

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

## Description

Adds Windows platform support to the `maui-copilot` CI pipeline (AzDO
definition 27723), enabling Copilot PR reviews on Windows-targeted PRs.

### Changes

**`eng/pipelines/ci-copilot.yml`**
- Add `catalyst` and `windows` to Platform parameter values
- Add per-platform pool selection (`androidPool`, `iosPool`, `macPool`,
`windowsPool`)
- Skip Xcode, Android SDK, simulator setup for Windows/Catalyst
- Add Windows-specific "Set screen resolution" step (1920x1080)
- Add MacCatalyst-specific "Disable Notification Center" step
- Fix `sed` command for `Directory.Build.Override.props` on Windows (Git
Bash uses GNU sed)
- Handle Copilot CLI PATH detection on Windows vs Unix
- Change `script:` steps to `bash:` for cross-platform consistency

**`.github/scripts/Review-PR.ps1`**
- Add `catalyst` to ValidateSet for Platform parameter

**`.github/scripts/BuildAndRunHostApp.ps1`**
- Add Windows test assembly directory for artifact collection

**`.github/scripts/post-ai-summary-comment.ps1` /
`post-pr-finalize-comment.ps1`**
- Various improvements for cross-platform comment posting

### Validation

Successfully ran the pipeline with `Platform=windows` on multiple
Windows-specific PRs:
- PR #27713 — ✅ Succeeded
- PR #34337 — ✅ Succeeded
- PR #26217, #27609, #27880, #28617, #29927, #30068 — Triggered and
running

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@Vignesh-SF3580
Copy link
Copy Markdown
Contributor

🤖 AI Summary

📊 Expand Full Reviewce189d0 · Update CarouselViewHandler.Windows.cs

The concern in the AI summary is that the fix is specific to Windows, but the test is restricted on Windows. The AI suggested adding a Windows test to validate the fix.
However, we are facing issue (#29245) on Windows when enabling loop, so the test has been restricted on that platform. I have already documented the details in the test case comments and updated the PR description.
Since a reliable Windows test is not currently feasible, the test remains restricted on Windows.

@MauiBot
Copy link
Copy Markdown
Collaborator

MauiBot commented Mar 29, 2026

🚦 Gate - Test Before and After Fix

📊 Expand Full Gatef26b850 · tests updated.

Gate Result: ❌ FAILED

Platform: WINDOWS · Base: main · Merge base: 794a9fa6

Test Without Fix (expect FAIL) With Fix (expect PASS)
🖥️ Issue27563 Issue27563 ✅ FAIL — 516s ❌ FAIL — 423s
🔴 Without fix — 🖥️ Issue27563: FAIL ✅ · 516s
  Determining projects to restore...
  Restored D:\a\1\s\src\Graphics\src\Graphics.Win2D\Graphics.Win2D.csproj (in 19.81 sec).
  Restored D:\a\1\s\src\Graphics\src\Graphics\Graphics.csproj (in 19.81 sec).
  Restored D:\a\1\s\src\Essentials\src\Essentials.csproj (in 9.44 sec).
  Restored D:\a\1\s\src\Core\src\Core.csproj (in 15.54 sec).
  Restored D:\a\1\s\src\Core\maps\src\Maps.csproj (in 13 sec).
  Restored D:\a\1\s\src\Controls\src\Xaml\Controls.Xaml.csproj (in 65 ms).
  Restored D:\a\1\s\src\Controls\src\Core\Controls.Core.csproj (in 19 ms).
  Restored D:\a\1\s\src\Controls\Maps\src\Controls.Maps.csproj (in 21 ms).
  Restored D:\a\1\s\src\Controls\tests\TestCases.HostApp\Controls.TestCases.HostApp.csproj (in 7.11 sec).
  Restored D:\a\1\s\src\Controls\Foldable\src\Controls.Foldable.csproj (in 13 ms).
  Restored D:\a\1\s\src\BlazorWebView\src\Maui\Microsoft.AspNetCore.Components.WebView.Maui.csproj (in 22 ms).
  3 of 14 projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Graphics -> D:\a\1\s\artifacts\bin\Graphics\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Essentials -> D:\a\1\s\artifacts\bin\Essentials\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Graphics.Win2D -> D:\a\1\s\artifacts\bin\Graphics.Win2D\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.Win2D.WinUI.Desktop.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Core -> D:\a\1\s\artifacts\bin\Core\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.dll
  Controls.BindingSourceGen -> D:\a\1\s\artifacts\bin\Controls.BindingSourceGen\Debug\netstandard2.0\Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Controls.Core -> D:\a\1\s\artifacts\bin\Controls.Core\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Maps -> D:\a\1\s\artifacts\bin\Maps\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Maps.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Microsoft.AspNetCore.Components.WebView.Maui -> D:\a\1\s\artifacts\bin\Microsoft.AspNetCore.Components.WebView.Maui\Debug\net10.0-windows10.0.19041.0\Microsoft.AspNetCore.Components.WebView.Maui.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Controls.Xaml -> D:\a\1\s\artifacts\bin\Controls.Xaml\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.Xaml.dll
  Controls.Foldable -> D:\a\1\s\artifacts\bin\Controls.Foldable\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.Foldable.dll
  Controls.Maps -> D:\a\1\s\artifacts\bin\Controls.Maps\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.Maps.dll
  Controls.TestCases.HostApp -> D:\a\1\s\artifacts\bin\Controls.TestCases.HostApp\Debug\net10.0-windows10.0.19041.0\win-x64\Controls.TestCases.HostApp.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:05:48.63
  Determining projects to restore...
  Restored D:\a\1\s\src\TestUtils\src\VisualTestUtils\VisualTestUtils.csproj (in 721 ms).
  Restored D:\a\1\s\src\TestUtils\src\UITest.NUnit\UITest.NUnit.csproj (in 1.32 sec).
  Restored D:\a\1\s\src\TestUtils\src\UITest.Core\UITest.Core.csproj (in 3 ms).
  Restored D:\a\1\s\src\TestUtils\src\UITest.Appium\UITest.Appium.csproj (in 1.05 sec).
  Restored D:\a\1\s\src\TestUtils\src\VisualTestUtils.MagickNet\VisualTestUtils.MagickNet.csproj (in 6.52 sec).
  Restored D:\a\1\s\src\TestUtils\src\UITest.Analyzers\UITest.Analyzers.csproj (in 5.16 sec).
  Restored D:\a\1\s\src\Controls\tests\CustomAttributes\Controls.CustomAttributes.csproj (in 10 ms).
  Restored D:\a\1\s\src\Controls\tests\TestCases.WinUI.Tests\Controls.TestCases.WinUI.Tests.csproj (in 3.5 sec).
  7 of 15 projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Graphics -> D:\a\1\s\artifacts\bin\Graphics\Debug\net10.0\Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Essentials -> D:\a\1\s\artifacts\bin\Essentials\Debug\net10.0\Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Controls.CustomAttributes -> D:\a\1\s\artifacts\bin\Controls.CustomAttributes\Debug\net10.0\Controls.CustomAttributes.dll
  Core -> D:\a\1\s\artifacts\bin\Core\Debug\net10.0\Microsoft.Maui.dll
  Controls.Core.Design -> D:\a\1\s\artifacts\bin\Controls.Core.Design\Debug\net472\Microsoft.Maui.Controls.DesignTools.dll
  Controls.BindingSourceGen -> D:\a\1\s\artifacts\bin\Controls.BindingSourceGen\Debug\netstandard2.0\Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Controls.Core -> D:\a\1\s\artifacts\bin\Controls.Core\Debug\net10.0\Microsoft.Maui.Controls.dll
  UITest.Core -> D:\a\1\s\artifacts\bin\UITest.Core\Debug\net10.0\UITest.Core.dll
  UITest.NUnit -> D:\a\1\s\artifacts\bin\UITest.NUnit\Debug\net10.0\UITest.NUnit.dll
  UITest.Appium -> D:\a\1\s\artifacts\bin\UITest.Appium\Debug\net10.0\UITest.Appium.dll
  VisualTestUtils -> D:\a\1\s\artifacts\bin\VisualTestUtils\Debug\netstandard2.0\VisualTestUtils.dll
  VisualTestUtils.MagickNet -> D:\a\1\s\artifacts\bin\VisualTestUtils.MagickNet\Debug\netstandard2.0\VisualTestUtils.MagickNet.dll
  UITest.Analyzers -> D:\a\1\s\artifacts\bin\UITest.Analyzers\Debug\netstandard2.0\UITest.Analyzers.dll
  Controls.TestCases.WinUI.Tests -> D:\a\1\s\artifacts\bin\Controls.TestCases.WinUI.Tests\Debug\net10.0\Controls.TestCases.WinUI.Tests.dll
Test run for D:\a\1\s\artifacts\bin\Controls.TestCases.WinUI.Tests\Debug\net10.0\Controls.TestCases.WinUI.Tests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
D:\a\1\s\artifacts\bin\Controls.TestCases.WinUI.Tests\Debug\net10.0\Controls.TestCases.WinUI.Tests.dll
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in D:\a\1\s\artifacts\bin\Controls.TestCases.WinUI.Tests\Debug\net10.0\Controls.TestCases.WinUI.Tests.dll
   NUnit3TestExecutor discovered 0 of 0 NUnit test cases using Current Discovery mode, Explicit run
NUnit Adapter 4.5.0.0: Test execution complete
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.08]   Discovering: Controls.TestCases.WinUI.Tests
[xUnit.net 00:00:00.28]   Discovered:  Controls.TestCases.WinUI.Tests
No test matches the given testcase filter `Issue27563` in D:\a\1\s\artifacts\bin\Controls.TestCases.WinUI.Tests\Debug\net10.0\Controls.TestCases.WinUI.Tests.dll


🟢 With fix — 🖥️ Issue27563: FAIL ❌ · 423s
  Determining projects to restore...
  All projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Graphics -> D:\a\1\s\artifacts\bin\Graphics\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Graphics.Win2D -> D:\a\1\s\artifacts\bin\Graphics.Win2D\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.Win2D.WinUI.Desktop.dll
  Essentials -> D:\a\1\s\artifacts\bin\Essentials\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Core -> D:\a\1\s\artifacts\bin\Core\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.dll
  Controls.BindingSourceGen -> D:\a\1\s\artifacts\bin\Controls.BindingSourceGen\Debug\netstandard2.0\Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Maps -> D:\a\1\s\artifacts\bin\Maps\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Maps.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Controls.Core -> D:\a\1\s\artifacts\bin\Controls.Core\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Controls.Maps -> D:\a\1\s\artifacts\bin\Controls.Maps\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.Maps.dll
  Controls.Xaml -> D:\a\1\s\artifacts\bin\Controls.Xaml\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.Xaml.dll
  Controls.Foldable -> D:\a\1\s\artifacts\bin\Controls.Foldable\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.Foldable.dll
  Microsoft.AspNetCore.Components.WebView.Maui -> D:\a\1\s\artifacts\bin\Microsoft.AspNetCore.Components.WebView.Maui\Debug\net10.0-windows10.0.19041.0\Microsoft.AspNetCore.Components.WebView.Maui.dll
  Controls.TestCases.HostApp -> D:\a\1\s\artifacts\bin\Controls.TestCases.HostApp\Debug\net10.0-windows10.0.19041.0\win-x64\Controls.TestCases.HostApp.dll

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:05:33.48
  Determining projects to restore...
  All projects are up-to-date for restore.
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Graphics -> D:\a\1\s\artifacts\bin\Graphics\Debug\net10.0\Microsoft.Maui.Graphics.dll
  Controls.CustomAttributes -> D:\a\1\s\artifacts\bin\Controls.CustomAttributes\Debug\net10.0\Controls.CustomAttributes.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Essentials -> D:\a\1\s\artifacts\bin\Essentials\Debug\net10.0\Microsoft.Maui.Essentials.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Core -> D:\a\1\s\artifacts\bin\Core\Debug\net10.0\Microsoft.Maui.dll
  Controls.Core.Design -> D:\a\1\s\artifacts\bin\Controls.Core.Design\Debug\net472\Microsoft.Maui.Controls.DesignTools.dll
  Controls.BindingSourceGen -> D:\a\1\s\artifacts\bin\Controls.BindingSourceGen\Debug\netstandard2.0\Microsoft.Maui.Controls.BindingSourceGen.dll
  ##vso[build.updatebuildnumber]10.0.60-ci+azdo.13750583
  Controls.Core -> D:\a\1\s\artifacts\bin\Controls.Core\Debug\net10.0\Microsoft.Maui.Controls.dll
  UITest.Core -> D:\a\1\s\artifacts\bin\UITest.Core\Debug\net10.0\UITest.Core.dll
  UITest.Appium -> D:\a\1\s\artifacts\bin\UITest.Appium\Debug\net10.0\UITest.Appium.dll
  UITest.NUnit -> D:\a\1\s\artifacts\bin\UITest.NUnit\Debug\net10.0\UITest.NUnit.dll
  VisualTestUtils -> D:\a\1\s\artifacts\bin\VisualTestUtils\Debug\netstandard2.0\VisualTestUtils.dll
  VisualTestUtils.MagickNet -> D:\a\1\s\artifacts\bin\VisualTestUtils.MagickNet\Debug\netstandard2.0\VisualTestUtils.MagickNet.dll
  UITest.Analyzers -> D:\a\1\s\artifacts\bin\UITest.Analyzers\Debug\netstandard2.0\UITest.Analyzers.dll
  Controls.TestCases.WinUI.Tests -> D:\a\1\s\artifacts\bin\Controls.TestCases.WinUI.Tests\Debug\net10.0\Controls.TestCases.WinUI.Tests.dll
Test run for D:\a\1\s\artifacts\bin\Controls.TestCases.WinUI.Tests\Debug\net10.0\Controls.TestCases.WinUI.Tests.dll (.NETCoreApp,Version=v10.0)
VSTest version 18.0.1 (x64)

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
D:\a\1\s\artifacts\bin\Controls.TestCases.WinUI.Tests\Debug\net10.0\Controls.TestCases.WinUI.Tests.dll
NUnit Adapter 4.5.0.0: Test execution started
Running selected tests in D:\a\1\s\artifacts\bin\Controls.TestCases.WinUI.Tests\Debug\net10.0\Controls.TestCases.WinUI.Tests.dll
   NUnit3TestExecutor discovered 0 of 0 NUnit test cases using Current Discovery mode, Explicit run
NUnit Adapter 4.5.0.0: Test execution complete
[xUnit.net 00:00:00.00] xUnit.net VSTest Adapter v2.8.2+699d445a1a (64-bit .NET 10.0.0)
[xUnit.net 00:00:00.07]   Discovering: Controls.TestCases.WinUI.Tests
[xUnit.net 00:00:00.28]   Discovered:  Controls.TestCases.WinUI.Tests
No test matches the given testcase filter `Issue27563` in D:\a\1\s\artifacts\bin\Controls.TestCases.WinUI.Tests\Debug\net10.0\Controls.TestCases.WinUI.Tests.dll


⚠️ Issues found
  • Issue27563 FAILED with fix (should pass)
📁 Fix files reverted (2 files)
  • eng/pipelines/ci-copilot.yml
  • src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs

@MauiBot MauiBot added s/agent-fix-win AI found a better alternative fix than the PR and removed s/agent-fix-pr-picked AI could not beat the PR fix - PR is the best among all candidates labels Mar 30, 2026
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.

The test is failing :/

@Vignesh-SF3580
Copy link
Copy Markdown
Contributor

The test is failing :/

@kubaflo The tests added in the PR are not executed on Windows, even though they address a Windows specific issue. This is due to a known issue (#29245) that occurs when using CarouselView with Loop = true. Therefore, the test has been restricted on Windows. This has already been documented in the test case comments and updated in the PR description.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-controls-collectionview CollectionView, CarouselView, IndicatorView 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-win AI found a better alternative fix than the PR s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) stale Indicates a stale issue/pr and will be closed soon

Projects

None yet

9 participants