Skip to content

[iOS] Fixed the Items are not displaying in CarouselView 2 #29397

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -110,5 +110,7 @@ public static void MapLoop(CarouselViewHandler2 handler, CarouselView carouselVi
(handler.Controller as CarouselViewController2)?.UpdateLoop();
}

public override Size GetDesiredSize(double widthConstraint, double heightConstraint) =>
this.GetDesiredSizeFromHandler(widthConstraint, heightConstraint);
Copy link
Member

Choose a reason for hiding this comment

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

Is this change needed?

When testing this PR if I just remove this override everything still works

}
}
18 changes: 1 addition & 17 deletions src/Controls/src/Core/Handlers/Items2/ItemsViewHandler2.iOS.cs
Original file line number Diff line number Diff line change
Expand Up @@ -161,23 +161,7 @@ NSIndexPath DetermineIndex(ScrollToRequestEventArgs args)

protected bool IsIndexPathValid(NSIndexPath indexPath)
{
if (indexPath.Item < 0 || indexPath.Section < 0)
{
return false;
}

var collectionView = Controller.CollectionView;
if (indexPath.Section >= collectionView.NumberOfSections())
{
return false;
}

if (indexPath.Item >= collectionView.NumberOfItemsInSection(indexPath.Section))
{
return false;
}

return true;
return LayoutFactory2.IsIndexPathValid(indexPath, Controller.CollectionView);
}

