Skip to content

[Testing] Additional Feature Matrix Event Test Cases for Slider and ScrollView#34352

Merged
kubaflo merged 2 commits intodotnet:inflight/currentfrom
nivetha-nagalingam:nivetha-ScrollView-Slider-events
Mar 25, 2026
Merged

[Testing] Additional Feature Matrix Event Test Cases for Slider and ScrollView#34352
kubaflo merged 2 commits intodotnet:inflight/currentfrom
nivetha-nagalingam:nivetha-ScrollView-Slider-events

Conversation

@nivetha-nagalingam
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!

Description of Change

This PR enhances the event handling support for multiple MAUI controls by adding comprehensive implementation and validation for control-specific events, along with corresponding test coverage.

The update includes the addition of events for Slider and ScrollView controls, ensuring proper event triggering and argument handling across different platforms.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 6, 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 -- 34352

Or

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

@Ahamed-Ali Ahamed-Ali added community ✨ Community Contribution area-testing Unit tests, device tests area-controls-stepper Stepper area-controls-slider Slider area-controls-scrollview ScrollView and removed area-controls-stepper Stepper labels Mar 6, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Hey there @@nivetha-nagalingam! 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 Mar 6, 2026
@sheiksyedm sheiksyedm marked this pull request as ready for review March 6, 2026 14:23
Copilot AI review requested due to automatic review settings March 6, 2026 14:23
@sheiksyedm
Copy link
Copy Markdown
Contributor

/azp run maui-pr-uitests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 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 extends the Feature Matrix coverage for MAUI controls by adding HostApp plumbing and UI tests that validate Slider.ValueChanged and ScrollView.ScrollToRequested event behavior and arguments.

Changes:

  • Added Slider.ValueChanged event tracking (raised/not-raised + old/new values) in the Slider Feature Matrix HostApp page and corresponding UI tests.
  • Added ScrollView.ScrollToRequested event tracking (requested X/Y/position/animate/mode/element) in the ScrollView Feature Matrix HostApp page and corresponding UI tests.
  • Updated one Android screenshot baseline impacted by the Slider page UI changes.

Reviewed changes

Copilot reviewed 8 out of 251 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/SliderFeatureTests.cs Adds UI tests validating Slider ValueChanged status and arguments.
src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/ScrollViewFeatureTests.cs Adds UI tests validating ScrollView ScrollToRequested event and argument values.
src/Controls/tests/TestCases.HostApp/FeatureMatrix/Slider/SliderViewModal.cs Adds bindable properties for Slider ValueChanged status + old/new values.
src/Controls/tests/TestCases.HostApp/FeatureMatrix/Slider/SliderControlPage.xaml.cs Hooks Slider ValueChanged to update the view model properties.
src/Controls/tests/TestCases.HostApp/FeatureMatrix/Slider/SliderControlPage.xaml Adds UI labels/AutomationIds for the new Slider event state/args.
src/Controls/tests/TestCases.HostApp/FeatureMatrix/ScrollView/ScrollViewViewModel.cs Adds bindable properties for ScrollToRequested status/args.
src/Controls/tests/TestCases.HostApp/FeatureMatrix/ScrollView/ScrollViewControlPage.xaml.cs Handles ScrollToRequested and adds a pixel-scroll trigger button handler.
src/Controls/tests/TestCases.HostApp/FeatureMatrix/ScrollView/ScrollViewControlPage.xaml Adds UI labels/AutomationIds for the new ScrollToRequested status/args and pixel-scroll trigger.
src/Controls/tests/TestCases.Android.Tests/snapshots/android/Slider_SetVisibilityToFalse_VerifyVisualState.png Updates Android screenshot baseline for a Slider visual test.

@sheiksyedm
Copy link
Copy Markdown
Contributor

/azp run maui-pr-uitests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented Mar 14, 2026

🤖 AI Summary

📊 Expand Full Review946b3ea · Addressed the copilot suggestions and added base images
🔍 Pre-Flight — Context & Validation

Issue: None (pure testing PR — no linked issue)
PR: #34352 - [Testing] Additional Feature Matrix Event Test Cases for Slider and ScrollView
Author: nivetha-nagalingam (community/Syncfusion partner)
Platforms Affected: All (test-only PR; tests run on Android, iOS, macOS)
Files Changed: 8 implementation/test code files + 403 PNG snapshot files

