Add confirmation dialog before deleting downloaded album

[rom4nik]

trafficstars

Fixes #165.

For some reason the delete button isn't replaced with download button until dialog is shown again, haven't been able to figure out why. Maybe a race condition between _downloadsHelper.deleteDownloads and _downloadsHelper.isAlbumDownloaded called a little bit too early?

https://user-images.githubusercontent.com/46846000/157121981-8539751a-874e-4f79-822c-0b915f416548.mp4

[jmshrv]

I'm pretty sure the method I used to switch between the download/delete button was very jank anyway, I'll have a look and see how I did it :)

[rom4nik]

I found a workaround - .whenComplete in DownloadButton runs after confirmation dialog disappears, so if you delay hiding dialog until all downloads get deleted, the button will change icon to trash can as expected. However, dialog staying on screen might be annoying if you want to delete an album with many songs.

[jmshrv]

Just realised a better way of doing this - the delete dialog could just return a bool specifying whether or not the delete was confirmed. Deleting downloads should be fast enough that someone can't go and start the download before the delete is done, but if we want to be super safe we could have the dialog return the deleteDownloads future and the download button could disable itself until that future is complete.

[Chaphasilor]

Interestingly I was able to push commits to this PR 😁
I updated the dialog logic, the dialog now accepts two callbacks, onConfirmed and onAborted, that can be used to perform an action based on which button was pressed. Also rebased it so that it can be merged.

Now I'd like to:

• [x] Make the dialog more generic, to support artists, playlists, etc.
• [x] Add proper localization for the used strings
• [x] Use the dialog on the "Downloads" screen as well

Sounds good?

[Chaphasilor]

This should be ready for prime time now. @rom4nik if you don't mind, please take another look at my changes and check if there's anything I've missed!

[Chaphasilor]

I just noticed that I completely glossed over the trash icon not changing back to the download icon after deletion. So I guess I'll take another look at this tomorrow...

[Chaphasilor]

Okay, should be fine now :)

[rom4nik]

Looks good to me, thanks for fixing up this PR!

Some things I've noticed:

1. There's no similar dialog when I'm removing album songs one by one on the Downloads screen, I'm not sure if that's intended.

2. No confirmation when I press the delete on artist view.
3. Delete button in artist view keeps the trashcan icon after deleting songs.
4. Same issue as 3 and 4 in genre view.

I'm not sure if 2-4 are in scope of this PR, just FYI.

[Chaphasilor]

1. Haha I didn't even know you could expand the items to show individual tracks 🙃
   I'll add the confirmation later, including the additional translation strings.

And yes, I considered 2-4 to be out-of-scope. I'll check how much work it would be to add this as well, I only noticed that the artist screen uses some different logic already...

[Chaphasilor]

Artists and genres used the same code, so I added the dialog to both. That means 1-4 should be fixed now.
Please confirm, then we can merge this :D

[rom4nik]

Thanks, spotted just two more issues:

1. Genre delete dialog uses text from artist dialog :sweat_smile:
2. At least on my device the confirmation button's text is wrapped into second line - wouldn't it be sufficient to just have Delete on the button, since the dialog text already says that it'll remove downloaded song/album/artist/playlist?

[Chaphasilor]

Good catch! About the wrapping text, yeah I struggled with this a bit but wanted to emphasize that the genre is not being deleted on the server. How would you feel about "Delete files?"

[Chaphasilor]

I'm going to merge this once I get home. If there's anything we missed we can fix it in another PR :)