Let NavigationViewItem respect CompactPane length for icon size (#2051)

* Initial commit of compact pane length handling

* Add interaction test

* CR feedback

* Fix issue with top mode not working

* Improve pane button sizing behavior

* CR feedback

* Remove this is null check from presenter

* Add const

* Code improvements

* Switch way of casting

* Update test code to adjust for new presenter template

* CR feedback

* CR feedback

* Update dev/NavigationView/NavigationView.cpp

Co-Authored-By: Stephen L Peters <stpete@microsoft.com>

Co-authored-by: Stephen L Peters <stpete@microsoft.com>

dev/NavigationView/NavigationView.cpp

@@ -30,6 +30,7 @@
 #include "NavigationViewItemExpandingEventArgs.h"
 #include "NavigationViewItemCollapsedEventArgs.h"
 
+// General items
 static constexpr auto c_togglePaneButtonName = L"TogglePaneButton"sv;
 static constexpr auto c_paneTitleHolderFrameworkElement = L"PaneTitleHolder"sv;
 static constexpr auto c_paneTitleFrameworkElement = L"PaneTitleTextBlock"sv;
@@ -42,6 +43,7 @@ static constexpr auto c_paneContentGridName = L"PaneContentGrid"sv;
 static constexpr auto c_rootGridName = L"RootGrid"sv;
 static constexpr auto c_contentGridName = L"ContentGrid"sv;
 static constexpr auto c_searchButtonName = L"PaneAutoSuggestButton"sv;
+static constexpr auto c_paneToggleButtonIconGridColumnName = L"PaneToggleButtonIconWidthColumn"sv;
 static constexpr auto c_togglePaneTopPadding = L"TogglePaneTopPadding"sv;
 static constexpr auto c_contentPaneTopPadding = L"ContentPaneTopPadding"sv;
 static constexpr auto c_contentLeftPadding = L"ContentLeftPadding"sv;
@@ -52,6 +54,7 @@ static constexpr auto c_navViewCloseButtonToolTip = L"NavigationViewCloseButtonT
 static constexpr auto c_paneShadowReceiverCanvas = L"PaneShadowReceiver"sv;
 static constexpr auto c_flyoutRootGrid = L"FlyoutRootGrid"sv;
 
+// DisplayMode Top specific items
 static constexpr auto c_topNavMenuItemsHost = L"TopNavMenuItemsHost"sv;
 static constexpr auto c_topNavOverflowButton = L"TopNavOverflowButton"sv;
 static constexpr auto c_topNavMenuItemsOverflowHost = L"TopNavMenuItemsOverflowHost"sv;
@@ -61,6 +64,7 @@ static constexpr auto c_leftNavPaneAutoSuggestBoxPresenter = L"PaneAutoSuggestBo
 static constexpr auto c_topNavPaneAutoSuggestBoxPresenter = L"TopPaneAutoSuggestBoxPresenter"sv;
 static constexpr auto c_paneTitlePresenter = L"PaneTitlePresenter"sv;
 
+// DisplayMode Left specific items
 static constexpr auto c_leftNavFooterContentBorder = L"FooterContentBorder"sv;
 static constexpr auto c_leftNavPaneHeaderContentBorder = L"PaneHeaderContentBorder"sv;
 static constexpr auto c_leftNavPaneCustomContentBorder = L"PaneCustomContentBorder"sv;
@@ -1358,6 +1362,36 @@ void NavigationView::UpdateIsClosedCompact()
     }
 }
 
