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.

Sign up for GitHub

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

Jump to bottom

Feature: Recycle Bin changing icons in the side bar #10113

Closed
wants to merge 36 commits into from
Closed

Feature: Recycle Bin changing icons in the side bar #10113

Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
78cb382
Sidebar now loads an icon for empty and filled recycle bin.
QuaintMako Sep 23, 2022
3b190f1
Recycle Bin Helpers now offer a method to retrieve the adapted recycl…
QuaintMako Sep 23, 2022
ffcdccc
Ordered image res in value order.
QuaintMako Sep 24, 2022
9fe3f7c
Moved RetrieveAdaptedRecycleBinIcon in a more adapted class.
QuaintMako Sep 25, 2022
0067b32
Eliminated NPR warnings.
QuaintMako Sep 25, 2022
facacc9
Extracted a if to remove redundancy
QuaintMako Sep 25, 2022
6f4ff9d
Extracted and simplified method to retrieve icon data.
QuaintMako Sep 25, 2022
c0cd9c3
Reworked locationItem.IsInvalid evaluation.
QuaintMako Sep 25, 2022
7717072
Merge branch 'files-community:main' into 9195_DifferentIconSideBarRec…
QuaintMako Sep 28, 2022
aa18a4f
Now retrieving the correct recycle bin icon when adding it to the sid…
QuaintMako Sep 28, 2022
df88672
An event is now raised when the recycle bin is modified.
QuaintMako Sep 28, 2022
5f7e8ef
Refactored the creation of sidebar sections.
QuaintMako Sep 28, 2022
aadac84
Merge branch '9195_DifferentIconSideBarRecycleBinState' of https://gi…
QuaintMako Sep 28, 2022
72efbe3
Merge branch 'files-community:main' into 9195_DifferentIconSideBarRec…
QuaintMako Sep 30, 2022
ac395c3
Revert "Refactored the creation of sidebar sections."
QuaintMako Sep 30, 2022
d7fe441
Sidebar recycle bin icon now updates after using restore and empty ac…
QuaintMako Oct 3, 2022
4279f2d
Recycle bin changes are now notified in the FileSystemHelpers.
QuaintMako Oct 3, 2022
f659904
Merge branch 'main' into 9195_DifferentIconSideBarRecycleBinState
QuaintMako Oct 3, 2022
1f3c2ec
Brought the modifications that were overlooked to simplify last merge.
QuaintMako Oct 3, 2022
0fe3b19
Fixed some tabulation issues.
QuaintMako Oct 3, 2022
08fe06f
Fixed some tabulation issues.
QuaintMako Oct 3, 2022
06766d6
Fixed some tabulation issues.
QuaintMako Oct 3, 2022
931768e
Removed SingleItem load icon, as it was deemed useless.
QuaintMako Oct 6, 2022
61b1287
Merge branch 'main' into 9195_DifferentIconSideBarRecycleBinState
QuaintMako Oct 6, 2022
db44a48
Merge branch 'main' into 9195_DifferentIconSideBarRecycleBinState
QuaintMako Oct 6, 2022
8b7c9ee
Merge branch 'main' into 9195_DifferentIconSideBarRecycleBinState
QuaintMako Oct 7, 2022
8bc21f2
Merge branch 'main' into 9195_DifferentIconSideBarRecycleBinState
QuaintMako Oct 12, 2022
d78cf52
Moved a test statement in an upper function.
QuaintMako Oct 12, 2022
3eb2d23
Modified the RaiseRecycleBinChangedEvent method to use ? operator.
QuaintMako Oct 12, 2022
cb89ba3
Made linq request into a foreach loop.
QuaintMako Oct 12, 2022
40ef8b8
Now correctly disposing of RecycleBinChanged event listener.
QuaintMako Oct 12, 2022
abd8775
Made RecycleBinHelpers a static class.
QuaintMako Oct 12, 2022
d1042ca
Merge branch 'main' into 9195_DifferentIconSideBarRecycleBinState
QuaintMako Oct 15, 2022
ad402df
Merge branch 'main' into 9195_DifferentIconSideBarRecycleBinState
QuaintMako Oct 16, 2022
e898abf
Merge branch 'main' into 9195_DifferentIconSideBarRecycleBinState
QuaintMako Oct 19, 2022
4e7a95e
Merge branch 'main' into 9195_DifferentIconSideBarRecycleBinState
QuaintMako Oct 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Files.App/Constants.cs
View file
Open in desktop
Expand Up @@ -28,7 +28,6 @@ public static class KnownImageFormats
{
public const string BITMAP_IMAGE_FORMAT = "bitmapimage";
}

