Skip to content

Commit

Permalink
fix: Fix NavigationView memory leak
Browse files Browse the repository at this point in the history
  • Loading branch information
@Youssef1313
Youssef1313 committed Jan 12, 2024
1 parent ea212dc commit a9182c9
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 21 deletions.
35 changes: 20 additions & 15 deletions src/Uno.UI.FluentTheme.v1/Resources/Version1/PriorityDefault/NavigationView_v1.xaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
</VisualState>
</VisualStateGroup>
</VisualStateManager.VisualStateGroups>

<!-- Content layouts -->
<Grid>
<Grid.RowDefinitions>
Expand All @@ -152,7 +152,7 @@
Height="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=TemplateSettings.TopPadding}"
Visibility="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=TemplateSettings.TopPaneVisibility}"/>

<Grid x:Name="TopNavGrid"
<Grid x:Name="TopNavGrid"
Height="{ThemeResource NavigationViewTopPaneHeight}"
Visibility="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=TemplateSettings.TopPaneVisibility}">
<Grid.ColumnDefinitions>
Expand All @@ -167,8 +167,8 @@
<ColumnDefinition Width="Auto"/>
</Grid.ColumnDefinitions>

<Grid
x:Name="TopNavLeftPadding"
<Grid
x:Name="TopNavLeftPadding"
Grid.Column="1"
Width="0"/>

Expand All @@ -194,7 +194,7 @@
HorizontalScrollBarVisibility="Hidden"
VerticalScrollMode="Disabled"
VerticalScrollBarVisibility="Hidden">
<local:ItemsRepeater
<local:ItemsRepeater
AutomationProperties.LandmarkType="Navigation"
AutomationProperties.Name="{TemplateBinding AutomationProperties.Name}"
AutomationProperties.AccessibilityView = "Content"
Expand All @@ -206,7 +206,7 @@
</ScrollViewer>
</local:ItemsRepeaterScrollHost>

<Button
<Button
x:Name="TopNavOverflowButton"
Grid.Column="4"
Content="More"
Expand Down Expand Up @@ -264,7 +264,12 @@
<ScrollViewer VerticalScrollBarVisibility="Auto">
<local:ItemsRepeater
AutomationProperties.AccessibilityView = "Content"
x:Name="TopNavMenuItemsOverflowHost"/>
x:Name="TopNavMenuItemsOverflowHost">
<!-- Uno specific: This matches Fluent v2 version and avoids a memory leak related to the use of the singleton StackLayout in ItemsRepeater -->
<local:ItemsRepeater.Layout>
<local:StackLayout />
</local:ItemsRepeater.Layout>
</local:ItemsRepeater>
</ScrollViewer>
</local:ItemsRepeaterScrollHost>
</Flyout>
Expand Down Expand Up @@ -325,11 +330,11 @@
IsTabStop="False"
OpenPaneLength="{TemplateBinding OpenPaneLength}"
PaneBackground="{ThemeResource NavigationViewDefaultPaneBackground}"

Grid.Row="1">

<SplitView.Pane>
<Grid
<Grid
x:Name="PaneContentGrid"
HorizontalAlignment="Left"
Visibility="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=TemplateSettings.LeftPaneVisibility}">
Expand Down Expand Up @@ -480,7 +485,7 @@
</Grid>
</SplitView.Content>
</SplitView>

</Grid>

<!-- Button grid -->
Expand Down Expand Up @@ -511,7 +516,7 @@
<ToolTip x:Name="NavigationViewBackButtonToolTip"/>
</ToolTipService.ToolTip>
</Button>
<Button
<Button
x:Name="NavigationViewCloseButton"
Style="{StaticResource NavigationBackButtonNormalStyle}"
VerticalAlignment="Top">
Expand All @@ -527,7 +532,7 @@
VerticalAlignment="Top"
MinWidth="{Binding RelativeSource={RelativeSource TemplatedParent}, Path=TemplateSettings.PaneToggleButtonWidth}">
<TextBlock
x:Name="PaneTitleTextBlock"
x:Name="PaneTitleTextBlock"
Grid.Column="0"
Text="{TemplateBinding PaneTitle}"
HorizontalAlignment="Left"
Expand Down Expand Up @@ -573,7 +578,7 @@
<RowDefinition Height="*"/>
<RowDefinition Height="Auto"/>
</Grid.RowDefinitions>

