Skip to content

Feature: Improved performance when opening path bar flyouts #16665

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file modified src/Files.App/Assets/FilesOpenDialog/Files.App.Launcher.exe
Binary file not shown.
Original file line number Diff line number Diff line change
@@ -1 +1 @@
7219ea9bd5832c77f1d6c8e57f97e6504a33f07bdef61122a171061f0dcb3509
cc957911eb0abac03d457a70f1c6ff03eeabd0d05548806ff801998d7218b48d
14 changes: 0 additions & 14 deletions src/Files.App/Data/EventArguments/ToolbarFlyoutOpenedEventArgs.cs

This file was deleted.

17 changes: 17 additions & 0 deletions src/Files.App/Data/EventArguments/ToolbarFlyoutOpeningEventArgs.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Copyright (c) Files Community
// Licensed under the MIT License.

using Microsoft.UI.Xaml.Controls;

namespace Files.App.Data.EventArguments
{
public sealed class ToolbarFlyoutOpeningEventArgs
{
public MenuFlyout OpeningFlyout { get; }

public ToolbarFlyoutOpeningEventArgs(MenuFlyout openingFlyout)
{
OpeningFlyout = openingFlyout;
}
}
}
2 changes: 2 additions & 0 deletions src/Files.App/UserControls/AddressToolbar.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@
<Button.ContextFlyout>
<MenuFlyout
x:Name="BackHistoryFlyout"
AreOpenCloseAnimationsEnabled="False"
Opening="BackHistoryFlyout_Opening"
Placement="BottomEdgeAlignedLeft"
ScrollViewer.VerticalScrollBarVisibility="Auto"
Expand Down Expand Up @@ -306,6 +307,7 @@
<Button.ContextFlyout>
<MenuFlyout
x:Name="ForwardHistoryFlyout"
AreOpenCloseAnimationsEnabled="False"
Opening="ForwardHistoryFlyout_Opening"
Placement="BottomEdgeAlignedLeft"
ScrollViewer.VerticalScrollBarVisibility="Auto"
Expand Down
28 changes: 20 additions & 8 deletions src/Files.App/UserControls/AddressToolbar.xaml.cs
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
// Copyright (c) Files Community
// Licensed under the MIT License.

using System.Windows.Input;
using Microsoft.UI.Input;
using Microsoft.UI.Xaml;
using Microsoft.UI.Xaml.Controls;
using Microsoft.UI.Xaml.Controls.Primitives;
using Microsoft.UI.Xaml.Input;
using Microsoft.UI.Xaml.Media;
using Windows.System;
using Microsoft.UI.Xaml.Navigation;
using System.Windows.Input;
using Windows.System;
using FocusManager = Microsoft.UI.Xaml.Input.FocusManager;

namespace Files.App.UserControls
Expand Down Expand Up @@ -161,37 +161,49 @@ private async void ForwardHistoryFlyout_Opening(object? sender, object e)
await AddHistoryItemsAsync(shellPage.ForwardStack, ForwardHistoryFlyout.Items, false);
}

private async Task AddHistoryItemsAsync(IEnumerable<PageStackEntry> items, IList<MenuFlyoutItemBase> destination, bool isBackMode)
private async Task AddHistoryItemsAsync(IEnumerable<PageStackEntry> items, IList<MenuFlyoutItemBase> flyoutItems, bool isBackMode)
{
// This may not seem performant, however it's the most viable trade-off to make.
// Instead of constantly keeping track of back/forward stack and performing lookups
// (which may degrade performance), we only add items in bulk when it's needed.
// There's also a high chance the user might not use the feature at all in which case
// the former approach would just waste extra performance gain

destination.Clear();
flyoutItems.Clear();
foreach (var item in items.Reverse())
{
if (item.Parameter is not NavigationArguments args || args.NavPathParam is null)
continue;

var imageSource = await NavigationHelpers.GetIconForPathAsync(args.NavPathParam);
var fileName = SystemIO.Path.GetFileName(args.NavPathParam);

// The fileName is empty if the path is (root) drive path
if (string.IsNullOrEmpty(fileName))
fileName = args.NavPathParam;

destination.Add(new MenuFlyoutItem()
var flyoutItem = new MenuFlyoutItem
{
Icon = new ImageIcon() { Source = imageSource },
Icon = new FontIcon { Glyph = "\uE8B7" }, // Use font icon as placeholder
Text = fileName,
Command = historyItemClickedCommand,
CommandParameter = new ToolbarHistoryItemModel(item, isBackMode)
});
};

flyoutItems?.Add(flyoutItem);

// Start loading the thumbnail in the background
_ = LoadFlyoutItemIconAsync(flyoutItem, args.NavPathParam);
}
}

