Skip to content

Conversation

@PureWeen
Copy link
Member

@PureWeen PureWeen commented Mar 7, 2025

Description of Change

This caused a regression for SR5 so we are going to revert for now and fix the original for SR6.

Issues Fixed

Fixes #28098

Copilot AI review requested due to automatic review settings March 7, 2025 14:20
@PureWeen PureWeen requested a review from a team as a code owner March 7, 2025 14:20
@PureWeen PureWeen added this to the .NET 9 SR5 milestone Mar 7, 2025
Copy link
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.

PR Overview

This PR reverts a regression fix for iOS that caused a blank screen when navigating back to MainPage and restores the original behavior for SR6. Key changes include:

  • Adding new test cases (Issue28098) in both Shared.Tests and HostApp to validate the restored behavior.
  • Updating the iOS handler in ObservableItemsSource by removing the collectionView.Window null check.
  • Removing group index boundary checks in ObservableGroupedSource and deleting the old Issue27797 tests.

Reviewed Changes

File Description
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue28098.cs New test case validating the blank screen issue on navigation back.
src/Controls/tests/TestCases.HostApp/Issues/Issue28098.xaml.cs New UI implementation supporting the Issue28098 test.
src/Controls/src/Core/Handlers/Items/iOS/ObservableItemsSource.cs Removed the check for collectionView.Window being null.
src/Controls/src/Core/Handlers/Items/iOS/ObservableGroupedSource.cs Removed boundary checks on group index values.
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue27797.cs Removed redundant tests related to the previous regression.
src/Controls/tests/TestCases.HostApp/Issues/Issue27797.xaml.cs Removed redundant host app tests for the previous issue.

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

src/Controls/src/Core/Handlers/Items/iOS/ObservableItemsSource.cs:317

  • Removing the check for collectionView.Window being null may lead to UI updates being attempted on a collectionView that is not attached to a window. Verify that this change does not introduce race conditions or unexpected behavior when the view is not fully loaded.
if (collectionView.Hidden)

src/Controls/src/Core/Handlers/Items/iOS/ObservableGroupedSource.cs:315

  • The removal of the boundary check for the groupIndex might lead to an IndexOutOfRangeException if groupIndex is not within a valid range. Please ensure that groupIndex is always valid before accessing _groupSource.
switch (_groupSource[groupIndex])

@github-project-automation github-project-automation bot moved this from Todo to Approved in MAUI SDK Ongoing Mar 7, 2025
@PureWeen PureWeen added p/0 Current heighest priority issues that we are targeting for a release. area-controls-collectionview CollectionView, CarouselView, IndicatorView platform/ios labels Mar 7, 2025
@PureWeen
Copy link
Member Author

PureWeen commented Mar 7, 2025

/rebase

@PureWeen
Copy link
Member Author

PureWeen commented Mar 8, 2025

/rebase

@PureWeen PureWeen merged commit 172dd41 into main Mar 10, 2025
120 of 128 checks passed
@PureWeen PureWeen deleted the revert_27991 branch March 10, 2025 04:11
@github-project-automation github-project-automation bot moved this from Approved to Done in MAUI SDK Ongoing Mar 10, 2025
rmarinho pushed a commit that referenced this pull request Mar 11, 2025
…he groups change - fix (#28246)

* Revert "CollectionView with grouped data crashes on iOS when the groups change - fix (#27991)"

This reverts commit 47077b1.

* - add tests to make sure we validate regression
@github-actions github-actions bot locked and limited conversation to collaborators Apr 9, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-controls-collectionview CollectionView, CarouselView, IndicatorView p/0 Current heighest priority issues that we are targeting for a release. platform/ios

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Returning back from navigation to My Recipes page would result in a blank screen.

3 participants