+void NavigationView::UpdatePaneButtonsWidths()
+{
+    auto newButtonWidths = [this]()
+    {
+        if (DisplayMode() == winrt::NavigationViewDisplayMode::Minimal)
+        {
+            return static_cast<double>(c_paneToggleButtonWidth);
+        }
+        return CompactPaneLength();
+    }();
+
+    if (auto&& backButton = m_backButton.get())
+    {
+        backButton.Width(newButtonWidths);
+    }
+    if (auto&& paneToggleButton = m_paneToggleButton.get())
+    {
+        paneToggleButton.MinWidth(newButtonWidths);
+        if (const auto iconGridColumnElement = paneToggleButton.GetTemplateChild(c_paneToggleButtonIconGridColumnName))
+        {
+            if (const auto paneToggleButtonIconColumn = iconGridColumnElement.try_as<winrt::ColumnDefinition>())
+            {
+                auto width = paneToggleButtonIconColumn.Width();
+                width.Value = newButtonWidths;
+                paneToggleButtonIconColumn.Width(width);
+            }
+        }
+    }
+}
+
 void NavigationView::OnBackButtonClicked(const winrt::IInspectable& sender, const winrt::RoutedEventArgs& args)
 {
     auto eventArgs = winrt::make_self<NavigationViewBackRequestedEventArgs>();
@@ -3343,6 +3377,7 @@ void NavigationView::OnPropertyChanged(const winrt::DependencyPropertyChangedEve
         UpdatePaneTitleFrameworkElementParents();
         UpdatePaneVisibility();
         UpdateVisualState();
+        UpdatePaneButtonsWidths();
     }
     else if (property == s_IsPaneVisibleProperty)
     {
@@ -3393,6 +3428,9 @@ void NavigationView::OnPropertyChanged(const winrt::DependencyPropertyChangedEve
     {
         // Need to update receiver margins when CompactPaneLength changes
         UpdatePaneShadow();
+
+        // Update pane-button-grid width when pane is closed and we are not in minimal
+        UpdatePaneButtonsWidths();
     }
     else if (property == s_IsTitleBarAutoPaddingEnabledProperty)
     {
@@ -3472,6 +3510,7 @@ void NavigationView::OnIsPaneOpenChanged()
             }
         }
     }
+
     SetPaneToggleButtonAutomationName();
     UpdatePaneTabFocusNavigation();
     UpdateSettingsItemToolTip();
@@ -3491,6 +3530,7 @@ void NavigationView::OnIsPaneOpenChanged()
             }
         }
     }
+    UpdatePaneButtonsWidths();
 }
 
 void NavigationView::UpdatePaneToggleButtonVisibility()

dev/NavigationView/NavigationView.h

@@ -221,6 +221,7 @@ class NavigationView :
 
     void CreateAndHookEventsToSettings(std::wstring_view settingsName);
     void OnIsPaneOpenChanged();
+    void UpdatePaneButtonsWidths();
     void UpdateHeaderVisibility();
     void UpdateHeaderVisibility(winrt::NavigationViewDisplayMode displayMode);
     void UpdatePaneToggleButtonVisibility();
@@ -328,6 +329,7 @@ class NavigationView :
     tracker_ref<winrt::SplitView> m_rootSplitView{ this };
     tracker_ref<winrt::NavigationViewItem> m_settingsItem{ this };
     tracker_ref<winrt::UIElement> m_paneContentGrid{ this };
+    tracker_ref<winrt::ColumnDefinition> m_paneToggleButtonIconGridColumn{ this };
     tracker_ref<winrt::FrameworkElement> m_paneTitleHolderFrameworkElement{ this };
     tracker_ref<winrt::FrameworkElement> m_paneTitleFrameworkElement{ this };
     tracker_ref<winrt::Button> m_paneSearchButton{ this };

dev/NavigationView/NavigationView.xaml

@@ -141,6 +141,7 @@
                             </VisualStateGroup>
                         </VisualStateManager.VisualStateGroups>
 
+                        <!-- Button grid -->
                         <Grid
                             x:Name="PaneToggleButtonGrid"
                             Margin="0,0,0,8"
@@ -178,7 +179,7 @@
                                 <Button
                                     x:Name="TogglePaneButton"
                                     Style="{TemplateBinding PaneToggleButtonStyle}"
-                                    AutomationProperties.LandmarkType="Navigation"
+                                    AutomationProperties.LandmarkType="Navigation" HorizontalAlignment="Center"
                                     Visibility="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=TemplateSettings.PaneToggleButtonVisibility}"
                                     VerticalAlignment="Top">
                                     <TextBlock
@@ -201,13 +202,15 @@
                                 </Grid>
                             </Grid>
                         </Grid>
-
+
+                        <!-- Content layouts -->
                         <Grid>
                             <Grid.RowDefinitions>
                                 <RowDefinition Height="Auto"/>
                                 <RowDefinition Height="*"/>
                             </Grid.RowDefinitions>
 
+                            <!-- DisplayMode top -->
                             <StackPanel
                                 x:Name="TopNavArea"
                                 Background="{ThemeResource NavigationViewTopPaneBackground}"
@@ -351,6 +354,7 @@
                                     Child="{TemplateBinding ContentOverlay}" />
                             </StackPanel>
 
+                            <!-- Displaymode (compact/minimal/normal) left -->
                             <SplitView
                                 x:Name="RootSplitView"
                                 Background="{TemplateBinding Background}"

dev/NavigationView/NavigationViewItem.cpp

