Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

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

wants to merge 15 commits into from

Conversation

NanthiniMahalingam
Copy link
Contributor

@NanthiniMahalingam NanthiniMahalingam commented Nov 22, 2024 •
edited
Loading

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
jsuarezruiz
jsuarezruiz reviewed Nov 22, 2024
View reviewed changes
src/Controls/src/Core/Handlers/Items/iOS/CarouselViewController.cs Outdated
@@ -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
@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@sheiksyedm sheiksyedm added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Dec 6, 2024
@rmarinho
Copy link
Member

rmarinho commented Dec 9, 2024

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@NanthiniMahalingam NanthiniMahalingam marked this pull request as ready for review December 10, 2024 15:26
@NanthiniMahalingam NanthiniMahalingam requested a review from a team as a code owner December 10, 2024 15:26
@rmarinho
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@rmarinho
Copy link
Member

rmarinho commented Jan 8, 2025

/rebase

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@dotnet dotnet deleted a comment from jsuarezruiz Feb 17, 2025
@rmarinho
Copy link
Member

rmarinho commented Feb 22, 2025 •
edited by jsuarezruiz
Loading

/rebase

@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsuarezruiz jsuarezruiz jsuarezruiz requested changes

@jfversluis jfversluis Awaiting requested review from jfversluis jfversluis is a code owner automatically assigned from dotnet/dotnet-maui-reviewers

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration
Projects
None yet
Milestone
No milestone
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
4 participants
@NanthiniMahalingam @jsuarezruiz @rmarinho @sheiksyedm