Skip to content

Commit d6b4251

Browse files
Felix-DevFelix-Dev
authored
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 (#3087)
* Fix overflow button not being collapsed. * Add interaction test. * Revoking m_footerItemsCollectionChangedRevoker in UnhookEventsAndClearFields() now. * Move NavigationViewTopNavOverflowButtonPage into TopMode folder. * CR feedback
1 parent 4981365 commit d6b4251

8 files changed

+187
-3
lines changed

‎dev/NavigationView/NavigationView.cpp

+46-2
Original file line numberDiff line numberDiff line change
@@ -151,10 +151,14 @@ void NavigationView::UnhookEventsAndClearFields(bool isFromDestructor)
151151
m_topNavFooterMenuRepeaterGettingFocusRevoker.revoke();
152152
m_topNavFooterMenuRepeater.set(nullptr);
153153

154+
m_footerItemsCollectionChangedRevoker.revoke();
155+
154156
m_topNavOverflowItemsRepeaterElementPreparedRevoker.revoke();
155157
m_topNavOverflowItemsRepeaterElementClearingRevoker.revoke();
156158
m_topNavRepeaterOverflowView.set(nullptr);
157159

160+
m_topNavOverflowItemsCollectionChangedRevoker.revoke();
161+
158162
if (isFromDestructor)
159163
{
160164
m_selectionChangedRevoker.revoke();
@@ -230,6 +234,14 @@ void NavigationView::OnFooterItemsSourceCollectionChanged(const winrt::IInspecta
230234
UpdateFooterRepeaterItemsSource(false /*sourceCollectionReset*/, true /*sourceCollectionChanged*/);
231235
}
232236

237+
void NavigationView::OnOverflowItemsSourceCollectionChanged(const winrt::IInspectable&, const winrt::IInspectable&)
238+
{
239+
if (m_topNavRepeaterOverflowView.get().ItemsSourceView().Count() == 0)
240+
{
241+
SetOverflowButtonVisibility(winrt::Visibility::Collapsed);
242+
}
243+
}
244+
233245
void NavigationView::OnSelectionModelSelectionChanged(const winrt::SelectionModel& selectionModel, const winrt::SelectionModelSelectionChangedEventArgs& e)
234246
{
235247
auto selectedItem = selectionModel.SelectedItem();
@@ -702,15 +714,47 @@ void NavigationView::UpdateTopNavRepeatersItemSource(const winrt::IInspectable&
702714
m_topDataProvider.SetDataSource(items);
703715

704716
// rebinding
717+
UpdateTopNavPrimaryRepeaterItemsSource(items);
718+
UpdateTopNavOverflowRepeaterItemsSource(items);
719+
}
720+
721+
void NavigationView::UpdateTopNavPrimaryRepeaterItemsSource(const winrt::IInspectable& items)
722+
{
705723
if (items)
706724
{
707725
UpdateItemsRepeaterItemsSource(m_topNavRepeater.get(), m_topDataProvider.GetPrimaryItems());
708-
UpdateItemsRepeaterItemsSource(m_topNavRepeaterOverflowView.get(), m_topDataProvider.GetOverflowItems());
709726
}
710727
else
711728
{
712729
UpdateItemsRepeaterItemsSource(m_topNavRepeater.get(), nullptr);
713-
UpdateItemsRepeaterItemsSource(m_topNavRepeaterOverflowView.get(), nullptr);
730+
}
731+
}
732+
733+
void NavigationView::UpdateTopNavOverflowRepeaterItemsSource(const winrt::IInspectable& items)
734+
{
735+
m_topNavOverflowItemsCollectionChangedRevoker.revoke();
736+
737+
if (const auto overflowRepeater = m_topNavRepeaterOverflowView.get())
738+
{
739+
if (items)
740+
{
741+
const auto itemsSource = m_topDataProvider.GetOverflowItems();
742+
overflowRepeater.ItemsSource(itemsSource);
743+
744+
// We listen to changes to the overflow menu item collection so we can set the visibility of the overflow button
745+
// to collapsed when it no longer has any items.
746+
//
747+
// Normally, MeasureOverride() kicks off updating the button's visibility, however, it is not run when the overflow menu
748+
// only contains a *single* item and we
749+
// - either remove that menu item or
750+
// - remove menu items displayed in the NavigationView pane until there is enough room for the single overflow menu item
751+
// to be displayed in the pane
752+
m_topNavOverflowItemsCollectionChangedRevoker = overflowRepeater.ItemsSourceView().CollectionChanged(winrt::auto_revoke, { this, &NavigationView::OnOverflowItemsSourceCollectionChanged });
753+
}
754+
else
755+
{
756+
overflowRepeater.ItemsSource(nullptr);
757+
}
714758
}
715759
}
716760

‎dev/NavigationView/NavigationView.h

+4
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ class NavigationView :
157157
inline bool IsNavigationViewListSingleSelectionFollowsFocus();
158158
inline void UpdateSingleSelectionFollowsFocusTemplateSetting();
159159
void OnFooterItemsSourceCollectionChanged(const winrt::IInspectable &, const winrt::IInspectable &);
160+
void OnOverflowItemsSourceCollectionChanged(const winrt::IInspectable&, const winrt::IInspectable&);
160161
void SetSelectedItemAndExpectItemInvokeWhenSelectionChangedIfNotInvokedFromAPI(winrt::IInspectable const& item);
161162
void ChangeSelectStatusForItem(winrt::IInspectable const& item, bool selected);
162163
void UnselectPrevItem(winrt::IInspectable const& prevItem, winrt::IInspectable const& nextItem);
@@ -236,6 +237,8 @@ class NavigationView :
236237
void UpdatePaneTitleMargins();
237238
void UpdateLeftRepeaterItemSource(const winrt::IInspectable& items);
238239
void UpdateTopNavRepeatersItemSource(const winrt::IInspectable& items);
240+
void UpdateTopNavPrimaryRepeaterItemsSource(const winrt::IInspectable& items);
241+
void UpdateTopNavOverflowRepeaterItemsSource(const winrt::IInspectable& items);
239242
static void UpdateItemsRepeaterItemsSource(const winrt::ItemsRepeater& listView, const winrt::IInspectable& itemsSource);
240243
void UpdateSelectionForMenuItems();
241244
bool UpdateSelectedItemFromMenuItems(const winrt::impl::com_ref<winrt::IVector<winrt::IInspectable>>& menuItems, bool foundFirstSelected = false);
@@ -423,6 +426,7 @@ class NavigationView :
423426

424427
winrt::ItemsSourceView::CollectionChanged_revoker m_footerItemsCollectionChangedRevoker{};
425428

429+
winrt::ItemsSourceView::CollectionChanged_revoker m_topNavOverflowItemsCollectionChangedRevoker{};
426430

427431
winrt::FlyoutBase::Closing_revoker m_flyoutClosingRevoker{};
428432

‎dev/NavigationView/NavigationView_InteractionTests/TopModeTests.cs

+50
Original file line numberDiff line numberDiff line change
@@ -673,5 +673,55 @@ public void VerifyNavigationViewContentChangeOnTopNavImpactsLayout()
673673
}
674674
}
675675

676+
[TestMethod]
677+
public void VerifyOverflowButtonIsCollapsedWhenOverflowMenuIsEmpty()
678+
{
679+
using (var setup = new TestSetupHelper(new[] { "NavigationView Tests", "Top NavigationView Overflow Button Test" }))
680+
{
681+
VerifyOverflowButtonIsVisibleAndOverflowMenuHasSingleItem();
682+
683+
Log.Comment("Verify that removing the final overflow menu item collapses the overflow button");
684+
FindElement.ByName<Button>("RemoveLastItemButton").InvokeAndWait();
685+
686+
// Refresh the cache to make sure that the overflow button we are going to be searching for
687+
// does not return as a false positive due to the caching mechanism.
688+
ElementCache.Clear();
689+
690+
VerifyOverflowButtonIsCollapsed();
691+
692+
Log.Comment("Add menu item to show the overflow button again with the overflow menu item containing a single item");
693+
FindElement.ByName<Button>("AddItem4Button").InvokeAndWait();
694+
695+
VerifyOverflowButtonIsVisibleAndOverflowMenuHasSingleItem();
696+
697+
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");
698+
FindElement.ByName<Button>("RemoveFirstItemButton").InvokeAndWait();
699+
700+
// Refresh the cache to make sure that the overflow button we are going to be searching for
701+
// does not return as a false positive due to the caching mechanism.
702+
ElementCache.Clear();
703+
704+
VerifyOverflowButtonIsCollapsed();
705+
706+
void VerifyOverflowButtonIsVisibleAndOverflowMenuHasSingleItem()
707+
{
708+
UIObject overflowButton = FindElement.ById("TopNavOverflowButton");
709+
Verify.IsNotNull(overflowButton, "The overflow button is required to be visible for this test");
710+
711+
Log.Comment("Verify that the overflow menu contains a single item");
712+
overflowButton.Click();
713+
Wait.ForIdle();
714+
715+
int numOverflowMenuItems = GetTopNavigationItems(TopNavPosition.Overflow).Count;
716+
Verify.IsTrue(numOverflowMenuItems == 1, "The overflow menu should only contain a single item");
717+
}
718+
719+
void VerifyOverflowButtonIsCollapsed()
720+
{
721+
UIObject overflowButton = FindElement.ById("TopNavOverflowButton");
722+
Verify.IsNull(overflowButton, "The overflow button should have been collapsed");
723+
}
724+
}
725+
}
676726
}
677727
}

