Bug 140227 - remove or modify D&D tags menu from icon view area
Summary: remove or modify D&D tags menu from icon view area
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Usability-Menus (show other bugs)
Version: unspecified
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-01-18 08:05 UTC by Arnd Baecker
Modified: 2017-08-02 21:08 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 0.9.1


Attachments
re-order menu-entries (1.11 KB, patch)
2007-02-27 09:40 UTC, Arnd Baecker
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arnd Baecker 2007-01-18 08:05:23 UTC
Version:           0.9.1-svn (using KDE KDE 3.5.5)
Installed from:    Debian testing/unstable Packages

This summarizes a discussion on the mailing list
on the menu arising when D&D a tag into the
icon view area:

Daniel suggested in http://bugs.kde.org/show_bug.cgi?id=139547

> In this regard I have a small suggestion: I'd change the order of
> the items in the drag and drop menu (1: assign to Selected 2: to
> Dropped, 3: to All), because I think usually you do an action for
> the selected items, only sometimes just for the dropped, and rarely
> for all.

On the other hand, Achim wants to get rid of this menu completely:

> I must have been on
> crack when I suggested it in http://bugs.kde.org/show_bug.cgi?id=115153
> Sorry :(
>
> IMHO, the menu is overkill.  In my experience in KDE a drop action on a
> selection or only the drop object if the object is outside the selection.
> (see http://bugs.kde.org/show_bug.cgi?id=117403)
>
> Select all is easy:  Control-A, then drop the tag(s).  IMHO if one
> drops to an object outside the selection the selection should be
> cleared
> This leaves us with only one action and cancel in RMB.
> Much easier and simplifies the workflow IMHO.

So the suggestions for the menu are
1.) change the ordering
2.) remove completely
Comment 1 Arnd Baecker 2007-01-18 08:13:33 UTC
Well, I should have said that I am also in favour of removing it.

Ad Achim:
> IMHO if one drops to an object outside the selection 
> the selection should be cleared

I am not sure about this - sometimes one makes a complicated
selection and if one then misses the selected region by chance,
the selection is gone...
(wasn't there also a B.K.O about not loosing selections?)

> and ignored and the tag(s) only assigned to he drag object.

Hmm, somehow I would prefer if this is like canceling
(maybe one could one tell this the user this by a warning box,
because otherwise he thinks that his D&D did something?)
Comment 2 Daniel Bauer 2007-01-18 12:24:18 UTC
I personally don't need the menu. If there is a menu, the ordering should be changed.

I'd suggest:

- drop on a selected picture->apply to all selected pictures
- drop on a not-selected pic->apply only to this one

but unlike Achim, I think:

- drop on a not-selected pic should leave the selection unchanged and intact
- dropping somewhere in between (on a "non-droppable" area, not on a picture) should do just nothing (i.e. cancel drag/drop action, but neither apply tags nor change selection)
Comment 3 Achim Bohnet 2007-01-18 23:24:11 UTC
Daniel, 

about leaving the selection intakt: if other apps do this too, fine with me.
If not, I would favor an undo or restore for the selection.

Personally I never had a use for preserved selection.  But is there
are use cases, that slowdown the workflow even with an implementet
restore/undo of the selection, this might overrule the a non-kde'sh
drop behaviour.

I agree completely about 'Daniel's droping in between' behaviour.
Additinally I would like/expect that the icon used for the drop
indicates that the drop will have no effect., or other way round
when the drop will have an effect, change the way the effected thumbnails
are shown.

Achim
Comment 4 Arnd Baecker 2007-02-27 09:40:01 UTC
Created attachment 19837 [details]
re-order menu-entries
Comment 5 Marcel Wiesweg 2007-02-27 16:32:05 UTC
SVN commit 637724 by mwiesweg:

AlbumIconView Tag-drop popup menu:
Reorder menu entries
	1) Assign to selected
	2) Assign to item dropped on
	3) Assign to all
Do not offer "Assign to selected" if the dropped-on item is the only selected item.

CCBUG: 140227


 M  +29 -22    albumiconview.cpp  


--- trunk/extragear/graphics/digikam/digikam/albumiconview.cpp #637723:637724
@@ -1178,27 +1178,33 @@
         {
             QPopupMenu popMenu(this);
 
-            bool itemsSelected = false;
-            for (IconItem *it = firstItem(); it; it = it->nextItem())
-                if (it->isSelected())
-                    itemsSelected = true;
-    
+            bool moreItemsSelected = false;
             bool itemDropped = false;
+
             AlbumIconItem *albumItem = findItem(event->pos());
-            if (albumItem) 
+            if (albumItem)
                 itemDropped = true;
 
-            popMenu.insertItem(SmallIcon("tag"), 
-                               i18n("Assign '%1' to &All Items").arg(talbum->tagPath().mid(1)),          11);
+            for (IconItem *it = firstItem(); it; it = it->nextItem())
+            {
+                if (it->isSelected() && it != albumItem)
+                {
+                    moreItemsSelected = true;
+                    break;
+                }
+            }
 
-            if (itemsSelected)
+            if (moreItemsSelected)
                 popMenu.insertItem(SmallIcon("tag"), 
                                    i18n("Assign '%1' to &Selected Items").arg(talbum->tagPath().mid(1)), 10);
-            
+
             if (itemDropped)
                 popMenu.insertItem(SmallIcon("tag"),
                                    i18n("Assign '%1' to &Dropped Item").arg(talbum->tagPath().mid(1)),   12);
 
+            popMenu.insertItem(SmallIcon("tag"), 
+                               i18n("Assign '%1' to &All Items").arg(talbum->tagPath().mid(1)),          11);
+
             popMenu.insertSeparator(-1);
             popMenu.insertItem(SmallIcon("cancel"), i18n("&Cancel"));
 
@@ -1257,29 +1263,30 @@
 
         QPopupMenu popMenu(this);
 
-        bool itemsSelected = false;
+        bool moreItemsSelected = false;
+        bool itemDropped = false;
+
+        AlbumIconItem *albumItem = findItem(event->pos());
+        if (albumItem)
+            itemDropped = true;
+
         for (IconItem *it = firstItem(); it; it = it->nextItem())
         {
-            if (it->isSelected())
+            if (it->isSelected() && it != albumItem)
             {
-                itemsSelected = true;
+                moreItemsSelected = true;
                 break;
             }
         }
 
-        bool itemDropped = false;
-        AlbumIconItem *albumItem = findItem(event->pos());
-        if (albumItem) 
-            itemDropped = true;
-
-        popMenu.insertItem(SmallIcon("tag"), i18n("Assign Tags to &All Items"),          11);
-
-        if (itemsSelected)
+        if (moreItemsSelected)
             popMenu.insertItem(SmallIcon("tag"), i18n("Assign Tags to &Selected Items"), 10);
 
         if (itemDropped)
             popMenu.insertItem(SmallIcon("tag"), i18n("Assign Tags to &Dropped Item"),   12);
-    
+
+        popMenu.insertItem(SmallIcon("tag"), i18n("Assign Tags to &All Items"),          11);
+
         popMenu.insertSeparator(-1);
         popMenu.insertItem(SmallIcon("cancel"), i18n("&Cancel"));
 
Comment 6 Arnd Baecker 2007-02-28 10:32:08 UTC
Thanks a lot Marcel!

There is one (minor) thing which could qualify as a buglet:
when one has a selection of several items and does a drop
on a different item, this dropped one als gets the tag assigned 
when choosing the "Assign to selected".
(So in principle this is a "Assign to selected and dropped",
but if I remember correctly, this was shortened some time ago
to the present version).
Comment 7 Marcel Wiesweg 2007-02-28 16:39:26 UTC
Yes I will fix that.

A different point:
I am not a native speaker, but I wonder if the language used is correct:
"Assign '%1' to Dropped Item"

The item is not dropped, but the tag is dropped on the item. It is the "dropped-on" item.
What about
"Assign '%1' to this item"
resp.
"Assign '%1' to this item only" (when others selected)
or other ideas?
(we are in string freeze anyway, but for after the release)
Comment 8 Gerhard Kulzer 2007-02-28 16:42:48 UTC
Am Wednesday 28 February 2007 schrieb Marcel Wiesweg:
[bugs.kde.org quoted mail]

I sent already a mail on this with the same suggestion: 'Assign to this item 
only', so I agree with you. But now we have a string freeze.

Gerhard
Comment 9 Marcel Wiesweg 2007-02-28 16:53:42 UTC
SVN commit 638027 by mwiesweg:

Do not include the "dropped-on" item in the list when "Selected items"
is chosen and the dropped item is not selected

CCBUG:140227


 M  +5 -16     albumiconview.cpp  


--- trunk/extragear/graphics/digikam/digikam/albumiconview.cpp #638026:638027
@@ -1212,18 +1212,13 @@
             int id = popMenu.exec(QCursor::pos());
             switch(id) 
             {
-                case 10:    // Selected and Dropped Items
+                case 10:    // Selected Items
                 {
                     emit signalProgressBarMode(StatusProgressBar::ProgressBarMode, 
                                                i18n("Assign tag to pictures. Please wait..."));
 
-                    // get selected image infos
-                    QPtrList<ImageInfo> infos = selectedImageInfos(true);
-                    // add droppted item
-                    AlbumIconItem *dropItem = findItem(event->pos());
-                    if (dropItem)
-                        infos.append(dropItem->imageInfo());
-                    changeTagOnImageInfos(infos, QValueList<int>() << tagID, true, true);
+                    // always give a copy of the image infos (the "true"). Else there were crashes reported.
+                    changeTagOnImageInfos(selectedImageInfos(true), QValueList<int>() << tagID, true, true);
 
                     emit signalProgressBarMode(StatusProgressBar::TextMode, QString());
                     break;
@@ -1294,18 +1289,12 @@
         int id = popMenu.exec(QCursor::pos());
         switch(id) 
         {
-            case 10:    // Selected and Dropped Items
+            case 10:    // Selected Items
             {
                 emit signalProgressBarMode(StatusProgressBar::ProgressBarMode, 
                                             i18n("Assign tags to pictures. Please wait..."));
 
-                // get selected image infos
-                QPtrList<ImageInfo> infos = selectedImageInfos(true);
-                // add droppted item
-                AlbumIconItem *dropItem = findItem(event->pos());
-                if (dropItem)
-                    infos.append(dropItem->imageInfo());
-                changeTagOnImageInfos(infos, tagIDs, true, true);
+                changeTagOnImageInfos(selectedImageInfos(true), tagIDs, true, true);
 
                 emit signalProgressBarMode(StatusProgressBar::TextMode, QString());
                 break;
Comment 10 caulier.gilles 2008-03-27 14:30:16 UTC
Arnd,

This file sound like already fixed. Fine to close it ?

Gilles Caulier
Comment 11 Arnd Baecker 2008-03-27 18:16:24 UTC
Yes, this is fixed.