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

[iOS] Fix CarouselView loading previous item #22280

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[iOS] Fix CarouselView loading previous item #22280

wants to merge 4 commits into from

Conversation

rmarinho
Copy link
Member

@rmarinho rmarinho commented May 7, 2024

Description of Change

Some performance enhancements that were done with Unbind, seem to have broken the previous cell on CarouselView.

This does't seem the correct fix yet , but is a start in reverting the regression.

Issues Fixed

Fixes #22015

@rmarinho rmarinho added platform/iOS 🍎 area-controls-collectionview CollectionView, CarouselView, IndicatorView labels May 7, 2024
@rmarinho rmarinho requested a review from a team as a code owner May 7, 2024 19:18
@rmarinho
Copy link
Member Author

rmarinho commented May 7, 2024

cc @filipnavara if we have ideas to fix this one better.

@@ -44,5 +44,10 @@ protected override bool AttributesConsistentWithConstrainedDimension(UICollectio
{
return false;
}

public override void PrepareForReuse()
Copy link
Contributor

Choose a reason for hiding this comment

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

This is reverting changes and fixing the issue or just include the reverts?. Could we include a comment here to avoid remove it by mistake?

Copy link
Member Author

Choose a reason for hiding this comment

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

this fixes the issue, but I still investigating a better way.

@@ -82,9 +82,8 @@
CurrentItem="{Binding Selected}">
<CarouselView.ItemTemplate>
<DataTemplate>
<Frame
<Border
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we create also an UITest?

Copy link
Member Author

@rmarinho rmarinho May 8, 2024

Choose a reason for hiding this comment

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

@filipnavara
Copy link
Member

cc @filipnavara if we have ideas to fix this one better.

Unfortunately I don't. I think we only used carousels in our app for full-page transitions and we ended up running into so many issues that we wrote a handler using UIPageViewController. That approach doesn't necessarily work for MAUI where the requirements of generic CarouselView are different.

@mattleibow
Copy link
Member

This one is a bit scary and weird. The item vanishes when scrolling, so this PR is going to prevent any release of things. But, why does it not come back? This is normal CV functionality.

If I go from page 1 to page 2, when I go back to page 1 - the normal things should be working to reload the view

A multiple loading may be a bit bad, but saying "just never undo/unsubscribe/release" is NOT the better option maybe?

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 platform/iOS 🍎
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG][iOS] Maui 8.0.10+ - CarouselView - Previous item content disappears after scrolling to next item
4 participants