NavigationView: Fix overflow button not being collapsed when the over…

…flow menu only contains one item and NavigationView menu items are removed to empty the overflow menu (microsoft/microsoft-ui-xaml#3087)

ModernWpf.Controls/NavigationView/NavigationView.cs

@@ -183,13 +183,19 @@ void UnhookEventsAndClearFields(bool isFromDestructor = false)
                 m_topNavFooterMenuRepeater = null;
             }
 
+            m_footerItemsCollectionChangedRevoker?.Revoke();
+            m_footerItemsCollectionChangedRevoker = null;
+
             if (m_topNavRepeaterOverflowView != null)
             {
                 m_topNavRepeaterOverflowView.ElementPrepared -= OnRepeaterElementPrepared;
                 m_topNavRepeaterOverflowView.ElementClearing -= OnRepeaterElementClearing;
                 m_topNavRepeaterOverflowView = null;
             }
 
+            m_topNavOverflowItemsCollectionChangedRevoker?.Revoke();
+            m_topNavOverflowItemsCollectionChangedRevoker = null;
+
             if (isFromDestructor)
             {
                 m_selectionModel.SelectionChanged -= OnSelectionModelSelectionChanged;
@@ -265,6 +271,14 @@ void OnFooterItemsSourceCollectionChanged(object sender, object e)
             UpdateFooterRepeaterItemsSource(false /*sourceCollectionReset*/, true /*sourceCollectionChanged*/);
         }
 
+        void OnOverflowItemsSourceCollectionChanged(object sender, object e)
+        {
+            if (m_topNavRepeaterOverflowView.ItemsSourceView.Count == 0)
+            {
+                SetOverflowButtonVisibility(Visibility.Collapsed);
+            }
+        }
+
         void OnSelectionModelSelectionChanged(SelectionModel selectionModel, SelectionModelSelectionChangedEventArgs e)
         {
             var selectedItem = selectionModel.SelectedItem;
@@ -767,15 +781,48 @@ void UpdateTopNavRepeatersItemSource(object items)
             m_topDataProvider.SetDataSource(items);
 
             // rebinding
+            UpdateTopNavPrimaryRepeaterItemsSource(items);
+            UpdateTopNavOverflowRepeaterItemsSource(items);
+        }
+
+        void UpdateTopNavPrimaryRepeaterItemsSource(object items)
+        {
             if (items != null)
             {
                 UpdateItemsRepeaterItemsSource(m_topNavRepeater, m_topDataProvider.GetPrimaryItems());
-                UpdateItemsRepeaterItemsSource(m_topNavRepeaterOverflowView, m_topDataProvider.GetOverflowItems());
             }
             else
             {
                 UpdateItemsRepeaterItemsSource(m_topNavRepeater, null);
-                UpdateItemsRepeaterItemsSource(m_topNavRepeaterOverflowView, null);
+            }
+        }
+
+        void UpdateTopNavOverflowRepeaterItemsSource(object items)
+        {
+            m_topNavOverflowItemsCollectionChangedRevoker?.Revoke();
+            m_topNavOverflowItemsCollectionChangedRevoker = null;
+
+            if (m_topNavRepeaterOverflowView is { } overflowRepeater)
+            {
+                if (items != null)
+                {
+                     var itemsSource = m_topDataProvider.GetOverflowItems();
+                    overflowRepeater.ItemsSource = itemsSource;
+
+                    // We listen to changes to the overflow menu item collection so we can set the visibility of the overflow button
+                    // to collapsed when it no longer has any items.
+                    //
+                    // Normally, MeasureOverride() kicks off updating the button's visibility, however, it is not run when the overflow menu
+                    // only contains a *single* item and we
+                    // - either remove that menu item or
+                    // - remove menu items displayed in the NavigationView pane until there is enough room for the single overflow menu item
+                    //   to be displayed in the pane
+                    m_topNavOverflowItemsCollectionChangedRevoker = new ItemsSourceView.CollectionChangedRevoker(overflowRepeater.ItemsSourceView, OnOverflowItemsSourceCollectionChanged);
+                }
+                else
+                {
+                    overflowRepeater.ItemsSource = null;
+                }
             }
         }
 
@@ -834,7 +881,7 @@ object init()
                 if (m_footerItemsSource is null)
                 {
                     m_footerItemsSource = new InspectingDataSource(itemsSource);
-                    m_footerItemsSource.CollectionChanged += OnFooterItemsSourceCollectionChanged;
+                    m_footerItemsCollectionChangedRevoker = new ItemsSourceView.CollectionChangedRevoker(m_footerItemsSource, OnFooterItemsSourceCollectionChanged);
                 }
 
                 if (m_footerItemsSource != null)
@@ -5597,6 +5644,10 @@ protected override void OnDpiChanged(DpiScale oldDpi, DpiScale newDpi)
         // Event Tokens
         bool m_layoutUpdatedToken;
 
