NavigationView: Fix overflow button not being collapsed when the overflow menu only contains one item and NavigationView menu items are removed to empty the overflow menu

dev/NavigationView/NavigationView.cpp

@@ -151,10 +151,14 @@ void NavigationView::UnhookEventsAndClearFields(bool isFromDestructor)
     m_topNavFooterMenuRepeaterGettingFocusRevoker.revoke();
     m_topNavFooterMenuRepeater.set(nullptr);
 
+    m_footerItemsCollectionChangedRevoker.revoke();
+
     m_topNavOverflowItemsRepeaterElementPreparedRevoker.revoke();
     m_topNavOverflowItemsRepeaterElementClearingRevoker.revoke();
     m_topNavRepeaterOverflowView.set(nullptr);
 
+    m_topNavOverflowItemsCollectionChangedRevoker.revoke();
+
     if (isFromDestructor)
     {
         m_selectionChangedRevoker.revoke();
@@ -230,6 +234,14 @@ void NavigationView::OnFooterItemsSourceCollectionChanged(const winrt::IInspecta
     UpdateFooterRepeaterItemsSource(false /*sourceCollectionReset*/, true /*sourceCollectionChanged*/);
 }
 
+void NavigationView::OnOverflowItemsSourceCollectionChanged(const winrt::IInspectable&, const winrt::IInspectable&)
+{
+    if (m_topNavRepeaterOverflowView.get().ItemsSourceView().Count() == 0)
+    {
+        SetOverflowButtonVisibility(winrt::Visibility::Collapsed);
+    }
+}
+
 void NavigationView::OnSelectionModelSelectionChanged(const winrt::SelectionModel& selectionModel, const winrt::SelectionModelSelectionChangedEventArgs& e)
 {
     auto selectedItem = selectionModel.SelectedItem();
@@ -696,15 +708,47 @@ void NavigationView::UpdateTopNavRepeatersItemSource(const winrt::IInspectable&
     m_topDataProvider.SetDataSource(items);
 
     // rebinding
+    UpdateTopNavPrimaryRepeaterItemsSource(items);
+    UpdateTopNavOverflowRepeaterItemsSource(items);
+}
+
+void NavigationView::UpdateTopNavPrimaryRepeaterItemsSource(const winrt::IInspectable& items)
+{
     if (items)
     {
         UpdateItemsRepeaterItemsSource(m_topNavRepeater.get(), m_topDataProvider.GetPrimaryItems());
-        UpdateItemsRepeaterItemsSource(m_topNavRepeaterOverflowView.get(), m_topDataProvider.GetOverflowItems());
     }
     else
     {
         UpdateItemsRepeaterItemsSource(m_topNavRepeater.get(), nullptr);
-        UpdateItemsRepeaterItemsSource(m_topNavRepeaterOverflowView.get(), nullptr);
+    }
+}
+
+void NavigationView::UpdateTopNavOverflowRepeaterItemsSource(const winrt::IInspectable& items)
+{
+    m_topNavOverflowItemsCollectionChangedRevoker.revoke();
+
+    if (const auto overflowRepeater = m_topNavRepeaterOverflowView.get())
+    {
+        if (items)
+        {
+            const auto 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 = overflowRepeater.ItemsSourceView().CollectionChanged(winrt::auto_revoke, { this, &NavigationView::OnOverflowItemsSourceCollectionChanged });
+        }
+        else
+        {
+            overflowRepeater.ItemsSource(nullptr);
+        }
     }
 }
 

dev/NavigationView/NavigationView.h

@@ -157,6 +157,7 @@ class NavigationView :
     inline bool IsNavigationViewListSingleSelectionFollowsFocus();
     inline void UpdateSingleSelectionFollowsFocusTemplateSetting();
     void OnFooterItemsSourceCollectionChanged(const winrt::IInspectable &, const winrt::IInspectable &);
+    void OnOverflowItemsSourceCollectionChanged(const winrt::IInspectable&, const winrt::IInspectable&);
     void SetSelectedItemAndExpectItemInvokeWhenSelectionChangedIfNotInvokedFromAPI(winrt::IInspectable const& item);
     void ChangeSelectStatusForItem(winrt::IInspectable const& item, bool selected);
     void UnselectPrevItem(winrt::IInspectable const& prevItem, winrt::IInspectable const& nextItem);
@@ -236,6 +237,8 @@ class NavigationView :
     void UpdatePaneTitleMargins();
     void UpdateLeftRepeaterItemSource(const winrt::IInspectable& items);
     void UpdateTopNavRepeatersItemSource(const winrt::IInspectable& items);
+    void UpdateTopNavPrimaryRepeaterItemsSource(const winrt::IInspectable& items);
+    void UpdateTopNavOverflowRepeaterItemsSource(const winrt::IInspectable& items);
     static void UpdateItemsRepeaterItemsSource(const winrt::ItemsRepeater& listView, const winrt::IInspectable& itemsSource);
     void UpdateSelectionForMenuItems();
     bool UpdateSelectedItemFromMenuItems(const winrt::impl::com_ref<winrt::IVector<winrt::IInspectable>>& menuItems, bool foundFirstSelected = false);
@@ -423,6 +426,7 @@ class NavigationView :
 
     winrt::ItemsSourceView::CollectionChanged_revoker m_footerItemsCollectionChangedRevoker{};
 
+    winrt::ItemsSourceView::CollectionChanged_revoker m_topNavOverflowItemsCollectionChangedRevoker{};
 
     winrt::FlyoutBase::Closing_revoker m_flyoutClosingRevoker{};
 

dev/NavigationView/NavigationView_InteractionTests/TopModeTests.cs

@@ -672,5 +672,55 @@ public void VerifyNavigationViewContentChangeOnTopNavImpactsLayout()
             }
         }
 
