xamarin / Xamarin.Forms

Xamarin.Forms is no longer supported. Migrate your apps to .NET MAUI.
https://aka.ms/xamarin-upgrade
Other
5.62k stars 1.87k forks source link

[Bug] XF 5.0 breaks Native UWP winui:NavigationView #13457

Closed Pinox closed 2 years ago

Pinox commented 3 years ago

Upgrading to XF 5.0 in my native UWP app breaks the winui:NavigationView.

I used UWP TemplateStudio to check results: This is navigationview with XF 4.8

UWP_Menu1

This is the navigationview in UWP when upgrading to XF 5.0

As you can see the NavigationView is displayed in a very compressed layout. Not sure if there is a way to override this default XF behaviour in version 5.0

UWP_Menu2

Source in UWP app.

List assembliesToInclude = new List(); Xamarin.Forms.Forms.Init((LaunchActivatedEventArgs)activationArgs, assembliesToInclude);

The above 2 lines of source creates the problem in the UWP app without using any other XF functionality.

PureWeen commented 3 years ago

@Pinox can you provide a sample? With NavigationView we are just pulling in WinUI dependencies and with 5.0 we bumped up the WinUI dependency but we aren't replacing any of the implicit styles for these controls

Pinox commented 3 years ago

@PureWeen Sure , please find attached the sample.

Just uncomment below to see effect.

List assembliesToInclude = new List(); Xamarin.Forms.Forms.Init((LaunchActivatedEventArgs)activationArgs, assembliesToInclude);

App8.zip

EmilAlipiev commented 3 years ago

but as my knowledge, XF doesnt implement navigationview for uwp. it is same custom framework element like a listview. Despite that yes, XF 5 messed up my master Detail page as it is now called flyout menu.

PureWeen commented 3 years ago

@Pinox my guess is that it's caused by this

https://github.com/xamarin/Xamarin.Forms/blob/5.0.0/Xamarin.Forms.Platform.UAP/Shell/ShellStyles.xaml#L11

If you do this?

                Xamarin.Forms.Forms.Init((LaunchActivatedEventArgs)activationArgs, assembliesToInclude);
                Application.Current.Resources["NavigationViewItemOnLeftMinHeight"] = 40;

does it fix?

Pinox commented 3 years ago

Hi @PureWeen ,

The styles you are suggesting seems to be included in a Xamarin Forms project that targets UWP. I don't have a Xamarin forms project in the sample app above. I simply reference the Xamarin Forms nuget and then initialize XF in native UWP.

I was planning to use a bing maps feature from XF in my native UWP app therefor the XF reference,
So I can't change styles in XF project as I don't have one.

This issue with styles result from the fact that I simply reference a XF nuget and then initialize that nuget in my native uwp app.

If your style that you suggest did not exist in XF v4.8 then I agree this is probably causing this result. Weird thing is XF interfered with native UWP app just by initializing the XF nuggets. It's like XF overrides the native UWP app behavior.

In one of my other native UWP apps I decided to take out this XF reference and it resulted in a fairly noticeable performance improvement in the views of this native UWP app. So there is massive performance degradation in native UWP by simply referencing the XF nugets that I think should be pointed out in the XF documentation.

EmilAlipiev commented 3 years ago

wait! silly question but does XF implement Native UWP navigationview as masterdetail(as previously known) or it is only for shell?

PureWeen commented 3 years ago

@EmilAlipiev only Shell. The FlyoutPage was really in name only, behavior is all the same. If it's not please log an issue.

@Pinox Right, but when you Init Forms it loads all of our resource files into your App ResourceDictionary so whatever we have specified that modifies global styles (implicit styles, etc...) will change things in your application. It's basically the same thing as including WinUI.

I added this to your code and it looks like that fixes it

                Xamarin.Forms.Forms.Init((LaunchActivatedEventArgs)activationArgs, assembliesToInclude);
                Application.Current.Resources["NavigationViewItemOnLeftMinHeight"] = 40;

As far as performance I could see it harming startup performance because of assembly scanning but it shouldn't change runtime performance. If you have an example of it harming runtime performance that would be helpful

EmilAlipiev commented 3 years ago

Not exactly the same. Xf non shell implements a custom framework element as masterdetail page which has never been a native navigation menu. For example Uwp navigation menu works on xbox app with controller pads perfectly which implements xy focus navigation but xf master detail menu doesn't. Beside that uep navigation menu has horizontal, footer and header features. Not sure if shell have does. I should give it a try

EmilAlipiev commented 3 years ago

i had an existing issue https://github.com/xamarin/Xamarin.Forms/issues/5183

Pinox commented 3 years ago

thanks @PureWeen. Just for reference is the Application.Current.Resources["NavigationViewItemOnLeftMinHeight"] = 0 the new default in XF 5 ? So we then have to manually change this settings.

if you have an example of it harming runtime performance that would be helpful

That is going to be more difficult as my UWP app is fairly big with lots of image rendering. If I can replicate the performance issue on a smaller uwp app I will let you know.

thanks again.

PureWeen commented 3 years ago

So we then have to manually change this settings.

With Shell the "FlyoutItems" are all generated from XAML. We remove the min size so users aren't confined by the platform but ideally it shouldn't be breaking your use case. I've moved this item to our shell/regression board so we can fix.

jfversluis commented 3 years ago

@Pinox and others, a PR (#14767) for this is open now, would you be able to grab the NuGet as described here and let us know if this fixes this issue? That will greatly speed up the review process.

Besides verifying if this particular issue is fixed also be sure to check other scenarios in the same area to make sure that this fix doesn't accidentally has side-effects 🙂

Thanks!