Key Findings

  • This is a pure testing PR — no production code changes. It adds Feature Matrix event test coverage for Slider and ScrollView.
  • Slider: Adds ValueChanged event handler in HostApp, properties (ValueChangedStatus, OldValue, NewValue) to SliderViewModel, and 4 new test methods verifying event raise/no-raise and argument values.
  • ScrollView: Adds ScrollToRequested event handler in HostApp, 7 new properties to ScrollViewViewModel (RequestedScrollX/Y, RequestedPosition, RequestedAnimate, Mode, RequestedElementTypeName), a "Scroll To Pixel" button, and 11 new test methods.
  • Copilot review feedback was addressed: stray ; removed, OnScrollToPixelClicked made async/awaited, Element property changed from object to string (RequestedElementTypeName), unused using System.Windows.Input; removed, timing wait added to async scroll assertions.
  • Remaining concerns:
    • ScrollView_ScrollToRequested_Position_IsCorrect tests assert immediately after tap with no WaitForElement for the updated label — potential flakiness.
    • ScrollView_ScrollToPixel_* tests compare RequestedScrollYLabel/ScrollYLabel without waiting for animation to complete.
    • Slider_ValueBelowMinimum_EventNotRaised_WithClampedValue test expects "Not Raised" but setting value to -10 (clamped to 0) is the same as the initial value — the event may still fire depending on platform behavior.
    • XAML in SliderControlPage.xaml still has ValueChanged="OnSliderValueChanged" in XAML and the code-behind registers slider.ValueChanged += OnSliderValueChanged — potential double subscription.

Fix Candidates (PR's Test Approach)

# Source Approach Test Result Files Changed Notes
PR PR #34352 Add HostApp event handlers + ViewModel properties + test methods ⏳ PENDING (Gate) 8 code files Original PR

Issue: None (pure testing PR — no linked issue)
PR: #34352 - [Testing] Additional Feature Matrix Event Test Cases for Slider and ScrollView
Author: nivetha-nagalingam (community / Syncfusion partner)
Platforms Affected: All (test-only PR — new tests run on Android, iOS, macOS)
Files Changed: 8 code files + ~403 PNG snapshot files

Key Findings

  • This is a pure testing PR — no production code changes. Adds Feature Matrix event test coverage for Slider and ScrollView.
  • Slider: Adds ValueChanged event handler in HostApp, properties (ValueChangedStatus, OldValue, NewValue) to SliderViewModel, and 4 new test methods verifying event raise/no-raise and argument values.
  • ScrollView: Adds ScrollToRequested event handler in HostApp, 7 new properties to ScrollViewViewModel, a "Scroll To Pixel" button, and 11 new test methods.
  • Copilot review feedback was addressed (prior review): stray ; removed, OnScrollToPixelClicked made async/awaited, Element property renamed from object to string (RequestedElementTypeName), unused using System.Windows.Input; removed, timing waits added.
  • Prior Gate (Android): ⚠️ SKIPPED — Android emulator environment failure (ADB broken pipe). Re-running Gate on iOS.
  • Remaining concerns identified by prior review:
    • ScrollView_ScrollToRequested_Position_IsCorrect may be flaky — asserts immediately after tap without polling for label update.
    • ScrollView_ScrollToPixel_* tests may have format ambiguity — double → string without StringFormat.
    • Slider_ValueBelowMinimum_EventNotRaised_WithClampedValue behavior is platform-specific.
    • XAML declares ValueChanged="OnSliderValueChanged" AND code-behind subscribes — but NOT a real double subscription (slider is recreated in ReInitializeSlider()).

Fix Candidates

# Source Approach Test Result Files Changed Notes
PR PR #34352 Add HostApp event handlers + ViewModel properties + test methods ⏳ PENDING (Gate) 8 code files Original PR

Prior Agent Review

  • Prior run detected: Yes (on Android — Gate was skipped)
  • Prior try-fix: 2 attempts, both passing — found real timing + format issues in new tests
  • Prior recommendation: REQUEST CHANGES — fix timing (WaitForTextToBePresentInElement) and add StringFormat to scroll coordinate labels

🚦 Gate — Test Verification

Gate Result: ⚠️ SKIPPED

Platform: Android
Mode: Testing PR — Verify tests PASS with PR changes

Note: This is a pure testing PR. No bug baseline exists. Verification: do new tests pass with PR's HostApp changes?

  • Tests PASS with PR changes: ⚠️ SKIPPED (environment issue)
  • Error details: Android emulator deployment failure due to transient ADB0010 broken-pipe errors on API 30 (see below)

Error Summary:
The Gate verification was skipped due to Android emulator environment issues:

  1. First Attempt: ADB0010 "Broken pipe" error during app installation

    • Error: Mono.AndroidTools.InstallFailedException: Unexpected install output: cmd: Failure calling service package: Broken pipe (32)
    • This is a known transient issue on Android API 30
  2. Second Attempt (Retry): App crashed or test framework timeout

    • The app was deployed but failed to locate the test button (GoToTestButton)
    • Device logs show repeated failed attempts to find the button via Appium
    • App was forcibly stopped after 60+ seconds of button lookup failures

Raw test output (last 30 lines):

03-14 08:12:35.213 18392 18461 W ft.maui.uitest: ClassLoaderContext classpath size mismatch. expected=14, found=2
03-14 08:12:36.255 18392 18461 W ft.maui.uitest: Found duplicated class when checking oat files: 'Landroid/support/v4/graphics/drawable/IconCompatParcelizer;'
03-14 08:12:37.897 11057 11070 I ActivityManager: Force stopping com.microsoft.maui.uitests appid=10154 user=0: from pid 18359
03-14 08:12:37.899 11057 11070 I ActivityManager: Killing 18206:com.microsoft.maui.uitests/u0a154 (adj 0): stop com.microsoft.maui.uitests due to from pid 18359
03-14 08:12:37.910 11057 11070 W ActivityTaskManager: Force removing ActivityRecord{b3c6d38 u0 com.microsoft.maui.uitests/.MainActivity t13 f}}: app died, no saved state