public static class ImageRes
yaira2 marked this conversation as resolved.
Show resolved Hide resolved
{
// See imageres.dll for more icon indexes to add
Expand All @@ -43,7 +42,8 @@ public static class ImageRes
public const int WindowsDrive = 36;
public const int ThisPC = 109;
public const int NetworkDrives = 25;
public const int RecycleBin = 55;
public const int RecycleBin = 54;
yaira2 marked this conversation as resolved.
Show resolved Hide resolved
public const int EmptyRecycleBin = 55;
public const int CloudDrives = 1040;
public const int OneDrive = 1043;
public const int Libraries = 1023;
Expand Down
38 changes: 27 additions & 11 deletions src/Files.App/DataModels/SidebarPinnedModel.cs
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@
using Files.App.Controllers;
using Files.App.DataModels.NavigationControlItems;
using Files.App.Filesystem;
using Files.App.Filesystem.StorageItems;
using Files.App.Helpers;
using Files.Backend.Services.Settings;
using System;
Expand Down Expand Up @@ -252,22 +253,23 @@ public async Task AddItemToSidebarAsync(string path)
Text = res.Result?.DisplayName ?? Path.GetFileName(path.TrimEnd('\\'))
};

locationItem.IsInvalid = res || (FilesystemResult)FolderHelpers.CheckFolderAccessWithWin32(path);

if (res || (FilesystemResult)FolderHelpers.CheckFolderAccessWithWin32(path))
{
locationItem.IsInvalid = false;
if (res)
if (locationItem.Path == CommonPaths.RecycleBinPath)
{
var iconData = await FileThumbnailHelper.LoadIconFromStorageItemAsync(res.Result, 24u, ThumbnailMode.ListView);
locationItem.IconData = iconData;
locationItem.Icon = await App.Window.DispatcherQueue.EnqueueAsync(() => locationItem.IconData.ToBitmapAsync());
int recycleBinIconIndex = UIHelpers.GetAdaptedRecycleBinIconIndex();
locationItem.IconData = UIHelpers.GetIconResourceInfo(recycleBinIconIndex).IconData;
}
if (locationItem.IconData == null)
else
{
locationItem.IconData = await FileThumbnailHelper.LoadIconWithoutOverlayAsync(path, 24u);
if (locationItem.IconData != null)
{
locationItem.Icon = await App.Window.DispatcherQueue.EnqueueAsync(() => locationItem.IconData.ToBitmapAsync());
}
locationItem.IconData = await RetrieveItemIconData(path, res);
}

if (locationItem.IconData != null)
{
locationItem.Icon = await App.Window.DispatcherQueue.EnqueueAsync(() => locationItem.IconData.ToBitmapAsync());
}
}
else
Expand All @@ -280,6 +282,20 @@ public async Task AddItemToSidebarAsync(string path)
AddLocationItemToSidebar(locationItem);
}

private async Task<byte[]?> RetrieveItemIconData(string itemPath, FilesystemResult<BaseStorageFolder> result)
{
byte[]? iconData = null;

if (result)
QuaintMako marked this conversation as resolved.
Show resolved Hide resolved
{
iconData = await FileThumbnailHelper.LoadIconFromStorageItemAsync(result.Result, 24u, Windows.Storage.FileProperties.ThumbnailMode.ListView);
}

iconData ??= await FileThumbnailHelper.LoadIconWithoutOverlayAsync(itemPath, 24u);

return iconData;
}

