Skip to content
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

Code Quality: Introduced HomePageContext #14115

Merged
merged 5 commits into from
Dec 19, 2023
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
49 changes: 49 additions & 0 deletions src/Files.App/Data/Contexts/HomePage/HomePageContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright (c) 2023 Files Community
// Licensed under the MIT License. See the LICENSE.

using Files.App.UserControls.Widgets;
using Files.App.ViewModels.Widgets;
using Microsoft.UI.Xaml.Controls;
using System.Collections.Immutable;

namespace Files.App.Data.Contexts
{
internal class HomePageContext : ObservableObject, IHomePageContext
{
private static readonly IImmutableList<FileTagsItemViewModel> emptyTaggedItems = Enumerable.Empty<FileTagsItemViewModel>().ToImmutableList();

public bool IsAnyItemRightClicked => rightClickedItem is not null;

private WidgetCardItem? rightClickedItem = null;
public WidgetCardItem? RightClickedItem => rightClickedItem;

private CommandBarFlyout? itemContextFlyoutMenu = null;
public CommandBarFlyout? ItemContextFlyoutMenu => itemContextFlyoutMenu;

private IReadOnlyList<FileTagsItemViewModel> selectedTaggedItems = emptyTaggedItems;
public IReadOnlyList<FileTagsItemViewModel> SelectedTaggedItems
{
get => selectedTaggedItems;
set => selectedTaggedItems = value ?? emptyTaggedItems;
}

public HomePageContext()
{
HomePageWidget.RightClickedItemChanged += HomePageWidget_RightClickedItemChanged;
FileTagsWidget.SelectedTaggedItemsChanged += FileTagsWidget_SelectedTaggedItemsChanged;
}

private void FileTagsWidget_SelectedTaggedItemsChanged(object? sender, IEnumerable<FileTagsItemViewModel> e)
{
SetProperty(ref selectedTaggedItems, e.ToList());
}

private void HomePageWidget_RightClickedItemChanged(object? sender, WidgetsRightClickedItemChangedEventArgs e)
{
if (SetProperty(ref rightClickedItem, e.Item, nameof(RightClickedItem)))
OnPropertyChanged(nameof(IsAnyItemRightClicked));

SetProperty(ref itemContextFlyoutMenu, e.Flyout, nameof(ItemContextFlyoutMenu));
}
}
}
32 changes: 32 additions & 0 deletions src/Files.App/Data/Contexts/HomePage/IHomePageContext.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright (c) 2023 Files Community
// Licensed under the MIT License. See the LICENSE.

using Files.App.UserControls.Widgets;
using Files.App.ViewModels.Widgets;
using Microsoft.UI.Xaml.Controls;