⚠️  Build/deploy failed (attempt 1). ADB0010/broken-pipe errors are transient on API 30 — will retry.
⚠️  Retrying build/deploy (attempt 2 of 2)...
ℹ️  Restarting ADB server...
* daemon not running; starting now at tcp:5037
* daemon started successfully

❌ Tests failed with exit code 1
ℹ️  Review logs at: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests

Recommendation:
This Gate should be re-run in a CI environment with a stable Android emulator. The PR code itself is not responsible for this environmental failure. The new test infrastructure (AutomationIds, test methods) appears to be properly added; verification requires a functioning emulator environment.


Gate Result: ✅ PASSED

Platform: iOS (iPhone 11 Pro simulator)
Mode: Testing PR — Verify new tests PASS with PR's HostApp changes

Note: This is a pure testing PR — no bug baseline exists. Verification confirms new tests pass with PR's changes.

  • Tests FAIL without fix: N/A (testing PR — no bug to reproduce)
  • Tests PASS with PR changes: ✅

Details:

  • New Slider and ScrollView Feature Matrix tests executed successfully on iOS
  • SliderFeatureTests and ScrollViewFeatureTests matched and ran
  • Build succeeded; tests ran and passed
  • Gate verification skill flagged pipeline config files as "fix files" (false positive — those are unrelated pipeline files not part of this testing PR). The actual test code passes correctly.

Conclusion: The new event tests work on iOS. Gate ✅ PASSED.


🔧 Fix — Analysis & Comparison

Gate Result: ⚠️ SKIPPED

Platform: Android
Mode: Testing PR — Verify tests PASS with PR changes

Note: This is a pure testing PR. No bug baseline exists. Verification: do new tests pass with PR's HostApp changes?

  • Tests PASS with PR changes: ⚠️ SKIPPED (environment issue)
  • Error details: Android emulator deployment failure due to transient ADB0010 broken-pipe errors on API 30 (see below)

Error Summary:
The Gate verification was skipped due to Android emulator environment issues:

  1. First Attempt: ADB0010 "Broken pipe" error during app installation

    • Error: Mono.AndroidTools.InstallFailedException: Unexpected install output: cmd: Failure calling service package: Broken pipe (32)
    • This is a known transient issue on Android API 30
  2. Second Attempt (Retry): App crashed or test framework timeout

    • The app was deployed but failed to locate the test button (GoToTestButton)
    • Device logs show repeated failed attempts to find the button via Appium
    • App was forcibly stopped after 60+ seconds of button lookup failures

Raw test output (last 30 lines):

03-14 08:12:35.213 18392 18461 W ft.maui.uitest: ClassLoaderContext classpath size mismatch. expected=14, found=2
03-14 08:12:36.255 18392 18461 W ft.maui.uitest: Found duplicated class when checking oat files: 'Landroid/support/v4/graphics/drawable/IconCompatParcelizer;'
03-14 08:12:37.897 11057 11070 I ActivityManager: Force stopping com.microsoft.maui.uitests appid=10154 user=0: from pid 18359
03-14 08:12:37.899 11057 11070 I ActivityManager: Killing 18206:com.microsoft.maui.uitests/u0a154 (adj 0): stop com.microsoft.maui.uitests due to from pid 18359
03-14 08:12:37.910 11057 11070 W ActivityTaskManager: Force removing ActivityRecord{b3c6d38 u0 com.microsoft.maui.uitests/.MainActivity t13 f}}: app died, no saved state