private async Task LoadFlyoutItemIconAsync(MenuFlyoutItem flyoutItem, string path)
{
var imageSource = await NavigationHelpers.GetIconForPathAsync(path);

if (imageSource is not null)
flyoutItem.Icon = new ImageIcon { Source = imageSource };
}

private void HistoryItemClicked(ToolbarHistoryItemModel? itemModel)
{
if (itemModel is null)
Expand Down
3 changes: 2 additions & 1 deletion src/Files.App/UserControls/PathBreadcrumb.xaml
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,9 @@
<Button.Flyout>
<MenuFlyout
x:Name="ExpandMenuFlyout"
AreOpenCloseAnimationsEnabled="False"
Closed="PathBoxItemFlyout_Closed"
Opened="PathBoxItemFlyout_Opened"
Opening="PathBoxItemFlyout_Opening"
Placement="BottomEdgeAlignedLeft"
ScrollViewer.VerticalScrollBarVisibility="Auto"
ScrollViewer.VerticalScrollMode="Auto">
Expand Down
4 changes: 2 additions & 2 deletions src/Files.App/UserControls/PathBreadcrumb.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ private void PathItemSeparator_DataContextChanged(FrameworkElement sender, DataC
ViewModel.PathItemSeparator_DataContextChanged(sender, args);
}

private void PathBoxItemFlyout_Opened(object sender, object e)
private void PathBoxItemFlyout_Opening(object sender, object e)
{
ViewModel.PathboxItemFlyout_Opened(sender, e);
ViewModel.PathboxItemFlyout_Opening(sender, e);
}

private void PathBoxItemFlyout_Closed(object sender, object e)
Expand Down
24 changes: 17 additions & 7 deletions src/Files.App/ViewModels/UserControls/AddressToolbarViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public sealed class AddressToolbarViewModel : ObservableObject, IAddressToolbarV

public delegate void ToolbarPathItemInvokedEventHandler(object sender, PathNavigationEventArgs e);

public delegate void ToolbarFlyoutOpenedEventHandler(object sender, ToolbarFlyoutOpenedEventArgs e);
public delegate void ToolbarFlyoutOpeningEventHandler(object sender, ToolbarFlyoutOpeningEventArgs e);

public delegate void ToolbarPathItemLoadedEventHandler(object sender, ToolbarPathItemLoadedEventArgs e);

Expand All @@ -43,7 +43,7 @@ public sealed class AddressToolbarViewModel : ObservableObject, IAddressToolbarV

public event ToolbarPathItemInvokedEventHandler? ToolbarPathItemInvoked;

public event ToolbarFlyoutOpenedEventHandler? ToolbarFlyoutOpened;
public event ToolbarFlyoutOpeningEventHandler? ToolbarFlyoutOpening;

public event ToolbarPathItemLoadedEventHandler? ToolbarPathItemLoaded;

Expand Down Expand Up @@ -456,9 +456,9 @@ public void PathItemSeparator_DataContextChanged(FrameworkElement sender, DataCo
});
}

public void PathboxItemFlyout_Opened(object sender, object e)
public void PathboxItemFlyout_Opening(object sender, object e)
{
ToolbarFlyoutOpened?.Invoke(this, new ToolbarFlyoutOpenedEventArgs() { OpenedFlyout = (MenuFlyout)sender });
ToolbarFlyoutOpening?.Invoke(this, new ToolbarFlyoutOpeningEventArgs((MenuFlyout)sender));
}