namespace Files.App.Data.Contexts
{
internal interface IHomePageContext
{
/// <summary>
/// The last right clicked item
/// </summary>
WidgetCardItem? RightClickedItem { get; }

/// <summary>
/// The last opened widget's context menu instance
/// </summary>
CommandBarFlyout? ItemContextFlyoutMenu { get; }

/// <summary>
/// An list containing all the selected tagged items
/// </summary>
IReadOnlyList<FileTagsItemViewModel> SelectedTaggedItems { get; }

/// <summary>
/// Tells whether any item has been right clicked
/// </summary>
bool IsAnyItemRightClicked { get; }
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Copyright (c) 2023 Files Community
// Licensed under the MIT License. See the LICENSE.

using Files.App.UserControls.Widgets;
using Microsoft.UI.Xaml.Controls;

namespace Files.App.Data.EventArguments
{
public class WidgetsRightClickedItemChangedEventArgs
{
public WidgetCardItem? Item { get; set; }

public CommandBarFlyout? Flyout { get; set; }

public WidgetsRightClickedItemChangedEventArgs(WidgetCardItem? item = null, CommandBarFlyout? flyout = null)
{
Item = item;
Flyout = flyout;
}
}
}
1 change: 1 addition & 0 deletions src/Files.App/Helpers/Application/AppLifecycleHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,7 @@ public static IHost ConfigureHost()
.AddSingleton<IPageContext, PageContext>()
.AddSingleton<IContentPageContext, ContentPageContext>()
.AddSingleton<IDisplayPageContext, DisplayPageContext>()
.AddSingleton<IHomePageContext, HomePageContext>()
.AddSingleton<IWindowContext, WindowContext>()
.AddSingleton<IMultitaskingContext, MultitaskingContext>()
.AddSingleton<ITagsContext, TagsContext>()
Expand Down
11 changes: 8 additions & 3 deletions src/Files.App/UserControls/Widgets/DrivesWidget.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public async Task LoadCardThumbnailAsync()
public sealed partial class DrivesWidget : HomePageWidget, IWidgetItemModel, INotifyPropertyChanged
{
public IUserSettingsService userSettingsService { get; } = Ioc.Default.GetRequiredService<IUserSettingsService>();
private IHomePageContext HomePageContext { get; } = Ioc.Default.GetRequiredService<IHomePageContext>();

private DrivesViewModel drivesViewModel = Ioc.Default.GetRequiredService<DrivesViewModel>();

Expand Down Expand Up @@ -294,13 +295,17 @@ private void FormatDrive(DriveCardItem? item)

private void OpenProperties(DriveCardItem item)
{
if (!HomePageContext.IsAnyItemRightClicked)
return;

EventHandler<object> flyoutClosed = null!;
flyoutClosed = async (s, e) =>
flyoutClosed = (s, e) =>
{
ItemContextMenuFlyout.Closed -= flyoutClosed;
HomePageContext.ItemContextFlyoutMenu!.Closed -= flyoutClosed;
FilePropertiesHelpers.OpenPropertiesWindow(item.Item, associatedInstance);
};
ItemContextMenuFlyout.Closed += flyoutClosed;

HomePageContext.ItemContextFlyoutMenu!.Closed += flyoutClosed;
}

private async void Button_Click(object sender, RoutedEventArgs e)
Expand Down
72 changes: 36 additions & 36 deletions src/Files.App/UserControls/Widgets/FileTagsWidget.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,12 @@
using System.Windows.Input;
using Windows.Storage;

// The User Control item template is documented at https://go.microsoft.com/fwlink/?LinkId=234236

namespace Files.App.UserControls.Widgets
{
public sealed partial class FileTagsWidget : HomePageWidget, IWidgetItemModel
{
private readonly IUserSettingsService userSettingsService;
private IHomePageContext HomePageContext { get; } = Ioc.Default.GetRequiredService<IHomePageContext>();

public FileTagsWidgetViewModel ViewModel
{
Expand All @@ -32,6 +31,7 @@ public FileTagsWidgetViewModel ViewModel
public delegate void FileTagsOpenLocationInvokedEventHandler(object sender, PathNavigationEventArgs e);
public delegate void FileTagsNewPaneInvokedEventHandler(object sender, QuickAccessCardInvokedEventArgs e);

public static event EventHandler<IEnumerable<FileTagsItemViewModel>>? SelectedTaggedItemsChanged;
public event FileTagsOpenLocationInvokedEventHandler FileTagsOpenLocationInvoked;
public event FileTagsNewPaneInvokedEventHandler FileTagsNewPaneInvoked;

Expand Down Expand Up @@ -69,10 +69,14 @@ public FileTagsWidget()

private void OpenProperties(WidgetCardItem? item)
{
if (!HomePageContext.IsAnyItemRightClicked)
return;

EventHandler<object> flyoutClosed = null!;
flyoutClosed = (s, e) =>
{
ItemContextMenuFlyout.Closed -= flyoutClosed;
HomePageContext.ItemContextFlyoutMenu!.Closed -= flyoutClosed;

ListedItem listedItem = new(null!)
{
ItemPath = (item.Item as FileTagsItemViewModel)?.Path ?? string.Empty,
Expand All @@ -82,7 +86,8 @@ private void OpenProperties(WidgetCardItem? item)
};
FilePropertiesHelpers.OpenPropertiesWindow(listedItem, AppInstance);
};
ItemContextMenuFlyout.Closed += flyoutClosed;

HomePageContext.ItemContextFlyoutMenu!.Closed += flyoutClosed;
}

private void OpenInNewPane(WidgetCardItem? item)
Expand All @@ -101,50 +106,45 @@ private async void FileTagItem_ItemClick(object sender, ItemClickEventArgs e)

private void AdaptiveGridView_RightTapped(object sender, RightTappedRoutedEventArgs e)
{
// Ensure values are not null
if (e.OriginalSource is not FrameworkElement element ||
element.DataContext is not FileTagsItemViewModel item)
{
return;
}

LoadContextMenu(
element,
e,
GetItemMenuItems(item, QuickAccessService.IsItemPinned(item.Path), item.IsFolder),
rightClickedItem: item);
}
// Create a new Flyout
var itemContextMenuFlyout = new CommandBarFlyout()
{
Placement = FlyoutPlacementMode.Full
};

private void LoadContextMenu(
FrameworkElement element,
RightTappedRoutedEventArgs e,
List<ContextMenuFlyoutItemViewModel> menuItems,
FileTagsItemViewModel? rightClickedItem = null)
{
var itemContextMenuFlyout = new CommandBarFlyout { Placement = FlyoutPlacementMode.Full };
// Hook events
itemContextMenuFlyout.Opening += (sender, e) => App.LastOpenedFlyout = sender as CommandBarFlyout;
itemContextMenuFlyout.Opened += (sender, e) => OnRightClickedItemChanged(null, null);

FlyoutItemPath = item.Path;

// Notify of the change on right clicked item
OnRightClickedItemChanged(item, itemContextMenuFlyout);

// Get items for the flyout
var menuItems = GetItemMenuItems(item, QuickAccessService.IsItemPinned(item.Path), item.IsFolder);
var (_, secondaryElements) = ItemModelListToContextFlyoutHelper.GetAppBarItemsFromModel(menuItems);

if (!UserSettingsService.GeneralSettingsService.MoveShellExtensionsToSubMenu)
secondaryElements.OfType<FrameworkElement>()
.ForEach(i => i.MinWidth = Constants.UI.ContextMenuItemsMaxWidth); // Set menu min width if the overflow menu setting is disabled
// Set max width of the flyout
secondaryElements
.OfType<FrameworkElement>()
.ForEach(i => i.MinWidth = Constants.UI.ContextMenuItemsMaxWidth);

secondaryElements.ForEach(i => itemContextMenuFlyout.SecondaryCommands.Add(i));
ItemContextMenuFlyout = itemContextMenuFlyout;
if (rightClickedItem is not null)
{
FlyouItemPath = rightClickedItem.Path;
ItemContextMenuFlyout.Opened += ItemContextMenuFlyout_Opened;
}
itemContextMenuFlyout.ShowAt(element, new FlyoutShowOptions { Position = e.GetPosition(element) });
// Add menu items to the secondary flyout
secondaryElements.ForEach(itemContextMenuFlyout.SecondaryCommands.Add);

e.Handled = true;
}
// Show the flyout
itemContextMenuFlyout.ShowAt(element, new() { Position = e.GetPosition(element) });

private async void ItemContextMenuFlyout_Opened(object? sender, object e)
{
ItemContextMenuFlyout.Opened -= ItemContextMenuFlyout_Opened;
await ShellContextmenuHelper.LoadShellMenuItemsAsync(FlyouItemPath, ItemContextMenuFlyout, showOpenWithMenu: true, showSendToMenu: true);
// Load shell menu items
_ = ShellContextmenuHelper.LoadShellMenuItemsAsync(FlyoutItemPath, itemContextMenuFlyout);

e.Handled = true;
}

public override List<ContextMenuFlyoutItemViewModel> GetItemMenuItems(WidgetCardItem item, bool isPinned, bool isFolder = false)
Expand Down
88 changes: 61 additions & 27 deletions src/Files.App/UserControls/Widgets/HomePageWidget.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,51 +13,81 @@ namespace Files.App.UserControls.Widgets
{
public abstract class HomePageWidget : UserControl
{
// Dependency injections

public IUserSettingsService UserSettingsService { get; } = Ioc.Default.GetRequiredService<IUserSettingsService>();
public IQuickAccessService QuickAccessService { get; } = Ioc.Default.GetRequiredService<IQuickAccessService>();
public IStorageService StorageService { get; } = Ioc.Default.GetRequiredService<IStorageService>();

public ICommand RemoveRecentItemCommand;
public ICommand ClearAllItemsCommand;
public ICommand OpenFileLocationCommand;
public ICommand OpenInNewTabCommand;
public ICommand OpenInNewWindowCommand;
public ICommand OpenPropertiesCommand;
public ICommand PinToFavoritesCommand;
public ICommand UnpinFromFavoritesCommand;
// Fields

protected string? FlyoutItemPath;

// Commands

public ICommand? RemoveRecentItemCommand { get; protected set; }
public ICommand? ClearAllItemsCommand { get; protected set; }
public ICommand? OpenFileLocationCommand { get; protected set; }
public ICommand? OpenInNewTabCommand { get; protected set; }
public ICommand? OpenInNewWindowCommand { get; protected set; }
public ICommand? OpenPropertiesCommand { get; protected set; }
public ICommand? PinToFavoritesCommand { get; protected set; }
public ICommand? UnpinFromFavoritesCommand { get; protected set; }

// Events

protected CommandBarFlyout ItemContextMenuFlyout;
protected string FlyouItemPath;
public static event EventHandler<WidgetsRightClickedItemChangedEventArgs>? RightClickedItemChanged;

// Abstract methods

public abstract List<ContextMenuFlyoutItemViewModel> GetItemMenuItems(WidgetCardItem item, bool isPinned, bool isFolder = false);

// Event methods

public void Button_RightTapped(object sender, RightTappedRoutedEventArgs e)
{
var itemContextMenuFlyout = new CommandBarFlyout { Placement = FlyoutPlacementMode.Full };
itemContextMenuFlyout.Opening += (sender, e) => App.LastOpenedFlyout = sender as CommandBarFlyout;
if (sender is not Button widgetCardItem || widgetCardItem.DataContext is not WidgetCardItem item)
// Ensure values are not null
if (sender is not Button widgetCardItem ||
widgetCardItem.DataContext is not WidgetCardItem item)
return;

// Create a new Flyout
var itemContextMenuFlyout = new CommandBarFlyout()
{
Placement = FlyoutPlacementMode.Full
};

// Hook events
itemContextMenuFlyout.Opening += (sender, e) => App.LastOpenedFlyout = sender as CommandBarFlyout;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this get garbage-collected?

Copy link
Member Author

@0x5bfa 0x5bfa Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont know why we've put the last opened flyout to the App.LastOpenedFlyout at the first place

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's not needed I'd advise to remove it. It's definitely not getting GC'd

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m gonna try to find who added this line and remove if it seems to be ok.

Copy link
Member

@yaira2 yaira2 Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use it to dismiss open context menus when closing the app, otherwise we found that it could cause the app to hang.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a workaround for #10110. This workaround is still necessary to avoid crashes.

itemContextMenuFlyout.Opened += (sender, e) => OnRightClickedItemChanged(null, null);

FlyoutItemPath = item.Path;

// Notify of the change on right clicked item
OnRightClickedItemChanged(item, itemContextMenuFlyout);

// Get items for the flyout
var menuItems = GetItemMenuItems(item, QuickAccessService.IsItemPinned(item.Path));
var (_, secondaryElements) = ItemModelListToContextFlyoutHelper.GetAppBarItemsFromModel(menuItems);

secondaryElements.OfType<FrameworkElement>()
.ForEach(i => i.MinWidth = Constants.UI.ContextMenuItemsMaxWidth);
// Set max width of the flyout
secondaryElements
.OfType<FrameworkElement>()
.ForEach(i => i.MinWidth = Constants.UI.ContextMenuItemsMaxWidth);

// Add menu items to the secondary flyout
secondaryElements.ForEach(itemContextMenuFlyout.SecondaryCommands.Add);

// Show the flyout
itemContextMenuFlyout.ShowAt(widgetCardItem, new() { Position = e.GetPosition(widgetCardItem) });

secondaryElements.ForEach(i => itemContextMenuFlyout.SecondaryCommands.Add(i));
ItemContextMenuFlyout = itemContextMenuFlyout;
FlyouItemPath = item.Path;
ItemContextMenuFlyout.Opened += ItemContextMenuFlyout_Opened;
itemContextMenuFlyout.ShowAt(widgetCardItem, new FlyoutShowOptions { Position = e.GetPosition(widgetCardItem) });
// Load shell menu items
_ = ShellContextmenuHelper.LoadShellMenuItemsAsync(FlyoutItemPath, itemContextMenuFlyout);

e.Handled = true;
}

private async void ItemContextMenuFlyout_Opened(object? sender, object e)
{
ItemContextMenuFlyout.Opened -= ItemContextMenuFlyout_Opened;
await ShellContextmenuHelper.LoadShellMenuItemsAsync(FlyouItemPath, ItemContextMenuFlyout);
}
// Command methods

public async Task OpenInNewTabAsync(WidgetCardItem item)
{
Expand All @@ -71,13 +101,17 @@ public async Task OpenInNewWindowAsync(WidgetCardItem item)

public virtual async Task PinToFavoritesAsync(WidgetCardItem item)
{
_ = QuickAccessService.PinToSidebarAsync(item.Path);
await QuickAccessService.PinToSidebarAsync(item.Path);
}

public virtual async Task UnpinFromFavoritesAsync(WidgetCardItem item)
{
_ = QuickAccessService.UnpinFromSidebarAsync(item.Path);
await QuickAccessService.UnpinFromSidebarAsync(item.Path);
}

protected void OnRightClickedItemChanged(WidgetCardItem? item, CommandBarFlyout? flyout)
{
RightClickedItemChanged?.Invoke(this, new WidgetsRightClickedItemChangedEventArgs(item, flyout));
}
}
}
Loading