⚠️  Build/deploy failed (attempt 1). ADB0010/broken-pipe errors are transient on API 30 — will retry.
⚠️  Retrying build/deploy (attempt 2 of 2)...
ℹ️  Restarting ADB server...
* daemon not running; starting now at tcp:5037
* daemon started successfully

❌ Tests failed with exit code 1
ℹ️  Review logs at: /home/vsts/work/1/s/CustomAgentLogsTmp/UITests

Recommendation:
This Gate should be re-run in a CI environment with a stable Android emulator. The PR code itself is not responsible for this environmental failure. The new test infrastructure (AutomationIds, test methods) appears to be properly added; verification requires a functioning emulator environment.


Fix Candidates

# Source Approach Test Result Files Changed Notes
1 try-fix (claude-sonnet-4.6) ViewModel int conversion + sync barrier on ScrollToRequestedLabel ✅ PASS 3 files Fixes format at data model + timing with different-element barrier
2 try-fix (claude-opus-4.6) Direct label.Text in code-behind (bypass ViewModel binding) ✅ PASS 3 files Fixes both issues at source; also improves Slider test
PR PR #34352 Add event handlers + VM properties + test methods ✅ Gate Passed 8 files Original PR — has latent format + timing issues

Cross-Pollination

Model Round New Ideas? Details
claude-sonnet-4.6 2 NO NEW IDEAS All layers covered: test, XAML, ViewModel, code-behind
claude-opus-4.6 2 NO NEW IDEAS Further approaches would be variants of existing 4

Exhausted: Yes — Both models agree problem space is fully explored

Best Fix Selection

Comparing the two ✅ PASS candidates:

Criterion Attempt 1 (sonnet) Attempt 2 (opus)
Simplicity ✅ Minimal (3 small changes) ⚠️ Larger (14+26+7 lines)
Robustness ✅ High — int type eliminates format, sync barrier ensures event fired ✅ High — direct assignment is synchronous
Layer ViewModel (cleanest data contract) Code-behind (couples UI to logic)
Codebase style ✅ MVVM pattern — ViewModel owns data types ⚠️ Bypasses MVVM; mixes View and logic
Risk Low Medium — removes binding makes maintenance harder

Selected Fix: Attempt 1 (claude-sonnet-4.6)int ViewModel conversion + sync barrier

  • Simpler (3 files, ~8 lines)
  • Cleaner architecture (MVVM pattern preserved)
  • int property type is semantically correct (pixel coordinates are integers)
  • Sync barrier using ScrollToRequestedLabel is more elegant than bypassing MVVM entirely

📋 Report — Final Recommendation

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase Status Notes
Pre-Flight ✅ COMPLETE Testing PR — no linked issue, 8 code files + 403 PNG snapshots
Gate ⚠️ SKIPPED Android emulator environment failure (ADB broken pipe); testing PR — no bug baseline
Try-Fix ✅ COMPLETE 2 attempts, 2 passing — both identified real improvements
Report ✅ COMPLETE

Summary

PR #34352 adds Feature Matrix event test cases for Slider (ValueChanged) and ScrollView (ScrollToRequested). This is a pure testing PR — no production code is changed. The PR is well-structured and addresses feedback from a prior Copilot review (removed stray ;, made OnScrollToPixelClicked async, renamed Element property to RequestedElementTypeName, removed unused using).

However, the try-fix exploration found two real quality issues in the new test code that could cause flaky failures in CI.

Root Cause

Two independent issues in the new test code:

  1. Timing issue (ScrollView_ScrollToRequested_Position_IsCorrect): The test taps a scroll button and immediately asserts RequestedPositionLabel without waiting for the async ScrollToRequested event to fire and update the label. Since ScrollToAsync is async/animated, the label update may lag, causing intermittent assertion failures.

  2. Format ambiguity (RequestedScrollX/RequestedScrollY labels): The ViewModel properties are typed as double, but the XAML bindings have no StringFormat. A double value of 150.0 may display as "150" or "150.0" depending on platform/runtime, while the tests assert against "150". This is a latent determinism issue.

Fix Quality

The PR's core approach is sound — the HostApp infrastructure (new labels, event handlers, ViewModel properties) is correctly implemented. The Copilot review feedback was addressed. Slider tests are correct and the ValueBelowMinimum behavior is confirmed accurate.

