From e3144481b91b24e9a6bd3fee02160e4149b0c1d5 Mon Sep 17 00:00:00 2001 From: Felix Date: Mon, 10 Aug 2020 20:50:15 +0200 Subject: [PATCH 1/5] Fix overflow button not being collapsed. --- dev/NavigationView/NavigationView.cpp | 46 +++++++++++++++++++++++++-- dev/NavigationView/NavigationView.h | 4 +++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/dev/NavigationView/NavigationView.cpp b/dev/NavigationView/NavigationView.cpp index 294d8fc10e..a54ed77f7e 100644 --- a/dev/NavigationView/NavigationView.cpp +++ b/dev/NavigationView/NavigationView.cpp @@ -1,4 +1,4 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. +// Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. See LICENSE in the project root for license information. #include "pch.h" @@ -155,6 +155,8 @@ void NavigationView::UnhookEventsAndClearFields(bool isFromDestructor) m_topNavOverflowItemsRepeaterElementClearingRevoker.revoke(); m_topNavRepeaterOverflowView.set(nullptr); + m_topNavOverflowItemsCollectionChangedRevoker.revoke(); + if (isFromDestructor) { m_selectionChangedRevoker.revoke(); @@ -230,6 +232,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(); @@ -688,14 +698,46 @@ void NavigationView::UpdateTopNavRepeatersItemSource(const winrt::IInspectable& m_topDataProvider.SetDataSource(items); // rebinding + UpdateTopNavRepeaterItemsSource(items); + UpdateTopNavOverflowRepeaterItemsSource(items); +} + +void NavigationView::UpdateTopNavRepeaterItemsSource(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); + } +} + +void NavigationView::UpdateTopNavOverflowRepeaterItemsSource(const winrt::IInspectable& items) +{ + m_topNavOverflowItemsCollectionChangedRevoker.revoke(); + + if (items) + { + UpdateItemsRepeaterItemsSource(m_topNavRepeaterOverflowView.get(), m_topDataProvider.GetOverflowItems()); + + if (const auto overflowRepeater = m_topNavRepeaterOverflowView.get()) + { + // 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 + { UpdateItemsRepeaterItemsSource(m_topNavRepeaterOverflowView.get(), nullptr); } } diff --git a/dev/NavigationView/NavigationView.h b/dev/NavigationView/NavigationView.h index 992af81bef..b6ad64dc14 100644 --- a/dev/NavigationView/NavigationView.h +++ b/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 UpdateTopNavRepeaterItemsSource(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>& 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{}; From 10e4c8c3f5a7e6b2d223f807729856e1957979cd Mon Sep 17 00:00:00 2001 From: Felix Date: Mon, 10 Aug 2020 20:54:30 +0200 Subject: [PATCH 2/5] Add interaction test. --- .../TopModeTests.cs | 50 +++++++++++++++++++ .../TestUI/NavigationViewCaseBundle.xaml | 1 + .../TestUI/NavigationViewCaseBundle.xaml.cs | 1 + ...avigationViewTopNavOverflowButtonPage.xaml | 34 +++++++++++++ ...gationViewTopNavOverflowButtonPage.xaml.cs | 43 ++++++++++++++++ .../TestUI/NavigationView_TestUI.projitems | 7 +++ 6 files changed, 136 insertions(+) create mode 100644 dev/NavigationView/TestUI/NavigationViewTopNavOverflowButtonPage.xaml create mode 100644 dev/NavigationView/TestUI/NavigationViewTopNavOverflowButtonPage.xaml.cs diff --git a/dev/NavigationView/NavigationView_InteractionTests/TopModeTests.cs b/dev/NavigationView/NavigationView_InteractionTests/TopModeTests.cs index 256d08af4c..6538ad7356 100644 --- a/dev/NavigationView/NavigationView_InteractionTests/TopModeTests.cs +++ b/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 + diff --git a/dev/NavigationView/TestUI/NavigationViewCaseBundle.xaml.cs b/dev/NavigationView/TestUI/NavigationViewCaseBundle.xaml.cs index aba8d7a32d..d3f592cff9 100644 --- a/dev/NavigationView/TestUI/NavigationViewCaseBundle.xaml.cs +++ b/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); }; diff --git a/dev/NavigationView/TestUI/NavigationViewTopNavOverflowButtonPage.xaml b/dev/NavigationView/TestUI/NavigationViewTopNavOverflowButtonPage.xaml new file mode 100644 index 0000000000..34e7beeaa0 --- /dev/null +++ b/dev/NavigationView/TestUI/NavigationViewTopNavOverflowButtonPage.xaml @@ -0,0 +1,34 @@ + + + + + + + + + + + + + + + +