Bug 126874

Summary: digikam does not support <shift+del>
Product: [Applications] digikam Reporter: Thorsten Schnebeck <thorsten.schnebeck>
Component: Usability-KeyboardAssignee: Digikam Developers <digikam-bugs-null>
Status: RESOLVED FIXED    
Severity: wishlist CC: ach, aironmail, caulier.gilles
Priority: NOR    
Version: 0.9.0   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In: 0.9.0
Sentry Crash Report:
Attachments: Suggestion for trash bin handling
Next version of the patch
Dialog with one image selected
Deleting albums
Current version of the patch
Corrected version
Corrected version
Current version

Description Thorsten Schnebeck 2006-05-07 01:44:03 UTC
Version:           0.9.0-svn (using KDE 3.5.2, compiled sources)
Compiler:          gcc version 3.4.5 (Gentoo 3.4.5-r1, ssp-3.4.5-1.0, pie-8.7.9)
OS:                Linux (i686) release 2.6.16-gentoo-r6

Standard KDE behavior is
<del> moves a file into the trashbin
<shift+del> deletes a file

No need for a visual command in the edit menu but as a keyboard command it would be nice add-on in digiKams albumview.

Bye

  Thorsten
Comment 1 Marcel Wiesweg 2006-06-03 23:38:07 UTC
Look at this:
http://amarok.kde.org/blog/archives/90-Concluding-the-Delete-Controversy.html
I guess with shift+del the checkbox at the bottom will be checked.
Comment 2 Thorsten Schnebeck 2006-06-04 00:54:18 UTC
I am completely for supporting "delete into trash-folder" in a GUI. But there should be a shortcut for really deleting. As I said in my wishlist: No need for a visual command only the shortcut for power users.
Comment 3 Marcel Wiesweg 2006-06-04 17:55:10 UTC
Yes, but there wont be a shortcut without GUI, its too dangerous.
If we change this and use the above GUI idea, which I like quite much (and it seems not only digikam has had a delete controversy) I'd propose

- Del shows the dialog without checkbox checked - currently, nothing
- Shift+Del shows the dialog with checkbox checked - currently, move to trash (or delete file, depending on setup option I think)
- context menu as today, not daring to touch the setup option - or remove the option.

As you point out, the current behavior differs from KDE standard, thats probably the most important point here.
Comment 4 Achim Bohnet 2006-06-13 13:57:29 UTC
Yeah, standard behaviour is very important IMO.
aka <del> move to trash, shift-<del> delete.

I don't care that much if a dialog pops up AS
LANG AS there is an option 'never show this again.
(and somwhere an option reset them so all dialogs are
shown again)

Achim
Comment 5 Marcel Wiesweg 2006-07-12 16:02:22 UTC
Created attachment 16967 [details]
Suggestion for trash bin handling

The attached patch contains the proposed delete dialog, boldly copied from
Amarok, (c) Ian Monroe and Micheal Pyne.
It's not complete, deleting an album from AlbumFolderView is missing, the
delete action in the main app and IE menus is missing, and the keyboard
shortcuts are missing.

The idea is the the use-trash setup option will go away, but the delete dialog
stores the user's preference.

Once again, I think that we won't have any sort of delete action without a
confirmation dialog. But that's a difficult issue, don't know what KDE
guidelines say about this - Konqueror e.g. has the don't ask me again checkbox.
Comment 6 Achim Bohnet 2006-07-12 18:19:47 UTC
Hi Marcel,

can you attach a screen shot.

> Marcel wrote:
>Once again, I think that we won't have any sort of delete action without a 
> confirmation dialog.

Well, uhm, argl! ;) As a compromisse, a delete-with-out-bothering-me-with-dialog action should be available so one can bind shift-del to it if one wants it.  I'm no fan of either always delete or always move to trash.

OSS is about freedom, and I and maybe other want the freedom to get rid 'sometimes' of some pics quickly.  If the binding is on by default
is a topic for a future bug report ;)

Achim
Comment 7 Marcel Wiesweg 2006-07-12 19:17:44 UTC
The link from #1 contains a screenshot. Currently it's exactly the same.

I like the idea of a hidden action, so power users can enable this. It must be documented somewhere. I am always favorable of allowing such choice, but I really think it won't be the default, it's so dangerous, some inaadvertent key press and your images are gone forever.
Comment 8 Marcel Wiesweg 2006-07-15 23:42:48 UTC
Created attachment 16998 [details]
Next version of the patch

