unoplatform / uno

Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported.
https://platform.uno
Apache License 2.0
8.76k stars 705 forks source link

[Android] Setting `NavigationView.IsPaneOpen` through code causes `Java.Lang.IllegalStateException` when `FooterItems` are bound #8809

Open marwalsch opened 2 years ago

marwalsch commented 2 years ago

Current behavior

Setting NavigationView.IsPaneOpen = true inside the code-behind results into a Java.Lang.IllegalStateException. It works on iOS and the WinSDK.

As of now clueless whether or to what extend this is related to similar known issues (such as #4836).

Here is the full stack trace:

StackTrace.txt

Expected behavior

No exception is thrown.

How to reproduce it (as minimally and precisely as possible)

NavMenuRepro.zip

Workaround

No response

Works on UWP/WinUI

Yes

Environment

Uno.WinUI

NuGet package version(s)

Uno.WinUI. 4.2.6 and later.

Affected platforms

Android

IDE

Visual Studio 2022

IDE version

17.2.1

jeromelaban commented 2 years ago

Thanks for the report. When opening issues, make sure to validate the behavior using both a latest stable (4.2.6) and dev (4.3-dev) packages. Try updating your sample first and let us know.

marwalsch commented 2 years ago

Pardon, I updated everything. The issue still occurs on 4.2.6 and 4.3-dev, though unlike in prior versions the pane actually expands shortly before crashing.

queequac commented 2 years ago

Tried to reproduce this with the provided sample. I do not experience the issue, neither with 4.2.6 nor with 4.3.6. The pane opens and closes as expected. @marwalsch What Android SDK version are you using? I gave it a try with API-28 running your sample.

In general I do heavily make use of setting IsPaneOpen via code-behind in my apps. There was no problem with that up to now.

marwalsch commented 2 years ago

@queequac Thanks for looking into this.

I tested against API versions 31, 30 and 25 with the exact same output respectively. During your reproduction could you confirm that:

Mere opening and closing as you mentioned still works with the provided UNO version.

Also, note this issue does only occurs when <XamlControlsResources xmlns="using:Microsoft.UI.Xaml.Controls"/> is applied in App.xaml. On regular styles it works fine (might explain why you lack this issue in your app).

queequac commented 2 years ago

@marwalsch Thanks for the clarification, indeed there was not content in the pane... you tricked me by writing "See the app crashing", which is actually not happening in your repro sample.😅 Now I can see the issue. Also interesting that this is related to the specific styles... indeed I am using the "regular" ones in my app.

Additional note: If I modify your sample and bind IsPaneOpen against a property, the issue is the same. Same is true if you try the app on a bigger form factor or in landscape. An empty compact pane is shown then, butthe same Java exception is thrown. That means it has nothing to do with setting it from code-behind.

And I can confirm that commenting out <XamlControlsResources xmlns="using:Microsoft.UI.Xaml.Controls"/> makes the menu items appear just like under Windows. Long story short, seems more to be related to WinUI style of the NavigationView and an error on rendering the menu items in the pane.

queequac commented 2 years ago

Digged a little deeper into it... it's related to FooterMenuItemsSoure. Removing that binding, it renders correctly without any exception.

Edit: Another interesting finding. While the error relates to the FooterMenuItemsSource, it works fine if you define your footer menu items within XAML in the FooterMenuItemscollection or if you add them there from the code-behind instead of binding them.

@marwalsch Since I do not expect your footer navigation items to be very dynamic, I'd propose to set them from code. As a workaround you may simply remove the binding and add the items like the following in your sample:

public AppLayout()
{
     InitializeComponent();
     foreach (var item in FooterItems)
     {
         xNavigationView.FooterMenuItems.Add(item);
     }
     xNavigationView.SelectedItem = MenuItems.OfType<NavigationViewItem>().First();
}
marwalsch commented 2 years ago

@queequac

you tricked me by writing "See the app crashing", which is actually not happening in your repro sample.

Agreed as for the confusing title, it referred to the original finding, not the updated Repro.

Digged a little deeper into it... it's related to FooterMenuItemsSoure. Removing that binding, it renders correctly without any exception.

Great job finding the culprit, never would have guessed this would be exclusively connected to the FooterItems and using x:Bind.

Assigning the entire collection to FooterMenuItemsSource worked perfectly fine for me, no need to iterate through all elements. It's really just XAML-binding which causes the error. Replacing

FooterMenuItemsSource="{x:Bind FooterItems}" with xNavigationView.FooterMenuItemsSource = FooterItems;

thus lasts out as a workaround.

I won't close on my behalf though as actual bug is still present.