+        ItemsSourceView.CollectionChangedRevoker m_footerItemsCollectionChangedRevoker;
+
+        ItemsSourceView.CollectionChangedRevoker m_topNavOverflowItemsCollectionChangedRevoker;
+
         bool m_wasForceClosed = false;
         bool m_isClosedCompact = false;
         bool m_blockNextClosingEvent = false;

ModernWpf.Controls/NavigationView/NavigationViewItem.cs

@@ -172,7 +172,7 @@ object init()
                 }
                 m_itemsSourceViewCollectionChangedRevoker?.Revoke();
                 repeater.ItemsSource = itemsSource;
-                m_itemsSourceViewCollectionChangedRevoker = new ItemsSourceView.CollectionChanged_revoker(repeater.ItemsSourceView, OnItemsSourceViewChanged);
+                m_itemsSourceViewCollectionChangedRevoker = new ItemsSourceView.CollectionChangedRevoker(repeater.ItemsSourceView, OnItemsSourceViewChanged);
             }
         }
 
@@ -920,7 +920,7 @@ void UnhookEventsAndClearFields()
 
         ItemsRepeaterElementPreparedRevoker m_repeaterElementPreparedRevoker;
         ItemsRepeaterElementClearingRevoker m_repeaterElementClearingRevoker;
-        ItemsSourceView.CollectionChanged_revoker m_itemsSourceViewCollectionChangedRevoker;
+        ItemsSourceView.CollectionChangedRevoker m_itemsSourceViewCollectionChangedRevoker;
 
         FlyoutBaseClosingRevoker m_flyoutClosingRevoker;
 

ModernWpf.Controls/Repeater/ItemsRepeater/ItemsSource/ItemsSourceView.cs

@@ -89,9 +89,9 @@ internal virtual int IndexOfCore(object value)
 
         private int m_cachedSize = -1;
 
-        internal class CollectionChanged_revoker : EventRevoker<ItemsSourceView, NotifyCollectionChangedEventHandler>
+        internal class CollectionChangedRevoker : EventRevoker<ItemsSourceView, NotifyCollectionChangedEventHandler>
         {
-            public CollectionChanged_revoker(ItemsSourceView source, NotifyCollectionChangedEventHandler handler) : base(source, handler)
+            public CollectionChangedRevoker(ItemsSourceView source, NotifyCollectionChangedEventHandler handler) : base(source, handler)
             {
             }
 

test/NavigationView_TestUI/NavigationViewCaseBundle.xaml

@@ -42,6 +42,7 @@
             <Button x:Name="NavigationViewTopNavPage" AutomationProperties.Name="NavigationView TopNav Test" Margin="2" HorizontalAlignment="Stretch">NavigationView TopNav Test</Button>
             <Button x:Name="NavigationViewTopNavOnlyPage" AutomationProperties.Name="Top NavigationView Test" Margin="2" HorizontalAlignment="Stretch">Top NavigationView Test</Button>
             <Button x:Name="NavigationViewTopNavStorePage" AutomationProperties.Name="Top NavigationView Store Test" Margin="2" HorizontalAlignment="Stretch">Top NavigationView Store Test</Button>
+            <Button x:Name="NavigateToTopNavOverflowButtonPage" AutomationProperties.Name="Top NavigationView Overflow Button Test" Margin="2" HorizontalAlignment="Stretch">Top NavigationView Overflow Button Test</Button>
         </StackPanel>
 
         <StackPanel>

test/NavigationView_TestUI/NavigationViewCaseBundle.xaml.cs

@@ -25,6 +25,7 @@ public NavigationViewCaseBundle()
             NavigationViewTopNavPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewTopNavPage), 0); };
             NavigationViewTopNavOnlyPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewTopNavOnlyPage), 0); };
             NavigationViewTopNavStorePage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewTopNavStorePage), 0); };
+            NavigateToTopNavOverflowButtonPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewTopNavOverflowButtonPage), 0); };
             NavigateToSelectedItemEdgeCasePage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewSelectedItemEdgeCasePage), 0); };
             NavigateToInitPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewInitPage), 0); };
             NavigateToStretchPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewStretchPage), 0); };

test/NavigationView_TestUI/NavigationView_TestUI.projitems

@@ -85,6 +85,10 @@
       <SubType>Designer</SubType>
       <Generator>MSBuild:Compile</Generator>
     </Page>
+    <Page Include="$(MSBuildThisFileDirectory)TopMode\NavigationViewTopNavOverflowButtonPage.xaml">
+      <SubType>Designer</SubType>
+      <Generator>MSBuild:Compile</Generator>
+    </Page>
     <Page Include="$(MSBuildThisFileDirectory)TopMode\NavigationViewTopNavPage.xaml">
       <SubType>Designer</SubType>
       <Generator>MSBuild:Compile</Generator>