Recommended changes (from try-fix):

Fix 1 — Replace immediate assertion with wait/poll in ScrollView_ScrollToRequested_Position_IsCorrect:

// Before (flaky):
Assert.That(
    App.FindElement("RequestedPositionLabel").GetText(),
    Is.EqualTo(expectedPosition));

// After (robust):
Assert.That(
    App.WaitForTextToBePresentInElement("RequestedPositionLabel", expectedPosition),
    Is.True,
    $"RequestedPositionLabel did not show '{expectedPosition}' in time");

Fix 2 — Add StringFormat to XAML bindings for scroll coordinate labels (ScrollViewControlPage.xaml):

<!-- Before: -->
<Label AutomationId="RequestedScrollXLabel" Text="{Binding RequestedScrollX}"/>
<Label AutomationId="RequestedScrollYLabel" Text="{Binding RequestedScrollY}"/>

<!-- After: -->
<Label AutomationId="RequestedScrollXLabel" Text="{Binding RequestedScrollX, StringFormat='{0:0}'}"/>
<Label AutomationId="RequestedScrollYLabel" Text="{Binding RequestedScrollY, StringFormat='{0:0}'}"/>

Fix comparison:

# Source Approach Test Result Simplicity Robustness
1 try-fix (sonnet) WaitForTextToBePresentInElement in test ✅ PASS Simple High — handles async timing
2 try-fix (opus) StringFormat='{0:0}' in XAML binding ✅ PASS Very simple High — eliminates format ambiguity
PR original Immediate assertion, no StringFormat ⚠️ Gate skipped Simple Low — fragile

Selected Fix: Both Attempt 1 + Attempt 2 — they address independent issues in different files and together make the new tests production-grade.

Additional Notes (No Code Change Required)

  • The Slider_ValueBelowMinimum_EventNotRaised_WithClampedValue test correctly expects "Not Raised" — when value -10 is clamped to 0 (same as initial), ValueChanged does not fire.
  • The MaximumEntry entry in Slider_ValueChangedEvent_VerifyArguments_FromOptions is empty on each Options navigation (fresh page), so entering "5" without ClearText is safe.
  • The apparent "double subscription" (ValueChanged in XAML + slider.ValueChanged in code-behind) is NOT a real double subscription: ReInitializeSlider() calls SliderGrid.Children.Clear() removing the XAML slider, then adds a fresh programmatic slider. Different objects, no double subscription.
  • 403 PNG snapshot files are updated — standard for visual regression baseline refresh when HostApp layout changes.

⚠️ Final Recommendation: REQUEST CHANGES

Phase Status

Phase Status Notes
Pre-Flight ✅ COMPLETE Testing PR — no linked issue; 8 code files + ~403 PNG snapshots; prior review imported
Gate ✅ PASSED iOS (iPhone 11 Pro) — new tests pass with PR's changes
Try-Fix ✅ COMPLETE 2 attempts (+ 2 from prior run), both passing; problem space exhausted
Report ✅ COMPLETE

Summary

PR #34352 adds Feature Matrix event test cases for Slider (ValueChanged) and ScrollView (ScrollToRequested). This is a pure testing PR — no production code changes. The PR is structurally sound and prior Copilot feedback was addressed. The new tests passed on iOS.

However, the try-fix exploration confirmed two real quality issues that could cause intermittent CI failures:

  1. RequestedScrollX/RequestedScrollY are double properties without StringFormat — tests assert "150" but 150.0 could render differently on some platforms
  2. ScrollView_ScrollToRequested_Position_IsCorrect asserts immediately after tap without waiting for the async ScrollToRequested event handler to update the ViewModel

A better fix was found that is simpler and more architecturally correct than the PR's current approach.


Root Cause

Two independent quality issues in new test code:

  1. Format ambiguity: ScrollViewViewModel.RequestedScrollX and RequestedScrollY are double properties. double 150.0 bound without StringFormat may render as "150", "150.0", or locale-specific strings depending on platform. The test asserts Is.EqualTo("150").

  2. Timing race: ScrollView_ScrollToRequested_Position_IsCorrect taps a scroll button and immediately calls App.FindElement("RequestedPositionLabel").GetText(). Since ScrollToAsync is animated and the ScrollToRequested event fires asynchronously, the ViewModel may not be updated yet when the assertion runs.


Fix Quality

Recommended changes (from try-fix Attempt 1 — claude-sonnet-4.6):

Fix 1 — Change RequestedScrollX/RequestedScrollY from double to int in ScrollViewViewModel.cs:

// Before:
private double _requestedScrollX;
public double RequestedScrollX { get => _requestedScrollX; set { ... } }

// After:
private int _requestedScrollX;
public int RequestedScrollX { get => _requestedScrollX; set { ... } }

And cast in event handler (ScrollViewControlPage.xaml.cs):

// Before:
vm.RequestedScrollX = e.ScrollX;
vm.RequestedScrollY = e.ScrollY;

// After:
vm.RequestedScrollX = (int)e.ScrollX;
vm.RequestedScrollY = (int)e.ScrollY;

Fix 2 — Add sync barrier in ScrollView_ScrollToRequested_Position_IsCorrect (ScrollViewFeatureTests.cs):

// Before:
App.Tap(buttonAutomationId);
Assert.That(App.FindElement("RequestedPositionLabel").GetText(), Is.EqualTo(expectedPosition));

// After:
App.Tap(buttonAutomationId);
App.WaitForTextToBePresentInElement("ScrollToRequestedLabel", "Raised");  // ← sync barrier
Assert.That(App.FindElement("RequestedPositionLabel").GetText(), Is.EqualTo(expectedPosition));

Why sync barrier on ScrollToRequestedLabel is better than waiting on RequestedPositionLabel itself:

  • ScrollToRequestedLabel and RequestedPositionLabel are both set in the same OnScrollViewScrollToRequested handler
  • Waiting for "Raised" on the event-fired indicator guarantees the position is also set
  • Waiting on RequestedPositionLabel directly could false-positive if it already shows the expected value from a prior test run

Fix comparison:

# Source Approach Result Simplicity Style
1 try-fix (sonnet) int ViewModel + sync barrier ✅ PASS ✅ 3 files, ~8 lines ✅ MVVM preserved
2 try-fix (opus) Direct label.Text in code-behind ✅ PASS ⚠️ 3 files, ~47 lines ⚠️ Bypasses MVVM
PR original double binding, immediate assertion ⚠️ Latent issues ✅ Simple ✅ MVVM

Selected Fix: Attempt 1 (claude-sonnet-4.6) — simpler, cleaner, architecturally aligned with MVVM pattern already used in this HostApp.


Additional Observations (No Code Change Required)

  • Slider_ValueBelowMinimum_EventNotRaised_WithClampedValue: Correctly expects "Not Raised" — when value -10 is clamped to 0 (same as initial), ValueChanged does not fire. ✅ Correct behavior.
  • XAML + code-behind event subscription: Not a double subscription — ReInitializeSlider() calls SliderGrid.Children.Clear() removing XAML slider before adding programmatic one.
  • SliderViewModal.cs filename: Typo ("Modal" vs "ViewModel") exists but is pre-existing pattern in that directory; minor cleanup.
  • Indentation in ScrollView_ScrollToRequested_X_IsNonZero_ForPixelScroll: Second Is.EqualTo("300") continuation is not indented — minor style issue.
  • ~403 PNG snapshot files: Updated as expected when HostApp layout changes; standard baseline refresh.

Changes Requested

  1. ScrollViewViewModel.cs — Change RequestedScrollX/RequestedScrollY from double to int
  2. ScrollViewControlPage.xaml.cs — Cast e.ScrollX/e.ScrollY to int in event handler
  3. ScrollViewFeatureTests.cs — Add App.WaitForTextToBePresentInElement("ScrollToRequestedLabel", "Raised") in ScrollView_ScrollToRequested_Position_IsCorrect before assertion

📋 Expand PR Finalization Review

PR #34352 Finalization Review

PR: #34352 — [Testing] Additional Feature Matrix Event Test Cases for Slider and ScrollView
Author: @nivetha-nagalingam
Status: Open · Not Merged · Changes Requested (agent label)


Phase 1: Title & Description

✅ Title: Acceptable — Minor Enhancement Optional

Current: [Testing] Additional Feature Matrix Event Test Cases for Slider and ScrollView

Assessment: Accurate and descriptive. The [Testing] prefix is appropriate since this is a pure testing PR. The title clearly names both controls. No change strictly required.

Optional improvement: [Testing] Feature Matrix: Add event test cases for Slider (ValueChanged) and ScrollView (ScrollToRequested) — more specific about which events.


⚠️ Description: Present but Generic — Enhancements Recommended

Current description:

"This PR enhances the event handling support for multiple MAUI controls by adding comprehensive implementation and validation for control-specific events, along with corresponding test coverage. The update includes the addition of events for Slider and ScrollView controls, ensuring proper event triggering and argument handling across different platforms."

