Bug 115153

Summary: digikam assigns tags not only to selected images with drag and drop
Product: [Applications] digikam Reporter: Achim Bohnet <ach>
Component: Usability-Drag&DropAssignee: Digikam Developers <digikam-bugs-null>
Status: RESOLVED FIXED    
Severity: normal    
Priority: NOR    
Version: 0.8.0   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In: 0.9.1
Sentry Crash Report:

Description Achim Bohnet 2005-10-26 22:54:36 UTC
Version:           0.8.0-beta2 (using KDE 3.4.3, Kubuntu Package 4:3.4.3-0ubuntu1 )
Compiler:          Target: i486-linux-gnu
OS:                Linux (i686) release 2.6.12-9-686

o select one (or more images) image
o Drag from 'Tags Filter' a tag over an
  image that is _not_ selected a menu with
  'Assign tags to selected images' and
  release.
=> the tag gets assigned to the selected
  images _and_ the not selected image over
  that the mouse was released.

In my case the selected image(s) were some-
times scrolled out of view and I assigned
via drag and drop to an unselected image.
Later I found that quite some pictures had
lots of wrongly tags  because they were still
selected.

The menu text 'Assign tags to selected images'
does not change if no image is selected
and one drop over an image that is not selected.

Suggestion for a fix: change menu text

  'Assign tag to the X selected images'
  'Assign tag to current image and X selected images'
  'Assing tag to current image'

I prefer a solutions that were those 3
cases are better visible than only when reading
the text.  Maybe the Tags icon with nontransparent
background:  blue, blue/white and white (blue is
really the selectioncolor).

Achim



next tag to 
  mou
Comment 1 caulier.gilles 2006-12-21 15:41:50 UTC
SVN commit 615438 by cgilles:

digikam from trunk : fix drag and drop behaviours between tags view and album icon view. All pop-up menu will ask you if you want to assign tags to :

- All album items.
- dropped item only.

If album have items selected, 2 options are also added:

- Selected Items only .
- Selected and dropped items only (if dropped item is not in selection).

BUG: 115153



 M  +149 -37   albumiconview.cpp  


--- trunk/extragear/graphics/digikam/digikam/albumiconview.cpp #615437:615438
@@ -134,30 +134,30 @@
         toolTip       = 0;
     }
 
-    QString albumTitle;
-    QString albumDate;
-    QString albumComments;
+    QString                       albumTitle;
+    QString                       albumDate;
+    QString                       albumComments;
 
-    QRect   itemRect;
-    QRect   itemRatingRect;
-    QRect   itemDateRect;
-    QRect   itemModDateRect;
-    QRect   itemPixmapRect;
-    QRect   itemNameRect;
-    QRect   itemCommentsRect;
-    QRect   itemResolutionRect;
-    QRect   itemSizeRect;
-    QRect   itemTagRect;
-    QRect   bannerRect;
+    QRect                         itemRect;
+    QRect                         itemRatingRect;
+    QRect                         itemDateRect;
+    QRect                         itemModDateRect;
+    QRect                         itemPixmapRect;
+    QRect                         itemNameRect;
+    QRect                         itemCommentsRect;
+    QRect                         itemResolutionRect;
+    QRect                         itemSizeRect;
+    QRect                         itemTagRect;
+    QRect                         bannerRect;
 
-    QPixmap itemRegPixmap;
-    QPixmap itemSelPixmap;
-    QPixmap bannerPixmap;
-    QPixmap ratingPixmap;
+    QPixmap                       itemRegPixmap;
+    QPixmap                       itemSelPixmap;
+    QPixmap                       bannerPixmap;
+    QPixmap                       ratingPixmap;
 
-    QFont   fnReg;
-    QFont   fnCom;
-    QFont   fnXtra;
+    QFont                         fnReg;
+    QFont                         fnCom;
+    QFont                         fnXtra;
 
     QDict<AlbumIconItem>          itemDict;
     
@@ -382,7 +382,7 @@
         if (!item->album())
         {
             DWarning() << "No album for item: " << item->name()
-                        << ", albumID: " << item->albumID() << endl;
+                       << ", albumID: " << item->albumID() << endl;
             continue;
         }
 