/// <summary>
/// Adds the location item to the navigation sidebar
/// </summary>
Expand Down
3 changes: 3 additions & 0 deletions src/Files.App/Filesystem/FilesystemOperations/Helpers/FilesystemHelpers.cs
View file
Open in desktop
Expand Up @@ -190,6 +190,7 @@ public async Task<ReturnResult> DeleteItemsAsync(IEnumerable<IStorageItemWithPat
sw.Stop();

PostBannerHelpers.PostBanner_Delete(source, returnStatus, permanently, token.IsCancellationRequested, itemsDeleted);
RecycleBinHelpers.RaiseRecycleBinChangedEvent();

return returnStatus;
}
Expand Down Expand Up @@ -239,6 +240,8 @@ public async Task<ReturnResult> RestoreItemsFromTrashAsync(IEnumerable<IStorageI

sw.Stop();

RecycleBinHelpers.RaiseRecycleBinChangedEvent();

return returnStatus;
}

Expand Down
13 changes: 13 additions & 0 deletions src/Files.App/Helpers/RecycleBinHelpers.cs
View file
Open in desktop
Expand Up @@ -14,6 +14,8 @@
using Files.App.Shell;
using Vanara.PInvoke;
using Files.App.Filesystem;
using System.ComponentModel;
using System.Runtime.CompilerServices;

namespace Files.App.Helpers
{
Expand All @@ -29,6 +31,8 @@ public class RecycleBinHelpers

#endregion Private Members

public static event EventHandler? RecycleBinChanged;

public async Task<List<ShellFileItem>> EnumerateRecycleBin()
{
return (await Win32Shell.GetShellFolderAsync(CommonPaths.RecycleBinPath, "Enumerate", 0, int.MaxValue)).Enumerate;
Expand Down Expand Up @@ -76,6 +80,7 @@ public async Task EmptyRecycleBin()
if (result == ContentDialogResult.Primary)
{
Shell32.SHEmptyRecycleBin(IntPtr.Zero, null, Shell32.SHERB.SHERB_NOCONFIRMATION | Shell32.SHERB.SHERB_NOPROGRESSUI);
RaiseRecycleBinChangedEvent();
Copy link
Member

Choose a reason for hiding this comment

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

You can use null check

RecycleBinChanged?.Invoke(null, null);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed inside the function that handles the event raising. But would you rather have the event raised directly without any method maybe?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. There's no need for additional method to raise this event

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We actuallty do need it to raise the event outside of the class. We need to raise it from RestoreItemsFromTrashAsync and DeleteItemsAsync from FilesystemHelpers.cs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@d2dyno1 Should I do something to indeed remove the need of the method entirely or should it stay that way?

Removing it would means to move methods from the FileSystemHelpers to the RecycleBinHelpers, albeit it wouldn't be their optimal locations.

Copy link
Member

Choose a reason for hiding this comment

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

I think moving it would make sense (?). In addition, it should be moved to instantiable helper (because it exposes an event) but that'd be part of a bigger refactor out of scope for this PR.

Copy link

Choose a reason for hiding this comment

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

I think that moving it is better and good

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure how to actually implement this. I have been turning the issue in my head for the past few days, but I cannot find a solution that feels actually clean for it. Consider this message as a formal request for help.

After giving it some thoughts, the methods from FilesystemHelpers.cs should not be moved. They are in their right places, they are not only linked to the recycle bin.

A possibility would to raise events in FilesystemHelpers.cs when Delete or Restore are called, and have methods to update the recycle bin icon subscribe to them. But then, an issue comes from the RecycleBinHelpers.EmptyRecycleBin() method that does not use the Delete method to clear the recycle bin.

I must be missing something obvious, and I'd like to see this PR through.

}
}

Expand Down Expand Up @@ -195,5 +200,13 @@ public virtual async Task DeleteItem(IShellPage associatedInstance)
item.PrimaryItemAttribute == StorageItemTypes.File ? FilesystemItemType.File : FilesystemItemType.Directory));
await associatedInstance.FilesystemHelpers.DeleteItemsAsync(items, true, false, true);
}

