Skip to content
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

Fix iOS] Fix CarouselViewLoopNoFreeze so that it's passing on both CV1 and CV2 sets of handlers #26053

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

NanthiniMahalingam
Copy link
Contributor

@NanthiniMahalingam NanthiniMahalingam commented Nov 22, 2024

Issue Details

  • When removing the last item in the carosuel view, zeroth index item updated instead of previous items of the last index.
  • When swiping to the last item and removing the all the item in the carousel view, argument output range exception raised.

Root cause:

Issue12574Test

  • Current index of the visible cell in carousel view loop layout does not updated When dynamically removing the items from last index of the carousel view,

RemoveItemsQuickly

  • Swiping till last element and removing all the items, while cell displaying the end of the item tried to get the index of the path. Here index return the negative value.

Description Changes:

Issue12574Test

  • Invoked the CollectionView.ReloadItems method to reload specific items while dynamically removing the item, using the helper method GetScrollToIndexPath(targetPosition) to identify the correct item to update the visible cell in carousel view the loop layout manager.
  • Removed the _goToPostion is equel to -1 condition from ScrollToPosition method to update the current items.

RemoveItemsQuickly

  • Return the index path as zero in GetCorrectedIndex method when items count is zero.

Validated the behaviour in the following platforms

  • [] Android
  • [] Windows
  • iOS
  • [] Mac

Fixes #25775

Output:

Issue12574Test

Before After
ViewHanlder1.mp4
CV1Updated.mp4

RemoveItemsQuickly
Before changes

Before After
image
AfterChanges.mp4

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Nov 22, 2024
@jsuarezruiz jsuarezruiz added the area-controls-collectionview CollectionView, CarouselView, IndicatorView label Nov 22, 2024
@@ -302,7 +302,7 @@ void CarouselViewScrolled(object sender, ItemsViewScrolledEventArgs e)
[UnconditionalSuppressMessage("Memory", "MEM0003", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
void CollectionViewUpdating(object sender, NotifyCollectionChangedEventArgs e)
{
if (ItemsView is not CarouselView carousel)
if (ItemsSource.ItemCount == 0 || ItemsView is not CarouselView carousel)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you include an UITest?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you include an UITest?

Hi @jsuarezruiz
The test case for this scenario is already present. I have updated it to use the default CarouselView instead of CarouselView2.

@NanthiniMahalingam NanthiniMahalingam changed the title Fix 25775 Fix iOS] Fix CarouselViewLoopNoFreeze so that it's passing on both CV1 and CV2 sets of handlers Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView community ✨ Community Contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] Fix CarouselViewLoopNoFreeze so that it's passing on both CV1 and CV2 sets of handlers
2 participants