Bug 140227

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?)

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)

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

Created attachment 19837 [details]
re-order menu-entries

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"));
 

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).

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)

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

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;

Arnd,

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

Gilles Caulier

Yes, this is fixed.