@@ -486,8 +486,7 @@
 
     // --------------------------------------------------------
 
-    KMimeType::Ptr mimePtr = KMimeType::findByURL(iconItem->imageInfo()->kurl(),
-                                                  0, true, true);
+    KMimeType::Ptr mimePtr = KMimeType::findByURL(iconItem->imageInfo()->kurl(), 0, true, true);
 
     QValueVector<KService::Ptr> serviceVector;
     KTrader::OfferList offers = KTrader::self()->query(mimePtr->name(), "Type == 'Application'");
@@ -1150,8 +1149,21 @@
         if (talbum)
         {
             QPopupMenu popMenu(this);
-            popMenu.insertItem(i18n("&Assign Tag '%1' to Selected Images")
-                .arg(talbum->prettyURL()), 10 );
+
+            bool itemsSelected = false;
+            for (IconItem *it = firstItem(); it; it = it->nextItem())
+                if (it->isSelected())
+                    itemsSelected = true;
+    
+            if (itemsSelected)
+            {
+                popMenu.insertItem(i18n("&Assign '%1' to Selected Items").arg(talbum->url()),         10);
+                popMenu.insertItem(i18n("&Assign '%1' to Selected/Dropped Items").arg(talbum->url()), 11);
+                popMenu.insertSeparator(-1);
+            }
+
+            popMenu.insertItem(i18n("&Assign '%1' to All Items").arg(talbum->url()),                  12);
+            popMenu.insertItem(i18n("&Assign '%1' to Dropped Item").arg(talbum->url()),               13);
             popMenu.insertSeparator(-1);
             popMenu.insertItem( SmallIcon("cancel"), i18n("C&ancel") );
 
@@ -1159,14 +1171,23 @@
             int id = popMenu.exec(QCursor::pos());
             switch(id) 
             {
-                case 10:
+                case 10:    // Selected Items
                 {
-                    AlbumIconItem *albumItem = findItem(event->pos());
-                    if (albumItem)
+                    for (IconItem *it = firstItem(); it; it = it->nextItem())
                     {
-                        albumItem->imageInfo()->setTag(tagID);
+                        if (it->isSelected())
+                        {
+                            AlbumIconItem *albumItem = static_cast<AlbumIconItem *>(it);
+                            albumItem->imageInfo()->setTag(tagID);
+                        }
                     }
     
+                    d->imageLister->refresh();
+                    updateContents();
+                    break;
+                }
+                case 11:    // Selected and Dropped Items
+                {
                     for (IconItem *it = firstItem(); it; it = it->nextItem())
                     {
                         if (it->isSelected())
@@ -1175,11 +1196,37 @@
                             albumItem->imageInfo()->setTag(tagID);
                         }
                     }
+
+                    AlbumIconItem *albumItem = findItem(event->pos());
+                    if (albumItem)
+                        albumItem->imageInfo()->setTag(tagID);
     
                     d->imageLister->refresh();
                     updateContents();
                     break;
                 }
+                case 12:    // All Items
+                {
+                    for (IconItem *it = firstItem(); it; it = it->nextItem())
+                    {
+                        AlbumIconItem *albumItem = static_cast<AlbumIconItem *>(it);
+                        albumItem->imageInfo()->setTag(tagID);
+                    }
+    
+                    d->imageLister->refresh();
+                    updateContents();
+                    break;
+                }
+                case 13:    // Dropped Item only.
+                {
+                    AlbumIconItem *albumItem = findItem(event->pos());
+                    if (albumItem)
+                        albumItem->imageInfo()->setTag(tagID);
+
+                    d->imageLister->refresh();
+                    updateContents();
+                    break;
+                }
                 default:
                     break;
             }
@@ -1193,7 +1240,21 @@
         ds >> tagIDs;
 
         QPopupMenu popMenu(this);
