maui icon indicating copy to clipboard operation
maui copied to clipboard

[iOS][CV2] Fix page can be dragged down, and it would cause an extra space between Header and EmptyView text

Open devanathan-vaithiyanathan opened this issue 3 months ago • 9 comments

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

On iOS, a CollectionView with Header, EmptyView shows incorrect spacing when dragging down. The EmptyView shifts downward by double its intended Y offset leaving extra space between the header and empty view.

Root Cause:

The extra spacing happens because the header offset is applied twice. On iOS, the native container (_emptyUIView) is shifted by the ScrollView during header space calculations, and at the same time the MAUI element inside that container is also arranged with the same offset. As a result, the element ends up positioned once by its container and again by MAUI, which doubles the intended Y offset.

Description of changes:

Previously, the MAUI element was being arranged using absolute coordinates of the frame via _emptyViewFormsElement.Arrange(frame.ToRectangle()), which caused the header offset to be applied twice — once by the native container and once by the MAUI element.

The fix changes the arrange call to _emptyViewFormsElement.Arrange(new Rect(0, 0, frame.Width, frame.Height)). This positions the MAUI element at the origin (0,0) within the native container while retaining its width and height.

Issues Fixed

Fixes #31465

Tested the behavior in the following platforms.

  • [x] Android
  • [x] Windows
  • [x] iOS
  • [x] Mac
Before After
iOS
iOS

/azp run

jsuarezruiz avatar Oct 02 '25 12:10 jsuarezruiz

Azure Pipelines successfully started running 3 pipeline(s).

azure-pipelines[bot] avatar Oct 02 '25 12:10 azure-pipelines[bot]

/azp run MAUI-UITests-public

jsuarezruiz avatar Oct 06 '25 05:10 jsuarezruiz

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines[bot] avatar Oct 06 '25 05:10 azure-pipelines[bot]

/rebase

PureWeen avatar Nov 07 '25 21:11 PureWeen

/azp run

sheiksyedm avatar Nov 11 '25 10:11 sheiksyedm

Azure Pipelines successfully started running 3 pipeline(s).

azure-pipelines[bot] avatar Nov 11 '25 10:11 azure-pipelines[bot]

Review Feedback: PR #31840 - Fix CollectionView EmptyView positioning with Header on iOS (CV2)

Recommendation

Approve with Minor Suggestion

Recommended changes (optional):

  1. Fix the unused BindingContext assignment in Issue31465.cs test page (line 41) - though this doesn't affect test functionality since ItemsSource is empty

📋 Full PR Review Details

Summary

This PR correctly fixes a double-offset bug in iOS CollectionView (CV2 handler) where EmptyView elements were positioned incorrectly when a Header was present. The fix changes the MAUI element arrangement from absolute coordinates to local container coordinates, eliminating the duplicate offset application.

Code Review

The Problem (Root Cause Analysis)

On iOS with CollectionView using the Items2 (CV2) handler, when both Header and EmptyView are present:

  1. Frame calculation (DetermineEmptyViewFrame() in StructuredItemsViewController2.cs:199-215):

    • Returns a CGRect with Y position already offset by header height
    • Example: If header is 50px tall, frame.Y = 50
  2. Native container positioning:

    • _emptyUIView.Frame = frame positions the native UIView at the offset Y coordinate
    • The container is now correctly positioned 50px down
  3. MAUI element arrangement (BEFORE this PR):

    _emptyViewFormsElement.Arrange(frame.ToRectangle());
    
    • This arranges the MAUI element at the absolute frame coordinates (Y=50)
    • But the MAUI element is inside the native container that's already at Y=50
    • Result: Element ends up at Y=100 (50 from container + 50 from arrangement = double offset)