Here is the next version of the patch. It adds the suggested, dangerous (and
thus hidden by default) actions which can be assigned shortcuts by power users.
Del and Shift+Del are supported. Album deletion is still not integrated in the
DeleteDialog
Comment 9 Marcel Wiesweg 2006-08-03 23:22:16 UTC
Created attachment 17215 [details]
Dialog with one image selected

Screenshot of the new delete dialog, one image was selected, <Del> was pressed
Comment 10 Marcel Wiesweg 2006-08-03 23:25:19 UTC
Created attachment 17216 [details]
Deleting albums

Screenshot of the new delete dialog.
Right-click on the albums folder view, Delete Album, the checkbox was checked
to delete permanently.

I can provide a patch if someone wants to test this.
But, unless I receive negative comments, I am going to commit this. Any
opinions?
The Delete Dialog is now used in main view (for albums and images) and in IE.
For images, hidden actions to delete without confirmation are available in IE
and main view.
Comment 11 Achim Bohnet 2006-08-04 02:12:16 UTC
Looks fine.  I'm already curious to try it out in -beta2

Thx a lot Marcel,
Achim
Comment 12 Thorsten Schnebeck 2006-08-05 00:39:32 UTC
I would like to test a patch this weekend if you can provide one. The older one can not be applied to svn-trunk without errors. I hope, it still provides a method to configure a simple delete shortcut without anoying GUI warnings for the "power users"(?)
I think this combined mv to trash/delete GUI is confusing for the inexperienced user. I would vote for a move to trash as a save default without offering a delete option.

Thanks for your work

  Thorsten
Comment 13 Marcel Wiesweg 2006-08-05 14:44:32 UTC
Created attachment 17235 [details]
Current version of the patch

Here is a patch from the current version.