@@ -152,6 +156,9 @@
     <Compile Include="$(MSBuildThisFileDirectory)TopMode\NavigationViewTopNavOnlyPage.xaml.cs">
       <DependentUpon>NavigationViewTopNavOnlyPage.xaml</DependentUpon>
     </Compile>
+    <Compile Include="$(MSBuildThisFileDirectory)TopMode\NavigationViewTopNavOverflowButtonPage.xaml.cs">
+      <DependentUpon>NavigationViewTopNavOverflowButtonPage.xaml</DependentUpon>
+    </Compile>
     <Compile Include="$(MSBuildThisFileDirectory)TopMode\NavigationViewTopNavPage.xaml.cs">
       <DependentUpon>NavigationViewTopNavPage.xaml</DependentUpon>
     </Compile>
@@ -163,7 +170,6 @@
     <Folder Include="$(MSBuildThisFileDirectory)Common\" />
     <Folder Include="$(MSBuildThisFileDirectory)CustomResources\" />
     <Folder Include="$(MSBuildThisFileDirectory)Hierarchical\" />
-    <Folder Include="$(MSBuildThisFileDirectory)TopMode\" />
     <Folder Include="$(MSBuildThisFileDirectory)Regression\" />
   </ItemGroup>
 </Project>

test/NavigationView_TestUI/TopMode/NavigationViewTopNavOverflowButtonPage.xaml

@@ -0,0 +1,34 @@
+<!-- Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. See LICENSE in the project root for license information. -->
+<local:TestPage
+    x:Class="MUXControlsTestApp.NavigationViewTopNavOverflowButtonPage"
+    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
+    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
+    xmlns:local="clr-namespace:MUXControlsTestApp"
+    xmlns:muxcontrols="http://schemas.modernwpf.com/2019"
+    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
+    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
+    mc:Ignorable="d">
+
+    <StackPanel Background="{ThemeResource ApplicationPageBackgroundThemeBrush}"
+                HorizontalAlignment="Left">
+        <!-- The width and number of NavigationView menu items have been set in such a way that the overflow menu button
+        is visible and the overflow menu only contains a single item. -->
+        <muxcontrols:NavigationView x:Name="NavView"
+                                    PaneDisplayMode="Top"
+                                    IsBackButtonVisible="Collapsed"
+                                    MinWidth="100" Width="499">
+            <muxcontrols:NavigationView.MenuItems>
+                <muxcontrols:NavigationViewItem Content="Menu Item 1"/>
+                <muxcontrols:NavigationViewItem Content="Menu Item 2" />
+                <muxcontrols:NavigationViewItem Content="Menu Item 3"/>
+                <muxcontrols:NavigationViewItem Content="Menu Item 4"/>
+            </muxcontrols:NavigationView.MenuItems>
+        </muxcontrols:NavigationView>
+
+        <StackPanel Orientation="Horizontal" Margin="8,10,0,0">
+            <Button x:Name="AddItem4Button" AutomationProperties.Name="AddItem4Button" Click="AddItem4Button_Click" Margin="5,0,0,0" Content="Add Menu Item 4"/>
+            <Button x:Name="RemoveFirstItemButton" AutomationProperties.Name="RemoveFirstItemButton" Click="RemoveFirstItemButton_Click" Margin="5,0,0,0" Content="Remove First Item"/>
+            <Button x:Name="RemoveLastItemButton" AutomationProperties.Name="RemoveLastItemButton" Click="RemoveLastItemButton_Click" Margin="5,0,0,0" Content="Remove Last Item"/>
+        </StackPanel>
+    </StackPanel>
+</local:TestPage>

test/NavigationView_TestUI/TopMode/NavigationViewTopNavOverflowButtonPage.xaml.cs

@@ -0,0 +1,43 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License. See LICENSE in the project root for license information.
+
+using System.Windows;
+
+using NavigationViewItem = ModernWpf.Controls.NavigationViewItem;
+
+namespace MUXControlsTestApp
+{
+    public sealed partial class NavigationViewTopNavOverflowButtonPage : TestPage
+    {
+        public NavigationViewTopNavOverflowButtonPage()
+        {
+            this.InitializeComponent();
+        }
+
+        private void AddItem4Button_Click(object sender, RoutedEventArgs e)
+        {
+            var menuItem = new NavigationViewItem 
+            {
+                Content = $"Menu Item 4",
+            };
+
+            this.NavView.MenuItems.Add(menuItem);
+        }
+
+        private void RemoveFirstItemButton_Click(object sender, RoutedEventArgs e)
+        {
+            if (NavView.MenuItems.Count > 0)
+            {
+                NavView.MenuItems.RemoveAt(0);
+            }
+        }
+
+        private void RemoveLastItemButton_Click(object sender, RoutedEventArgs e)
+        {
+            if (NavView.MenuItems.Count > 0)
+            {
+                NavView.MenuItems.RemoveAt(NavView.MenuItems.Count - 1);
+            }
+        }
+    }
+}