adospace / reactorui-maui

MauiReactor is a MVU UI framework built on top of .NET MAUI
MIT License
568 stars 47 forks source link

Listview does not work as expected #106

Closed FynnSchapdick closed 1 year ago

FynnSchapdick commented 1 year ago

I've build the following Component using a ListView to display some buttons with actions. If i open the menu and calling a Action for 23rd time, i will cause an Exception saying 'system.exception: itemtemplate count has exceeded the limit of 23'. CollectionView instead will not cause any Exception. Is this a Maui bug or a Reactor-Maui bug? Even using the MauiControls.ListViewCachingStrategy.RecycleElementAndDataTemplate will not prevent this exception.

`public sealed class ActionMenuOverlay : Component { private IEnumerable _menuActions = Enumerable.Range(0, 10).Select(x => new MenuAction {Text = "Im an action"});

public override VisualNode Render()
{
    var parameter = GetParameter<ActionMenuOverlayParameters>();
    return new ContentView
    {
        new Grid("2*,5*", "1*,1*")
        {
            //new CollectionView()
                //.ItemsSource(_menuActions, RenderMenuAction2)
                //.ItemsLayout(new VerticalLinearItemsLayout())
                //.BackgroundColor(Colors.White)
                //.GridColumn(1)

            new ListView(MauiControls.ListViewCachingStrategy.RecycleElementAndDataTemplate)
                .ItemsSource(_menuActions, RenderMenuAction)
            .BackgroundColor(Colors.White)
                .GridColumn(1)
        }
        .OnTapped(OnGridTapped)
        .BackgroundColor(Colors.Transparent)
    }
    .AbsoluteLayoutFlags(AbsoluteLayoutFlags.SizeProportional)
    .AbsoluteLayoutBounds(0,0,1,1)
    .IsVisible(parameter.Value.IsVisible)
    .ZIndex(25);
}

private ViewCell RenderMenuAction(MenuAction menuAction)
{
    return new ViewCell
    {
        new Button()
            .BackgroundColor(Colors.Transparent)
            .Text(menuAction.Text)
            .TextColor(Colors.Black)
            .OnClicked(() =>
            {
                menuAction.Action.Invoke();
                HideActionMenuOverlay();
            })
    };
}

private VisualNode RenderMenuAction2(MenuAction menuAction)
    => new Button()
    .BackgroundColor(Colors.Transparent)
    .Text(menuAction.Text)
    .TextColor(Colors.Black)
    .OnClicked(() =>
{
    menuAction.Action.Invoke();
    HideActionMenuOverlay();
});

private void OnGridTapped()
{
    System.Diagnostics.Debug.WriteLine("Grid tapped");
    HideActionMenuOverlay();
}

private void HideActionMenuOverlay()
{
    var parameter = GetParameter<ActionMenuOverlayParameters>();
    parameter.Set(x => x.IsVisible = false);
}

}`

Code-DJ commented 1 year ago

I think a lot of it is underlying MAUI issue.

I see issues regularly with ListView + Hot Reload where the app randomly crashes when you make changes and save.

Here's one issue - you make a change, save - hot reloads fine. Repeat and you will see a crash in the next couple of changes.

Unhandled Exception:
System.InvalidCastException: Specified cast is not valid.
   at ObjCRuntime.Runtime.ThrowException(IntPtr gchandle)
   at UIKit.UIApplication.UIApplicationMain(Int32 argc, String[] argv, IntPtr principalClassName, IntPtr delegateClassName)
   at UIKit.UIApplication.Main(String[] args, Type principalClass, Type delegateClass)

I tried using CollectionView but the headers aren't sticky like ListView, also uneven rows don't display correctly when you scroll - it's as though it's reusing the smaller height of previous rows.

Another issue I have had is, if you load data in OnMounted - when Hot Reload happens, it reloads an empty ListView. My fix is to load the data before navigating to the current page and passing the data as Props.

sorry for the rant

Code-DJ commented 1 year ago

I tried it on iOS as a ContentPage and it seems to click fine for more than 23 times.

class MenuAction
{
    public string Text { get; set; } = default!;
    public Action Action { get; set; } = default!;
}

class TestPage : Component
{
    private readonly List<MenuAction> _menuActions = new()
    {
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
        new() { Text = "Title 1", Action = () => Console.WriteLine("Title 1") },
    };

    public override VisualNode Render()
    {
        // var parameter = GetParameter<ActionMenuOverlayParameters>();

        return new ContentPage
        {
            new Grid("2*,5*", "1*,1*")
            {
                //new CollectionView()
                    //.ItemsSource(_menuActions, RenderMenuAction2)
                    //.ItemsLayout(new VerticalLinearItemsLayout())
                    //.BackgroundColor(Colors.White)
                    //.GridColumn(1)

                new ListView(MauiControls.ListViewCachingStrategy.RecycleElementAndDataTemplate)
                    .ItemsSource(_menuActions, RenderMenuAction)
                    .BackgroundColor(Colors.White)
                    .GridColumn(1)
            }
            .OnTapped(OnGridTapped)
            .BackgroundColor(Colors.Transparent)
        }
        .AbsoluteLayoutFlags(AbsoluteLayoutFlags.SizeProportional)
        .AbsoluteLayoutBounds(0,0,1,1)
        // .IsVisible(parameter.Value.IsVisible)
        .ZIndex(25);
    }

    private ViewCell RenderMenuAction(MenuAction menuAction)
    {
        return new ViewCell
        {
            new Button()
                .BackgroundColor(Colors.Transparent)
                .Text(menuAction.Text)
                .TextColor(Colors.Black)
                .OnClicked(() =>
                {
                    menuAction.Action.Invoke();
                    HideActionMenuOverlay();
                })
        };
    }

    private VisualNode RenderMenuAction2(MenuAction menuAction)
        => new Button()
        .BackgroundColor(Colors.Transparent)
        .Text(menuAction.Text)
        .TextColor(Colors.Black)
        .OnClicked(() =>
    {
        menuAction.Action.Invoke();
        HideActionMenuOverlay();
    });

    private void OnGridTapped()
    {
        System.Diagnostics.Debug.WriteLine("Grid tapped");
        HideActionMenuOverlay();
    }

    private void HideActionMenuOverlay()
    {
        // var parameter = GetParameter<ActionMenuOverlayParameters>();
        // parameter.Set(x => x.IsVisible = false);
    }
}
adospace commented 1 year ago

Hi, in Android there is a limit of 23 for the maximum number of DataTemplate that can be created and linked to a ListView (https://github.com/xamarin/Xamarin.Forms/issues/2931). Current implementation of the ListView in MauiReactor could easily reach that limit if you are going to pass to the list view a different items source at each render call (in you case you are re-rendering the component everytime a menu item is clicked).

So I can suggest a couple of things: 1) Wait for the next version of MauiReactor that will reuse the datatemplate even if the ItemsSource changes. 2) Better IMHO do not use a ListView or CollectionView at all: in your case (probably you're implementing a ContextMenu-like UI) I think it's better to stick with a VStack() containing the list of menu items (optionally inside a scrollview) as, I guess, the number of menu items are gereally small

adospace commented 1 year ago

As the ContextMenu is open/closed often in a application lifetime, for better perfomance, I suggest you, also, to look at the MauiReactor CanvasView that will render the menu items within a single Canvas.

adospace commented 1 year ago

Starting from 1.0.133, MauiReactor tries to reuse the data template as much as possible. The limit under Android could be anyway hit under certain circumstances like when the list view is hot reloaded more than 23 times.