Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

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

Closed
Pinox opened this issue Jan 19, 2021 · 12 comments · Fixed by #14767
Closed

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

Pinox opened this issue Jan 19, 2021 · 12 comments · Fixed by #14767

Comments

@Pinox
Copy link

Pinox commented Jan 19, 2021

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.

@Pinox Pinox added s/unverified New report that has yet to be verified t/bug 🐛 labels Jan 19, 2021
@samhouts samhouts added this to New in Triage Jan 19, 2021
@PureWeen
Copy link
Contributor

@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

@PureWeen PureWeen added s/needs-info ❓ A question has been asked that requires an answer before work can continue on this issue. s/needs-repro ❔ This reported issue doesn't include a sample project reproducing the issue. Please provide one. labels Jan 20, 2021
@PureWeen PureWeen moved this from New to Needs Info in Triage Jan 20, 2021
@PureWeen PureWeen added 5.0.0 Regression on 5.0.0 i/regression labels Jan 20, 2021
@Pinox
Copy link
Author

Pinox commented Jan 21, 2021

@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

@Redth Redth removed s/needs-info ❓ A question has been asked that requires an answer before work can continue on this issue. s/needs-repro ❔ This reported issue doesn't include a sample project reproducing the issue. Please provide one. labels Jan 21, 2021
@PureWeen PureWeen moved this from Needs Info to New in Triage Jan 22, 2021
@EmilAlipiev
Copy link
Contributor

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
Copy link
Contributor

PureWeen commented Jan 28, 2021

@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?

@PureWeen PureWeen added this to the 5.0.0 milestone Jan 28, 2021
@PureWeen PureWeen added this to To do in vNext+1 (5.0.0) via automation Jan 28, 2021
@PureWeen PureWeen moved this from New to Ready For Work in Triage Jan 28, 2021
@PureWeen PureWeen removed the s/unverified New report that has yet to be verified label Jan 28, 2021
@Pinox
Copy link
Author

Pinox commented Jan 29, 2021

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
Copy link
Contributor

EmilAlipiev commented Jan 29, 2021

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

@PureWeen
Copy link
Contributor

@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
Copy link
Contributor

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
Copy link
Contributor

i had an existing issue #5183

@Pinox
Copy link
Author

Pinox commented Jan 30, 2021

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
Copy link
Contributor

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.

@Redth Redth moved this from Ready For Work to Needs Estimate in Triage Feb 2, 2021
@hartez hartez added this to To Do in 5.0.0 SR 3 Feb 2, 2021
@hartez hartez removed this from To do in vNext+1 (5.0.0) Feb 2, 2021
@hartez hartez added this to To Fix in 5.0.0 SR 4 (Planning) Feb 9, 2021
@Redth Redth removed this from To Fix in 5.0.0 SR 3 Feb 9, 2021
@Redth Redth moved this from To Fix to Issue In progress in 5.0.0 SR 4 (Planning) Jun 28, 2021
@Redth Redth moved this from Issue In progress to Meets Bar in 5.0.0 SR 4 (Planning) Jun 28, 2021
@jfversluis jfversluis removed this from Meets Bar in 5.0.0 SR 4 (Planning) Oct 6, 2021
@jsuarezruiz jsuarezruiz moved this from To Fix to Issues in Progress in 5.0.0 SR7 (Planning) - Target Date Nov. 10th Oct 20, 2021
@jfversluis jfversluis moved this from Issues in Progress to PR Needs Review in 5.0.0 SR7 (Planning) - Target Date Nov. 10th Oct 27, 2021
@jfversluis
Copy link
Member

@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!

@jfversluis jfversluis moved this from To Fix to PR Needs Review in 5.0.0 SR8 (Planning) - Target Date Dec. 15th Nov 17, 2021
@jfversluis jfversluis moved this from To Fix to PR Needs Review in 5.0.0 SR9 (Planning) - Target Date Jan. 19th Dec 23, 2021
Triage automation moved this from Needs Estimate to Closed Dec 23, 2021
5.0.0 SR9 (Planning) - Target Date Jan. 19th automation moved this from PR Needs Review to Done Dec 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Triage
  
Closed
Development

Successfully merging a pull request may close this issue.

6 participants