Ask for confirmation before deleting a wallpaper.
Review Request #123211 - Created April 1, 2015 and submitted
This patch is adding a confirmation dialog which is being called before we remove a wallpaper.
the problem in using a framesvgitem mixed with QtControls is that while with the default themes is ok (and i think it looks great/prefer how it looks inline this way over a separate dialog window) you would risk to have things like black text on black background with different plasma themes or desktop color schemes.
The complex part of the wallpapers is in some cases it actually removes the file, sometimes it doesn't depending on whether we got it from GHNS or the user just selected a file.
Do we want the dialog every time when it's a local file and no deleting actually occurs? I'm not sure either way.
I certainly think we should avoid saying "delete" in the message when we're not deleting the file.
I don't think the wallpaper dialog should ever delete an actual image file, especially not when it's in the user's Pictures folder. Only exception is when it has been installed through GHNS in which case it is in some hidden dot folder.
As for the confirmation, I think we should rather provide an undo functionality so you don't accidentally remove the wallpaper from the list and then have to search it on the filesystem again.
I agree with Kai: Asking for confirmation in this case is not the approach we should take.
What this definitely calls for is an undo function, which should certainly not be impossible to do.
Asking for confirmation should only be the last resort if undoing an action is simply not possible (e.g. when deleting a file permanently). A confirmation adds another step to the 99% of actions that were intentional, only to protect from the 1% accidents.
Undo, on the other hand, doesn't bother users in 99% of the cases while helping them in the 1%.
Yes, an undo feature may sound overblown in this case, but we must get into the habit of moving away from confirmation dialogs and towards undo.
An undo feature can be done as following:
the wallpaper model would have a role like "pendingDeletion" that would be set by the remove button on the thumbnail. At that point the thumbnail can show an undo button based on the role.
Upon apply or ok, a "commitDeletion" method would be called on the model instance. this would go trough all the items and do the removeWallpaper on the ones that have the PendingDeletion role set.
Cancel would not delete anything.
Antonis, would you feel giving a try on it?
This patch is implementing an undo feature for the deleted wallpapers.
Revision 2 (+93 -7)
I think it's on the right track, biggest complain is the neaming of the methods so far
It's great that you've implemented the undo function, awesome!
What I cannot really tell from the screenshots is how a wallpaper which is marked to be removed is visualized. Can you maybe add a screenshot where the distinction between those which are marked for removal and those which are not becomes clear?
Status: Closed (submitted)
Submitted with commit 50942e2618f3ed26f8eac8f264656617bb113ec6 by Antonis Tsiapaliokas to branch master.