Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Bug] [iOS] [Android] ListView bug for CachingStrategy="RecycleElementAndDataTemplate" #9998

Open
avorobjovs opened this issue Mar 17, 2020 · 9 comments
Labels
a/listview Problems with the ListView/TableView e/4 🕓 4 p/Android p/iOS 🍎 t/bug 🐛 up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!

Comments

@avorobjovs
Copy link

avorobjovs commented Mar 17, 2020

Description

One of the pages of our application contains a ListView control. And we try to use CachingStrategy="RecycleElementAndDataTemplate". Our item model has the Type property.
Also, we have a DataTemplateSelector and 3 typed ViewCells. The DataTemplateSelector selects one of data templates with typed ViewCell according to an item model type.
Each of ViewCells has a little bit different xaml layout and different ViewCell.ContextActions menu items.

And the bug is that the ListView control uses the first typed ViewCell for all items no matter what the type has this item and what the typed of ViewCell is selected by the DataTemplateSelector.

This bug appears only we set the CachingStrategy="RecycleElementAndDataTemplate" for the ListView control. If we set "RetainElement" or "RecycleElement", all works as expected.

This bug appears on both iOS and Android platforms. Moreover, on iOS platform the ViewCell.ContextActions menu items are swapped with each other.

Basic Information

Xamarin.Forms version 4.4.0.991757

When I debug the app from the Visual Studio, the following versions are used:

Microsoft Visual Studio Enterprise 2019, Version 16.4.5
Xamarin 16.4.000.313 (d16-4@793df40)
Xamarin Designer 16.4.0.475 (remotes/origin/d16-4@ac250f5aa)
Xamarin Templates 16.4.25 (579ee62)
Xamarin.Android SDK 10.1.4.0 (d16-4/e44d1ae)
Xamarin.Android Reference Assemblies and MSBuild support.
Mono: fd9f379
Java.Interop: xamarin/java.interop@c4e569f
ProGuard: xamarin/proguard@905836d
SQLite: xamarin/sqlite@46204c4
Xamarin.Android Tools: xamarin/xamarin-android-tools@9f4ed4b
Xamarin.iOS and Xamarin.Mac SDK 13.10.0.21 (02c4b3b)

When I build the app on Azure DevOps on the Hosted MacOS build agent, the following versions are used:

Mono version 6.6.0.155
Xamarin.Android 10.1.3
Xamarin.iOS 13.10.0.21
Xcode 11.3.1

Reproduction Link

I created a small application that demonstrates this bug. The source code of this project is provided.

ListViewDataTemplateIssue_demo_app_source_code.zip

Large screenshots

Android_screenshots.zip

iOS_screenshots.zip

@avorobjovs avorobjovs added s/unverified New report that has yet to be verified t/bug 🐛 labels Mar 17, 2020
@pauldipietro pauldipietro added this to New in Triage Mar 17, 2020
@avorobjovs
Copy link
Author

Source code of typed ViewCells

Type1Cell:

<?xml version="1.0" encoding="UTF-8"?>
<ViewCell xmlns="http://xamarin.com/schemas/2014/forms" 
          xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
          xmlns:d="http://xamarin.com/schemas/2014/forms/design"
          xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
          mc:Ignorable="d"
          x:Class="ListViewDataTemplateIssue.Templates.Type1Cell">

  <ViewCell.ContextActions>
    <MenuItem Text="Action1" Command="{Binding Action1Command}" CommandParameter="{Binding .}" />
    <MenuItem Text="Action2" Command="{Binding Action2Command}" CommandParameter="{Binding .}" />
  </ViewCell.ContextActions>

  <ViewCell.View>
    <StackLayout MinimumHeightRequest="60" Padding="20">
      <StackLayout Orientation="Horizontal">
        <Label Text="ItemModel type = " />
        <Label Text="{Binding TypeName}" />
      </StackLayout>
      <Label Text="DataTemplate type = Type1" />
      <Label Text="The item should have Action1 and Action2 context menu items" />
    </StackLayout>
  </ViewCell.View>
</ViewCell>

Type2Cell:

