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

CarouselView layout bugs in iOS #18223

Closed
halfninja opened this issue Oct 21, 2023 · 9 comments · Fixed by #20001
Closed

CarouselView layout bugs in iOS #18223

halfninja opened this issue Oct 21, 2023 · 9 comments · Fixed by #20001
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView fixed-in-8.0.10 fixed-in-9.0.0-preview.2.10293 migration-compatibility Xamarin.Forms to .NET MAUI Migration, Upgrade Assistant, Try-Convert platform/iOS 🍎 t/bug Something isn't working
Milestone

Comments

@halfninja
Copy link

halfninja commented Oct 21, 2023

Description

A basic CarouselView, in a fresh .NET MAUI project, with 4 sample items, will render as expected on Windows and Android. On iOS the initial layout is off the bottom of the screen, and then when dragging starts it relayouts, but the content is laid out too high and clips off the top of the screen.

Carousel.mp4

Steps to Reproduce

  1. Create a new MAUI project in Visual Studio
  2. Replace the MainPage content with a CarouselView that has some basic template and a few items.

Link to public reproduction project repository

https://github.com/halfninja/CarouselDemo

Version with bug

7.0.96

Is this a regression from previous behavior?

Yes, this used to work in Xamarin.Forms

Last version that worked well

Unknown/Other

Affected platforms

iOS

Affected platform versions

Tested iOS 14 and 17

Did you find any workaround?

Remove Loop="False" if you can deal with looping.

Relevant log output

This gets logged after you start dragging and it figures out the correct layout

2023-10-21 23:26:45.768227+0100 AppName[79937:653172] The behavior of the UICollectionViewFlowLayout is not defined because:
2023-10-21 23:26:45.768383+0100 AppName[79937:653172] the item height must be less than the height of the UICollectionView minus the section insets top and bottom values, minus the content insets top and bottom values.
2023-10-21 23:26:45.768641+0100 AppName[79937:653172] The relevant UICollectionViewFlowLayout instance is <Microsoft_Maui_Controls_Handlers_Items_CarouselViewLayout: 0x10dd629d0>, and it is attached to <UICollectionView: 0x11797ba00; frame = (0 0; 320 504); clipsToBounds = YES; autoresize = W+H; gestureRecognizers = <NSArray: 0x600000c75530>; backgroundColor = UIExtendedGrayColorSpace 0 0; layer = <CALayer: 0x60000067a960>; contentOffset: {254, 0}; contentSize: {1280, 504}; adjustedContentInset: {0, 0, 0, 0}; layout: <Microsoft_Maui_Controls_Handlers_Items_CarouselViewLayout: 0x10dd629d0>; dataSource: <Microsoft_Maui_Controls_Handlers_Items_CarouselViewController: 0x10dd5f450>>.
2023-10-21 23:26:45.768777+0100 AppName[79937:653172] Make a symbolic breakpoint at UICollectionViewFlowLayoutBreakForInvalidSizes to catch this in the debugger.
@halfninja halfninja added the t/bug Something isn't working label Oct 21, 2023
@halfninja
Copy link
Author

Bug is the same in dotnet 8 rc2

@halfninja
Copy link
Author

halfninja commented Oct 21, 2023

Seems like this may be triggered by Loop="False". It behaves better with Loop="True" (or the attribute omitted) though this only helps on this demo project - in my real project, while it fixes the jump in layout when drag starts, the content is still laid out too far down (update: I found that some of these layout problems were a different issue, solved by switching from StackLayout to Grid, so that just leaves the initial bad layout)

@jsuarezruiz jsuarezruiz added platform/iOS 🍎 area-controls-collectionview CollectionView, CarouselView, IndicatorView labels Oct 23, 2023
@SimonBrettschneider
Copy link

I see similar issues, and changing Loop="False" to Loop="True" fixes the offset.

@Kamholtz
Copy link

I encountered similar issues and found that rolling back the changes from #15652 in commit 20097e8 fixed the issue for me (i.e. git checkout 20097e8e2dbb6eeced5500fa486097536a84408a~1 the maui repo and then rebuilding my app using the process described in https://github.com/dotnet/maui/blob/main/.github/DEVELOPMENT.md#pack). My layout was different (The CarouselView is nested inside a CollectionView), but was also resolved by setting Loop="True" so I suspect it's related given the changes are in:

src/Controls/src/Core/Handlers/Items/ItemsViewHandler.iOS.cs
src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs

