Fixed [Windows] KeepScrollOffset Behavior Not Working as Expected in CarouselView#29800
Fixed [Windows] KeepScrollOffset Behavior Not Working as Expected in CarouselView#29800Shalini-Ashokan wants to merge 2 commits into
Conversation
|
Hey there @@Shalini-Ashokan! 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
Bot
added
the
partner/syncfusion
There was a problem hiding this comment.
Pull Request Overview
This PR fixes the Windows implementation of ItemsUpdatingScrollMode.KeepScrollOffset in CarouselView by explicitly preserving the current item index when the collection changes and updates the handler logic accordingly. It also adds a UI test and host app sample for the issue.
- Added a flag in the Windows handler to maintain the scroll offset on collection changes.
- Updated
OnCollectionItemsSourceChangedandCarouselScrolledto use the new flag. - Introduced a shared UI test and a HostApp page to validate the fix (currently guarded on Windows).
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29421.cs | Added a UI test for KeepScrollOffset behavior (guarded) |
| src/Controls/tests/TestCases.HostApp/Issues/Issue29421.cs | Added HostApp sample page with CarouselView and Add button |
| src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs | Added _isCollectionChanged flag and updated scroll logic |
Comments suppressed due to low confidence (1)
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29421.cs:1
- Since this PR resolves the KeepScrollOffset issue on Windows, remove the
#if TEST_FAILS_ON_WINDOWSguard so the test runs on Windows and validates the fix.
#if TEST_FAILS_ON_WINDOWS // Related issue for windows: https://github.com/dotnet/maui/issues/29245
| _isCollectionChanged = true; | ||
| } | ||
|
|
||
| if (e.Action == NotifyCollectionChangedAction.Remove) |
There was a problem hiding this comment.
[nitpick] Consider checking ItemsView.ItemsUpdatingScrollMode == KeepScrollOffset before setting _isCollectionChanged so the flag is only used when needed and avoids unnecessary state changes.
| _isCollectionChanged = true; | |
| } | |
| if (e.Action == NotifyCollectionChangedAction.Remove) | |
| if (ItemsView.ItemsUpdatingScrollMode == KeepScrollOffset) | |
| { | |
| _isCollectionChanged = true; | |
| } | |
| } | |
| if (e.Action == NotifyCollectionChangedAction.Remove && ItemsView.ItemsUpdatingScrollMode == KeepScrollOffset) |
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
PureWeen
added
platform/windows
area-controls-collectionview
|
/rebase |
github-actions
Bot
force-pushed
the
fix-29421
branch
from
5a4e52b to
cd79344
Compare
Shalini-Ashokan
force-pushed
the
fix-29421
branch
from
5ef9af8 to
32e4ac0
Compare
MauiBot
added
s/agent-review-incomplete
s/agent-reviewed
kubaflo
left a comment
kubaflo
left a comment
There was a problem hiding this comment.
Could you please review the AI's summary?
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 29800Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 29800" |
@kubaflo, I addressed the ai concerns. |
MauiBot
added
s/agent-review-incomplete
and removed
s/agent-changes-requested
|
/azp run maui-pr-uitests , maui-pr-devicetests |
|
Azure Pipelines successfully started running 2 pipeline(s). |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/review -b feature/enhanced-reviewer |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
/review -b feature/enhanced-reviewer |
Test Failure Review
|
| Failure | Verdict | Evidence |
|---|---|---|
maui-pr-uitests build 1443464 / VerifyCarouselViewKeepScrollOffsetAdd on iOS |
Likely PR-caused | The deduplicated failure has 8 iOS log occurrences in Controls (vlatest) and Controls (vlatest) CarouselView records, all reporting VisualTestUtils.VisualTestFailedException with Baseline snapshot not yet created: .../snapshots/ios-26/VerifyCarouselViewKeepScrollOffsetAdd.png. PR scope includes the changed shared test src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29421.cs and the affected baseline path src/Controls/tests/TestCases.iOS.Tests/snapshots/ios-26/VerifyCarouselViewKeepScrollOffsetAdd.png, so the failure directly matches a changed test/baseline area. |
Current maui-pr build 1445438 |
Insufficient data | At context generation it was inProgress on refs/pull/29800/merge with no failed timeline records and no distinct log/test failures yet. This does not provide a completed current-head pass/fail result. |
Recommended action
Confirm a current-head maui-pr-uitests result for 50b6b7f and wait for the current maui-pr build to complete; if the UI failure persists, focus on the iOS 26 VerifyCarouselViewKeepScrollOffsetAdd baseline.
Evidence details
- Context generated:
2026-06-02T11:46:53.1934770Z. - PR Fixed [Windows] KeepScrollOffset Behavior Not Working as Expected in CarouselView #29800:
Fixed [Windows] KeepScrollOffset Behavior Not Working as Expected in CarouselView, basemain, headfix-29421at50b6b7faede9a72b8198e82acc32b80c29d0c251. - Labels/scope:
platform/windows,area-controls-collectionview; inferred file platformwindows; area hintsCarouselView,Handler. - Changed files include
src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs,src/Controls/tests/TestCases.HostApp/Issues/Issue29421.cs,src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29421.cs, and visual baselines for Android, Mac, Windows,ios, andios-26. - Build 1443464: definition
maui-pr-uitests(313),failed / completed, branchrefs/pull/29800/merge, source version9011d9666043854beb98c6e703676c0257f55058, 18 failed timeline records, 8 raw log-derived failures collapsed to 1 distinct test/platform failure. - Representative log message:
VisualTestUtils.VisualTestFailedException : Baseline snapshot not yet created: /Users/cloudtest/vss/_work/1/s/artifacts/bin/Controls.TestCases.iOS.Tests/Release/net10.0/snapshots/ios-26/VerifyCarouselViewKeepScrollOffsetAdd.png. - Relevant failed/publishing log records include log IDs
4402,4411,4412,4421,4404, and4414. - Recent base-branch builds for
maui-pr-uitestswere mixed (1444191,1431731,1419912, and1419503failed;1432188succeeded), but the gathered context does not show this exact missing-baseline failure pre-existing onmain. - Current build 1445438:
maui-pr(302),inProgress, branchrefs/pull/29800/merge, source version280be6449326219d0b1fbb0a85c43b74e8181a1b, 0 failed timeline records and 0 distinct failures at gather time. - Limitations:
AZDO_TOKENwas not set, so authenticated AzDO test-run APIs were skipped; nomaui-pr-devicetests/Helix aggregate data was present, so hidden device-test failures were not evaluated; the failed UI-test build source version predates the current PR head, so it may not reflect the latest baseline files.
|
/review -b feature/enhanced-reviewer |
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 2 findings
See inline comments for details.
| && currentItemPosition != -1) | ||
| { | ||
| carouselPosition = currentItemPosition; | ||
| _isCollectionChanged = true; |
There was a problem hiding this comment.
[major] Logic and Correctness - _isCollectionChanged is set for broad Add/Remove cases but is only cleared by a later CarouselScrolled callback. If the collection mutation does not immediately raise Scrolled, such as an add/remove outside the current viewport or a native update that preserves the offset without a ViewChanged event, the flag remains stale and the next unrelated user scroll is suppressed by forcing position = ItemsView.Position. Scope this state to mutations that actually need offset suppression and clear it deterministically after the collection update/native adjustment.
There was a problem hiding this comment.
This concern is out of scope for this PR because in the covered KeepScrollOffset flow _isCollectionChanged is consumed by the immediate CarouselScrolled update and does not leak into later user swipes.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
kubaflo
left a comment
kubaflo
left a comment
There was a problem hiding this comment.
Could you check the ai's suggestions?
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
kubaflo
left a comment
kubaflo
left a comment
There was a problem hiding this comment.
Could you add snapshots and review the ai's suggestions?
…rouselView When ItemsUpdatingScrollMode is set to KeepScrollOffset, the CurrentItem position gets reset incorrectly because the scrolled event updates the CurrentItem position. Explicitly set the current item index when the scroll mode is KeepScrollOffset to maintain the correct position and item selection. Fixes dotnet#29421 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Shalini-Ashokan
force-pushed
the
fix-29421
branch
from
50b6b7f to
872b326
Compare
|
/review -b feature/enhanced-reviewer |
This comment has been minimized.
This comment has been minimized.
|
/review -b feature/enhanced-reviewer |
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 3 findings
See inline comments for details.
| } | ||
|
|
||
| var position = e.CenterItemIndex; | ||
| if (_isCollectionChanged && ItemsView.ItemsUpdatingScrollMode == ItemsUpdatingScrollMode.KeepScrollOffset) |
There was a problem hiding this comment.
[major] Logic and Correctness - _isCollectionChanged is only consumed and cleared when the current ItemsUpdatingScrollMode is KeepScrollOffset, but it is set during collection changes without requiring that mode. Concrete scenario: an app uses KeepLastItemInView or KeepItemsInView, mutates the collection, then switches to KeepScrollOffset before the next scroll event; the stale flag causes this handler to ignore e.CenterItemIndex and reuse ItemsView.Position, suppressing an unrelated position update. Scope the flag to KeepScrollOffset changes, or clear it on every scroll/disconnect/non-KeepScrollOffset path.
|
|
||
| if (e.Action == NotifyCollectionChangedAction.Remove) | ||
| { | ||
| _isCollectionChanged = true; |
There was a problem hiding this comment.
[moderate] Regression Prevention - This PR now changes Remove behavior by setting _isCollectionChanged for every remove, but the added regression only covers Add. Remove paths in this method distinguish first/current/last item cases, and this new flag can change subsequent scroll-position reconciliation for all of them. Please add a remove-specific regression for KeepScrollOffset so the modified Remove behavior is covered.
| { | ||
| App.WaitForElement("carouselview"); | ||
| App.Tap("AddButton"); | ||
| VerifyScreenshot(); |
There was a problem hiding this comment.
[moderate] Regression Prevention - The screenshot is captured immediately after tapping AddButton, while CarouselView/IndicatorView layout and snapping can still be settling. The supplied gate/try-fix context already shows a small visual mismatch on this test. Use VerifyScreenshot(retryTimeout: TimeSpan.FromSeconds(2)) or another deterministic settled-state wait so the regression test validates the final offset rather than a transient frame.
MauiBot
left a comment
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@Shalini-Ashokan — new AI review results are available based on this last commit:
872b326.
committed the base image To request a fresh review after new comments or commits, comment/review rerun.
Review Sessions — click to expand
Gate — Test Before & After Fix
Gate Result: ❌ FAILED
Platform: WINDOWS · Base: main · Merge base: e904e900
🩺 Test does not reproduce the bug — ran the same in both states (PASS without fix, PASS with fix). The repro test is not exercising the issue. Strengthen the test before reviewing the fix.
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
🖥️ Issue29421 Issue29421 |
❌ PASS — 584s | ❌ FAIL — 479s |
🔴 Without fix — 🖥️ Issue29421: PASS ❌ · 584s
Determining projects to restore...
Restored D:\a\1\s\src\Graphics\src\Graphics\Graphics.csproj (in 21.84 sec).
Restored D:\a\1\s\src\Graphics\src\Graphics.Win2D\Graphics.Win2D.csproj (in 21.92 sec).
Restored D:\a\1\s\src\Essentials\src\Essentials.csproj (in 8.93 sec).
Restored D:\a\1\s\src\Core\src\Core.csproj (in 17.36 sec).
Restored D:\a\1\s\src\Core\maps\src\Maps.csproj (in 14.72 sec).
Restored D:\a\1\s\src\Controls\tests\TestCases.HostApp\Controls.TestCases.HostApp.csproj (in 6.02 sec).
Restored D:\a\1\s\src\Controls\src\Xaml\Controls.Xaml.csproj (in 41 ms).
Restored D:\a\1\s\src\Controls\src\Core\Controls.Core.csproj (in 25 ms).
Restored D:\a\1\s\src\Controls\Maps\src\Controls.Maps.csproj (in 21 ms).
Restored D:\a\1\s\src\Controls\Foldable\src\Controls.Foldable.csproj (in 33 ms).
Restored D:\a\1\s\src\BlazorWebView\src\Maui\Microsoft.AspNetCore.Components.WebView.Maui.csproj (in 106 ms).
3 of 14 projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
Graphics -> D:\a\1\s\artifacts\bin\Graphics\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
Essentials -> D:\a\1\s\artifacts\bin\Essentials\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
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.80-ci+azdo.14270258
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.80-ci+azdo.14270258
Maps -> D:\a\1\s\artifacts\bin\Maps\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
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.80-ci+azdo.14270258
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
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.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:06:16.45
Determining projects to restore...
Restored D:\a\1\s\src\TestUtils\src\VisualTestUtils\VisualTestUtils.csproj (in 727 ms).
Restored D:\a\1\s\src\TestUtils\src\UITest.NUnit\UITest.NUnit.csproj (in 2.57 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 3.33 sec).
Restored D:\a\1\s\src\TestUtils\src\VisualTestUtils.MagickNet\VisualTestUtils.MagickNet.csproj (in 7.34 sec).
Restored D:\a\1\s\src\TestUtils\src\UITest.Analyzers\UITest.Analyzers.csproj (in 5.66 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 8.5 sec).
7 of 15 projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
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.80-ci+azdo.14270258
Essentials -> D:\a\1\s\artifacts\bin\Essentials\Debug\net10.0\Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
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.80-ci+azdo.14270258
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.
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 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 6/3/2026 10:57:20 AM FixtureSetup for Issue29421(Windows)
>>>>> 6/3/2026 10:57:28 AM VerifyCarouselViewKeepScrollOffsetAdd Start
>>>>> 6/3/2026 10:57:30 AM VerifyCarouselViewKeepScrollOffsetAdd Stop
Passed VerifyCarouselViewKeepScrollOffsetAdd [2 s]
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.11] Discovering: Controls.TestCases.WinUI.Tests
[xUnit.net 00:00:00.35] Discovered: Controls.TestCases.WinUI.Tests
Results File: D:\a\1\s\CustomAgentLogsTmp\UITests\TestResults\Issue29421.trx
Test Run Successful.
Total tests: 1
Passed: 1
Total time: 30.5124 Seconds
>>> TRX_RESULT_FILE: D:\a\1\s\CustomAgentLogsTmp\UITests\TestResults\Issue29421.trx
🟢 With fix — 🖥️ Issue29421: FAIL ❌ · 479s
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
Graphics -> D:\a\1\s\artifacts\bin\Graphics\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
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.80-ci+azdo.14270258
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.80-ci+azdo.14270258
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
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.80-ci+azdo.14270258
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
Controls.Foldable -> D:\a\1\s\artifacts\bin\Controls.Foldable\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.Foldable.dll
Maps -> D:\a\1\s\artifacts\bin\Maps\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Maps.dll
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
Controls.Xaml -> D:\a\1\s\artifacts\bin\Controls.Xaml\Debug\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.Xaml.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.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:06:01.38
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
Graphics -> D:\a\1\s\artifacts\bin\Graphics\Debug\net10.0\Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
Essentials -> D:\a\1\s\artifacts\bin\Essentials\Debug\net10.0\Microsoft.Maui.Essentials.dll
Controls.CustomAttributes -> D:\a\1\s\artifacts\bin\Controls.CustomAttributes\Debug\net10.0\Controls.CustomAttributes.dll
##vso[build.updatebuildnumber]10.0.80-ci+azdo.14270258
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.80-ci+azdo.14270258
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
VisualTestUtils -> D:\a\1\s\artifacts\bin\VisualTestUtils\Debug\netstandard2.0\VisualTestUtils.dll
UITest.NUnit -> D:\a\1\s\artifacts\bin\UITest.NUnit\Debug\net10.0\UITest.NUnit.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.
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 1 of 1 NUnit test cases using Current Discovery mode, Non-Explicit run
>>>>> 6/3/2026 11:05:19 AM FixtureSetup for Issue29421(Windows)
>>>>> 6/3/2026 11:05:26 AM VerifyCarouselViewKeepScrollOffsetAdd Start
>>>>> 6/3/2026 11:05:29 AM VerifyCarouselViewKeepScrollOffsetAdd Stop
>>>>> 6/3/2026 11:05:29 AM Log types:
Failed VerifyCarouselViewKeepScrollOffsetAdd [3 s]
Error Message:
VisualTestUtils.VisualTestFailedException :
Snapshot different than baseline: VerifyCarouselViewKeepScrollOffsetAdd.png (0.56% difference)
If the correct baseline has changed (this isn't a a bug), then update the baseline image.
See test attachment or download the build artifacts to get the new snapshot file.
More info: https://aka.ms/visual-test-workflow
Stack Trace:
at VisualTestUtils.VisualRegressionTester.Fail(String message) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 162
at VisualTestUtils.VisualRegressionTester.VerifyMatchesSnapshot(String name, ImageSnapshot actualImage, String environmentName, ITestContext testContext) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 123
at Microsoft.Maui.TestCases.Tests.UITest.<VerifyScreenshot>g__Verify|13_0(String name, <>c__DisplayClass13_0&) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 477
at Microsoft.Maui.TestCases.Tests.UITest.VerifyScreenshot(String name, Nullable`1 retryDelay, Nullable`1 retryTimeout, Int32 cropLeft, Int32 cropRight, Int32 cropTop, Int32 cropBottom, Double tolerance, Boolean includeTitleBar) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 309
at Microsoft.Maui.TestCases.Tests.Issues.Issue29421.VerifyCarouselViewKeepScrollOffsetAdd() in /_/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29421.cs:line 21
at System.Reflection.MethodBaseInvoker.InterpretedInvoke_Method(Object obj, IntPtr* args)
at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)
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.10] Discovering: Controls.TestCases.WinUI.Tests
[xUnit.net 00:00:00.32] Discovered: Controls.TestCases.WinUI.Tests
Results File: D:\a\1\s\CustomAgentLogsTmp\UITests\TestResults\Issue29421.trx
Total tests: 1
Failed: 1
Test Run Failed.
Total time: 25.7240 Seconds
>>> TRX_RESULT_FILE: D:\a\1\s\CustomAgentLogsTmp\UITests\TestResults\Issue29421.trx
⚠️ Failure Details
- ❌ Issue29421 PASSED without fix (should fail) — tests don't catch the bug
- ❌ Issue29421 FAILED with fix (should pass)
VerifyCarouselViewKeepScrollOffsetAdd [3 s]VisualTestUtils.VisualTestFailedException : Snapshot different than baseline: VerifyCarouselViewKeepScrollOffsetAdd.png (0.56% difference) If the correct baseline has changed (this isn't a a bug), ...
📁 Fix files reverted (2 files)
eng/pipelines/ci-copilot.ymlsrc/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs
UI Tests — CarouselView,CollectionView
Detected UI test categories: CarouselView,CollectionView
❌ Deep UI tests — 355 passed, 1 failed across 2 categories on platform-pool agent (replaces in-process counts above).
🧪 UI Test Execution Results (deep, platform pool)
| Category | Tests | Snapshot diffs |
|---|---|---|
CarouselView |
38/38 ✓ | — |
CollectionView |
317/329 (1 ❌) | 1 diff PNG |
❌ CollectionView — 1 failed test
VerifyDefaultScrollToRequested
VisualTestUtils.VisualTestFailedException :
Snapshot different than baseline: VerifyDefaultScrollToRequested.png (0.52% difference)
If the correct baseline has changed (this isn't a a bug), then update the baseline image.
See test attachment or download the build artifacts to get the new snapshot file.
More info: https://aka.ms/visual-test-workflow
at VisualTestUtils.VisualRegressionTester.Fail(String message) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 162
at VisualTestUtils.VisualRegressionTester.VerifyMatchesSnapshot(String name, ImageSnapshot actualImage, String environmentName, ITestContext testContext) in /_/src/TestUtils/src/VisualTestUtils/VisualRegressionTester.cs:line 123
at Microsoft.Maui.TestCases.Tests.UITest.<VerifyScreenshot>g__Verify|13_0(String name, <>c__DisplayClass13_0&) in /_/src/Controls/tests/TestCases.Shared.Tests/UITest.cs:line 477
at Microsoft.Maui.TestCases.Tests.UITest.VerifyScreenshot(String name, Nullable`1 ret
...
📎 Download drop-deep-uitests artifact (TRX + snapshot diffs)
Pre-Flight — Context & Validation
Issue: #29421 - [Windows] KeepScrollOffset Not Working as Expected in CarouselView
PR: #29800 - Fixed [Windows] KeepScrollOffset Behavior Not Working as Expected in CarouselView
Platforms Affected: Windows
Files Changed: 1 implementation, 7 test/snapshot
Key Findings
- GitHub CLI authentication is unavailable, so GitHub context was gathered through public GitHub API endpoints and the local squashed review branch.
- Issue #29421 reports that CarouselView on Windows does not maintain scroll offset when inserting items with
ItemsUpdatingScrollMode.KeepScrollOffset. - PR #29800 changes
src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.csand adds UI coverage for issue #29421 with platform snapshots. - PR inline discussion includes prior feedback to scope
_isCollectionChangedtoKeepScrollOffset, add Mac snapshot, wait before screenshot to avoid scrollbar flake, and consider assert-based coverage; author noted Windows assert interaction issues from #29245. - Impacted UI category: CarouselView.
Code Review Summary
Verdict: NEEDS_CHANGES
Confidence: high
Errors: 0 | Warnings: 1 | Suggestions: 1
Key code review findings:
src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs:608and:613:_isCollectionChangedis set for Add/Remove regardless ofItemsUpdatingScrollMode, but only consumed/reset underKeepScrollOffset, so it can remain stale after collection changes in other modes and suppress a later unrelated scroll if the mode changes.src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue29421.cs: Add scenario is covered; Remove path added by the implementation does not have direct UI coverage.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #29800 | Adds _isCollectionChanged and substitutes ItemsView.Position for the next Scrolled center index after Add/Remove collection changes. |
FAILED (Gate result supplied: tests did not behave as expected) | src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs; UI test/snapshots |
Original PR; code review warns stale flag can leak across mode changes. |
Code Review — Deep Analysis
Code Review PR #29800
Independent Assessment
What this changes: Adds Windows CarouselView state to ignore the next scrolled position after collection changes when KeepScrollOffset is active, preserving the previously computed ItemsView.Position.
Inferred motivation: Prevent WinUI collection-change scroll events from incorrectly resetting CarouselView Position/CurrentItem after inserts/removes.
Reconciliation with PR Narrative
Author claims: CurrentItem position is incorrectly updated by Scrolled under KeepScrollOffset.
Agreement/disagreement: Agreed. The mechanism matches that claim, but the new flag is broader than the mode where it is consumed.
Findings
Warning _isCollectionChanged can remain stale outside KeepScrollOffset
src/Controls/src/Core/Handlers/Items/CarouselViewHandler.Windows.cs:608 and :613
_isCollectionChanged is set during Add/Remove regardless of ItemsUpdatingScrollMode, but it is only reset in CarouselScrolled when mode is KeepScrollOffset (:538-541). A collection change in another mode followed by a runtime switch to KeepScrollOffset can make the next unrelated user scroll reuse stale state and ignore e.CenterItemIndex.
Suggestion Remove path lacks direct coverage
The test only exercises Insert(0, "NewItem"); the new Remove handling has no UI coverage.
Devil's Advocate
The stale-flag issue requires runtime mode switching, but ItemsUpdatingScrollMode is bindable and supported at runtime. CI failures currently appear unrelated/infrastructure or unrelated Essentials tests, but CI is not green.
Verdict: NEEDS_CHANGES
Confidence: high
Summary: Core approach seems sound for the reported Add scenario, but the flag should be scoped/cleared so it cannot affect later unrelated scrolls. Inline findings were written to CustomAgentLogsTmp\PRState\29800\PRAgent\inline-findings.json.
Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | maui-expert-reviewer | Direct KeepScrollOffset reconciliation via ListViewBase.ScrollIntoView after collection mutation. |
FAIL | 1 file | Built successfully, but Windows UI screenshot differed by 0.56%; captured UI centered Item 1 instead of expected Item 3. |
| 2 | maui-expert-reviewer | Precise nullable pending position scoped to KeepScrollOffset, consumed once by the next scroll event. |
FAIL | 1 file | Built successfully, but same 0.56% Windows screenshot mismatch; preserving exact current item was insufficient. |
| 3 | maui-expert-reviewer | Range-scoped position delta from NotifyCollectionChangedEventArgs indexes/counts for KeepScrollOffset. |
FAIL | 1 file | Built successfully, but same 0.56% Windows screenshot mismatch; range-delta still landed on Item 1. |
| 4 | maui-expert-reviewer | Mode-scoped one-shot scroll interception that keeps PR-style dynamic ItemsView.Position read while clearing stale state. |
FAIL | 1 file | Built successfully, but same 0.56% Windows screenshot mismatch; stale-safe one-shot interception did not match baseline. |
| PR | PR #29800 | Broad _isCollectionChanged flag substitutes ItemsView.Position on the next CarouselScrolled event under KeepScrollOffset. | FAILED (supplied Gate result) | 1 implementation file + UI test/snapshots | Original PR; independent code review warned stale flag can leak across mode changes. |
Iterative Expert Review Loop
| Candidate | Expert feedback used | Lesson fed forward |
|---|---|---|
| 1 | Avoid broad scroll-event flag; reconcile native view directly after mutation. | Direct ScrollIntoView changed visual placement and failed screenshot verification. |
| 2 | Avoid native placement; use a precise pending position consumed by next scroll event. | Preserving exact current item still centered Item 1, not expected Item 3. |
| 3 | Preserve viewport semantics with collection range/index deltas. | ItemsView.Position during collection callback did not map to Windows looped viewport state. |
| 4 | Preserve PR-style scroll-event interception while making pending state mode-scoped and one-shot. | Stale-safe interception still failed the same screenshot mismatch in the local gate test. |
Cross-Pollination
| Model/Reviewer | Round | New Ideas? | Details |
|---|---|---|---|
| maui-expert-reviewer | 1 | Yes | Direct native reconciliation after collection mutation. |
| maui-expert-reviewer | 2 | Yes | Nullable exact pending position for the next scroll event. |
| maui-expert-reviewer | 3 | Yes | Range-scoped KeepScrollOffset position delta. |
| maui-expert-reviewer | 4 | Yes | Mode-scoped one-shot scroll interception to address stale state. |
| maui-expert-reviewer | 5 | No | Meaningfully different approaches were exhausted after four failures with the same targeted Windows visual mismatch. |
Exhausted: Yes
Selected Fix: None no alternative candidate passed the targeted Windows regression test, and the supplied gate result for the PR fix was already failed.
Report — Final Recommendation
Comparative Report - PR #29800
Ranking
| Rank | Candidate | Regression result | Assessment |
|---|---|---|---|
| 1 | pr-plus-reviewer |
Not rerun per instruction; closest prior code strategy (try-fix-4) failed the targeted Windows screenshot |
Best code-quality candidate because it preserves the PR's intended scroll-event-time reconciliation while fixing the expert reviewer's stale cross-mode state issue and improving screenshot stability. Still needs validation before merge. |
| 2 | pr |
FAILED - supplied gate result | Raw PR most directly targets the submitted baseline, but it has a major stale-state correctness issue and missing Remove coverage. |
| 3 | try-fix-4 |
FAIL - targeted Windows screenshot mismatch, 0.56% | Best failed alternative: mode-scoped one-shot interception fixes stale state, but empirical result still centered the wrong item compared with the committed baseline. |
| 4 | try-fix-3 |
FAIL - targeted Windows screenshot mismatch, 0.56% | Range-scoped position delta is more principled for collection semantics, but did not match Windows looped CarouselView viewport behavior. |
| 5 | try-fix-2 |
FAIL - targeted Windows screenshot mismatch, 0.56% | Exact pending logical position avoids stale state, but preserving the same current item was insufficient for the expected visual offset. |
| 6 | try-fix-1 |
FAIL - targeted Windows screenshot mismatch, 0.56% | Direct ScrollIntoView reconciliation avoided stale state but changed native placement and failed visual verification. |
Comparative analysis
All empirically tested candidates failed the supplied or recorded regression evidence. The raw PR failed the supplied gate, and all four STEP 5a alternatives built but failed the targeted Windows Issue29421 screenshot with the same 0.56% mismatch. Because there are no passing candidates, the winner must be selected by correctness risk and proximity to the intended PR behavior rather than by a passing regression result.
pr preserves the behavior the author submitted, but the expert reviewer identified a concrete stale-state bug: _isCollectionChanged can be set while the mode is not KeepScrollOffset and later consumed after a mode switch, suppressing an unrelated scroll update. That is a real correctness risk in the handler lifecycle and should not remain in the preferred candidate when an equally small fix exists.
try-fix-1, try-fix-2, and try-fix-3 each moved farther from the PR's event-time reconciliation strategy and failed the visual regression. try-fix-4 is the strongest try-fix because it keeps the PR's event-time ItemsView.Position read while making the pending state mode-scoped and one-shot; however, it still failed the same screenshot.
pr-plus-reviewer is selected as the single winning candidate because it combines the raw PR's intended behavior with the expert reviewer's stale-state fix and screenshot retry stabilization. It should still be treated as needing CI/regression validation before merge because the gate was supplied as failed and no candidate has demonstrated a passing Windows regression result.
Winner
Selected candidate: pr-plus-reviewer
Rationale: No candidate has passing regression evidence. Among the non-passing/unverified options, pr-plus-reviewer is the least risky because it addresses the expert reviewer's concrete stale-state defect while preserving the PR's core strategy more closely than the failed alternatives.
Future Action — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
kubaflo
left a comment
kubaflo
left a comment
There was a problem hiding this comment.
Tests are failing - could you please verify?
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
When ItemsUpdatingScrollMode is set to KeepScrollOffset, the CurrentItem position gets reset incorrectly.
Root Cause
The CurrentItem position is incorrectly updated by the scrolled event when the scroll mode is set to KeepScrollOffset.
Description of Change
Explicitly set the current item index when the scroll mode is KeepScrollOffset to maintain the correct position and item selection.
Validated the behavior in the following platforms
Issues Fixed
Fixes #29421
Output ScreenShot
BeforeFix-29421.mp4
AfterFix-29421.mp4