The Fix (Why It's Correct)

Changed to:

_emptyViewFormsElement.Arrange(new Rect(0, 0, frame.Width, frame.Height));

Why this works:

  • Native container (_emptyUIView) is positioned by iOS at the correct offset (Y=50)
  • MAUI element arranges at (0,0) within its container
  • No double offset - element appears exactly where it should

Additional safety: Added dimension validation before Measure/Arrange:

if (frame.Width > 0 && frame.Height > 0)

This prevents unnecessary layout calculations for zero-sized frames.

Code Quality

Correctness: Fix addresses the exact root cause
Minimal changes: Only modifies the problematic Arrange call
Well-commented: Clear explanation of the coordinate space logic
Platform-specific: Correctly isolated to iOS handler
No breaking changes: Only affects EmptyView positioning, nothing else

Comparison to Alternative Approaches

Why not modify DetermineEmptyViewFrame()?

  • That would break the native container positioning
  • The frame coordinates are correct for the native UIView

Why not change the native container logic?

  • iOS expects the frame in absolute coordinates
  • Changing that would require extensive refactoring

This fix is the minimal, correct solution.

Test Coverage

Tests Included in PR

Test page: TestCases.HostApp/Issues/Issue31465.cs

  • Reproduces the exact scenario from the issue report
  • CollectionView with Header and EmptyView
  • Empty ItemsSource to trigger EmptyView display

NUnit test: TestCases.Shared.Tests/Tests/Issues/Issue31465.cs

  • Waits for elements to load
  • Clicks header button (validates UI responsiveness)
  • Screenshot verification for visual regression testing

Platform coverage: Baseline snapshots provided for:

  • iOS (primary affected platform)
  • Android (ensures no regression)
  • Windows (ensures no regression)
  • MacCatalyst (ensures no regression)

Test Quality Assessment

Strengths:

  • Directly tests the reported issue scenario
  • Follows MAUI test conventions (AutomationIds, _IssuesUITest base class)
  • Proper category: UITestCategories.CollectionView

Minor Issue Found (non-blocking):

  • Line 41 in Issue31465.cs: BindingContext = "{Binding .}"
    • This assigns a string literal instead of creating a binding
    • Should either be removed (DataTemplate items get BindingContext automatically) or use SetBinding()
    • Impact: None - ItemsSource is empty, so this DataTemplate never executes

Edge Cases Considered

During deep analysis, I identified these edge cases:

  1. EmptyView without Header - Does basic EmptyView still work?

    • Fix only affects the arrangement call, which runs regardless of header presence
    • Should work correctly (native container would be at Y=0)
  2. EmptyView with Footer only - Different offset calculation

    • DetermineEmptyViewFrame() accounts for footer height in available space
    • Fix doesn't change this calculation, only the arrangement
  3. EmptyView with Header AND Footer - Combined scenario

    • Both offsets calculated in DetermineEmptyViewFrame()
    • Fix applies same local coordinate logic
  4. Dynamic header addition/removal - Runtime changes

    • PR author noted this causes issues (tracked separately in #31899)
    • Not a regression from this PR - pre-existing limitation
  5. SafeArea on notched devices - Screen insets

    • PR author confirmed tested with video evidence
    • Works correctly with notched devices

Testing Results

Platform: iOS Simulator (iPhone Xs, iOS 18.2)
Tests Run: Issue31465.VerifyCollectionViewEmptyView
Result: ✅ PASS

Test execution:

  • App launched successfully
  • EmptyView rendered correctly positioned
  • No double-offset observed
  • Screenshot baseline created and verified

Device logs: Clean - no exceptions or warnings related to layout

Issues Found

Must Fix

None - The fix is correct and addresses the root cause properly.

Should Fix (Optional)

  1. Test page code cleanup: Remove the unused BindingContext = "{Binding .}" assignment in Issue31465.cs line 41
    • Why: Assigns string literal instead of creating binding
    • Impact: None currently (ItemsSource is empty)
    • Fix: Remove the line entirely
    • Priority: Low - doesn't affect functionality

Approval Checklist

  • [x] Code solves the stated problem
  • [x] Minimal, focused changes
  • [x] Appropriate test coverage
  • [x] No security concerns
  • [x] Follows .NET MAUI conventions
  • [x] Platform-specific code properly isolated
  • [x] No PublicAPI changes (none needed for this fix)
  • [x] Well-documented with comments
  • [x] Includes regression tests
  • [x] Tested on all affected platforms

Additional Context

Issue History

  • Reported: Issue #31465 - iOS CollectionView regression in .NET 10 preview 7
  • Regression: Yes - worked in preview 6, 9.0.100, and 9.0.82
  • Priority: p/0 (highest priority)
  • Milestone: .NET 10.0 SR2

PR Discussion Summary

  • PR author (devanathan-vaithiyanathan from Syncfusion) provided video evidence of testing on notched devices
  • Maintainer (jsuarezruiz) requested snapshot baselines - author provided for all platforms
  • PR author noted related issue with dynamic header/footer in #31899 (separate work in progress)

Technical Excellence

This PR demonstrates:

  • Deep understanding of iOS UIView coordinate systems
  • Proper root cause analysis
  • Minimal surgical fix targeting exact issue
  • Comprehensive testing across platforms
  • Clear communication in code comments

Review Metadata

  • Reviewer: PR Review Agent
  • Date: 2025-12-04
  • PR: #31840
  • Issue: #31465
  • Platforms Tested: iOS (primary validation)
  • Test Framework: Appium + NUnit
  • Handler: Items2 (CollectionView CV2)

Final Recommendation

✅ APPROVE - This PR correctly fixes the reported issue with a minimal, well-understood change. The fix addresses the root cause (double offset in coordinate systems) with the appropriate solution (use local coordinates for MAUI element arrangement). Test coverage is comprehensive and the code is well-documented.

The optional cleanup suggestion (test page BindingContext) is truly optional and doesn't affect functionality.

kubaflo avatar Dec 04 '25 00:12 kubaflo

Verified the fix:

kubaflo avatar Dec 10 '25 01:12 kubaflo