It might be useful if someone could repeat this for the reproduction repo provided by @halfninja. I have not been successful in building it for unrelated issues ( Workload installation failed: Could not find file '/Users/xxx/maui/bin/temp/dotnet-sdk-advertising-temp/microsoft.netcore.app.runtime.mono.tvos-arm64/7.0.6/microsoft.netcore.app.runtime.mono.tvos-arm64.7.0.6.nupkg'.

@halfninja
Copy link
Author

Thanks for investigating @Kamholtz. I might try a workaround of overriding the handler, or failing that, copying CarouselView wholesale into my project, where I can tweak and see what helps. Will update if I figure anything out.

@PureWeen PureWeen added the migration-compatibility Xamarin.Forms to .NET MAUI Migration, Upgrade Assistant, Try-Convert label Oct 24, 2023
@PureWeen PureWeen added this to the Backlog milestone Oct 24, 2023
@ghost
Copy link

ghost commented Oct 24, 2023

We've added this issue to our backlog, and we will work to address it as time and resources allow. If you have any additional information or questions about this issue, please leave a comment. For additional info about issue management, please read our Triage Process.

@hlogmans
Copy link

With this "loop" option set to false, somehow also the transition to a new carousel item is slow and ugly. On a real device it is almost not visible, but it runs fluently when "loop = true".

bagusandryan referenced this issue in bagusandryan/NET-MAUI-OnBoardingPage-Example Dec 11, 2023
@asi-evin
Copy link

Can confirm that setting Loop to "True" worked-around the issue for me, would prefer if it didn't loop though.

@NielsKnaap
Copy link
Contributor

NielsKnaap commented Jan 10, 2024

For the people who don't want to use the Loop = true workaround or sometimes have only one item in the carouselview (than the loop workaround doesn't work). You can also create a custom handler and override the default CarouselViewHandler like I will show below.
I will take another look at the best fix and try to create a PR in the next few days. Till then you can use the following:

CustomItemsViewHandler.cs

public class CustomItemsViewHandler : CarouselViewHandler
{
    public override Size GetDesiredSize(double widthConstraint, double heightConstraint)
    {
	    var virtualView = VirtualView;
	    var platformView = PlatformView;
			
	    // The measurements ran in SizeThatFits percolate down to child views
	    // So if MaximumWidth/Height are not taken into account for constraints, the children may have wrong dimensions
	    widthConstraint = Math.Min(widthConstraint, ((IView)virtualView).MaximumWidth);
	    heightConstraint = Math.Min(heightConstraint, ((IView)virtualView).MaximumHeight);
	    
	    CGSize sizeThatFits;
			
	    sizeThatFits = platformView.SizeThatFits(new CGSize((float)widthConstraint, (float)heightConstraint));
			
	    var size = new Size(
		    sizeThatFits.Width == float.PositiveInfinity ? double.PositiveInfinity : sizeThatFits.Width,
		    sizeThatFits.Height == float.PositiveInfinity ? double.PositiveInfinity : sizeThatFits.Height);
			
	    if (double.IsInfinity(size.Width) || double.IsInfinity(size.Height))
	    {
		    platformView.SizeToFit();
		    size = new Size(platformView.Frame.Width, platformView.Frame.Height);
	    }
			
	    var finalWidth = ResolveConstraints(size.Width, virtualView.Width, ((IView)virtualView).MinimumWidth, (
		    (IView)virtualView).MaximumWidth);
	    var finalHeight = ResolveConstraints(size.Height, virtualView.Height, ((IView)virtualView).MinimumHeight, (
		    (IView)virtualView).MaximumHeight);
			
	    return new Size(finalWidth, finalHeight);
    }
    
    internal static double ResolveConstraints(double measured, double exact, double min, double max)
    {
	    var resolved = measured;

	    min = !double.IsNaN(min) ? min : 0;

	    if (!double.IsNaN(exact))
	    {
		    // If an exact value has been specified, try to use that
		    resolved = exact;
	    }

	    if (resolved > max)
	    {
		    // Apply the max value constraint (if any)
		    // If the exact value is in conflict with the max value, the max value should win
		    resolved = max;
	    }

	    if (resolved < min)
	    {
		    // Apply the min value constraint (if any)
		    // If the exact or max value is in conflict with the min value, the min value should win
		    resolved = min;
	    }

	    return resolved;
    }
}

Then add the handler in your MauiProgram.cs:

mauiAppBuilder.ConfigureMauiHandlers(handlers => {
    handlers.AddHandler(typeof(CarouselView), typeof(Platforms.iOS.Handlers.CustomItemsViewHandler));
});

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView fixed-in-8.0.10 fixed-in-9.0.0-preview.2.10293 migration-compatibility Xamarin.Forms to .NET MAUI Migration, Upgrade Assistant, Try-Convert platform/iOS 🍎 t/bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

10 participants