‎dev/NavigationView/TestUI/NavigationViewCaseBundle.xaml

+1
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
<Button x:Name="NavigationViewTopNavPage" AutomationProperties.Name="NavigationView TopNav Test" Margin="2" HorizontalAlignment="Stretch">NavigationView TopNav Test</Button>
4343
<Button x:Name="NavigationViewTopNavOnlyPage" AutomationProperties.Name="Top NavigationView Test" Margin="2" HorizontalAlignment="Stretch">Top NavigationView Test</Button>
4444
<Button x:Name="NavigationViewTopNavStorePage" AutomationProperties.Name="Top NavigationView Store Test" Margin="2" HorizontalAlignment="Stretch">Top NavigationView Store Test</Button>
45+
<Button x:Name="NavigateToTopNavOverflowButtonPage" AutomationProperties.Name="Top NavigationView Overflow Button Test" Margin="2" HorizontalAlignment="Stretch">Top NavigationView Overflow Button Test</Button>
4546
</StackPanel>
4647

4748
<StackPanel>

‎dev/NavigationView/TestUI/NavigationViewCaseBundle.xaml.cs

+1
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ public NavigationViewCaseBundle()
2525
NavigationViewTopNavPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewTopNavPage), 0); };
2626
NavigationViewTopNavOnlyPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewTopNavOnlyPage), 0); };
2727
NavigationViewTopNavStorePage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewTopNavStorePage), 0); };
28+
NavigateToTopNavOverflowButtonPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewTopNavOverflowButtonPage), 0); };
2829
NavigateToSelectedItemEdgeCasePage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewSelectedItemEdgeCasePage), 0); };
2930
NavigateToInitPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewInitPage), 0); };
3031
NavigateToStretchPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewStretchPage), 0); };

