Skip to content

Commit 779c0bd

Browse files
committed
NavigationView ItemInvoked returning incorrect item fix (microsoft/microsoft-ui-xaml#2682)
1 parent d69e104 commit 779c0bd

4 files changed

Lines changed: 148 additions & 18 deletions

File tree

ModernWpf.Controls/NavigationView/NavigationViewItemBase.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,10 +152,20 @@ internal bool IsTopLevelItem
152152
set => m_isTopLevelItem = value;
153153
}
154154

155+
internal bool CreatedByNavigationViewItemsFactory
156+
{
157+
get => m_createdByNavigationViewItemsFactory;
158+
set => m_createdByNavigationViewItemsFactory = value;
159+
}
160+
155161
private protected WeakReference<NavigationView> m_navigationView;
156162

157163
NavigationViewRepeaterPosition m_position = NavigationViewRepeaterPosition.LeftNav;
158164
int m_depth = 0;
159165
bool m_isTopLevelItem = false;
166+
167+
// Flag to keep track of whether this item was created by the custom internal NavigationViewItemsFactory.
168+
// This is required in order to achieve proper recycling
169+
bool m_createdByNavigationViewItemsFactory = false;
160170
}
161171
}

ModernWpf.Controls/NavigationView/NavigationViewItemsFactory.cs

Lines changed: 88 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License. See LICENSE in the project root for license information.
33

4+
using System.Collections.Generic;
45
using System.Windows;
56
using System.Windows.Controls;
67

@@ -24,8 +25,12 @@ public void UserElementFactory(object newValue)
2425
m_itemTemplateWrapper = new ItemTemplateWrapper(selector);
2526
}
2627
}
28+
29+
navigationViewItemPool = new List<NavigationViewItem>();
2730
}
2831

32+
// Retrieve the element that will be displayed for a specific data item.
33+
// If the resolved element is not derived from NavigationViewItemBase, wrap in a NavigationViewItem before returning.
2934
protected override UIElement GetElementCore(ElementFactoryGetArgs args)
3035
{
3136
object newContent;
@@ -41,39 +46,106 @@ object init()
4146
newContent = init();
4247
}
4348

49+
// Element is already of expected type, just return it
4450
if (newContent is NavigationViewItemBase newItem)
4551
{
4652
return newItem;
4753
}
4854

49-
// Create a wrapping container for the data
50-
var nvi = new NavigationViewItem();
51-
nvi.Content = newContent;
52-
return nvi;
53-
}
55+
// Get or create a wrapping container for the data
56+
NavigationViewItem nvi;
57+
{
58+
nvi = init();
59+
NavigationViewItem init()
60+
{
61+
if (navigationViewItemPool.Count > 0)
62+
{
63+
var nvi = navigationViewItemPool.Last();
64+
navigationViewItemPool.RemoveLast();
65+
return nvi;
66+
}
67+
return new NavigationViewItem();
68+
}
69+
}
70+
var nviImpl = nvi;
71+
nviImpl.CreatedByNavigationViewItemsFactory = true;
5472

55-
protected override void RecycleElementCore(ElementFactoryRecycleArgs args)
56-
{
73+
// If a user provided item template exists, just pass the template and data down to the ContentPresenter of the NavigationViewItem
5774
if (m_itemTemplateWrapper != null)
5875
{
59-
m_itemTemplateWrapper.RecycleElement(args);
76+
if (m_itemTemplateWrapper is ItemTemplateWrapper itemTemplateWrapper)
77+
{
78+
// Recycle newContent
79+
var tempArgs = new ElementFactoryRecycleArgs();
80+
tempArgs.Element = newContent as UIElement;
81+
m_itemTemplateWrapper.RecycleElement(tempArgs);
82+
83+
84+
nviImpl.Content = args.Data;
85+
nviImpl.ContentTemplate = itemTemplateWrapper.Template;
86+
nviImpl.ContentTemplateSelector = itemTemplateWrapper.TemplateSelector;
87+
return nviImpl;
88+
}
6089
}
61-
else
90+
91+
nviImpl.Content = newContent;
92+
return nviImpl;
93+
}
94+
95+
protected override void RecycleElementCore(ElementFactoryRecycleArgs args)
96+
{
97+
if (args.Element is { } element)
6298
{
63-
// We want to unlink the containers from the parent repeater
64-
// in case we are required to move it to a different repeater.
65-
if (args.Parent is Panel panel)
99+
if (element is NavigationViewItem nvi)
66100
{
67-
var children = panel.Children;
68-
int childIndex = 0;
69-
if (children.IndexOf(args.Element, out childIndex))
101+
var nviImpl = nvi;
102+
// Check whether we wrapped the element in a NavigationViewItem ourselves.
103+
// If yes, we are responsible for recycling it.
104+
if (nviImpl.CreatedByNavigationViewItemsFactory)
70105
{
71-
children.RemoveAt(childIndex);
106+
nviImpl.CreatedByNavigationViewItemsFactory = false;
107+
UnlinkElementFromParent(args);
108+
args.Element = null;
109+
110+
// Retain the NVI that we created for future re-use
111+
navigationViewItemPool.Add(nvi);
112+
113+
// Retrieve the proper element that requires recycling for a user defined item template
114+
// and update the args correspondingly
115+
if (m_itemTemplateWrapper != null)
116+
{
117+
// TODO: Retrieve the element and add to the args
118+
}
72119
}
73120
}
121+
122+
if (m_itemTemplateWrapper != null)
123+
{
124+
m_itemTemplateWrapper.RecycleElement(args);
125+
}
126+
else
127+
{
128+
UnlinkElementFromParent(args);
129+
}
130+
}
131+
}
132+
133+
void UnlinkElementFromParent(ElementFactoryRecycleArgs args)
134+
{
135+
// We want to unlink the containers from the parent repeater
136+
// in case we are required to move it to a different repeater.
137+
if (args.Parent is Panel panel)
138+
{
139+
var children = panel.Children;
140+
int childIndex = 0;
141+
if (children.IndexOf(args.Element, out childIndex))
142+
{
143+
children.RemoveAt(childIndex);
144+
}
74145
}
75146
}
76147