<VisualStateManager.VisualStateGroups>
<VisualStateGroup x:Name="ItemOnNavigationViewListPositionStates">
<VisualState x:Name="OnLeftNavigation">
Expand Down Expand Up @@ -807,7 +812,7 @@
</VisualState>
</VisualStateGroup>
</VisualStateManager.VisualStateGroups>
<Rectangle
<Rectangle
x:Name="SeparatorLine"
Height="{ThemeResource NavigationViewItemSeparatorHeight}"
Margin="{ThemeResource NavigationViewItemSeparatorMargin}"
Expand All @@ -817,4 +822,4 @@
</Setter.Value>
</Setter>
</Style>
</ResourceDictionary>
</ResourceDictionary>
7 changes: 6 additions & 1 deletion src/Uno.UI.FluentTheme.v1/themeresources_v1.xaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15998,7 +15998,12 @@
</Flyout.FlyoutPresenterStyle>
<controls:ItemsRepeaterScrollHost>
<ScrollViewer VerticalScrollBarVisibility="Auto">
<controls:ItemsRepeater AutomationProperties.AccessibilityView="Content" x:Name="TopNavMenuItemsOverflowHost" />
<controls:ItemsRepeater AutomationProperties.AccessibilityView="Content" x:Name="TopNavMenuItemsOverflowHost">
<!-- Uno specific: This matches Fluent v2 version and avoids a memory leak related to the use of the singleton StackLayout in ItemsRepeater -->
<controls:ItemsRepeater.Layout>
<controls:StackLayout />
</controls:ItemsRepeater.Layout>
</controls:ItemsRepeater>
</ScrollViewer>
</controls:ItemsRepeaterScrollHost>
</Flyout>
Expand Down
2 changes: 0 additions & 2 deletions src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_FrameworkElement_And_Leak.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@ public class Given_FrameworkElement_And_Leak
[DataRow(typeof(Microsoft/* UWP don't rename */.UI.Xaml.Controls.Primitives.MonochromaticOverlayPresenter), 15)]
[DataRow(typeof(Microsoft/* UWP don't rename */.UI.Xaml.Controls.NavigationViewItem), 15)]
[DataRow(typeof(Microsoft/* UWP don't rename */.UI.Xaml.Controls.Primitives.NavigationViewItemPresenter), 15)]
#if false // Disabled for #10309
[DataRow(typeof(Microsoft/* UWP don't rename */.UI.Xaml.Controls.NavigationView), 15)]
#endif
[DataRow(typeof(Microsoft/* UWP don't rename */.UI.Xaml.Controls.NumberBox), 15)]
#if !WINAPPSDK
[DataRow(typeof(Microsoft/* UWP don't rename */.UI.Xaml.Controls.PagerControl), 15)]
Expand Down
9 changes: 7 additions & 2 deletions src/Uno.UI/Microsoft/UI/Xaml/Controls/NavigationView/NavigationView_v1.xaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,12 @@
<ScrollViewer VerticalScrollBarVisibility="Auto">
<local:ItemsRepeater
AutomationProperties.AccessibilityView = "Content"
x:Name="TopNavMenuItemsOverflowHost"/>
x:Name="TopNavMenuItemsOverflowHost">
<!-- Uno specific: This matches Fluent v2 version and avoids a memory leak related to the use of the singleton StackLayout in ItemsRepeater -->
<local:ItemsRepeater.Layout>
<local:StackLayout />
</local:ItemsRepeater.Layout>
</local:ItemsRepeater>
</ScrollViewer>
</local:ItemsRepeaterScrollHost>
</Flyout>
Expand Down Expand Up @@ -817,4 +822,4 @@
</Setter.Value>
</Setter>
</Style>
</ResourceDictionary>
</ResourceDictionary>
7 changes: 6 additions & 1 deletion src/Uno.UI/UI/Xaml/Style/mergedstyles.xaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5053,7 +5053,12 @@
</Flyout.FlyoutPresenterStyle>
<controls:ItemsRepeaterScrollHost>
<ScrollViewer VerticalScrollBarVisibility="Auto">
<controls:ItemsRepeater AutomationProperties.AccessibilityView="Content" x:Name="TopNavMenuItemsOverflowHost" />
<controls:ItemsRepeater AutomationProperties.AccessibilityView="Content" x:Name="TopNavMenuItemsOverflowHost">
<!-- Uno specific: This matches Fluent v2 version and avoids a memory leak related to the use of the singleton StackLayout in ItemsRepeater -->
<controls:ItemsRepeater.Layout>
<controls:StackLayout />
</controls:ItemsRepeater.Layout>
</controls:ItemsRepeater>
</ScrollViewer>
</controls:ItemsRepeaterScrollHost>
</Flyout>
Expand Down

0 comments on commit a9182c9

Please sign in to comment.