-        popMenu.insertItem(i18n("&Assign Tags to Selected Images"), 10);
+
+        bool itemsSelected = false;
+        for (IconItem *it = firstItem(); it; it = it->nextItem())
+            if (it->isSelected())
+                itemsSelected = true;
+
+        if (itemsSelected)
+        {
+            popMenu.insertItem(i18n("&Assign Tags to Selected Items"),         10);
+            popMenu.insertItem(i18n("&Assign Tags to Selected/Dropped Items"), 11);
+            popMenu.insertSeparator(-1);
+        }
+
+        popMenu.insertItem(i18n("&Assign Tags to All Items"),                  12);
+        popMenu.insertItem(i18n("&Assign Tags to Dropped Item"),               13);
         popMenu.insertSeparator(-1);
         popMenu.insertItem( SmallIcon("cancel"), i18n("C&ancel") );
 
@@ -1201,18 +1262,27 @@
         int id = popMenu.exec(QCursor::pos());
         switch(id) 
         {
-            case 10:
+            case 10:    // Selected Items
             {
-                AlbumIconItem *albumItem = findItem(event->pos());
-                if (albumItem)
+                for (IconItem *it = firstItem(); it; it = it->nextItem())
                 {
-                    for (QValueList<int>::iterator it = tagIDs.begin();
-                        it != tagIDs.end(); ++it)
+                    if (it->isSelected())
                     {
-                        albumItem->imageInfo()->setTag(*it);
+                        AlbumIconItem *albumItem = static_cast<AlbumIconItem*>(it);
+                        for (QValueList<int>::iterator it = tagIDs.begin();
+                            it != tagIDs.end(); ++it)
+                        {
+                            albumItem->imageInfo()->setTag(*it);
+                        }
                     }
                 }
     
+                d->imageLister->refresh();
+                updateContents();
+                break;
+            }
+            case 11:    // Selected and Dropped Items
+            {
                 for (IconItem *it = firstItem(); it; it = it->nextItem())
                 {
                     if (it->isSelected())
@@ -1225,11 +1295,53 @@
                         }
                     }
                 }
+
+                AlbumIconItem *albumItem = findItem(event->pos());
+                if (albumItem)
+                {
+                    for (QValueList<int>::iterator it = tagIDs.begin();
+                        it != tagIDs.end(); ++it)
+                    {
+                        albumItem->imageInfo()->setTag(*it);
+                    }
+                }
     
                 d->imageLister->refresh();
                 updateContents();
                 break;
             }
+            case 12:    // All Items
+            {
+                for (IconItem *it = firstItem(); it; it = it->nextItem())
+                {
+                    AlbumIconItem *albumItem = static_cast<AlbumIconItem*>(it);
+                    for (QValueList<int>::iterator it = tagIDs.begin();
+                        it != tagIDs.end(); ++it)
+                    {
+                        albumItem->imageInfo()->setTag(*it);
+                    }
+                }
+    
+                d->imageLister->refresh();
+                updateContents();
+                break;
+            }
+            case 13:    // Dropped item only.
+            {
+                AlbumIconItem *albumItem = findItem(event->pos());
+                if (albumItem)
+                {
+                    for (QValueList<int>::iterator it = tagIDs.begin();
+                        it != tagIDs.end(); ++it)
+                    {
+                        albumItem->imageInfo()->setTag(*it);
+                    }
+                }
+
+                d->imageLister->refresh();
+                updateContents();
+                break;
+            }
             default:
                 break;
         }
Comment 2 Achim Bohnet 2006-12-21 20:50:48 UTC
*** Bug 117403 has been marked as a duplicate of this bug. ***
Comment 3 caulier.gilles 2006-12-21 20:57:15 UTC
Achim,

115153 is a closed file... Do not duplicate another one here...

