<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/123211/">https://git.reviewboard.kde.org/r/123211/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 1st, 2015, 4:44 nachm. UTC, <b>Marco Martin</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Antonis, would you feel giving a try on it?</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Interesting approach! All in all, I see three possible approaches here:
1. The one you described.
Advantage: Users can make any modification to their decisions before they are committed
Disadvantage: If only one wallpaper is removed and nothing else is changed, it still adds another action which would otherwise not be needed
2. An undo function that only applies to the most recently removed wallpaper, so really just an "Oops, I didn't mean to do that, Ctrl-Z!" kind of thing, similar to the Plasmoid removal undo. As soon as another one is removed or the dialog is closed, it would not be possible to undo it anymore
Advantage: Adds no additional action in case one just removes one wallpaper (not by accident) and does nothing else
Disadvantage: Users could still lose their wallpapers if they realize their mistake too late
3. Establishing a Trash for wallpapers which contains both the file and the entry in the "database" of wallpapers, which stays until actively emptied by the user
Advantage: Adds no further action and it's possible to delete multiple wallpapers in a row and restore them all
Disadvantage: Probably the most complex one implementation wise, plus would need a way to show and empty the trash</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">In the end, we should look at this in the context of other areas where undo functionality makes sense and choose the approach that can be applied (at least in a similar way from a user perspective) to the broadest range of cases. The problem this solves is not important enough to create a solution just for this situation alone. It's only worth doing if it can be a model for similar situations (and there are plenty of them)</p></pre>
<br />
<p>- Thomas</p>
<br />
<p>On April 1st, 2015, 3:27 nachm. UTC, Antonis Tsiapaliokas wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Antonis Tsiapaliokas.</div>
<p style="color: grey;"><i>Updated April 1, 2015, 3:27 nachm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=338729">338729</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
plasma-workspace
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch is adding a confirmation dialog which is being called before we remove a wallpaper.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>wallpapers/image/imagepackage/contents/ui/WallpaperDelegate.qml <span style="color: grey">(aee2d3f)</span></li>
<li>wallpapers/image/imagepackage/contents/ui/config.qml <span style="color: grey">(2108082)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123211/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/04/01/5bfd2d7c-8baa-4b80-ad20-0844aafdb3a9__deletion_dialog.png">dialog</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>