Feature: Recycle Bin changing icons in the side bar #10113
Conversation
…thub.com/QuaintMako/Files into 9195_DifferentIconSideBarRecycleBinState
This reverts commit 5f7e8ef.
yaira2
changed the title
| @@ -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(); | |||
There was a problem hiding this comment.
You can use null check
RecycleBinChanged?.Invoke(null, null);There was a problem hiding this comment.
Changed inside the function that handles the event raising. But would you rather have the event raised directly without any method maybe?
There was a problem hiding this comment.
Yes. There's no need for additional method to raise this event
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I think that moving it is better and good
There was a problem hiding this comment.
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.
|
@QuaintMako Could you resolve conflicts before it's merged? |
|
Marking that PR as a draft for now, I won't be able to take care of it for a few days. |
|
Would it be possible to get a |
|
After discussions with Yair on Discord, I'm closing this PR for now. Waiting for the big refactoring coming up soon, which should solve the event issue. |
Resolved / Related Issues
Items resolved / related issues by this PR.
What have been done
src/Files.App/DataModels/SidebarPinnedModel.cshas been done to the functionAddItemToSidebarAsync.Validation
How did you test these changes?
Screenshots (optional)
When the recycle bin is empty:
When the recycle bin is not empty:
