Fix submenu navigation in NavigationViewItem

[koal44]

The feature for submenus in NavigationViewTop was previously implemented but left unused prolly because of odd behavior such as stack overflow errors in AutoSuggest.

The problem was traced to the NavigationViewItem.MenuItems dependency property, where a new ObservableCollection was incorrectly used as the default value. According to MS Docs, "dependency property metadata shouldn't include a default reference-type value because that value will be assigned to all instances of the class, creating a singleton class."

Pull request type

Please check the type of change your PR introduces:

• Update
• Bugfix
• Feature
• Code style update (formatting, renaming)
• Refactoring (no functional changes, no api changes)
• Build related changes
• Documentation content changes

What is the current behavior?

In the Gallery app if you add submenu items to NavigationView.Top you will get stack overflow
errors on AddItemsToAutoSuggestBoxItems(IList list) for menuItems.

<ui:NavigationView PaneDisplayMode="Top">
ui:NavigationView.MenuItems
<ui:NavigationViewItem Content="Menu Item 1"
ui:NavigationViewItem.MenuItems
<ui:NavigationViewItem Content="Menu SubItem 1" />
<ui:NavigationViewItem Content="Menu SubItem 2" />
</ui:NavigationViewItem.MenuItems>
</ui:NavigationViewItem>

Issue Number: N/A

What is the new behavior?

• Change MenuItems to an ObservableCollection and initialize in ctor
• Fix related properties such as HasMenuItems and MenuItemsSource (removed)
• Add demo in the Gallery app

Other information

[syntax-tm]

Why did you remove the MenuItemsSource?

Items is for defining in XAML. This is usually an object[] but it contains FrameworkElements.
ItemsSource is for databinding to a collection. Typically uses a DataTemplate to display the items.

• ItemsControl.Items Property
• ItemsControl.ItemsSource Property

If one of those was broken, then it's better to fix it instead of removing one of the options. You should be able to define a menu directly in XAML or bind to an IEnumerable source (with support for INotifyCollectionChanged).

[koal44]

Ah, thx for the feedback. My intent wasn't to limit options but to keep MenuItems to a single source of truth. I agree that NavigationViewItem is fundamentally an ItemsControl, and retaining MenuItemsSource makes sense. To test compatibility with view models, I made an experimental branch here and basically ran smack into a bug where binding to MenuItems nullified the collection. I'll keep working on it in the morning.

[koal44]

Here's an update on MenuItems working with both xaml and viewmodels. I'm still trying to understand why MS uses a source property for ItemsControl; it seems unneeded here. But i prolly lack the experience to make that call.