From 2a97c5cd36b646467be1d3247edcaa8c8ed9e76a Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 6 Feb 2023 12:46:36 -0600 Subject: [PATCH 1/2] [listview] fixes for various null/empty DataTemplate Fixes: https://github.com/dotnet/maui/issues/11203 The following breaks XAML Hot Reload: 1. Setup a working `ListView` and `DataTemplate` 2. Delete the `DataTemplate`'s body (as if I'm about to type a completely new view there). 3. Trigger a XAML Hot Reload. This can happen automatically if you just pause typing. 4. MAUI crashes at runtime: in Android.Views.ViewGroup.Layout at /Users/runner/work/1/s/xamarin-android/src/Mono.Android/obj/Release/net7.0/android-33/mcw/Android.Views.ViewGroup.cs:3369,5 in Microsoft.Maui.Controls.Handlers.Compatibility.VisualElementRenderer.OnLayout at D:\a\_work\1\s\src\Controls\src\Core\Compatibility\Handlers\Android\VisualElementRenderer.cs:54,6 in Microsoft.Maui.Controls.Handlers.Compatibility.ListViewRenderer.OnLayout at D:\a\_work\1\s\src\Controls\src\Core\Compatibility\Handlers\ListView\Android\ListViewRenderer.cs:298,4 You can also create this problem in C#, like I did in a unit test: listView.ItemTemplate = new DataTemplate(() => /* valid template */); listView.ItemTemplate = new DataTemplate(); // broken listView.ItemTemplate = new DataTemplate(() => null); //broken I could also get a slightly different crash: System.InvalidCastException: Specified cast is not valid. at Microsoft.Maui.Controls.Internals.TemplatedItemsList`2[[Microsoft.Maui.Controls.ItemsView`1[[Microsoft.Maui.Controls.Cell, Microsoft.Maui.Controls, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]], Microsoft.Maui.Controls, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null],[Microsoft.Maui.Controls.Cell, Microsoft.Maui.Controls, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null]].ActivateContent(Int32 index, Object item) The "invalid cast" appears to be due to the changes in c57858f. It returns a `new Label()` in some cases, which does not work with a `ListView`. You need a `Cell` instead of `View` in that case. The fixes appear to be in two places: * `VisualElementRenderer.OnLayout`: use `is`, also simplifies the code * `TemplatedItemsList.ActivateContent` : use `is` and call the default data template. Now my tests pass, and I can't reproduce the issue in XAML Hot Reload either! --- .../Core/src/Android/VisualElementRenderer.cs | 8 +-- src/Controls/src/Core/TemplatedItemsList.cs | 10 +++- .../Elements/ListView/ListViewTests.cs | 57 +++++++++++++++++++ 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/src/Compatibility/Core/src/Android/VisualElementRenderer.cs b/src/Compatibility/Core/src/Android/VisualElementRenderer.cs index ed5fa9bfb339..335af78c9828 100644 --- a/src/Compatibility/Core/src/Android/VisualElementRenderer.cs +++ b/src/Compatibility/Core/src/Android/VisualElementRenderer.cs @@ -300,10 +300,10 @@ protected virtual void OnElementPropertyChanged(object sender, PropertyChangedEv protected override void OnLayout(bool changed, int l, int t, int r, int b) { - if (Element == null) - return; - - UpdateLayout(((IElementController)Element).LogicalChildren); + if (Element is IElementController controller) + { + UpdateLayout(controller.LogicalChildren); + } } public override void Draw(Canvas canvas) diff --git a/src/Controls/src/Core/TemplatedItemsList.cs b/src/Controls/src/Core/TemplatedItemsList.cs index 00d336c30066..d66d82b6d9f5 100644 --- a/src/Controls/src/Core/TemplatedItemsList.cs +++ b/src/Controls/src/Core/TemplatedItemsList.cs @@ -534,8 +534,14 @@ public DataTemplate SelectDataTemplate(object item) public TItem ActivateContent(int index, object item) { - TItem content = ItemTemplate != null ? (TItem)ItemTemplate.CreateContent(item, _itemsView) : _itemsView.CreateDefault(item); - + if (ItemTemplate?.CreateContent(item, _itemsView) is TItem content) + { + // content is valid + } + else + { + content = _itemsView.CreateDefault(item); + } content = UpdateContent(content, index, item); return content; diff --git a/src/Controls/tests/DeviceTests/Elements/ListView/ListViewTests.cs b/src/Controls/tests/DeviceTests/Elements/ListView/ListViewTests.cs index 3d2cbb29dfcc..5af794fac7a8 100644 --- a/src/Controls/tests/DeviceTests/Elements/ListView/ListViewTests.cs +++ b/src/Controls/tests/DeviceTests/Elements/ListView/ListViewTests.cs @@ -20,6 +20,7 @@ void SetupBuilder() builder.ConfigureMauiHandlers(handlers => { handlers.AddHandler(); + handlers.AddHandler(); handlers.AddHandler(); handlers.AddHandler(); handlers.AddHandler(); @@ -152,5 +153,61 @@ await CreateHandlerAndAddToWindow(layout, async (handler) => await Task.Delay(100); }); } + + [Fact] + public async Task NullTemplateDoesntCrash() + { + SetupBuilder(); + ObservableCollection data = new ObservableCollection() + { + "cat", + "dog", + "catdog" + }; + + var listView = new ListView(ListViewCachingStrategy.RecycleElement) + { + ItemTemplate = new DataTemplate(() => + { + return new ViewCell + { + View = new VerticalStackLayout + { + new Label() + } + }; + }), + ItemsSource = data + }; + + var layout = new VerticalStackLayout + { + listView + }; + + await CreateHandlerAndAddToWindow(layout, async _ => + { + await Task.Delay(100); + ValidatePlatformCells(listView); + }); + + // Default ctor + listView.ItemTemplate = new DataTemplate(); + + await CreateHandlerAndAddToWindow(layout, async _ => + { + await Task.Delay(100); + ValidatePlatformCells(listView); + }); + + // Return null + listView.ItemTemplate = new DataTemplate(() => null); + + await CreateHandlerAndAddToWindow(layout, async _ => + { + await Task.Delay(100); + ValidatePlatformCells(listView); + }); + } } } \ No newline at end of file From c1133aaf7fd2fb8afe9d90c8c7177651c1f5be47 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 6 Feb 2023 21:19:57 -0600 Subject: [PATCH 2/2] Use `is not`, save a few lines of code --- src/Controls/src/Core/TemplatedItemsList.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Controls/src/Core/TemplatedItemsList.cs b/src/Controls/src/Core/TemplatedItemsList.cs index d66d82b6d9f5..b6f71bbe8a58 100644 --- a/src/Controls/src/Core/TemplatedItemsList.cs +++ b/src/Controls/src/Core/TemplatedItemsList.cs @@ -534,11 +534,7 @@ public DataTemplate SelectDataTemplate(object item) public TItem ActivateContent(int index, object item) { - if (ItemTemplate?.CreateContent(item, _itemsView) is TItem content) - { - // content is valid - } - else + if (ItemTemplate?.CreateContent(item, _itemsView) is not TItem content) { content = _itemsView.CreateDefault(item); }