Quality assessment:

  • ✅ NOTE block present
  • ✅ Covers basic scope
  • ❌ Does not name which events were added
  • ❌ No breakdown of new test methods or HostApp changes
  • ❌ Issues Fixed section empty (acceptable for testing-only PRs)

Suggested description additions:

### Description of Change

Adds Feature Matrix test coverage for events on two controls:

**Slider** (`SliderControlPage`, `SliderFeatureTests`):
- Added `ValueChanged` event tracking to `SliderViewModel` (`ValueChangedStatus`, `OldValue`, `NewValue`)
- Wired `slider.ValueChanged += OnSliderValueChanged` in `ReInitializeSlider()`
- New tests: event not raised initially, arguments verified from Options, clamped-to-maximum args, value-below-minimum not raised

**ScrollView** (`ScrollViewControlPage`, `ScrollViewViewModel`, `ScrollViewFeatureTests`):
- Added `ScrollToRequested` and `Scrolled` event handling
- New `ScrollViewViewModel` properties: `ScrollToRequestedText`, `RequestedScrollX/Y`, `RequestedPosition`, `RequestedAnimate`, `Mode`, `RequestedElementTypeName`
- New tests: event raised, position enum, animated flag, mode (Element vs Position), X/Y coordinates, element type for all content types, pixel-scroll actual vs requested comparison

**Addressed review comments (all 7 resolved):**
- Removed stray `;` in `OnScrollViewScrollToRequested`
- Made `OnScrollToPixelClicked` `async void` and awaited `ScrollToAsync`
- Renamed `Element` property to `RequestedElementTypeName` (typed as `string`)
- Removed unused `using System.Windows.Input` from `ScrollViewViewModel`
- Fixed parameter indentation in `ScrollView_ScrollToRequested_Position_IsCorrect`
- Added `App.WaitForElement` before ScrollToRequested assertions

### Issues Fixed
None — pure test coverage addition.

Phase 2: Code Review

🔴 Critical Issues

1. Filename Typo: SliderViewModal.cs should be SliderViewModel.cs

  • File: src/Controls/tests/TestCases.HostApp/FeatureMatrix/Slider/SliderViewModal.cs
  • Problem: The file is named SliderViewModal.cs ("Modal") but the class inside is SliderViewModel. Every other similar file in the repo uses the ViewModel suffix (e.g., ScrollViewViewModel.cs). This naming inconsistency makes the file hard to discover and violates naming conventions.
  • Recommendation: Rename the file to SliderViewModel.cs to match the class name and follow existing conventions.

🟡 Suggestions

2. Missing App.PressEnter() after MaximumEntry in event test

  • File: SliderFeatureTests.csSlider_ValueChangedEvent_VerifyArguments_FromOptions
  • Problem: The test calls App.EnterText("MaximumEntry", "5") without a subsequent App.PressEnter(). Later tests (e.g., Slider_SetMaximumValue_VerifyMinimumLabel) consistently call App.PressEnter() after entering text. If the OptionsPage uses a keyboard-submit pattern, skipping PressEnter could result in the Maximum not being applied.
  • Recommendation: Add App.PressEnter() after App.EnterText("MaximumEntry", "5") for consistency and reliability.

3. Test method name mismatch: X test also asserts Y

  • File: ScrollViewFeatureTests.csScrollView_ScrollToRequested_X_IsNonZero_ForPixelScroll
  • Problem: The test name implies only X is validated, but the test also asserts RequestedScrollYLabel == "150". Wait — actually it asserts RequestedScrollXLabel == "150" and RequestedScrollYLabel == "300". The method name mentions only X being non-zero, but Y (300) is also asserted. A reader scanning tests would assume this method only checks X.
  • Recommendation: Rename to ScrollView_ScrollToRequested_XY_AreNonZero_ForPixelScroll or split into two tests.

4. Inconsistent indentation in pixel scroll test

  • File: ScrollViewFeatureTests.csScrollView_ScrollToRequested_X_IsNonZero_ForPixelScroll
  • Problem: The second Assert.That(App.FindElement("RequestedScrollYLabel")...) is indented differently than the first assertion in the same method.
  • Recommendation: Align both assertions to the same indent level.

5. Missing WaitForElement in horizontal pixel scroll test

  • File: ScrollViewFeatureTests.csScrollView_ScrollToPixel_Horizontal_Updates_RequestedX_And_ScrollX
  • Problem: The vertical counterpart (ScrollView_ScrollToPixel_Vertical_Updates_RequestedY_And_ScrollY) correctly calls App.WaitForElement("ScrollYLabel") before reading scroll values. The horizontal test reads ScrollXLabel and ScrollYLabel without any explicit wait after tapping. Since the Scrolled event is asynchronous, this is inconsistent and potentially flaky.
  • Recommendation: Add App.WaitForElement("ScrollXLabel") after tapping ScrollToPixelButton, matching the vertical test pattern.

