microsoft / TemplateStudio

Template Studio accelerates the creation of new WinUI 3, WPF, and UWP apps using a wizard-based experience.
Other
2.68k stars 457 forks source link

NavigationViewService: GetSelectedItem is looking on MenuItems, but not on MenuItemsSource #4616

Open tsvietOK opened 1 year ago

tsvietOK commented 1 year ago

Describe the bug

When you use binding in NavigationView with property MenuItemsSource, GetSelectedItem is looking for this selected item only in _navigationView.MenuItems and _navigationView.FooterMenuItems. But in case of binding usage, these two properties will be empty. image As a result we have broken navigation selector (when moving back), but navigation still works. Also, after startup there is no nav item selected. Of course I've tried to change

return GetSelectedItem(_navigationView.MenuItems, pageType)

to

return GetSelectedItem((IEnumerable<object>)_navigationView.MenuItemsSource, pageType)

But in the moment, when app starts, MenuItemSource is null. image

To Reproduce

I've created a test app. App in master branch is just output form TemplateStudio + added MenuItemsSource to NavView. App in broken branch is the same as master branch, but with changed GetSelectedItem(_navigationView.MenuItems, pageType) to GetSelectedItem((IEnumerable<object>)_navigationView.MenuItemsSource, pageType).

Additional context

No response

Applies to the following platforms:

About your setup

mikebattista commented 1 year ago

Is this a timing issue? Is MenuItems eventually populated when you use MenuItemsSource?

Also you're trying to cast a MenuItemsSource to an IEnumerable to get the list of MenuItems. Is that documented behavior or just something you tried? I wouldn't expect that to work.

tsvietOK commented 1 year ago

Is this a timing issue? Is MenuItems eventually populated when you use MenuItemsSource?

Yeah, I think so. MenuItemsSource is not available only at the moment when app starts. I think this this the reason why there is no selected item after startup. As temporary fix I've added if (_navigationView != null && _navigationView.MenuItemsSource != null) MenuItemsSource null check.

Also you're trying to cast a MenuItemsSource to an IEnumerable to get the list of MenuItems. Is that documented behavior or just something you tried? I wouldn't expect that to work.

Yes, this is because GetSelectedItem is expecting to get IEnumerable<>, but MenuItemsSource is an object, not collection.

mikebattista commented 1 year ago

GetSelectedItem is expecting a collection, yes, but MenuItemsSource isn't a collection as you said. It's an object.

Are you saying that you've confirmed that if MenuItemsSource isn't null, casting it to an IEnumerable will return a collection of MenuItems?

tsvietOK commented 1 year ago

No, I am saying that when you use MenuItems defined in XAML, then it (navigation view) will use _navigationView.MenuItems property (MenuItemsSource is null in this case) and navigation works fine. If you use MenuItemsSource in XAML, it (navigation view) will use _navigationView.MenuItemsSource property and _navigationView.MenuItems will be empty. But method GetSelectedItem still looking for selected item in MenuItems, which is empty when you use MenuItemsSource, so navigation is broken.

tsvietOK commented 1 year ago

Of course in order to fix that when moved from MenuItems to MenuItemsSource in method GetSelectedItem I've added null check for MenuItemsSource and added cast to IEnumerable<> and now it is working (there is still no selected item on startup).

mikebattista commented 1 year ago

I understand the problem you reported. I don't expect the workaround you tried (to cast MenuItemsSource to an IEnumerable) is a valid workaround.

If it's just a timing issue, there are a couple tactics to try:

  1. Use DispatcherQueue with Low priority to queue the operation. MenuItem properties may be populated when the queued callback executes. The sample App Notification code in the templates demonstrates this.
  2. Wire up a Page.OnLoaded event and update the selected item then. The ShellPage.xaml.cs in the templates already wires up this event to update the TitleBar and KeyboardAccelerators for example so you could add some code there.