‎dev/NavigationView/TestUI/NavigationView_TestUI.projitems

+8-1
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,10 @@
7878
<SubType>Designer</SubType>
7979
<Generator>MSBuild:Compile</Generator>
8080
</Page>
81+
<Page Include="$(MSBuildThisFileDirectory)TopMode\NavigationViewTopNavOverflowButtonPage.xaml">
82+
<SubType>Designer</SubType>
83+
<Generator>MSBuild:Compile</Generator>
84+
</Page>
8185
<Page Include="$(MSBuildThisFileDirectory)TopMode\NavigationViewTopNavPage.xaml">
8286
<SubType>Designer</SubType>
8387
<Generator>MSBuild:Compile</Generator>
@@ -121,7 +125,7 @@
121125
<Compile Include="$(MSBuildThisFileDirectory)Common\NavigationViewIsPaneOpenPage.xaml.cs">
122126
<DependentUpon>NavigationViewIsPaneOpenPage.xaml</DependentUpon>
123127
</Compile>
124-
<Compile Include="$(MSBuildThisFileDirectory)Common\NavigationViewMenuItemStretchPage.xaml.cs" >
128+
<Compile Include="$(MSBuildThisFileDirectory)Common\NavigationViewMenuItemStretchPage.xaml.cs">
125129
<DependentUpon>NavigationViewMenuItemStretchPage.xaml</DependentUpon>
126130
</Compile>
127131
<Compile Include="$(MSBuildThisFileDirectory)Common\NavigationViewMinimalPage.xaml.cs">
@@ -151,6 +155,9 @@
151155
<Compile Include="$(MSBuildThisFileDirectory)Common\NavigationViewInitPage.xaml.cs">
152156
<DependentUpon>NavigationViewInitPage.xaml</DependentUpon>
153157
</Compile>
158+
<Compile Include="$(MSBuildThisFileDirectory)TopMode\NavigationViewTopNavOverflowButtonPage.xaml.cs">
159+
<DependentUpon>NavigationViewTopNavOverflowButtonPage.xaml</DependentUpon>
160+
</Compile>
154161
<Compile Include="$(MSBuildThisFileDirectory)TopMode\NavigationViewTopNavPage.xaml.cs">
155162
<DependentUpon>NavigationViewTopNavPage.xaml</DependentUpon>
156163
</Compile>