6. App.WaitForElement does not verify label text content

  • File: ScrollViewFeatureTests.csScrollView_ScrollToRequested_Event_IsRaised
  • Problem: App.WaitForElement("ScrollToRequestedLabel") was added as the fix for the review comment about flaky assertions. However, WaitForElement only waits for the element to be present/visible — not for its text to equal "Raised". The element exists before the tap too, so this wait provides minimal timing protection.
  • Recommendation: Use App.WaitForTextToBePresentInElement("ScrollToRequestedLabel", "Raised") if available, or poll the text value.

7. Slider_ValueChangedEvent_NotRaised_Initially — test name is slightly misleading

  • File: SliderFeatureTests.cs
  • Problem: The test navigates to Options, taps Apply, then asserts "Not Raised". The word "Initially" suggests the check happens without user interaction, but the test first visits the Options page (which resets ValueChangedStatus = "Not Raised") before checking. Technically this tests the reset behavior, not the truly initial state.
  • Recommendation: Either rename to Slider_ValueChangedEvent_NotRaised_AfterReset or simplify by checking the label directly without navigating to Options.

✅ Looks Good

  • NOTE block present — Required block is at the top of the description.
  • Event handler cleanup — Previous review issues resolved: stray ; removed, OnScrollToPixelClicked is now async void with await, Element property correctly typed as string.
  • using System.Windows.Input in SliderViewModal.cs is correctly keptICommand is used for DragStartedCommand and DragCompletedCommand properties, so this import is necessary (unlike ScrollViewViewModel.cs where it was unused).
  • AutomationId naming — All AutomationIds are clear and consistent (ScrollToRequestedLabel, RequestedPositionLabel, ValueChangedEventStatus, etc.).
  • ScrollToRequested testsScrollToRequested fires synchronously before ScrollToAsync animation begins, so immediate assertions after tap are generally safe for those labels.
  • Scrolled event testsScrollView_ScrollToPixel_Vertical_Updates_RequestedY_And_ScrollY correctly waits for ScrollYLabel before comparing, handling the async nature of the Scrolled event.
  • [Test, Order(...)] for Slider events — Test ordering makes sense given value state carries across re-renders.
  • Category attributes — All tests correctly use [Category(UITestCategories.Slider)] or [Category(UITestCategories.ScrollView)].
  • Snapshot updates — Updated ~400 snapshot PNGs indicate the UI changes have been properly baselined.

Summary

Item Status
NOTE block in description ✅ Present
Title accuracy ✅ Acceptable
Description quality ⚠️ Generic — enhancements recommended
SliderViewModal.cs filename typo 🔴 Should be SliderViewModel.cs
Missing PressEnter in Slider event test 🟡 Potential reliability issue
Pixel scroll test name mismatch (X only but asserts Y too) 🟡 Misleading
Missing wait in horizontal pixel scroll test 🟡 Inconsistent with vertical test
WaitForElement doesn't check text value 🟡 Incomplete fix for flakiness
Slider events (ValueChanged) — implementation ✅ Correct
ScrollView events (ScrollToRequested) — implementation ✅ Correct
Prior review comments all resolved ✅ 7/7 resolved

@kubaflo kubaflo added s/agent-changes-requested AI agent recommends changes - found a better alternative or issues s/agent-reviewed PR was reviewed by AI agent workflow (full 4-phase review) labels Mar 14, 2026
@kubaflo
Copy link
Copy Markdown
Contributor

kubaflo commented Mar 15, 2026

/azp run maui-pr-uitests

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@kubaflo kubaflo changed the base branch from main to inflight/current March 19, 2026 23:38
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?

@nivetha-nagalingam nivetha-nagalingam force-pushed the nivetha-ScrollView-Slider-events branch from 946b3ea to a1222a8 Compare March 25, 2026 05:38
@nivetha-nagalingam
Copy link
Copy Markdown
Contributor Author

Could you please resolve conflicts?

@kubaflo , The conflicts have been resolved.

@kubaflo kubaflo merged commit e23c4a2 into dotnet:inflight/current Mar 25, 2026
31 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-controls-scrollview ScrollView area-controls-slider Slider area-testing Unit tests, device tests 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-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.

6 participants