@@ -172,6 +172,12 @@ void NavigationViewItem::UpdateCompactPaneLength()
     if (auto splitView = GetSplitView())
     {
         SetValue(s_CompactPaneLengthProperty, winrt::PropertyValue::CreateDouble(splitView.CompactPaneLength()));
+
+        // Only update when on left
+        if (auto presenter = GetPresenter())
+        {
+            presenter->UpdateCompactPaneLength(splitView.CompactPaneLength(), IsOnLeftNav());
+        }
     }
 }
 

dev/NavigationView/NavigationViewItemPresenter.cpp

@@ -12,13 +12,17 @@ static constexpr auto c_expandCollapseChevron = L"ExpandCollapseChevron"sv;
 static constexpr auto c_expandCollapseRotateExpandedStoryboard = L"ExpandCollapseRotateExpandedStoryboard"sv;
 static constexpr auto c_expandCollapseRotateCollapsedStoryboard = L"ExpandCollapseRotateCollapsedStoryboard"sv;
 
+static constexpr auto c_iconBoxColumnDefinitionName = L"IconColumn"sv;
+
 NavigationViewItemPresenter::NavigationViewItemPresenter()
 {
     SetDefaultStyleKey(this);
 }
 
 void NavigationViewItemPresenter::OnApplyTemplate()
 {
+    winrt::IControlProtected controlProtected = *this;
+
     // Retrieve pointers to stable controls 
     m_helper.Init(*this);
 
@@ -36,6 +40,13 @@ void NavigationViewItemPresenter::OnApplyTemplate()
         }
 
         navigationViewItem->UpdateVisualStateNoTransition();
+
+
+        // We probably switched displaymode, so restore width now, otherwise the next time we will restore is when the CompactPaneLength changes
+        if (navigationViewItem->GetNavigationView().PaneDisplayMode() != winrt::NavigationViewPaneDisplayMode::Top)
+        {
+            UpdateCompactPaneLength(m_compactPaneLengthValue, true);
+        }
     }
 
     m_chevronExpandedStoryboard.set(GetTemplateChildT<winrt::Storyboard>(c_expandCollapseRotateExpandedStoryboard, *this));
@@ -110,3 +121,18 @@ void NavigationViewItemPresenter::UpdateMargin()
         grid.Margin({ m_leftIndentation, oldGridMargin.Top, oldGridMargin.Right, oldGridMargin.Bottom });
     }
 }
+
+
+void NavigationViewItemPresenter::UpdateCompactPaneLength(double compactPaneLength, bool shouldUpdate)
+{
+    m_compactPaneLengthValue = compactPaneLength;
+    if (shouldUpdate)
+    {
+        if (auto iconGridColumn = GetTemplateChildT<winrt::ColumnDefinition>(c_iconBoxColumnDefinitionName, *this))
+        {
+            auto gridLength = iconGridColumn.Width();
+            gridLength.Value = compactPaneLength;
+            iconGridColumn.Width(gridLength);
+        }
+    }
+}

dev/NavigationView/NavigationViewItemPresenter.h

@@ -27,10 +27,14 @@ class NavigationViewItemPresenter:
 
     void RotateExpandCollapseChevron(bool isExpanded);
 
+    void UpdateCompactPaneLength(double compactPaneLength,bool shouldUpdate);
+
 private:
     NavigationViewItem * GetNavigationViewItem();
     void UpdateMargin();
 
+    double m_compactPaneLengthValue { 40 };
+
     NavigationViewItemHelper<NavigationViewItemPresenter> m_helper{ this };
     tracker_ref<winrt::Grid> m_contentGrid{ this };
     tracker_ref<winrt::Grid> m_expandCollapseChevron{ this };

dev/NavigationView/NavigationView_InteractionTests/NavigationViewTests.cs

@@ -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.
 
 using Common;
@@ -4248,6 +4248,42 @@ public void VerifyEventsReturnExpectedDataTypesMenuItemsSource()
             }
         }
 
