-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[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
Conversation
There was a problem hiding this 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])
/rebase |
da6f6fe
to
666280a
Compare
/rebase |
666280a
to
7bc65bf
Compare
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