+        [TestMethod]
+        public void VerifyOverflowButtonIsCollapsedWhenOverflowMenuIsEmpty()
+        {
+            using (var setup = new TestSetupHelper(new[] { "NavigationView Tests", "Top NavigationView Overflow Button Test" }))
+            {
+                VerifyOverflowButtonIsVisibleAndOverflowMenuHasSingleItem();
+
+                Log.Comment("Verify that removing the final overflow menu item collapses the overflow button");
+                FindElement.ByName<Button>("RemoveLastItemButton").InvokeAndWait();
+
+                // Refresh the cache to make sure that the overflow button we are going to be searching for
+                // does not return as a false positive due to the caching mechanism.
+                ElementCache.Clear();
+
+                VerifyOverflowButtonIsCollapsed();
+
+                Log.Comment("Add menu item to show the overflow button again with the overflow menu item containing a single item");
+                FindElement.ByName<Button>("AddItem4Button").InvokeAndWait();
+
+                VerifyOverflowButtonIsVisibleAndOverflowMenuHasSingleItem();
+
+                Log.Comment("Verify that removing a primary menu item which creates enough room for the single overflow menu item to be moved into collapses the overflow button");
+                FindElement.ByName<Button>("RemoveFirstItemButton").InvokeAndWait();
+
+                // Refresh the cache to make sure that the overflow button we are going to be searching for
+                // does not return as a false positive due to the caching mechanism.
+                ElementCache.Clear();
+
+                VerifyOverflowButtonIsCollapsed();
+
+                void VerifyOverflowButtonIsVisibleAndOverflowMenuHasSingleItem()
+                {
+                    UIObject overflowButton = FindElement.ById("TopNavOverflowButton");
+                    Verify.IsNotNull(overflowButton, "The overflow button is required to be visible for this test");
+
+                    Log.Comment("Verify that the overflow menu contains a single item");
+                    overflowButton.Click();
+                    Wait.ForIdle();
+
+                    int numOverflowMenuItems = GetTopNavigationItems(TopNavPosition.Overflow).Count;
+                    Verify.IsTrue(numOverflowMenuItems == 1, "The overflow menu should only contain a single item");
+                }
+
+                void VerifyOverflowButtonIsCollapsed()
+                {
+                    UIObject overflowButton = FindElement.ById("TopNavOverflowButton");
+                    Verify.IsNull(overflowButton, "The overflow button should have been collapsed");
+                }
+            }
+        }
     }
 }

dev/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>

dev/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); };

dev/NavigationView/TestUI/NavigationView_TestUI.projitems

@@ -74,6 +74,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>
@@ -114,7 +118,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Common\NavigationViewIsPaneOpenPage.xaml.cs">
       <DependentUpon>NavigationViewIsPaneOpenPage.xaml</DependentUpon>
     </Compile>
-    <Compile Include="$(MSBuildThisFileDirectory)Common\NavigationViewMenuItemStretchPage.xaml.cs" >
+    <Compile Include="$(MSBuildThisFileDirectory)Common\NavigationViewMenuItemStretchPage.xaml.cs">
       <DependentUpon>NavigationViewMenuItemStretchPage.xaml</DependentUpon>
     </Compile>
     <Compile Include="$(MSBuildThisFileDirectory)Common\NavigationViewMinimalPage.xaml.cs">
@@ -144,6 +148,9 @@
     <Compile Include="$(MSBuildThisFileDirectory)Common\NavigationViewInitPage.xaml.cs">
       <DependentUpon>NavigationViewInitPage.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>

dev/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="using:MUXControlsTestApp"
+    xmlns:muxcontrols="using:Microsoft.UI.Xaml.Controls"
+    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="500">
+            <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>

dev/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 Windows.UI.Xaml;
+
+using NavigationViewItem = Microsoft.UI.Xaml.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);
+            }
+        }
+    }
+}