Skip to content

Commit

Permalink
[listview] fixes for various null/empty DataTemplate (#13146) Fixes: #…
Browse files Browse the repository at this point in the history
…11203

### Description of Change

Fixes: #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<Microsoft.Maui.Controls.ListView>.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!

Manually testing, I'm able to alternative commenting out the two XAML
elements:


![image](https://user-images.githubusercontent.com/840039/217080323-113e4d5d-2b22-429c-87fa-c3bedbfb73de.png)


![image](https://user-images.githubusercontent.com/840039/217080982-1adea02f-a0eb-4ffd-8a67-922dc95d504f.png)

This didn't work before.

### Issues Fixed

Fixes: #11203
  • Loading branch information
rmarinho authored Feb 7, 2023
2 parents f822230 + c1133aa commit 9ab7067
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 6 deletions.
8 changes: 4 additions & 4 deletions src/Compatibility/Core/src/Android/VisualElementRenderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions src/Controls/src/Core/TemplatedItemsList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -534,8 +534,10 @@ 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 not TItem content)
{
content = _itemsView.CreateDefault(item);
}
content = UpdateContent(content, index, item);

return content;
Expand Down
57 changes: 57 additions & 0 deletions src/Controls/tests/DeviceTests/Elements/ListView/ListViewTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ void SetupBuilder()
builder.ConfigureMauiHandlers(handlers =>
{
handlers.AddHandler<ViewCell, ViewCellRenderer>();
handlers.AddHandler<TextCell, TextCellRenderer>();
handlers.AddHandler<ListView, ListViewRenderer>();
handlers.AddHandler<VerticalStackLayout, LayoutHandler>();
handlers.AddHandler<Label, LabelHandler>();
Expand Down Expand Up @@ -152,5 +153,61 @@ await CreateHandlerAndAddToWindow<LayoutHandler>(layout, async (handler) =>
await Task.Delay(100);
});
}

[Fact]
public async Task NullTemplateDoesntCrash()
{
SetupBuilder();
ObservableCollection<string> data = new ObservableCollection<string>()
{
"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<LayoutHandler>(layout, async _ =>
{
await Task.Delay(100);
ValidatePlatformCells(listView);
});

// Default ctor
listView.ItemTemplate = new DataTemplate();

await CreateHandlerAndAddToWindow<LayoutHandler>(layout, async _ =>
{
await Task.Delay(100);
ValidatePlatformCells(listView);
});

// Return null
listView.ItemTemplate = new DataTemplate(() => null);

await CreateHandlerAndAddToWindow<LayoutHandler>(layout, async _ =>
{
await Task.Delay(100);
ValidatePlatformCells(listView);
});
}
}
}

0 comments on commit 9ab7067

Please sign in to comment.