-
Notifications
You must be signed in to change notification settings - Fork 2k
[iOS/Mac][CV2] Fix CarouselView2 freezes with infinite loop when IsScrollAnimated=False #35848
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,7 @@ public class CarouselViewController2 : ItemsViewController2<CarouselView> | |
| bool _isInternalCollectionUpdate = false; | ||
| int _section = 0; | ||
| bool _wasDetachedFromWindow = false; | ||
| int _gotoPosition = -1; | ||
| CarouselViewLoopManager _carouselViewLoopManager; | ||
| CancellationTokenSource _scrollDebounce; | ||
|
|
||
|
|
@@ -333,7 +334,7 @@ void CollectionViewUpdated(object sender, NotifyCollectionChangedEventArgs e) | |
| return; | ||
| } | ||
|
|
||
| //_gotoPosition = -1; | ||
| _gotoPosition = -1; | ||
|
|
||
| // We need to update the position while modifying the collection. | ||
| targetPosition = GetTargetPosition(); | ||
|
|
@@ -472,7 +473,7 @@ void ScrollToPosition(int goToPosition, int carouselPosition, bool animate, bool | |
| return; | ||
| } | ||
|
|
||
| if (goToPosition != carouselPosition || forceScroll) | ||
| if (_gotoPosition == -1 && (goToPosition != carouselPosition || forceScroll)) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| { | ||
| UICollectionViewScrollPosition uICollectionViewScrollPosition = IsHorizontal ? UICollectionViewScrollPosition.CenteredHorizontally : UICollectionViewScrollPosition.CenteredVertically; | ||
| var goToIndexPath = GetScrollToIndexPath(goToPosition); | ||
|
|
@@ -482,6 +483,7 @@ void ScrollToPosition(int goToPosition, int carouselPosition, bool animate, bool | |
| return; | ||
| } | ||
|
|
||
| _gotoPosition = goToPosition; | ||
| CollectionView.ScrollToItem(goToIndexPath, uICollectionViewScrollPosition, animate); | ||
| } | ||
| } | ||
|
|
@@ -509,6 +511,19 @@ internal void SetPosition(int position) | |
| return; | ||
| } | ||
|
|
||
| if (_gotoPosition != -1) | ||
| { | ||
| if (position == _gotoPosition) | ||
| { | ||
| _gotoPosition = -1; | ||
| } | ||
| else | ||
| { | ||
| // Suppress intermediate positions while scrolling to target | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| ItemsView.SetValueFromRenderer(CarouselView.PositionProperty, position); | ||
| SetCurrentItem(position); | ||
| UpdateVisualStates(); | ||
|
|
@@ -552,6 +567,11 @@ internal void UpdateFromCurrentItem() | |
| return; | ||
| } | ||
|
|
||
| if (currentItemIndex.Row == _gotoPosition) | ||
| { | ||
| _gotoPosition = -1; | ||
| } | ||
|
|
||
| ScrollToPosition(currentItemIndex.Row, carousel.Position, carousel.AnimateCurrentItemChanges); | ||
|
|
||
| UpdateVisualStates(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| [Issue(IssueTracker.Github, 35675, "[iOS] CarouselView freezes with infinite loop when IsScrollAnimated=False", PlatformAffected.iOS)] | ||
| public class Issue35675 : ContentPage | ||
| { | ||
| readonly string[] _initialItems = ["Item 0", "Item 1", "Item 2"]; | ||
| readonly string[] _updatedItems = ["Item 0b", "Item 1b", "Item 2b"]; | ||
|
|
||
| public Issue35675() | ||
| { | ||
| Label instructionLabel = new Label | ||
| { | ||
| AutomationId = "InstructionLabel", | ||
| Text = "The test passes if the CarouselView is not frozen after the button click and the current item is updated properly.", | ||
| HorizontalOptions = LayoutOptions.Center, | ||
| HorizontalTextAlignment = TextAlignment.Center, | ||
| FontSize = 18 | ||
| }; | ||
|
|
||
| CarouselView carouselView = new CarouselView | ||
| { | ||
| AutomationId = "CarouselView", | ||
| HeightRequest = 300, | ||
| IsScrollAnimated = false, | ||
| BackgroundColor = Colors.LightGray, | ||
| HorizontalScrollBarVisibility = ScrollBarVisibility.Never, | ||
| ItemsSource = _initialItems, | ||
| ItemTemplate = new DataTemplate(() => | ||
| { | ||
| Label label = new Label | ||
| { | ||
| FontSize = 24 | ||
| }; | ||
| label.SetBinding(Label.TextProperty, "."); | ||
| label.SetBinding(Label.AutomationIdProperty, "."); | ||
| return label; | ||
| }) | ||
| }; | ||
|
|
||
| Button scrollButton = new Button | ||
| { | ||
| Text = "Change Items And Scroll", | ||
| AutomationId = "ScrollButton" | ||
| }; | ||
|
|
||
| scrollButton.Clicked += (s, e) => | ||
| { | ||
| carouselView.ItemsSource = _updatedItems; | ||
| carouselView.CurrentItem = "Item 2b"; | ||
| }; | ||
|
|
||
| Content = new VerticalStackLayout | ||
| { | ||
| Padding = 20, | ||
| Spacing = 15, | ||
| Children = { instructionLabel, carouselView, scrollButton } | ||
| }; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| #if TEST_FAILS_ON_ANDROID && TEST_FAILS_ON_WINDOWS // Android Issue: https://github.com/dotnet/maui/issues/35643, Windows PR: https://github.com/dotnet/maui/pull/35398 | ||
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
|
|
||
| public class Issue35675 : _IssuesUITest | ||
| { | ||
| public Issue35675(TestDevice device) : base(device) | ||
| { | ||
| } | ||
|
|
||
| public override string Issue => "[iOS] CarouselView freezes with infinite loop when IsScrollAnimated=False"; | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.CarouselView)] | ||
| public void CV2DoesNotFreezeWhenSettingCurrentItemWithIsScrollAnimatedFalse() | ||
| { | ||
| App.WaitForElement("ScrollButton"); | ||
| App.WaitForElement("Item 0"); | ||
| App.Tap("ScrollButton"); | ||
|
|
||
| App.WaitForElement("Item 2b"); | ||
| } | ||
| } | ||
| #endif |
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.
_gotoPositionis new lifecycle state but it is not reset in teardown/dispose or ItemsSource replacement/empty-source paths. If the CarouselView detaches while a programmatic scroll is pending, the stale target can survive reattach and causeSetPositionto suppress valid positions for the new view/source. Please clear this with the other lifecycle flags.