Skip to content

Commit 04cfbe6

Browse files
ojhadojhad
authored
Fixed HNav NVI spacing, top nav issues (#2280)
* fixed spacing, top nav reparenting on load, showhidechildren * fixed pane switching bug * test page update * updated masters
1 parent 8c9f7ad commit 04cfbe6

31 files changed

+25261
-325
lines changed

‎dev/NavigationView/NavigationView.cpp

+18-31
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,9 @@ void NavigationView::SelectandMoveOverflowItem(winrt::IInspectable const& select
278278
}
279279

280280
// We only need to close the flyout if the selected item is a leaf node
281-
void NavigationView::CloseFlyoutIfRequired()
281+
void NavigationView::CloseFlyoutIfRequired(const winrt::NavigationViewItem& selectedItem)
282282
{
283+
auto const selectedIndex = m_selectionModel.SelectedIndex();
283284
bool isInModeWithFlyout = [this]()
284285
{
285286
if (auto splitView = m_rootSplitView.get())
@@ -292,31 +293,18 @@ void NavigationView::CloseFlyoutIfRequired()
292293
return false;
293294
}();
294295

295-
if (isInModeWithFlyout)
296+
if (isInModeWithFlyout && selectedIndex && !DoesNavigationViewItemHaveChildren(selectedItem))
296297
{
297-
if (auto const selectedIndex = m_selectionModel.SelectedIndex())
298+
// Item selected is a leaf node, find top level parent and close flyout
299+
if (auto const rootItem = GetContainerForIndex(selectedIndex.GetAt(0)))
298300
{
299-
if (auto const selectedItem = GetContainerForIndexPath(selectedIndex))
301+
if (auto const nvi = rootItem.try_as<winrt::NavigationViewItem>())
300302
{
301-
if (auto const selectedNVI = selectedItem.try_as<winrt::NavigationViewItem>())
303+
auto const nviImpl = winrt::get_self<NavigationViewItem>(nvi);
304+
if (nviImpl->ShouldRepeaterShowInFlyout())
302305
{
303-
if (!DoesNavigationViewItemHaveChildren(selectedNVI))
304-
{
305-
// Item selected is a leaf node, find top level parent and close flyout
306-
if (auto const rootItem = GetContainerForIndex(selectedIndex.GetAt(0)))
307-
{
308-
if (auto const nvi = rootItem.try_as<winrt::NavigationViewItem>())
309-
{
310-
auto const nviImpl = winrt::get_self<NavigationViewItem>(nvi);
311-
if (nviImpl->ShouldRepeaterShowInFlyout())
312-
{
313-
nviImpl->ShowChildren(false);
314-
}
315-
}
316-
}
317-
}
306+
nvi.IsExpanded(false);
318307
}
319-
320308
}
321309
}
322310
}
@@ -773,7 +761,7 @@ void NavigationView::OnNavigationViewItemInvoked(const winrt::NavigationViewItem
773761

774762
if (updateSelection)
775763
{
776-
CloseFlyoutIfRequired();
764+
CloseFlyoutIfRequired(nvi);
777765
}
778766
}
779767

@@ -1231,7 +1219,7 @@ void NavigationView::OpenPane()
12311219
// Call this when you want an uncancellable close
12321220
void NavigationView::ClosePane()
12331221
{
1234-
CollapseAllMenuItemsUnderRepeater(m_leftNavRepeater.get());
1222+
CollapseMenuItemsInRepeater(m_leftNavRepeater.get());
12351223
auto scopeGuard = gsl::finally([this]()
12361224
{
12371225
m_isOpenPaneForInteraction = false;
@@ -3371,7 +3359,7 @@ void NavigationView::OnPropertyChanged(const winrt::DependencyPropertyChangedEve
33713359
// When PaneDisplayMode is changed, reset the force flag to make the Pane can be opened automatically again.
33723360
m_wasForceClosed = false;
33733361

3374-
CollapseAllMenuItems(auto_unbox(args.OldValue()));
3362+
CollapseTopLevelMenuItems(auto_unbox(args.OldValue()));
33753363
UpdatePaneToggleButtonVisibility();
33763364
UpdatePaneDisplayMode(auto_unbox(args.OldValue()), auto_unbox(args.NewValue()));
33773365
UpdatePaneTitleFrameworkElementParents();
@@ -4678,7 +4666,7 @@ void NavigationView::ShowHideChildrenItemsRepeater(const winrt::NavigationViewIt
46784666
{
46794667
auto nviImpl = winrt::get_self<NavigationViewItem>(nvi);
46804668

4681-
nviImpl->ShowChildren(nvi.IsExpanded());
4669+
nviImpl->ShowHideChildren();
46824670

46834671
if (nviImpl->ShouldRepeaterShowInFlyout())
46844672
{
@@ -4820,29 +4808,28 @@ winrt::IInspectable NavigationView::GetChildrenForItemInIndexPath(const winrt::U
48204808
return nullptr;
48214809
}
48224810

4823-
void NavigationView::CollapseAllMenuItems(winrt::NavigationViewPaneDisplayMode oldDisplayMode)
4811+
void NavigationView::CollapseTopLevelMenuItems(winrt::NavigationViewPaneDisplayMode oldDisplayMode)
48244812
{
48254813
// We want to make sure only top level items are visible when switching pane modes
48264814
if (oldDisplayMode == winrt::NavigationViewPaneDisplayMode::Top)
48274815
{
4828-
CollapseAllMenuItemsUnderRepeater(m_topNavRepeater.get());
4829-
CollapseAllMenuItemsUnderRepeater(m_topNavRepeaterOverflowView.get());
4816+
CollapseMenuItemsInRepeater(m_topNavRepeater.get());
4817+
CollapseMenuItemsInRepeater(m_topNavRepeaterOverflowView.get());
48304818
}
48314819
else
48324820
{
4833-
CollapseAllMenuItemsUnderRepeater(m_leftNavRepeater.get());
4821+
CollapseMenuItemsInRepeater(m_leftNavRepeater.get());
48344822
}
48354823
}
48364824

4837-
void NavigationView::CollapseAllMenuItemsUnderRepeater(const winrt::ItemsRepeater& ir)
4825+
void NavigationView::CollapseMenuItemsInRepeater(const winrt::ItemsRepeater& ir)
48384826
{
48394827
for (int index = 0; index < GetContainerCountInRepeater(ir); index++)
48404828
{
48414829
if (auto const element = ir.TryGetElement(index))
48424830
{
48434831
if (auto const nvi = element.try_as<winrt::NavigationViewItem>())
48444832
{
4845-
CollapseAllMenuItemsUnderRepeater(winrt::get_self<NavigationViewItem>(nvi)->GetRepeater());
48464833
ChangeIsExpandedNavigationViewItem(nvi, false /*isExpanded*/);
48474834
}
48484835
}

‎dev/NavigationView/NavigationView.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,11 @@ class NavigationView :
137137
winrt::NavigationViewItem FindLowestLevelContainerToDisplaySelectionIndicator();
138138
void UpdateIsChildSelectedForIndexPath(const winrt::IndexPath& ip, bool isChildSelected);
139139
void UpdateIsChildSelected(const winrt::IndexPath& prevIP, const winrt::IndexPath& nextIP);
140-
void CollapseAllMenuItems(winrt::NavigationViewPaneDisplayMode oldDisplayMode);
141-
void CollapseAllMenuItemsUnderRepeater(const winrt::ItemsRepeater& ir);
140+
void CollapseTopLevelMenuItems(winrt::NavigationViewPaneDisplayMode oldDisplayMode);
141+
void CollapseMenuItemsInRepeater(const winrt::ItemsRepeater& ir);
142142
void RaiseExpandingEvent(const winrt::NavigationViewItemBase& container);
143143
void RaiseCollapsedEvent(const winrt::NavigationViewItemBase& container);
144-
void CloseFlyoutIfRequired();
144+
void CloseFlyoutIfRequired(const winrt::NavigationViewItem& selectedItem);
145145

146146
// Force realization functions
147147
winrt::NavigationViewItemBase ResolveContainerForItem(const winrt::IInspectable& item, int index);

‎dev/NavigationView/NavigationViewItem.cpp

+9-1
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ void NavigationViewItem::OnApplyTemplate()
122122
m_appliedTemplate = true;
123123
UpdateItemIndentation();
124124
UpdateVisualStateNoTransition();
125+
ReparentRepeater();
125126

126127
auto visual = winrt::ElementCompositionPreview::GetElementVisual(*this);
127128
NavigationView::CreateAndAttachHeaderAnimation(visual);
@@ -513,15 +514,22 @@ NavigationViewItemPresenter * NavigationViewItem::GetPresenter()
513514
return presenter;
514515
}
515516

516-
void NavigationViewItem::ShowChildren(bool shouldShowChildren)
517+
void NavigationViewItem::ShowHideChildren()
517518
{
519+
bool shouldShowChildren = IsExpanded();
518520
auto visibility = shouldShowChildren ? winrt::Visibility::Visible : winrt::Visibility::Collapsed;
519521
m_repeater.get().Visibility(visibility);
520522

521523
if (ShouldRepeaterShowInFlyout())
522524
{
523525
if (shouldShowChildren)
524526
{
527+
// Verify that repeater is parented correctly
528+
if (!m_isRepeaterParentedToFlyout)
529+
{
530+
ReparentRepeater();
531+
}
532+
525533
// There seems to be a race condition happening which sometimes
526534
// prevents the opening of the flyout. Queue callback as a workaround.
527535
SharedHelpers::QueueCallbackForCompositionRendering(

‎dev/NavigationView/NavigationViewItem.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class NavigationViewItem :
5757
void OnPresenterPointerCanceled(const winrt::IInspectable& sender, const winrt::PointerRoutedEventArgs& args);
5858
void OnPresenterPointerCaptureLost(const winrt::IInspectable& sender, const winrt::PointerRoutedEventArgs& args);
5959

60-
void ShowChildren(bool shouldShowChildren);
60+
void ShowHideChildren();
6161
bool ShouldRepeaterShowInFlyout();
6262

6363
winrt::ItemsRepeater GetRepeater() { return m_repeater.get(); };

‎dev/NavigationView/NavigationView_rs1_themeresources.xaml

+6-4
Original file line numberDiff line numberDiff line change
@@ -658,21 +658,22 @@
658658

659659
<ContentPresenter x:Name="ContentPresenter"
660660
Grid.Column="1"
661+
Margin="0,0,16,0"
661662
ContentTransitions="{TemplateBinding ContentTransitions}"
662663
ContentTemplate="{TemplateBinding ContentTemplate}"
663664
Content="{TemplateBinding Content}"
664665
HorizontalAlignment="{TemplateBinding HorizontalContentAlignment}"
665666
VerticalAlignment="{TemplateBinding VerticalContentAlignment}"
666667
ContentTemplateSelector="{TemplateBinding ContentTemplateSelector}"
667-
Margin="{TemplateBinding Padding}"
668+
Padding="{TemplateBinding Padding}"
668669
AutomationProperties.AccessibilityView="Raw"/>
669670

670671
<Grid x:Name="ExpandCollapseChevron"
671672
Grid.Column="2"
672673
Visibility="Collapsed"
673674
HorizontalAlignment="Right"
674675
Width="40"
675-
Padding="12,0,12,0"
676+
Margin="-16,0,0,0"
676677
Background="Transparent">
677678
<TextBlock
678679
Width="12" Height="12"
@@ -818,15 +819,16 @@
818819
HorizontalAlignment="{TemplateBinding HorizontalContentAlignment}"
819820
VerticalAlignment="{TemplateBinding VerticalContentAlignment}"
820821
ContentTemplateSelector="{TemplateBinding ContentTemplateSelector}"
821-
Margin="{TemplateBinding Padding}"
822+
Margin="0,0,16,0"
823+
Padding="{TemplateBinding Padding}"
822824
AutomationProperties.AccessibilityView="Raw"/>
823825

824826
<Grid x:Name="ExpandCollapseChevron"
825827
Grid.Column="2"
826828
Visibility="Collapsed"
827829
HorizontalAlignment="Right"
828830
Width="40"
829-
Padding="12,0,12,0"
831+
Margin="-16,0,0,0"
830832
Background="Transparent">
831833
<TextBlock Foreground="{ThemeResource NavigationViewItemForeground}"
832834
RenderTransformOrigin="0.5, 0.5"

‎dev/NavigationView/TestUI/HierarchicalNavigationViewMarkup.xaml

+3-6
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,9 @@
5353
</muxcontrols:NavigationViewItem>
5454
<muxcontrols:NavigationViewItem Content="Menu Item 15" x:Name="MI15" SelectsOnInvoked="False" Icon="Shuffle">
5555
<muxcontrols:NavigationViewItem.MenuItems>
56-
<muxcontrols:NavigationViewItem Content="Menu Item 16 (Selectable)" Icon="Save" x:Name="MI16">
57-
<muxcontrols:NavigationViewItem.MenuItems>
58-
<muxcontrols:NavigationViewItem Content="Menu Item 17" Icon="Cut" x:Name="MI17" />
59-
<muxcontrols:NavigationViewItem Content="Menu Item 18" Icon="Paste" x:Name="MI18" />
60-
</muxcontrols:NavigationViewItem.MenuItems>
61-
</muxcontrols:NavigationViewItem>
56+
<muxcontrols:NavigationViewItem Content="Menu Item 16" Icon="Save" x:Name="MI16" />
57+
<muxcontrols:NavigationViewItem Content="Menu Item 17" Icon="Cut" x:Name="MI17" />
58+
<muxcontrols:NavigationViewItem Content="Menu Item 18" Icon="Paste" x:Name="MI18" />
6259
</muxcontrols:NavigationViewItem.MenuItems>
6360
</muxcontrols:NavigationViewItem>
6461
<muxcontrols:NavigationViewItem Content="Menu Item 19" x:Name="MI19" SelectsOnInvoked="True"/>

0 commit comments

Comments
 (0)