77148
IElementFactoryShim m_itemTemplateWrapper;
149+
List<NavigationViewItem> navigationViewItemPool = new List<NavigationViewItem>();
78150
}
79151
}

test/NavigationView_TestUI/NavigationViewItemTemplatePage.xaml

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@
2222
<controls:NavigationView
2323
x:Name="NavView"
2424
PaneDisplayMode="Top"
25-
SelectionChanged="NavView_SelectionChanged"
2625
MenuItemsSource="{StaticResource customers}"
26+
ItemInvoked="NavView_ItemInvoked"
27+
SelectionChanged="NavView_SelectionChanged"
2728
Grid.Row="1">
2829
<controls:NavigationView.MenuItemTemplate>
2930
<DataTemplate>
@@ -37,6 +38,18 @@
3738
<StackPanel Orientation="Vertical" Margin="8,0,0,0">
3839
<Button x:Name="FlipOrientation" AutomationProperties.Name="FlipOrientationButton" Content="Flip Orientation" Click="FlipOrientation_Click"/>
3940
<TextBlock x:Name="SelectionEventResult" AutomationProperties.Name="SelectionEventResult"></TextBlock>
41+
<StackPanel Orientation="Vertical">
42+
<StackPanel Orientation="Horizontal">
43+
<TextBlock Margin="5" HorizontalAlignment="Center" VerticalAlignment="Center">ItemInvoked Arguments (InvokedItem | InvokedItemContainer):</TextBlock>
44+
<TextBox Margin="42,0,0,0" x:Name="ItemInvokedItemType" AutomationProperties.Name="ItemInvokedItemType" Text="N/A"/>
45+
<TextBox x:Name="ItemInvokedItemContainerType" AutomationProperties.Name="ItemInvokedItemContainerType" Text="N/A"/>
46+
</StackPanel>
47+
<StackPanel Orientation="Horizontal">
48+
<TextBlock Margin="5" HorizontalAlignment="Center" VerticalAlignment="Center">SelectionChanged Arguments (SelectedItem | SelectedItemContainer):</TextBlock>
49+
<TextBox x:Name="SelectionChangedItemType" AutomationProperties.Name="SelectionChangedItemType" Text="N/A"/>
50+
<TextBox x:Name="SelectionChangedItemContainerType" AutomationProperties.Name="SelectionChangedItemContainerType" Text="N/A"/>
51+
</StackPanel>
52+
</StackPanel>
4053
</StackPanel>
4154
</controls:NavigationView>
4255
</Grid>

test/NavigationView_TestUI/NavigationViewItemTemplatePage.xaml.cs

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212

1313
using NavigationViewPaneDisplayMode = ModernWpf.Controls.NavigationViewPaneDisplayMode;
1414
//using MaterialHelperTestApi = Microsoft.UI.Private.Media.MaterialHelperTestApi;
15+
using NavigationView = ModernWpf.Controls.NavigationView;
16+
using NavigationViewItemInvokedEventArgs = ModernWpf.Controls.NavigationViewItemInvokedEventArgs;
17+
using NavigationViewSelectionChangedEventArgs = ModernWpf.Controls.NavigationViewSelectionChangedEventArgs;
1518

1619
namespace MUXControlsTestApp
1720
{
@@ -58,7 +61,7 @@ private void FlipOrientation_Click(object sender, RoutedEventArgs e)
5861

5962
private void NavView_SelectionChanged(ModernWpf.Controls.NavigationView sender, ModernWpf.Controls.NavigationViewSelectionChangedEventArgs args)
6063
{
61-
var children = (StackPanel)args.SelectedItemContainer.Content;
64+
var children = (Customer)args.SelectedItemContainer.Content;
6265
var customer = (Customer)args.SelectedItem;
6366
if(children != null && customer != null)
6467
{
@@ -68,6 +71,38 @@ private void NavView_SelectionChanged(ModernWpf.Controls.NavigationView sender,
6871
{
6972
SelectionEventResult.Text = "Failed";
7073
}
74+
75+
// Reset argument type indicatiors
76+
SelectionChangedItemType.Text = "null";
77+
SelectionChangedItemContainerType.Text = "null";
78+
79+
// Update argument type indicators
80+
if (args.SelectedItem != null)
81+
{
82+
SelectionChangedItemType.Text = args.SelectedItem.GetType().ToString();
83+
}
84+
85+
if (args.SelectedItemContainer != null)
86+
{
87+
SelectionChangedItemContainerType.Text = args.SelectedItemContainer.GetType().ToString();
88+
}
89+
}
90+
91+
private void NavView_ItemInvoked(NavigationView sender, NavigationViewItemInvokedEventArgs args)
92+
{
93+
// Reset argument type indicatiors
94+
ItemInvokedItemType.Text = "null";
95+
ItemInvokedItemContainerType.Text = "null";
96+
97+
if (args.InvokedItem != null)
98+
{
99+
ItemInvokedItemType.Text = args.InvokedItem.GetType().ToString();
100+
}
101+
102+
if (args.InvokedItemContainer != null)
103+
{
104+
ItemInvokedItemContainerType.Text = args.InvokedItemContainer.GetType().ToString();
105+
}
71106
}
72107
}
73108
}

0 commit comments

Comments
 (0)