-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Add Delete functionality for Peek #35418
base: main
Are you sure you want to change the base?
Conversation
Removed package updates, as these are not relevant to the new functionality.
…rce text. Also altered the text style to be consistent with the FailedFallbackPreviewControl error page.
…into peek-delete
{ | ||
wFunc = FO_DELETE, | ||
pFrom = path + "\0\0", | ||
fFlags = (ushort)(FOF_NOCONFIRMATION | (permanent ? 0 : FOF_ALLOWUNDO)), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this override the recycle bin property? (It is possible to configure the delete confirmation in the recycle bin properties.)
How does Fotos app handle it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This replicates how the Photos application handles deletes. Pressing Delete immediately sends the file to the Recycle Bin without popping up a message, and then it moves on to the next file.
The permanent
argument is there for possible future use, but is always false for now.
@daverayment |
This reverts commit 3a10918.
This comment has been minimized.
This comment has been minimized.
Thanks for your comments and suggestions. I've updated the code with an error InfoBar, which is shown when a delete fails: I've added user-friendly messages for the most common failures. Others result in a generic message being shown on the InfoBar. The failure is also logged, with the code and a more technical error message (taken from If the file still exists after the deletion attempt, it is kept in the items collection for previewing; otherwise it is removed. An aside: Windows Photo handles files being renamed or deleted differently to Peek, in that it seems to constantly monitor the folder. (Try changing the filename of a photo you're previewing and you can see it update in the UI after a second or two.) We may wish to replicate this functionality in the future, as it would be increase robustness, but that it outside of the scope of this issue. |
Summary of the Pull Request
This PR adds the ability for users to delete the file currently being previewed in Peek.
PR Checklist
Detailed Description of the Pull Request / Additional comments
There are several new and changed elements in this PR:
Items
collection (which could be very large). There are nowDisplayIndex
andDisplayItemCount
properties, which reflect the calculated position and total. The oldCurrentIndex
still points to the actual index of the current item in the collection, but this is now internal only.Shift + Delete
could do a permanent delete, and this is certainly possible in the future, but this release errs on the side of caution, especially as it's a new feature and involves deleting things.NativeMethods.cs
for the API call, struct, flags, and error messages.Screenshots
No More Files message
File Count For Single Remaining File
Validation Steps Performed
(Manual tests only.)
Tested: