Add Delete functionality for Peek

[daverayment]

Summary of the Pull Request

This PR adds the ability for users to delete the file currently being previewed in Peek.

PR Checklist

• [x] Closes: #22584
• [x] Communication: I've discussed this with core contributors already. If work hasn't been agreed, this work might be rejected
• [ ] Tests: Added/updated and all pass
• [ ] Localization: All end user facing strings can be localized
• [ ] Dev docs: Added/updated
• [ ] New binaries: Added on the required places
  • [ ] JSON for signing for new binaries
  • [ ] WXS for installer for new binaries and localization folder
  • [ ] YML for CI pipeline for new test projects
  • [ ] YML for signed pipeline
• [x] Documentation updated: If checked, please file a pull request on our docs repo and link it here: #5168

Detailed Description of the Pull Request / Additional comments

There are several new and changed elements in this PR:

• Navigation has been updated to track forward and backward movement through files. This is necessary so deleting a file always shows the next item in the natural sequence, i.e. the previous item if the user was moving backwards. This replicates Windows Photo and other media viewers.
• The indexes of deleted items are recorded rather than altering the Items collection (which could be very large). There are now DisplayIndex and DisplayItemCount properties, which reflect the calculated position and total. The old CurrentIndex still points to the actual index of the current item in the collection, but this is now internal only.
• The MainWindow ViewModel includes quite a bit of navigation-related code. We may wish to split this out into a separate model or provider in the future.
• When previewing a selection of items (where the number and total are shown in the title bar), these counts will always be shown. Previously, this info would not show if the count was 1. (This was likely because opening Peek while a single item is selected expands the Items collection to cover the entire folder and turns off the "(position/total)" display.)
• A text message has been added for when there are no more files remaining, i.e. the user has deleted the last one from their selection (or from the current folder if Peek wasn't opened via multi-selection). It may be worthwhile to add a simple icon by the side here - perhaps an image placeholder graphic? - but I've gone with the simple approach for now.
• Changes were made to the title bar code to ensure that the last filename and "open in application" details were not shown when there are no more files to preview. This was because there was an (entirely reasonable!) assumption in the code that there would always be one or more files being previewed.
• The delete operation sends the item to the Recycle Bin only. It was considered whether pressing 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.
• The Delete function itself relies upon a shell file operation, reflecting the fact that we're previewing shell items. This required adding some code to NativeMethods.cs for the API call, struct, flags, and error messages.
• Only file deletion is allowed. I considered allowing for empty folder deletes, too, but, again, this first release is deliberately conservative.
• Updated Peek documentation.

Screenshots

No More Files message

File Count For Single Remaining File

Validation Steps Performed

(Manual tests only.)

Tested:

• Single and multi-selection methods of opening Peek.
• Deleting from beginning, end and middle of a sequence (after opening Peek via multi-selection).
• Deleting until no files remained.
• Previewing a folder containing a mix of files and folders.
• Previewing a folder containing a mix of files which can be previewed and unsupported file types.
• Attempting to delete folders via Peek.
• Deleting a file in the selected list via Explorer before it could be previewed.
• Deleting a file from an external USB drive which doesn't support the Recycle Bin, to confirm that the system confirmation dialog is displayed which warns against permanent file deletion.
• Cancelling a permanent delete, confirming the file is re-added to the current list.
• Changing the "Don't show this warning again" setting in the delete confirmation dialog updates the settings.
• Updating the "Ask for confirmation before deleting files" setting in the Settings app is reflected in Peek immediately, and vice-versa. (The Settings app requires navigating to another page then returning to the Peek settings.)

[htcfreek]

@daverayment Please fix the merge conflicts. (This is the reason for the failing test.)

[github-actions[bot]]

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view, the :scroll:action log, or :memo: job summary for details.

Unrecognized words (1)

NOCONFIRMATION

Previously acknowledged words that are now absent

applayout appsfolder buildtask cswinrt directshow DOPUS GBarm netcore nugets QDir SYSTEMSETTINGS SYSTEMWOW telem TOTALCMD USEPOSITION USESIZE winappdriver xplorer 🫥

To accept these unrecognized words as correct and remove the previously acknowledged and now absent words, you could run the following commands

... in a clone of the [email protected]:daverayment/PowerToys.git repository on the peek-delete branch (:information_source: how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.22/apply.pl' |
perl - 'https://github.com/microsoft/PowerToys/actions/runs/11335144030/attempts/1'

Available :books: dictionaries could cover words (expected and unrecognized) not in the :blue_book: dictionary

This includes both expected items (1931) from .github/actions/spell-check/expect.txt and unrecognized words (1)

Dictionary	Entries	Covers	Uniquely
cspell:r/src/r.txt	543	1	1
cspell:cpp/src/people.txt	23	1
cspell:cpp/src/ecosystem.txt	51	1

Consider adding them (in .github/workflows/spelling2.yml) for uses: check-spelling/[email protected] in its with:

      with:
        extra_dictionaries:
          cspell:r/src/r.txt
          cspell:cpp/src/people.txt
          cspell:cpp/src/ecosystem.txt

To stop checking additional dictionaries, add (in .github/workflows/spelling2.yml) for uses: check-spelling/[email protected] in its with:

check_extra_dictionaries: ''

Warnings (1)

See the :open_file_folder: files view, the :scroll:action log, or :memo: job summary for details.

:information_source: Warnings	Count
:information_source: non-alpha-in-dictionary	1

See :information_source: Event descriptions for more information.

If the flagged items are :exploding_head: false positives

If items relate to a ...

• binary file (or some other file you wouldn't want to check at all).

  Please add a file path to the excludes.txt file matching the containing file.

  File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

  ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

• well-formed pattern.

  If you can write a pattern that would match it, try adding it to the patterns.txt file.

  Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

  Note that patterns can't match multiline strings.

[daverayment]

@htcfreek

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 winerror.h).

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.

[Jay-o-Way]

Even though it ends up in the Recycling Bin, I think having a Message Box Prompt first would be safer.

Agree. Could also try to get/use the setting for the Recycle Bin itself.

[htcfreek]

Even though it ends up in the Recycling Bin, I think having a Message Box Prompt first would be safer.

Agree. Could also try to get/use the setting for the Recycle Bin itself.

Doesn't the code does this yet? Or better asked: Isn't this handled by Recycle Bin itself?

[Jay-o-Way]

Doesn't the code does this yet? Or better asked: Isn't this handled by Recycle Bin itself?

Nope, the code uses DeleteFile(item) -> FO_DELETE and that's an explicit action. I'd say the setting is an Explorer setting.

[daverayment]

@jaimecbernardo @htcfreek @Jay-o-Way

Thank you for the review and guidance. However, I disagree with displaying a prompt.

The default Windows functionality for file deletions is not to show a confirmation prompt. The whole purpose of the Recycle Bin is to to protect against accidental deletions. The behaviour in Peek was deliberately chosen to be consistent with both Windows Explorer and other file viewer tools such as the Windows Photo App and third party apps like IrfanView. I believe users would find a confirmation dialog unnecessary or redundant every time they want to delete a file, which could be a common action when sorting through a large folder of files. Also, PowerToys is supposed to be for power users, so I don't think we should go out of our way to provide guardrails for easily-reversible common mistakes such as accidentally pressing the delete key (which is also not near the default Peek navigation keys).

To summarise, I don't think we should provide non-default behaviour which would slow down a user's workflow.

[jaimecbernardo]

I've tried the Windows "Photos" app, it does show a confirmation prompt:

[jaimecbernardo]

I imagine you might've already clicked that "Don't show this warning again" @daverayment . What I'm proposing is having a prompt such as this one from the Windows "Photos" app. But also a setting to allow the prompt to not be shown, but I think the prompt should be shown by default.

[jaimecbernardo]

I've also just tried the feature on an external drive. As expected, files were permanently deleted without going to the recycle bin. I don't feel easy causing potential data loss with a feature that wasn't there before without some initial guardrails so the user understands that feature is there 😅 Hope this makes sense to you.

[daverayment]

Thanks, @jaimecbernardo

Sorry, yes, you're right! I must have clicked the "Don't Show" checkbox when I initially started using Windows Photo. Please forgive the lapse in memory - it was a long time ago... 😬

My main concern was that presenting dialogs every time a delete was requested would get in the way, but I'm fine with your suggestion of replicating the Photo app's behaviour with the "Don't Show" checkbox. This will give me a chance to have a play with the Settings code, too, which I've been avoiding.

I'm very concerned that the delete operation skipped the Recycle Bin for a file on your external device. Would you mind seeing what your Recycle Bin properties are for that drive, please? It is possible to turn off the Recycle Bin on a per-drive basis:

The Peek code just calls the underlying Shell API, so it should be consistent with doing a delete from Explorer (without holding Shift down); it is a bug if it behaves differently for external drives.

[Jay-o-Way]

@daverayment using a delete function in most code languages (like DeleteFile(item) -> FO_DELETE) is very explicit - it doesn't care about a recycling bin. Code languages often have a "FileRecycle" function too, but...

Sadly, C# doesn't have a native API to delete files and folders to Recycle Bin. But Visual Basic has.

https://www.c-sharpcorner.com/blogs/extension-methods-for-delete-files-and-folders-to-recycle-bin

Also check out

• https://stackoverflow.com/questions/3282418/send-a-file-to-the-recycle-bin
• https://www.google.com/search?q=FileRecycle+function+in+c%23&oq=FileRecycle+function+in+c%23

[jaimecbernardo]

I'm very concerned that the delete operation skipped the Recycle Bin for a file on your external device. Would you mind seeing what your Recycle Bin properties are for that drive, please? It is possible to turn off the Recycle Bin on a per-drive basis:

@daverayment There's nothing on Recycle Bin for this drive and another I've tested. I'm guessing because their file system is FAT32.

[jaimecbernardo]

Here's the last two Settings added to Peek as a reference. Thank you 😉 https://github.com/microsoft/PowerToys/pull/26308 https://github.com/microsoft/PowerToys/pull/26364

[htcfreek]

@daverayment , @jaimecbernardo For best user behavior the setting should be controllable from the dialogue in Peek (disable message) and the settings page (disable and enable) in PT Settings. Is this possible?

[daverayment]

@daverayment using a delete function in most code languages (like DeleteFile(item) -> FO_DELETE) is very explicit - it doesn't care about a recycling bin. Code languages often have a "FileRecycle" function too, but...

Sadly, C# doesn't have a native API to delete files and folders to Recycle Bin. But Visual Basic has.

Hi, @Jay-o-Way !

Thanks for the message. It is certainly a shame that System.IO doesn't include recycling options when deleting files.

As you noticed, this update to Peek uses the Shell File Operation FO_DELETE. This is also used internally by Microsoft.VisualBasic.FileIO.FileSystem.DeleteFile() (see the VB Core library source here). FO_DELETE does support a flag to control whether it attempts to move the file to the Recycle Bin (FOF_ALLOWUNDO) and various others to control which progress and error messages are shown.

This isn't enough for our needs, though, so I will add the dialog box to prompt the user when they try to delete a file, as previously discussed. Giving users the option to permanently dismiss this warning replicates what the Windows Photo App does, and is a good compromise for power users.

The reason @jaimecbernardo experienced the issue with the FAT32 volume is because I needed to include a flag called FOF_WANTNUKEWARNING, which is very well named! I will correct that in the PR. Thank you for the catch, Jaime!

The VB Core library containing DeleteFile() is 1.2 MB, so I'm keen to just call the Shell API directly to reduce the potential bloat.

[daverayment]

@daverayment , @jaimecbernardo For best user behavior the setting should be controllable from the dialogue in Peek (disable message) and the settings page (disable and enable) in PT Settings. Is this possible?

@htcfreek

Yes, that's what I assumed we'd be doing. I haven't used the Settings system before, but I'll give it a go and I'll ask if I get stuck!

[htcfreek]

@daverayment Are you working on the outstanding tasks (delete message with setting, using recycle bin, fat32 bug)?

/needinfo

[daverayment]

@htcfreek Yes, I'm still working on this. Sorry about the delay.

[htcfreek]

@daverayment No problem. Only wondering about the 0.88 lable. I removed it now.

[github-actions[bot]]

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view, the :scroll:action log, or :memo: job summary for details.

Unrecognized words (3)

shellapi WANTNUKEWARNING winerror

These words are not needed and should be removed

AMPROPERTY AMPROPSETID Breadcrumb CDEF comdef ddf devenum DEVMON DEVSOURCE DGR DIIRFLAG dshow DVH DVHD DVSD DVSL EData ERole fdw FILEINFOSIG Filtergraph Filterx HCERTSTORE IKs iljxck IYUV KSPROPERTY lcb ldx lld LONGLONG LTRB majortype makecab MEDIASUBTYPE mediatype mfplat mic mjpg Msimg msiquery ORAW outpin overlaywindow PAUDIO PINDIR Pnp ppmt previouscamera PROPBAG propvarutil reencode reencoded REFGUID REGFILTER REGFILTERPINS REGPINTYPES regsvr shmem sizeread stl strsafe strutil subquery SYNCMFT TMPVAR vcdl vdi vid VIDCAP VIDEOINFOHEADER vih webcam wistd WVC

To accept these unrecognized words as correct and remove the previously acknowledged and now absent words, you could run the following commands

... in a clone of the [email protected]:daverayment/PowerToys.git repository on the peek-delete branch (:information_source: how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.24/apply.pl' |
perl - 'https://github.com/microsoft/PowerToys/actions/runs/12981752301/attempts/1'

If the flagged items are :exploding_head: false positives

If items relate to a ...

• binary file (or some other file you wouldn't want to check at all).

  Please add a file path to the excludes.txt file matching the containing file.

  File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

  ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

• well-formed pattern.

  If you can write a pattern that would match it, try adding it to the patterns.txt file.

  Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

  Note that patterns can't match multiline strings.

[daverayment]

Update: Please note that I've since simplified this dialog, and removed the workaround, as I'm unsure whether it is actually a WinUI issue or an intentional inconsistency.

I've updated the PR to include the Delete confirmation dialog and the setting to control whether it is shown or not.

The dialog looks like this:

And the new setting:

I had an issue during a rebase, so I apologise for the force-push.

A note about one of the implementation details: there is an issue with the focus rectangle being shown on ContentDialogs when they are shown in response to a keyboard input, but not a button press or other click/select event. This affects us here because we show the confirmation dialog in response to Delete being pressed. For a workaround, I settled on zeroing the focus thickness before showing the dialog and restoring it after the user begins interacting with the controls via the keyboard. This is described in more detail here: https://github.com/microsoft/microsoft-ui-xaml/issues/10315. If someone finds a better fix or objects to this approach, please let me know.

I've updated the Peek documentation to describe the new functionality, the dialog and the settings.

[github-actions[bot]]

@check-spelling-bot Report

:red_circle: Please review

See the :open_file_folder: files view, the :scroll:action log, or :memo: job summary for details.

:x: Errors	Count
:x: forbidden-pattern	1
:warning: non-alpha-in-dictionary	2

See :x: Event descriptions for more information.

These words are not needed and should be removed

AMPROPERTY AMPROPSETID Breadcrumb CDEF comdef ddf devenum DEVMON DEVSOURCE DGR DIIRFLAG dshow DVH DVHD DVSD DVSL EData ERole fdw FILEINFOSIG Filtergraph Filterx HCERTSTORE IKs iljxck IYUV KSPROPERTY lcb ldx lld LONGLONG LTRB majortype makecab MEDIASUBTYPE mediatype mfplat mic mjpg Msimg msiquery ORAW outpin overlaywindow PAUDIO PINDIR Pnp ppmt previouscamera PROPBAG propvarutil reencode reencoded REFGUID REGFILTER REGFILTERPINS REGPINTYPES regsvr shmem sizeread stl strsafe strutil subquery SYNCMFT TMPVAR vcdl vdi vid VIDCAP VIDEOINFOHEADER vih webcam wistd WVC

If the flagged items are :exploding_head: false positives

If items relate to a ...

• binary file (or some other file you wouldn't want to check at all).

  Please add a file path to the excludes.txt file matching the containing file.

  File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

  ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

• well-formed pattern.

  If you can write a pattern that would match it, try adding it to the patterns.txt file.

  Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

  Note that patterns can't match multiline strings.

[daverayment]

Update: the bug is now fixed. Please disregard.

In further testing I have found a couple of issues with the new dialog, so please don't merge for the time being. The problem can be replicated by following these steps:

1. Open an image for preview in Peek
2. Press Delete. This shows the new confirmation dialog
3. Close down the main Peek window while the delete confirmation dialog is still being displayed
4. Open Peek again, but for a different image

The confirmation dialog is still displayed, but now refers to the new image.

This is because the dialog is not removed when Peek's MainWindow is hidden, and is bound to the current item being displayed. This would lead to user confusion, so I will fix and re-test.

[daverayment]

I have updated the delete confirmation dialog to be simpler:

There was no need to include the filename, as it is obvious which file is current.

This revision includes changes so the system permanent deletion warnings are correctly shown:

(The flag FOF_NO_CONFIRMATION must not be included for this to work, even though the documentation suggests otherwise.)

I have removed the workaround for the focus issue on the delete confirmation dialog because:

• The Windows Photo app shows the same inconsistency
• It may not be a bug, but a deliberate WinUI design decision when the keyboard is being used to initiate an action. I shouldn't include a workaround until the issue is confirmed

[drawbyperpetual]

Any reason not to include an explicit button for this action? The Windows Photos app has one and the other Peek action that one can perform with files (open in default app) also has one. Another reason to include an explicit button is that the Delete key does not seem to work well with text files because of this issue

[daverayment]

Any reason not to include an explicit button for this action? The Windows Photos app has one and the other Peek action that one can perform with files (open in default app) also has one. Another reason to include an explicit button is that the Delete key does not seem to work well with text files because of this issue

Thanks for your review, @drawbyperpetual 😊

The intention was to keep the UI minimal, similar to why there are no icons for basic navigation. I also wasn't keen on having a destructive action button in the title bar. However, I do realise that's personal preference.

With regard to the WebView-related focus, the current behaviour is a good thing with regard to deletes. There is a difference in user expectation between pressing escape (should exit the viewer) and pressing delete while viewing a text file. When viewing text, Peek gives the user a caret and allows them to select text, so it is reasonable to assume that some of them will try to edit the file contents, including pressing delete to remove a paragraph or whatever. These delete keystrokes shouldn't bubble up to the KeyUp handler.

In any case, I'm happy to add the button or wait to see if there is demand for it from the community after the functionality is released. Please let me know.

[drawbyperpetual]

Any reason not to include an explicit button for this action? The Windows Photos app has one and the other Peek action that one can perform with files (open in default app) also has one. Another reason to include an explicit button is that the Delete key does not seem to work well with text files because of this issue

Thanks for your review, @drawbyperpetual 😊

The intention was to keep the UI minimal, similar to why there are no icons for basic navigation. I also wasn't keen on having a destructive action button in the title bar. However, I do realise that's personal preference.

With regard to the WebView-related focus, the current behaviour is a good thing with regard to deletes. There is a difference in user expectation between pressing escape (should exit the viewer) and pressing delete while viewing a text file. When viewing text, Peek gives the user a caret and allows them to select text, so it is reasonable to assume that some of them will try to edit the file contents, including pressing delete to remove a paragraph or whatever. These delete keystrokes shouldn't bubble up to the KeyUp handler.

In any case, I'm happy to add the button or wait to see if there is demand for it from the community after the functionality is released. Please let me know.

Sure; it can always be added later.

[jaimecbernardo]

/azp run

[azure-pipelines[bot]]

Azure Pipelines successfully started running 1 pipeline(s).