<?xml version="1.0" encoding="UTF-8"?>
<ViewCell xmlns="http://xamarin.com/schemas/2014/forms" 
          xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
          xmlns:d="http://xamarin.com/schemas/2014/forms/design"
          xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
          mc:Ignorable="d"
          x:Class="ListViewDataTemplateIssue.Templates.Type2Cell">

  <ViewCell.View>
    <StackLayout MinimumHeightRequest="60" Padding="20">
      <StackLayout Orientation="Horizontal">
        <Label Text="ItemModel type = " />
        <Label Text="{Binding TypeName}" />
      </StackLayout>
      <Label Text="DataTemplate type = Type2" />
      <Label Text="The item should have not any context menu items" />
    </StackLayout>
  </ViewCell.View>
</ViewCell>

Type3Cell:

<?xml version="1.0" encoding="UTF-8"?>
<ViewCell xmlns="http://xamarin.com/schemas/2014/forms" 
          xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
          xmlns:d="http://xamarin.com/schemas/2014/forms/design"
          xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
          mc:Ignorable="d"
          x:Class="ListViewDataTemplateIssue.Templates.Type3Cell">

  <ViewCell.ContextActions>
    <MenuItem Text="Action3" Command="{Binding Action3Command}" CommandParameter="{Binding .}" />
  </ViewCell.ContextActions>

  <ViewCell.View>
    <StackLayout MinimumHeightRequest="60" Padding="20">
      <StackLayout Orientation="Horizontal">
        <Label Text="ItemModel type = " />
        <Label Text="{Binding TypeName}" />
      </StackLayout>
      <Label Text="DataTemplate type = Type3" />
      <Label Text="The item should have only Action3 context menu item" />
    </StackLayout>
  </ViewCell.View>
</ViewCell>

DataTemplateSelector:

public class TypeTemplateSelector : DataTemplateSelector
{
    private readonly DataTemplate Type1Template = new DataTemplate(typeof(Type1Cell));
    private readonly DataTemplate Type2Template = new DataTemplate(typeof(Type2Cell));
    private readonly DataTemplate Type3Template = new DataTemplate(typeof(Type3Cell));

    protected override DataTemplate OnSelectTemplate(object item, BindableObject container)
    {
        ItemModel model = item as ItemModel;

        if (model.Type == ItemModelType.Type1)
        {
            return Type1Template;
        }
        else if (model.Type == ItemModelType.Type2)
        {
            return Type2Template;
        }
        else
        {
            return Type3Template;
        }
    }
}

@avorobjovs
Copy link
Author

avorobjovs commented Mar 17, 2020

Screenshots of actual behavior on Android:

Android

Screenshots of actual behavior on iOS:

iOS

@jfversluis
Copy link
Member

What I also notice is that the description ("This item should have...") is the same each time?
I do see the behavior as described. For the ContextItems it reminds me a little bit of #8284 which is probably somehow related. Verified with latest nightlies.

@jfversluis jfversluis added a/listview Problems with the ListView/TableView e/4 🕓 4 p/Android p/iOS 🍎 and removed s/unverified New report that has yet to be verified labels Mar 19, 2020
@jfversluis jfversluis moved this from New to Ready For Work in Triage Mar 19, 2020
@avorobjovs
Copy link
Author

avorobjovs commented Mar 19, 2020

If you have a look on my demo project source code, you will see that there are 3 items added to the ListView with 3 different types.

this.Items = new ObservableCollection<ItemModel>
{
    new ItemModel() { Type = ItemModelType.Type1, Action1Command = this.Action1GlobalCommand,  Action2Command = this.Action2GlobalCommand,  Action3Command = this.Action3GlobalCommand },
    new ItemModel() { Type = ItemModelType.Type2, Action1Command = this.Action1GlobalCommand,  Action2Command = this.Action2GlobalCommand,  Action3Command = this.Action3GlobalCommand },
    new ItemModel() { Type = ItemModelType.Type3, Action1Command = this.Action1GlobalCommand,  Action2Command = this.Action2GlobalCommand,  Action3Command = this.Action3GlobalCommand }
};

this.MyListView.ItemsSource = this.Items;

And, according to the DataTemplateSelector code (see it above), 3 different data templates should be selected.
But, the ListView all-time applies only the first data template to all items.

issue_explanation

@samhouts samhouts added this to To do in iOS Ready For Work Mar 24, 2020
@samhouts samhouts removed this from Ready For Work in Triage Mar 24, 2020
@samhouts samhouts added this to the 5.0.0 milestone Aug 13, 2020
@samhouts samhouts added this to To do in vNext+1 (5.0.0) Aug 13, 2020
@samhouts samhouts added inactive Issue is older than 6 months and needs to be retested help wanted We welcome community contributions to any issue, but these might be a good place to start! up-for-grabs We welcome community contributions to any issue, but these might be a good place to start! and removed inactive Issue is older than 6 months and needs to be retested help wanted We welcome community contributions to any issue, but these might be a good place to start! labels Sep 18, 2020
@EnglishDave
Copy link

EnglishDave commented Oct 17, 2020

I have seen this as well. Using a DataTemplateSelector that allocates static readonly DataTemplates created using the DataTemplate(Type t) constructor. With a ListView using CachingStrategy="RecycleElement" everything works as expected, using CachingStrategy="RecycleElementAndDataTemplate" the first DataTemplate used gets used everywhere irrespective of the type that should be allocated.

In short CachingStrategy="RecycleElementAndDataTemplate" is completely broken when using DataTemplateSelectors. It probably still works if using a single DataRemplate.

@BrayanKhosravian
Copy link
Contributor

BrayanKhosravian commented Nov 6, 2020

Ok I created an IssuePage in the control gallery and could verify the issue.

I can clearly see that you use the same Model/ViewModel for every DataTemplate and you are not using polymorphism to have different relationships between DataTemplates and ViewModels/Models.
So let me explain what's going on here and why this intents this way.

The thing is that the DataTemplateSelector caches the type of your Model/ViewModel and the DataTemplate.

Dictionary<Type, DataTemplate> _dataTemplates = new Dictionary<Type, DataTemplate>();

When the first item is added then the overriden OnSelectTemplate is called and adds your ViewModel type and the DataTemplate to the cache.
When we add the second item then the DataTemplateSelector checks if that type of the Model already exsists in the cache and returns its DataTemplate. (When placing a breakpoint in the overriden OnSelectTemplate then you will see that this method is called only once!)

DataTemplate dataTemplate = null;
if (recycle && _dataTemplates.TryGetValue(item.GetType(), out dataTemplate))
	return dataTemplate;

This is why you always get the first DataTemplate.

And this happens for all platforms as this DataTemplateSelector is defined in the Xamarin.Froms assembly.

However, there are 2 ways to solve this (as far as I know).
1.) Do not use RecycleElementAndDataTemplate as you are using a single ViewModel/Model foreach template.

2.) Use RecycleElementAndDataTemplate but have a separate ViewModel foreach DataTemplate.
This solution means that all ViewModels foreach DataTemplate should have an unique type.

Example:

class ItemModel1 : ItemModel{}
class ItemModel2 : ItemModel{}
class ItemModel3 : ItemModel{}

var items = new ObservableCollection<ItemModel>()
{
	new ItemModel1() { Type = ItemModel.ItemModelType.Type1, Action1Command = Action1GlobalCommand,  Action2Command = Action2GlobalCommand,  Action3Command = Action3GlobalCommand },
	new ItemModel2() { Type = ItemModel.ItemModelType.Type2, Action1Command = Action1GlobalCommand,  Action2Command = Action2GlobalCommand,  Action3Command = Action3GlobalCommand },
	new ItemModel3() { Type = ItemModel.ItemModelType.Type3, Action1Command = Action1GlobalCommand,  Action2Command = Action2GlobalCommand,  Action3Command = Action3GlobalCommand }		
};

How should this be handled?

I currently don't know what's the best solution for this issue.
But I at least thought about some:

1.) Update the docs and tell that the ViewModels/Models should be unique when using a DataTemplateSelector while the CachingStrategy is set to RecycleElementAndDataTemplate
And maybe change the DataTemplateSelector in a way that all VmTypes and DataTemplates could be get during runtime. This would enable to throw and exception when we use RecycleElementAndDataTemplate and all ViewModels/Models are not unique.

2.) Enable to functionality to use one vm foreach DataTemplate when using RecycleElementAndDataTemplate.
But I think that would be the same as using RecycleElement and we would not cache anything.

Waiting for feedback.
Please ask when something is not clear.

@BrayanKhosravian
Copy link
Contributor

Here is the page for the control gallery, just in case someone wants to try it.

using System;
using System.Threading.Tasks;
using System.Windows.Input;
using Xamarin.Forms.CustomAttributes;
using Xamarin.Forms.Internals;

namespace Xamarin.Forms.Controls.Issues
{
	[Preserve(AllMembers = true)]
	[Issue(IssueTracker.Github, 9998, "[Bug] [iOS] [Android] ListView bug for CachingStrategy='RecycleElementAndDataTemplate'", PlatformAffected.Android & PlatformAffected.iOS)]
	public class Issue9998 : ContentPage
	{
		public Issue9998()
		{
			Action1GlobalCommand = new Command<ItemModel>(async (model) => await Action1GlobalCommandHandlerAsync(model));
			Action2GlobalCommand = new Command<ItemModel>(async (model) => await Action2GlobalCommandHandlerAsync(model));
			Action3GlobalCommand = new Command<ItemModel>(async (model) => await Action3GlobalCommandHandlerAsync(model));

			var items = new ImprovedObservableCollection<ItemModel>()
			{
				new ItemModel() { Type = ItemModel.ItemModelType.Type1, Action1Command = Action1GlobalCommand,  Action2Command = Action2GlobalCommand,  Action3Command = Action3GlobalCommand },
				new ItemModel() { Type = ItemModel.ItemModelType.Type2, Action1Command = Action1GlobalCommand,  Action2Command = Action2GlobalCommand,  Action3Command = Action3GlobalCommand },
				new ItemModel() { Type = ItemModel.ItemModelType.Type3, Action1Command = Action1GlobalCommand,  Action2Command = Action2GlobalCommand,  Action3Command = Action3GlobalCommand }
			};
			
			var listView = new ListView(ListViewCachingStrategy.RecycleElementAndDataTemplate)
			{
				HasUnevenRows = true,
			};
			listView.ItemsSource = items;
			listView.ItemTemplate = new ItemModelTemplateSelector();

			Content = new StackLayout
			{
				Children = {
					new Label { Text = "Welcome to Xamarin.Forms!" },
					listView
				}
			};

		}

		public ICommand Action1GlobalCommand { get; }
		public ICommand Action2GlobalCommand { get; }
		public ICommand Action3GlobalCommand { get; }

		async Task Action1GlobalCommandHandlerAsync(ItemModel model) => 
			await DisplayAlert("Information", $"Action 1 for {model.TypeName}", "OK");

		async Task Action2GlobalCommandHandlerAsync(ItemModel model) => 
			await DisplayAlert("Information", $"Action 2 for {model.TypeName}", "OK");

		async Task Action3GlobalCommandHandlerAsync(ItemModel model) => 
			await DisplayAlert("Information", $"Action 3 for {model.TypeName}", "OK");

		class ItemModel
		{
			public enum ItemModelType
			{
				Type1,
				Type2,
				Type3
			}

			public string TypeName { get { return this.Type.ToString(); } }

			public ItemModelType Type { get; set; }

			public ICommand Action1Command { get; set; }
			public ICommand Action2Command { get; set; }
			public ICommand Action3Command { get; set; }
		}

		[Preserve(AllMembers = true)]
		class ItemModelTemplateSelector : DataTemplateSelector
		{
			readonly DataTemplate Type1Template = new DataTemplate(typeof(Type1Cell));
			readonly DataTemplate Type2Template = new DataTemplate(typeof(Type2Cell));
			readonly DataTemplate Type3Template = new DataTemplate(typeof(Type3Cell));

			protected override DataTemplate OnSelectTemplate(object item, BindableObject container)
			{
				if(!(item is ItemModel itemModel)) throw new ArgumentException();

				var template = itemModel.Type switch
				{
					ItemModel.ItemModelType.Type1 => Type1Template,
					ItemModel.ItemModelType.Type2 => Type2Template,
					ItemModel.ItemModelType.Type3 => Type3Template,
					_ => throw new ArgumentException()
				};

				return template;

			}
		}

		[Preserve(AllMembers = true)]
		class Type1Cell : ViewCell
		{
			public Type1Cell()
			{
				var menuItem1 = new MenuItem() { Text = "Action1" };
				var menuItem2 = new MenuItem() { Text = "Action2" };

				menuItem1.SetBinding(MenuItem.CommandProperty, $"{nameof(ItemModel.Action1Command)}");
				menuItem1.SetBinding(MenuItem.CommandParameterProperty, $".");

				menuItem2.SetBinding(MenuItem.CommandProperty, $"{nameof(ItemModel.Action2Command)}");
				menuItem2.SetBinding(MenuItem.CommandParameterProperty, $".");

				ContextActions.Add(menuItem1);
				ContextActions.Add(menuItem2);

				var l1 = new Label();
				l1.SetBinding(Label.TextProperty, $"{nameof(ItemModel.TypeName)}");

				View = new StackLayout()
				{
					Orientation = StackOrientation.Vertical,
					MinimumHeightRequest = 60,
					Padding = 20,
					Children =
					{
						new StackLayout()
						{
							Orientation = StackOrientation.Horizontal,
							Children =
							{
								new Label(){Text = "ItemModel Type = "},
								l1
							}
						},
						new Label(){Text="Template1"},
						new Label(){Text="Should have ContextActions 1 and 2"},
					}
				};

			}
		}