You can configure your own shortcuts for delete-without-confirmation using the
standard KDE shortcut configuration. These actions have no menu entry, toolbar
entry, shortcut assigned by default.
Default is <Del> -> Show dialog
For those who are used to <Shift+Del>, this shows the dialog with the checkbox
checked.
For those who are not used to shortcuts, and that's the main point of this idea
(Ian Monroe's), the dialog will save the user's preference. That's the
equivalent for the UseTrash setup entry.
Comment 14 Thorsten Schnebeck 2006-08-06 16:29:02 UTC
Hmm, seems like uic does not generate header files:
cat del.patch | patch -p0 && cd .. && ./configure --prefix=/usr/kde/3.5 && cd digikam && make 
In file included from imagewindow.cpp:91:
../../../../digikam/digikam/deletedialog.h:24:30: deletedialogbase.h: No such file or directory
In file included from imagewindow.cpp:91:
../../../../digikam/digikam/deletedialog.h:53: error: expected class-name before '{' token
../../../../digikam/digikam/deletedialog.h: In member function `bool Digikam::DeleteDialog::shouldDelete() const':
../../../../digikam/digikam/deletedialog.h:92: error: 'class Digikam::DeleteWidget' has no member named 'ddShouldDelete'
make[3]: *** [imagewindow.lo] Fehler 1
make[3]: Leaving directory `/usr/local/src/kde-svn/extragear/graphics/digikam/utilities/imageeditor/editor'

Bye

  Thorsten
Comment 15 Marcel Wiesweg 2006-08-06 23:14:16 UTC
Created attachment 17250 [details]
Corrected version

You are right. I moved it to libs/dialogs, thats more appropriate anyway.
Comment 16 Thorsten Schnebeck 2006-08-07 07:56:04 UTC
Hi Marcel,

I have still problems:

albumfolderview.cpp:59:26: deletedialog.h: No such file or directory
albumfolderview.cpp: In member function `void Digikam::AlbumFolderView::albumDelete(Digikam::AlbumFolderViewItem*)':
albumfolderview.cpp:596: error: `DeleteDialog' was not declared in this scope
albumfolderview.cpp:596: error: expected `;' before "dialog"
albumfolderview.cpp:599: error: `dialog' was not declared in this scope
albumfolderview.cpp:601: error: `DeleteDialogMode' has not been declared
albumfolderview.cpp:601: error: `Albums' was not declared in this scope
albumfolderview.cpp:601: error: `DeleteDialogMode' has not been declared
albumfolderview.cpp:601: error: `Subalbums' was not declared in this scope
albumfolderview.cpp:599: warning: unused variable 'dialog'
albumfolderview.cpp:601: warning: unused variable 'Albums'
albumfolderview.cpp:601: warning: unused variable 'Subalbums'
albumfolderview.cpp:604: error: `dialog' was not declared in this scope
albumfolderview.cpp:596: warning: unused variable 'DeleteDialog'
albumfolderview.cpp:604: warning: unused variable 'dialog'
make[1]: *** [albumfolderview.lo] Fehler 1
make[1]: Leaving directory `/usr/local/src/kde-svn/extragear/graphics/digikam/digikam'

Bye

  Thorsten
Comment 17 Marcel Wiesweg 2006-08-07 23:14:20 UTC
Created attachment 17277 [details]
Corrected version

Ok, next try. This time I did a make clean all to test that it compiles!
Comment 18 Thorsten Schnebeck 2006-08-08 00:35:08 UTC
Hmm, I have still the same error(?)

I deleted my extragear/graphics/digikam folder,
made svn-update to recreate a fresh digikam,
applied the patch (md5sum 8e2f454068de025c0577e0d38e62229e  del.patch)
make -f Makefile.cvs
./configure
cd digikam
make
...
error

Bye

 Thorsten
Comment 19 Marcel Wiesweg 2006-08-08 16:18:54 UTC
That is strange. I did a fresh complete checkout, applied the patch, and it works.

The part to fix the compilation problem is:

svn diff digikam/digikam/Makefile.am

Index: digikam/digikam/Makefile.am
===================================================================
--- digikam/digikam/Makefile.am (revision 571069)
+++ digikam/digikam/Makefile.am (working copy)
@@ -14,6 +14,7 @@
           -I$(top_srcdir)/digikam/libs/dmetadata \
           -I$(top_srcdir)/digikam/libs/imageproperties \
           -I$(top_srcdir)/digikam/libs/threadimageio \
+          -I$(top_srcdir)/digikam/libs/dialogs \
           -I$(top_srcdir)/digikam/utilities/cameragui \
           -I$(top_srcdir)/digikam/utilities/imageeditor/editor \
           -I$(top_srcdir)/digikam/utilities/imageeditor/canvas \
Comment 20 Thorsten Schnebeck 2006-08-13 00:29:06 UTC
(new weekend - new time for some tests)

so, I was able to apply the patch, which BTW in the meantime with one rej out of sync to SVN-trunk.
The patch gives me the same dialog for "move to trash" and "delete". There is one option allowing to switch visually between both modes with a simple mouse click.

So now, how do I comment this patch... I want to remark that I help some users that are "real users only". They dont know anything deeper about computer but they know how to work with their apps. I stipped down all advanced features and till now they like to use digikam for managing their photos. OTOH there are some power users with advanced knowledge about KDE like myself.

Of course its harder to maintain systems of the first group as things have to be easy. But this patch does not make things easier! 

Everytime a window appears based on user action of unexperienced users they are afraid that they have made a mistage. Believe me, if this type of user deletes a file they really think about this action as "delete" means "its away". If this is done by mistake its normally a mouse handling error and they are happy when I magically recover the file from the trash.

We all know this kind of "warning windows" that are anyoing and noone reads it and we click "next" or "ok". A warning window with and option I think is worse.

So I suggest to add an "dont show this window again" option and leave the "del"-action as "move to trash" by default. There should be no way to make a "del" to a "shift+del"-action like the option in the dialog allows now. "shift+del" should be an advanced feature you have to know about. Having huge harddisks today there sould be no reason to make make deleting more prominent.

(I really, really suggest to ask KDEs usabillity group for their opinion for such a basic operation) 

My 2Ct

  Thorsten
Comment 21 Marcel Wiesweg 2006-08-15 15:47:25 UTC
My opinion on delete-without-dialog can be found in comment #3.
We use dialogs everywhere, so a fear of dialogs is not good. (and nothing I will orient my coding at)

I will definitely wait for Gilles' opinion on this.
I did not find any relevant HIG guidelines or relevant discussions on usability mailinglists.
Comment 22 caulier.gilles 2006-08-17 02:49:08 UTC
Marcel, 

The screenshots sound good for me. I will try your patch and give an advanced feedback when it possible.

Gilles
Comment 23 Marcel Wiesweg 2006-08-17 23:08:38 UTC
Created attachment 17406 [details]
Current version

Same code as before, but is tested to apply cleanly.
(One thing I noted missing: set focus to buttons in DeleteDialog)
Comment 24 Marcel Wiesweg 2006-08-20 22:55:54 UTC
*** Bug 132691 has been marked as a duplicate of this bug. ***
Comment 25 Diego Moya 2006-08-21 08:18:13 UTC
There's some further discussion at the usability list:
http://lists.kde.org/?l=kde-usability&m=115581518925528&w=2
Comment 26 Antonio E. 2006-08-21 14:13:47 UTC
I've tested the patch. In my opinion now it works much better. Now digikam behaves in the way that I expect from an application when trying to remove files, preventing to remove files from the disk by user errors. I just wanted give some feedback.

I'm just curious about one thing. In the editor, I can use Del or Shift+Del to delete a file. In the main interface (the album browser,...) I only can use Shift+Del. After applying the patch, "Del only" has no effect. I don't know if this is intended.
Comment 27 Marcel Wiesweg 2006-08-21 22:19:04 UTC
To sum up my current state:

- <Del> brings up the trash dialog with a "Do not show this again"
- <Shift+Del> brings up the permanently-delete dialog without an option
- we can keep the hidden action for permanently-delete without confirmation
- remove the current setup option whether to trash or permanently delete

Problems:
1) The above is for single images. What about albums? We only have the context menu in the sidebar, no shortcut. Use the dialog as it is currently for this, with the option to switch to delete permanently?
2) Add a setup option to make the dialog reachable again when "Do not show this again" has been checked?
Comment 28 Marcel Wiesweg 2006-08-25 15:20:20 UTC
SVN commit 577066 by mwiesweg:

File Deletion handling

After long discussion (#126874) and input from KDE usability team:
- <Del> brings up the trash dialog with a "Do not ask again" 
- <Shift+Del> brings up the permanently-delete dialog without an option 
- there are hidden actions for delete without confirmation to which power users can assign shortcuts
- the current setup option whether to trash or permanently delete is removed from the setup
  - it is kept internally and used for album deletion, see below
- add a new setup option which is identical to the "Do not ask again" option is the trash
  dialog, so that this setting is reversible
  - added to AlbumSettings as well
- in the AlbumFolderView, if deleting a whole album,
  only the context menu can be used to select a delete operation - Shift+Del is reserved for images.
  Here, the user can choose in the dialog whether to trash or delete permanently. This setting is stored.
  No "Do not ask again" will be available.
  - all subalbums will be included in the list in the DeleteDialog to make the user aware that
    he deletes them as well
- fix a bug in ImageWindow slotDeleteCurrent with directories that contain a "#"
- The DeleteDialog has now five modes of operation, see above. The checkboxes
  are in a widget stack, all the functionality that was there is still there for future needs.

BUG: 126874
CCMAIL: digikam-devel@kde.org
 



 M  +1 -0      digikam/Makefile.am  
 M  +34 -57    digikam/albumfolderview.cpp  
 M  +2 -0      digikam/albumfolderview.h  
 M  +38 -30    digikam/albumiconview.cpp  
 M  +8 -7      digikam/albumiconview.h  
 M  +14 -0     digikam/albumsettings.cpp  
 M  +3 -0      digikam/albumsettings.h  
 M  +37 -29    digikam/digikamapp.cpp  
 M  +3 -1      digikam/digikamapp.h  
 M  +16 -1     digikam/digikamview.cpp  
 M  +3 -0      digikam/digikamview.h  
 M  +5 -5      digikam/dio.cpp  
 M  +2 -2      digikam/dio.h  
 M  +5 -15     digikam/syncjob.cpp  
 M  +2 -11     digikam/syncjob.h  
 M  +1 -1      libs/dialogs/Makefile.am  
 M  +1 -0      utilities/imageeditor/editor/Makefile.am  
 M  +1 -1      utilities/imageeditor/editor/editorwindow.cpp  
 M  +75 -25    utilities/imageeditor/editor/imagewindow.cpp  
 M  +9 -0      utilities/imageeditor/editor/imagewindow.h