public static void RaiseRecycleBinChangedEvent()
{
if(RecycleBinChanged != null)
{
RecycleBinChanged.Invoke(null, null);
}
}
}
}
14 changes: 12 additions & 2 deletions src/Files.App/Helpers/UIHelpers.cs
View file
Open in desktop
Expand Up @@ -109,7 +109,7 @@ public static void CloseAllDialogs()

private static IEnumerable<IconFileInfo> IconResources = UIHelpers.LoadSidebarIconResources();

public static IconFileInfo GetIconResourceInfo(int index)
public static IconFileInfo? GetIconResourceInfo(int index)
{
var icons = UIHelpers.IconResources;
if (icons != null)
Expand All @@ -119,7 +119,7 @@ public static IconFileInfo GetIconResourceInfo(int index)
return null;
}

public static async Task<BitmapImage> GetIconResource(int index)
public static async Task<BitmapImage?> GetIconResource(int index)
{
var iconInfo = GetIconResourceInfo(index);
if (iconInfo != null)
Expand All @@ -134,6 +134,7 @@ private static IEnumerable<IconFileInfo> LoadSidebarIconResources()
string imageres = Path.Combine(CommonPaths.SystemRootPath, "System32", "imageres.dll");
var imageResList = Win32API.ExtractSelectedIconsFromDLL(imageres, new List<int>() {
Constants.ImageRes.RecycleBin,
Constants.ImageRes.EmptyRecycleBin,
Constants.ImageRes.NetworkDrives,
Constants.ImageRes.Libraries,
Constants.ImageRes.ThisPC,
Expand All @@ -143,5 +144,14 @@ private static IEnumerable<IconFileInfo> LoadSidebarIconResources()

return imageResList;
}

public static int GetAdaptedRecycleBinIconIndex()
{
if (new RecycleBinHelpers().RecycleBinHasItems())
d2dyno1 marked this conversation as resolved.
Show resolved Hide resolved
{
return Constants.ImageRes.RecycleBin;
}
return Constants.ImageRes.EmptyRecycleBin;
}
}
}
21 changes: 21 additions & 0 deletions src/Files.App/ViewModels/SidebarViewModel.cs
View file
Open in desktop
Expand Up @@ -236,6 +236,7 @@ public SidebarViewModel()
SideBarItems = new BulkConcurrentObservableCollection<INavigationControlItem>();
EmptyRecycleBinCommand = new RelayCommand<RoutedEventArgs>(EmptyRecycleBin);
UserSettingsService.OnSettingChangedEvent += UserSettingsService_OnSettingChangedEvent;
RecycleBinHelpers.RecycleBinChanged += OnRecycleBinChanged;
QuaintMako marked this conversation as resolved.
Show resolved Hide resolved
CreateItemHome();

Manager_DataChanged(SectionType.Favorites, new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Reset));
Expand Down Expand Up @@ -542,6 +543,26 @@ public async void EmptyRecycleBin(RoutedEventArgs e)
await RecycleBinHelpers.S_EmptyRecycleBin();
}

public async void OnRecycleBinChanged(object sender, EventArgs e)
{
var recycleBinLocationItem = SideBarItems
QuaintMako marked this conversation as resolved.
Show resolved Hide resolved
.OfType<LocationItem>()
.Where(x => x.Section == SectionType.Favorites)
.SelectMany(favoriteSection => favoriteSection.ChildItems)
.OfType<LocationItem>()
.FirstOrDefault(favoriteItem => favoriteItem.Path == CommonPaths.RecycleBinPath);

if(recycleBinLocationItem != null)
{
int recycleBinIconIndex = UIHelpers.GetAdaptedRecycleBinIconIndex();
recycleBinLocationItem.IconData = UIHelpers.GetIconResourceInfo(recycleBinIconIndex).IconData;
if (recycleBinLocationItem.IconData != null)
{
recycleBinLocationItem.Icon = await App.Window.DispatcherQueue.EnqueueAsync(() => recycleBinLocationItem.IconData.ToBitmapAsync());
}
}
}

private void UserSettingsService_OnSettingChangedEvent(object sender, SettingChangedEventArgs e)
{
switch (e.SettingName)
Expand Down