‎dev/NavigationView/TestUI/TopMode/NavigationViewTopNavOverflowButtonPage.xaml

+34
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<!-- Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT License. See LICENSE in the project root for license information. -->
2+
<local:TestPage
3+
x:Class="MUXControlsTestApp.NavigationViewTopNavOverflowButtonPage"
4+
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
5+
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
6+
xmlns:local="using:MUXControlsTestApp"
7+
xmlns:muxcontrols="using:Microsoft.UI.Xaml.Controls"
8+
xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
9+
xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
10+
mc:Ignorable="d">
11+
12+
<StackPanel Background="{ThemeResource ApplicationPageBackgroundThemeBrush}"
13+
HorizontalAlignment="Left">
14+
<!-- The width and number of NavigationView menu items have been set in such a way that the overflow menu button
15+
is visible and the overflow menu only contains a single item. -->
16+
<muxcontrols:NavigationView x:Name="NavView"
17+
PaneDisplayMode="Top"
18+
IsBackButtonVisible="Collapsed"
19+
MinWidth="100" Width="500">
20+
<muxcontrols:NavigationView.MenuItems>
21+
<muxcontrols:NavigationViewItem Content="Menu Item 1"/>
22+
<muxcontrols:NavigationViewItem Content="Menu Item 2" />
23+
<muxcontrols:NavigationViewItem Content="Menu Item 3"/>
24+
<muxcontrols:NavigationViewItem Content="Menu Item 4"/>
25+
</muxcontrols:NavigationView.MenuItems>
26+
</muxcontrols:NavigationView>
27+
28+
<StackPanel Orientation="Horizontal" Margin="8,10,0,0">
29+
<Button x:Name="AddItem4Button" AutomationProperties.Name="AddItem4Button" Click="AddItem4Button_Click" Margin="5,0,0,0" Content="Add Menu Item 4"/>
30+
<Button x:Name="RemoveFirstItemButton" AutomationProperties.Name="RemoveFirstItemButton" Click="RemoveFirstItemButton_Click" Margin="5,0,0,0" Content="Remove First Item"/>
31+
<Button x:Name="RemoveLastItemButton" AutomationProperties.Name="RemoveLastItemButton" Click="RemoveLastItemButton_Click" Margin="5,0,0,0" Content="Remove Last Item"/>
32+
</StackPanel>
33+
</StackPanel>
34+
</local:TestPage>

‎dev/NavigationView/TestUI/TopMode/NavigationViewTopNavOverflowButtonPage.xaml.cs

+43
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License. See LICENSE in the project root for license information.
3+
4+
using Windows.UI.Xaml;
5+
6+
using NavigationViewItem = Microsoft.UI.Xaml.Controls.NavigationViewItem;
7+
8+
namespace MUXControlsTestApp
9+
{
10+
public sealed partial class NavigationViewTopNavOverflowButtonPage : TestPage
11+
{
12+
public NavigationViewTopNavOverflowButtonPage()
13+
{
14+
this.InitializeComponent();
15+
}
16+
17+
private void AddItem4Button_Click(object sender, RoutedEventArgs e)
18+
{
19+
var menuItem = new NavigationViewItem
20+
{
21+
Content = $"Menu Item 4",
22+
};
23+
24+
this.NavView.MenuItems.Add(menuItem);
25+
}
26+
27+
private void RemoveFirstItemButton_Click(object sender, RoutedEventArgs e)
28+
{
29+
if (NavView.MenuItems.Count > 0)
30+
{
31+
NavView.MenuItems.RemoveAt(0);
32+
}
33+
}
34+
35+
private void RemoveLastItemButton_Click(object sender, RoutedEventArgs e)
36+
{
37+
if (NavView.MenuItems.Count > 0)
38+
{
39+
NavView.MenuItems.RemoveAt(NavView.MenuItems.Count - 1);
40+
}
41+
}
42+
}
43+
}

0 commit comments

Comments
 (0)