Skip to content

[Revert] [iOS] CollectionView with grouped data crashes on iOS when the groups change - fix #28246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 10, 2025

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 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 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 Work that we can't release without 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 Work that we can't release without 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.
2 participants