+        [TestMethod]
+        [TestProperty("TestSuite","D")]
+        public void VerifyIconsRespectCompactPaneLength()
+        {
+            using (var setup = new TestSetupHelper(new[] { "NavigationView Tests", "NavigationView compact pane length test" }))
+            {
+                var checkMenuItemsButton = FindElement.ByName("CheckMenuItemsOffset");
+                var compactpaneCheckbox = new ComboBox(FindElement.ByName("CompactPaneLenghtComboBox"));
+                var displayModeToggle = new ComboBox(FindElement.ByName("PaneDisplayModeCombobox"));
+                var currentStatus = new CheckBox(FindElement.ByName("MenuItemsCorrectOffset"));
+
+                checkMenuItemsButton.Click();
+                Wait.ForIdle();
+                Verify.IsTrue(currentStatus.ToggleState == ToggleState.On);
+
+                compactpaneCheckbox.SelectItemByName("96");
+                Wait.ForIdle();
+
+                checkMenuItemsButton.Click();
+                Wait.ForIdle();
+                Verify.IsTrue(currentStatus.ToggleState == ToggleState.On);
+
+                // Check if changing displaymode to top and then changing length gets used correctly
+                displayModeToggle.SelectItemByName("Top");
+                compactpaneCheckbox.SelectItemByName("48");
+                Wait.ForIdle();
+
+                displayModeToggle.SelectItemByName("Left");
+                Wait.ForIdle();
+
+                checkMenuItemsButton.Click();
+                Wait.ForIdle();
+                Verify.IsTrue(currentStatus.ToggleState == ToggleState.On);
+            }
+        }
+
         [TestMethod]
         [TestProperty("TestSuite", "D")]
         public void VerifyExpandCollpaseFunctionality()

dev/NavigationView/NavigationView_rs1_themeresources.xaml

@@ -257,7 +257,7 @@
         <Setter Property="MinHeight" Value="{StaticResource PaneToggleButtonHeight}" />
         <Setter Property="MinWidth" Value="{StaticResource PaneToggleButtonWidth}" />
         <Setter Property="Padding" Value="0" />
-        <Setter Property="HorizontalAlignment" Value="Left" />
+        <Setter Property="HorizontalAlignment" Value="Center" />
         <Setter Property="VerticalAlignment" Value="Top" />
         <Setter Property="HorizontalContentAlignment" Value="Center" />
         <Setter Property="VerticalContentAlignment" Value="Center" />
@@ -275,7 +275,7 @@
                         Background="{TemplateBinding Background}"
                         HorizontalAlignment="Stretch">
                         <Grid.ColumnDefinitions>
-                            <ColumnDefinition Width="{ThemeResource PaneToggleButtonWidth}"/>
+                            <ColumnDefinition x:Name="PaneToggleButtonIconWidthColumn" Width="{ThemeResource PaneToggleButtonWidth}"/>
                             <ColumnDefinition Width="*"/>
                         </Grid.ColumnDefinitions>
                         <Grid.RowDefinitions>
@@ -336,7 +336,7 @@
                             x:Name="IconHost"
                             Width="16"
                             Height="16"
-                            HorizontalAlignment="{TemplateBinding HorizontalContentAlignment}"
+                            HorizontalAlignment="Center"
                             VerticalAlignment="{TemplateBinding VerticalContentAlignment}"
                             AutomationProperties.AccessibilityView="Raw">
 

dev/NavigationView/TestUI/NavigationViewCaseBundle.xaml

@@ -22,6 +22,7 @@
         <StackPanel>
             <TextBlock Text="Common tests"/>
             <Button x:Name="NavigationViewPage" AutomationProperties.Name="NavigationView Test" Margin="2" HorizontalAlignment="Stretch">NavigationView Test</Button>
+            <Button x:Name="NavigationViewCompactPaneLengthTestPage" AutomationProperties.Name="NavigationView compact pane length test" Margin="2" HorizontalAlignment="Stretch">NavigationView compact pane length test</Button>
             <Button x:Name="NavigationViewPageDataContext" AutomationProperties.Name="NavigationViewPageDataContext" Margin="2" HorizontalAlignment="Stretch">NavigationView DataContext</Button>
             <Button x:Name="NavigateToSelectedItemEdgeCasePage" AutomationProperties.Name="SelectedItem edge case test" Margin="2" HorizontalAlignment="Stretch">SelectedItem edge Case Test</Button>
             <Button x:Name="NavigateToInitPage" AutomationProperties.Name="NavigationView Init Test" Margin="2" HorizontalAlignment="Stretch">NavigationView Init Test</Button>

dev/NavigationView/TestUI/NavigationViewCaseBundle.xaml.cs

@@ -19,6 +19,7 @@ public NavigationViewCaseBundle()
         {
             this.InitializeComponent();
             NavigationViewPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewPage), 0); };
+            NavigationViewCompactPaneLengthTestPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewCompactPaneLengthTestPage), 0); };
             NavigationViewRS4Page.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewRS4Page), 0); };
             NavigationViewPageDataContext.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewPageDataContext), 0); };
             NavigationViewTopNavPage.Click += delegate { Frame.NavigateWithoutAnimation(typeof(NavigationViewTopNavPage), 0); };