-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[listview] fixes for various null/empty DataTemplate #13146
[listview] fixes for various null/empty DataTemplate #13146
Conversation
Fixes: dotnet#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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, maybe invert the if (I'm not a UI developer)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Failing test on iOS
NavigatingBackViaBackButtonFiresNavigatedEvent
@jonathanpeppers CI merges main before running tests. It looks like main broke with one of the merges from yesterday. Taking a look now |
@rmarinho so maybe we can merge this one? It seems like it should be an improvement, adding |
Description of Change
Fixes #11203
The following breaks XAML Hot Reload:
Setup a working
ListView
andDataTemplate
Delete the
DataTemplate
's body (as if I'm about to type a completely new view there).Trigger a XAML Hot Reload. This can happen automatically if you just pause typing.
MAUI crashes at runtime:
You can also create this problem in C#, like I did in a unit test:
I could also get a slightly different crash:
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 aListView
. You need aCell
instead ofView
in that case.The fixes appear to be in two places:
VisualElementRenderer.OnLayout
: useis
, also simplifies the codeTemplatedItemsList.ActivateContent
: useis
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:
This didn't work before.
Issues Fixed
Fixes: #11203