		[Preserve(AllMembers = true)]
		class Type2Cell : ViewCell
		{
			public Type2Cell()
			{
				var l1 = new Label();
				l1.SetBinding(Label.TextProperty, $"{nameof(ItemModel.TypeName)}");

				View = new StackLayout()
				{
					Orientation = StackOrientation.Vertical,
					MinimumHeightRequest = 60,
					Padding = 20,
					Children =
					{
						new StackLayout()
						{
							Orientation = StackOrientation.Horizontal,
							Children =
							{
								new Label() { Text = "ItemModel Type = " },
								l1
							}
						},
						new Label() { Text = "Template2" },
						new Label() { Text = "Should have NO ContextActions" },
					}
				};

			}
		}

		[Preserve(AllMembers = true)]
		class Type3Cell : ViewCell
		{
			public Type3Cell()
			{
				var menuItem3 = new MenuItem() { Text = "Action3" };

				menuItem3.SetBinding(MenuItem.CommandProperty, $"{nameof(ItemModel.Action3Command)}");
				menuItem3.SetBinding(MenuItem.CommandParameterProperty, $".");

				ContextActions.Add(menuItem3);

				var l1 = new Label();
				l1.SetBinding(Label.TextProperty, $"{nameof(ItemModel.TypeName)}");

				View = new StackLayout()
				{
					Orientation = StackOrientation.Vertical,
					MinimumHeightRequest = 60,
					Padding = 20,
					Children =
					{
						new StackLayout()
						{
							Orientation = StackOrientation.Horizontal,
							Children =
							{
								new Label() { Text = "ItemModel Type = " },
								l1
							}
						},
						new Label() { Text = "Template3" },
						new Label() { Text = "Should have ContextAction 3" },
					}
				};
			}
		}
	}
}

@avorobjovs
Copy link
Author

Thank you, @BrayanKhosravian! Now it's clear to me.

How should this be handled?

I think that option 1 is better:
1.) Update the docs and tell that the ViewModels/Models should be unique when using a DataTemplateSelector while the CachingStrategy is set to RecycleElementAndDataTemplate
And maybe change the DataTemplateSelector in a way that all VmTypes and DataTemplates could be get during runtime. This would enable to throw and exception when we use RecycleElementAndDataTemplate and all ViewModels/Models are not unique.

@BrayanKhosravian
Copy link
Contributor

Yes I also think that option 1 is better. I wonder how the xamarin.forms team sees this, as this change would have API impact.
But there could also be other solutions.

But I think that we could use the modified DataTemplateSelector for the iOS CollectionView. I spent a lot of time with this issue #10842.

The iOS CollectionView PreRegisters all DataTemplate Types and does not do that during runtime.
We can see that in the method Xamarin.Forms.Platform.iOS.ItemsViewController<TItemsView>.RegisterViewTypes(). (Added the code snipped below)
When we would enable to get the data from the TemplateSelector then we could register them in the ios CollectionVIew.
I think a lot of issues are also caused by this.

protected virtual void RegisterViewTypes()
{
	CollectionView.RegisterClassForCell(typeof(HorizontalDefaultCell), HorizontalDefaultCell.ReuseId);
	CollectionView.RegisterClassForCell(typeof(VerticalDefaultCell), VerticalDefaultCell.ReuseId);
	CollectionView.RegisterClassForCell(typeof(HorizontalCell), HorizontalCell.ReuseId);
	CollectionView.RegisterClassForCell(typeof(VerticalCell), VerticalCell.ReuseId);
}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/listview Problems with the ListView/TableView e/4 🕓 4 p/Android p/iOS 🍎 t/bug 🐛 up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!
Projects
Development

No branches or pull requests

5 participants