public override Size GetDesiredSize(double widthConstraint, double heightConstraint)
Expand Down
25 changes: 25 additions & 0 deletions src/Controls/src/Core/Handlers/Items2/iOS/LayoutFactory2.cs
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,11 @@ public static UICollectionViewLayout CreateCarouselLayout(

var goToIndexPath = cv2Controller.GetScrollToIndexPath(carouselPosition);

if (!IsIndexPathValid(goToIndexPath, cv2Controller.CollectionView))
{
return;
}

//This will move the carousel to fake the loop
cv2Controller.CollectionView.ScrollToItem(
NSIndexPath.FromItemSection(pageIndex, 0),
Expand All @@ -396,6 +401,26 @@ public static UICollectionViewLayout CreateCarouselLayout(
return layout;
}
#nullable enable

public static bool IsIndexPathValid(NSIndexPath indexPath, UICollectionView collectionView)
{
if (indexPath.Item < 0 || indexPath.Section < 0)
{
return false;
}

if (indexPath.Section >= collectionView.NumberOfSections())
{
return false;
}

if (indexPath.Item >= collectionView.NumberOfItemsInSection(indexPath.Section))
{
return false;
}

return true;
}
class CustomUICollectionViewCompositionalLayout : UICollectionViewCompositionalLayout
{
LayoutSnapInfo _snapInfo;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Microsoft.Maui.Controls.FlexLayout.CrossPlatformMeasure(double widthConstraint,
override Microsoft.Maui.Controls.ContentPresenter.OnSizeAllocated(double width, double height) -> void
override Microsoft.Maui.Controls.ScrollView.OnSizeAllocated(double width, double height) -> void
override Microsoft.Maui.Controls.TemplatedView.OnSizeAllocated(double width, double height) -> void
override Microsoft.Maui.Controls.Handlers.Items2.CarouselViewHandler2.GetDesiredSize(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size
virtual Microsoft.Maui.Controls.BindableProperty.CreateDefaultValueDelegate<TDeclarer, TPropertyType>.Invoke(TDeclarer bindable) -> TPropertyType
~virtual Microsoft.Maui.Controls.BindableProperty.BindingPropertyChangedDelegate.Invoke(Microsoft.Maui.Controls.BindableObject bindable, object oldValue, object newValue) -> void
~virtual Microsoft.Maui.Controls.BindableProperty.BindingPropertyChangedDelegate<TPropertyType>.Invoke(Microsoft.Maui.Controls.BindableObject bindable, TPropertyType oldValue, TPropertyType newValue) -> void
Expand All @@ -16,4 +17,4 @@ virtual Microsoft.Maui.Controls.BindableProperty.CreateDefaultValueDelegate<TDec
~virtual Microsoft.Maui.Controls.BindableProperty.ValidateValueDelegate<TPropertyType>.Invoke(Microsoft.Maui.Controls.BindableObject bindable, TPropertyType value) -> bool
~virtual Microsoft.Maui.Controls.CollectionSynchronizationCallback.Invoke(System.Collections.IEnumerable collection, object context, System.Action accessMethod, bool writeAccess) -> void
~virtual Microsoft.Maui.Controls.Internals.EvaluateJavaScriptDelegate.Invoke(string script) -> System.Threading.Tasks.Task<string>
~virtual Microsoft.Maui.Controls.PropertyChangingEventHandler.Invoke(object sender, Microsoft.Maui.Controls.PropertyChangingEventArgs e) -> void
~virtual Microsoft.Maui.Controls.PropertyChangingEventHandler.Invoke(object sender, Microsoft.Maui.Controls.PropertyChangingEventArgs e) -> void
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ Microsoft.Maui.Controls.FlexLayout.CrossPlatformMeasure(double widthConstraint,
override Microsoft.Maui.Controls.ContentPresenter.OnSizeAllocated(double width, double height) -> void
override Microsoft.Maui.Controls.ScrollView.OnSizeAllocated(double width, double height) -> void
override Microsoft.Maui.Controls.TemplatedView.OnSizeAllocated(double width, double height) -> void
override Microsoft.Maui.Controls.Handlers.Items2.CarouselViewHandler2.GetDesiredSize(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size
virtual Microsoft.Maui.Controls.BindableProperty.CreateDefaultValueDelegate<TDeclarer, TPropertyType>.Invoke(TDeclarer bindable) -> TPropertyType
~virtual Microsoft.Maui.Controls.BindableProperty.BindingPropertyChangedDelegate.Invoke(Microsoft.Maui.Controls.BindableObject bindable, object oldValue, object newValue) -> void
~virtual Microsoft.Maui.Controls.BindableProperty.BindingPropertyChangedDelegate<TPropertyType>.Invoke(Microsoft.Maui.Controls.BindableObject bindable, TPropertyType oldValue, TPropertyType newValue) -> void
Expand All @@ -16,4 +17,4 @@ virtual Microsoft.Maui.Controls.BindableProperty.CreateDefaultValueDelegate<TDec
~virtual Microsoft.Maui.Controls.BindableProperty.ValidateValueDelegate<TPropertyType>.Invoke(Microsoft.Maui.Controls.BindableObject bindable, TPropertyType value) -> bool
~virtual Microsoft.Maui.Controls.CollectionSynchronizationCallback.Invoke(System.Collections.IEnumerable collection, object context, System.Action accessMethod, bool writeAccess) -> void
~virtual Microsoft.Maui.Controls.Internals.EvaluateJavaScriptDelegate.Invoke(string script) -> System.Threading.Tasks.Task<string>
~virtual Microsoft.Maui.Controls.PropertyChangingEventHandler.Invoke(object sender, Microsoft.Maui.Controls.PropertyChangingEventArgs e) -> void
~virtual Microsoft.Maui.Controls.PropertyChangingEventHandler.Invoke(object sender, Microsoft.Maui.Controls.PropertyChangingEventArgs e) -> void
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,8 @@ protected override void Init()
SnapPointsType = SnapPointsType.MandatorySingle,
SnapPointsAlignment = SnapPointsAlignment.Center
};
// This functionality failed in CarouselView2. Reference: https://github.com/dotnet/maui/issues/29310
// TODO: Replace CarouselView1 with CarouselView once the issues are resolved.
var carouselView = new CarouselView1

var carouselView = new CarouselView
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be nice to update some of the related Issue7678 tests to verify some snapshots.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Random colors are used for the border background inside the CarouselView template in the Issue7678 test, so I used a validation test instead of relying on screenshots. @jsuarezruiz

{
AutomationId = "carouselView",
ItemsLayout = itemsLayout,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ protected override void Init()

// This functionality failed in CarouselView2. Reference: https://github.com/dotnet/maui/issues/29310
// TODO: Replace CarouselView1 with CarouselView once the issues mentioned in the GitHub issue are resolved.
var carouselView = new CarouselView1
var carouselView = new CarouselView
{
AutomationId = "carouselView",
ItemsLayout = itemsLayout,
Expand Down Expand Up @@ -119,7 +119,7 @@ public async Task LoadItemsAsync()
});
}

await Task.Delay(500);
await Task.Delay(50);

_items = new ObservableCollection<Issue7678Model_1>(items);
OnPropertyChanged(nameof(Items));
Expand Down
Loading