Gilles
Comment 4 Achim Bohnet 2006-12-21 21:02:21 UTC
Sorry, my suggestion with several selections was a bad one.
Much too hard use and totally different how other apps deal with it :(

As already noted in duplicate #117403.  It's much easier and
used  in other apps too that a drop outside an selection, clears
the selection and just gets applied to the dropped on object.

This way only one menu entry (+ cancel?) is necessary

Assign '...' to image

when dropped on an image (outside of an existing selection, which gets
cleared), or

Assign '...' to selection

when dropped on a selection.

This speeds up the workflow considerably.

Please reopen.

Sorry for my bad advise,
Achim
Comment 5 caulier.gilles 2006-12-22 07:38:11 UTC
Achim, 

First , the duplicate file is right. I have not read indeep the content of 117403. Sorry for the sound.

In second, about the "dropped on object" stuff that i have fixed by #115153 (Assigns tags not only to selected images with drag and drop), i will polish implementation and simplified the workflow...

Gilles
Comment 6 caulier.gilles 2006-12-22 08:24:32 UTC
SVN commit 615664 by cgilles:

digikam from trunk : polish Album Icon View pop-up menu contents when tags are dropped on album items.
CCBUGS: 115153



 M  +35 -57    albumiconview.cpp  
 M  +2 -2      albumiconview.h  


--- trunk/extragear/graphics/digikam/digikam/albumiconview.cpp #615663:615664
@@ -1,11 +1,12 @@
 /* ============================================================
  * Authors: Renchi Raju <renchi@pooh.tam.uiuc.edu>
  *          Caulier Gilles <caulier dot gilles at kdemail dot net>
+ *          Marcel Wiesweg <marcel.wiesweg@gmx.de>
  * Date   : 2002-16-10
  * Description : album icon view 
  * 
  * Copyright 2002-2005 by Renchi Raju and Gilles Caulier
- * Copyright      2006 by Gilles Caulier
+ * Copyright      2006 by Gilles Caulier and Marcel Wiesweg
  *
  * This program is free software; you can redistribute it
  * and/or modify it under the terms of the GNU General
@@ -1155,23 +1156,30 @@
                 if (it->isSelected())
                     itemsSelected = true;
     
+            bool itemDropped = false;
+            AlbumIconItem *albumItem = findItem(event->pos());
+            if (albumItem) 
+                itemDropped = true;
+
+            popMenu.insertItem(SmallIcon("tag"), 
+                               i18n("Assign '%1' to &All Items").arg(talbum->url().mid(1)),          11);
+
             if (itemsSelected)
-            {
-                popMenu.insertItem(i18n("&Assign '%1' to Selected Items").arg(talbum->url()),         10);
-                popMenu.insertItem(i18n("&Assign '%1' to Selected/Dropped Items").arg(talbum->url()), 11);
-                popMenu.insertSeparator(-1);
-            }
+                popMenu.insertItem(SmallIcon("tag"), 
+                                   i18n("Assign '%1' to &Selected Items").arg(talbum->url().mid(1)), 10);
+            
+            if (itemDropped)
+                popMenu.insertItem(SmallIcon("tag"),
+                                   i18n("Assign '%1' to &Dropped Item").arg(talbum->url().mid(1)),   12);
 
-            popMenu.insertItem(i18n("&Assign '%1' to All Items").arg(talbum->url()),                  12);
-            popMenu.insertItem(i18n("&Assign '%1' to Dropped Item").arg(talbum->url()),               13);
             popMenu.insertSeparator(-1);
-            popMenu.insertItem( SmallIcon("cancel"), i18n("C&ancel") );
+            popMenu.insertItem(SmallIcon("cancel"), i18n("&Cancel"));
 
             popMenu.setMouseTracking(true);
             int id = popMenu.exec(QCursor::pos());
             switch(id) 
             {
-                case 10:    // Selected Items
+                case 10:    // Selected and Dropped Items
                 {
                     for (IconItem *it = firstItem(); it; it = it->nextItem())
                     {
@@ -1181,21 +1189,6 @@
                             albumItem->imageInfo()->setTag(tagID);
                         }
                     }
-    
-                    d->imageLister->refresh();
-                    updateContents();
-                    break;
-                }
-                case 11:    // Selected and Dropped Items
-                {
-                    for (IconItem *it = firstItem(); it; it = it->nextItem())
-                    {
-                        if (it->isSelected())
-                        {
-                            AlbumIconItem *albumItem = static_cast<AlbumIconItem *>(it);
-                            albumItem->imageInfo()->setTag(tagID);
-                        }
-                    }
 
                     AlbumIconItem *albumItem = findItem(event->pos());
                     if (albumItem)
@@ -1205,7 +1198,7 @@
                     updateContents();
                     break;
                 }
-                case 12:    // All Items
+                case 11:    // All Items
                 {
                     for (IconItem *it = firstItem(); it; it = it->nextItem())
                     {
@@ -1217,7 +1210,7 @@
                     updateContents();
                     break;
                 }
-                case 13:    // Dropped Item only.
+                case 12:    // Dropped Item only.
                 {
                     AlbumIconItem *albumItem = findItem(event->pos());
                     if (albumItem)
@@ -1246,23 +1239,27 @@
             if (it->isSelected())
                 itemsSelected = true;
 
+        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)
-        {
-            popMenu.insertItem(i18n("&Assign Tags to Selected Items"),         10);
-            popMenu.insertItem(i18n("&Assign Tags to Selected/Dropped Items"), 11);
-            popMenu.insertSeparator(-1);
-        }
+            popMenu.insertItem(SmallIcon("tag"), i18n("Assign Tags to &Selected Items"), 10);
 
-        popMenu.insertItem(i18n("&Assign Tags to All Items"),                  12);
-        popMenu.insertItem(i18n("&Assign Tags to Dropped Item"),               13);
+        if (itemDropped)
+            popMenu.insertItem(SmallIcon("tag"), i18n("Assign Tags to &Dropped Item"),   12);
+    
         popMenu.insertSeparator(-1);
-        popMenu.insertItem( SmallIcon("cancel"), i18n("C&ancel") );
+        popMenu.insertItem(SmallIcon("cancel"), i18n("&Cancel"));
 
         popMenu.setMouseTracking(true);
         int id = popMenu.exec(QCursor::pos());
         switch(id) 
         {
-            case 10:    // Selected Items
+            case 10:    // Selected and Dropped Items
             {
                 for (IconItem *it = firstItem(); it; it = it->nextItem())
                 {
@@ -1276,25 +1273,6 @@
                         }
                     }
                 }
-    
-                d->imageLister->refresh();
-                updateContents();
-                break;
-            }
-            case 11:    // Selected and Dropped Items
-            {
-                for (IconItem *it = firstItem(); it; it = it->nextItem())
-                {
-                    if (it->isSelected())
-                    {
-                        AlbumIconItem *albumItem = static_cast<AlbumIconItem*>(it);
-                        for (QValueList<int>::iterator it = tagIDs.begin();
-                            it != tagIDs.end(); ++it)
-                        {
-                            albumItem->imageInfo()->setTag(*it);
-                        }
-                    }
-                }
 
                 AlbumIconItem *albumItem = findItem(event->pos());
                 if (albumItem)
@@ -1310,7 +1288,7 @@
                 updateContents();
                 break;
             }
-            case 12:    // All Items
+            case 11:    // All Items
             {
                 for (IconItem *it = firstItem(); it; it = it->nextItem())
                 {
@@ -1326,7 +1304,7 @@
                 updateContents();
                 break;
             }
-            case 13:    // Dropped item only.
+            case 12:    // Dropped item only.
             {
                 AlbumIconItem *albumItem = findItem(event->pos());
                 if (albumItem)
--- trunk/extragear/graphics/digikam/digikam/albumiconview.h #615663:615664
@@ -1,11 +1,12 @@
 /* ============================================================
  * Authors: Renchi Raju <renchi@pooh.tam.uiuc.edu>
  *          Caulier Gilles <caulier dot gilles at kdemail dot net>
+ *          Marcel Wiesweg <marcel.wiesweg@gmx.de>
  * Date   : 2002-16-10
  * Description : album icon view 
  * 
  * Copyright 2002-2005 by Renchi Raju and Gilles Caulier
- * Copyright      2006 by Gilles Caulier
+ * Copyright      2006 by Gilles Caulier and Marcel Wiesweg
  *
  * This program is free software; you can redistribute it
  * and/or modify it under the terms of the GNU General
@@ -185,7 +186,6 @@
 private:
 
     AlbumIconViewPrivate *d;
-
 };
 
 }  // namespace Digikam