public void PathBoxItemFlyout_Closed(object sender, object e)
Expand Down Expand Up @@ -651,11 +651,9 @@ public async Task SetPathBoxDropDownFlyoutAsync(MenuFlyout flyout, PathBoxItem p

foreach (var childFolder in childFolders)
{
var imageSource = await NavigationHelpers.GetIconForPathAsync(childFolder.Path);

var flyoutItem = new MenuFlyoutItem
{
Icon = new ImageIcon() { Source = imageSource },
Icon = new FontIcon { Glyph = "\uE8B7" }, // Use font icon as placeholder
Text = childFolder.Item.Name,
FontSize = 12,
};
Expand All @@ -670,9 +668,21 @@ public async Task SetPathBoxDropDownFlyoutAsync(MenuFlyout flyout, PathBoxItem p
}

flyout.Items?.Add(flyoutItem);

// Start loading the thumbnail in the background
_ = LoadFlyoutItemIconAsync(flyoutItem, childFolder.Path);
}
}

private async Task LoadFlyoutItemIconAsync(MenuFlyoutItem flyoutItem, string path)
{
var imageSource = await NavigationHelpers.GetIconForPathAsync(path);

if (imageSource is not null)
flyoutItem.Icon = new ImageIcon { Source = imageSource };
}


private static string NormalizePathInput(string currentInput, bool isFtp)
{
if (currentInput.Contains('/') && !isFtp)
Expand Down
10 changes: 5 additions & 5 deletions src/Files.App/Views/Shells/BaseShellPage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public BaseShellPage(CurrentInstanceViewModel instanceViewModel)
FlowDirection = FlowDirection.RightToLeft;

ToolbarViewModel.ToolbarPathItemInvoked += ShellPage_NavigationRequested;
ToolbarViewModel.ToolbarFlyoutOpened += ShellPage_ToolbarFlyoutOpened;
ToolbarViewModel.ToolbarFlyoutOpening += ShellPage_ToolbarFlyoutOpening;
ToolbarViewModel.ToolbarPathItemLoaded += ShellPage_ToolbarPathItemLoaded;
ToolbarViewModel.AddressBarTextEntered += ShellPage_AddressBarTextEntered;
ToolbarViewModel.PathBoxItemDropped += ShellPage_PathBoxItemDropped;
Expand Down Expand Up @@ -431,12 +431,12 @@ protected async void ShellPage_ToolbarPathItemLoaded(object sender, ToolbarPathI
await ToolbarViewModel.SetPathBoxDropDownFlyoutAsync(e.OpenedFlyout, e.Item, this);
}

protected async void ShellPage_ToolbarFlyoutOpened(object sender, ToolbarFlyoutOpenedEventArgs e)
protected async void ShellPage_ToolbarFlyoutOpening(object sender, ToolbarFlyoutOpeningEventArgs e)
{
var pathBoxItem = ((Button)e.OpenedFlyout.Target).DataContext as PathBoxItem;
var pathBoxItem = ((Button)e.OpeningFlyout.Target).DataContext as PathBoxItem;

if (pathBoxItem is not null)
await ToolbarViewModel.SetPathBoxDropDownFlyoutAsync(e.OpenedFlyout, pathBoxItem, this);
await ToolbarViewModel.SetPathBoxDropDownFlyoutAsync(e.OpeningFlyout, pathBoxItem, this);
}

protected async void NavigationToolbar_QuerySubmitted(object sender, ToolbarQuerySubmittedEventArgs e)
Expand Down Expand Up @@ -828,7 +828,7 @@ public virtual void Dispose()
drivesViewModel.PropertyChanged -= DrivesManager_PropertyChanged;

ToolbarViewModel.ToolbarPathItemInvoked -= ShellPage_NavigationRequested;
ToolbarViewModel.ToolbarFlyoutOpened -= ShellPage_ToolbarFlyoutOpened;
ToolbarViewModel.ToolbarFlyoutOpening -= ShellPage_ToolbarFlyoutOpening;
ToolbarViewModel.ToolbarPathItemLoaded -= ShellPage_ToolbarPathItemLoaded;
ToolbarViewModel.AddressBarTextEntered -= ShellPage_AddressBarTextEntered;
ToolbarViewModel.PathBoxItemDropped -= ShellPage_PathBoxItemDropped;
Expand Down
Loading