Bug 113807

Summary: Is it possible to have the tags of the contextual menus "Assign Tag" and "Remove Tag" sorted ?
Product: [Applications] digikam Reporter: Tung NGUYEN <ntung>
Component: Usability-MenusAssignee: Digikam Developers <digikam-bugs-null>
Status: RESOLVED FIXED    
Severity: normal CC: caulier.gilles
Priority: NOR    
Version: 0.8.0   
Target Milestone: ---   
Platform: Compiled Sources   
OS: Linux   
Latest Commit: Version Fixed In: 0.9.0
Attachments: Sort tag context menu popups

Description Tung NGUYEN 2005-10-03 22:08:32 UTC
Version:           0.8.0-beta2 (using KDE KDE 3.4.2)
Installed from:    Compiled From Sources
OS:                Linux

Currently, all the tags are displayed sorted in the "Tags" view and in the "Tag Filters" but not in the contextual menus "Assign Tag" and "Remove Tag" (creation order). It can be useful to have these menus sorted in order to select easily the tags.
Comment 1 Craig Howard 2005-10-03 22:14:21 UTC
Created attachment 12834 [details]
Sort tag context menu popups

I posted this patch to the dev list a while ago, but didn't get any responses.
Comment 2 caulier.gilles 2005-10-04 10:44:03 UTC
Thanks for this patch, we will take a look when it's possible.

Gilles Caulier
Comment 3 Joern Ahrens 2005-12-19 11:53:52 UTC
SVN commit 489627 by jahrens:

Added the patch from Craig Howard.
Thank you Craig!

CCMAIL: kde@choward.ca
BUG: 113807


 M  +10 -4     tagfolderview.cpp  
 M  +28 -33    tagspopupmenu.cpp  
 M  +3 -0      tagspopupmenu.h  


--- trunk/extragear/graphics/digikam/digikam/tagfolderview.cpp #489626:489627
@@ -350,11 +350,17 @@
 
     int counter = 100;
     KABC::AddressBook* ab = KABC::StdAddressBook::self();
-    KABC::AddressBook::Iterator it;
-    for ( it = ab->begin(); it != ab->end(); ++it )
+    QStringList names;
+    for ( KABC::AddressBook::Iterator it = ab->begin(); it != ab->end(); ++it )
     {
-        KABC::Addressee addr = (*it);
-        QString name = addr.formattedName();
+        names.push_back(it->formattedName());
+    }
+
+    qHeapSort(names);
+
+    for ( QStringList::Iterator it = names.begin(); it != names.end(); ++it )
+    {
+        QString name = *it;
         if ( !name.isNull() )
             d->ABCMenu->insertItem( name, ++counter );
     }
--- trunk/extragear/graphics/digikam/digikam/tagspopupmenu.cpp #489626:489627
@@ -28,6 +28,7 @@
 #include <qstring.h>
 #include <qpainter.h>
 #include <qstyle.h>
+#include <qvaluevector.h>
 
 #include "albumiconview.h"
 #include "albumiconitem.h"
@@ -169,37 +170,8 @@
         }
     }
     
-    for (Album* a = album->firstChild(); a; a = a->next())
-    {
-        if (m_mode == REMOVE)
-        {
-            IntList::iterator it = qFind(m_assignedTags.begin(),
-                                         m_assignedTags.end(),
-                                         a->id());
-            if (it == m_assignedTags.end())
-                continue;
-        }
+    iterateAndBuildMenu(popup, album);
 
-        QPixmap pix = SyncJob::getTagThumbnail(((TAlbum*)a)->icon(), KIcon::SizeSmall);
-        if (a->firstChild())
-        {
-            popup->insertItem(pix, a->title(), buildSubMenu(a->id()));
-        }
-        else
-        {
-            //popup->insertItem(pix, a->title(), m_addToID + a->id());
-            if ((m_mode == ASSIGN) && (m_assignedTags.contains(a->id())))
-            {
-                popup->insertItem(new TagsPopupCheckedMenuItem(popup, a->title(), pix),
-                                  m_addToID + a->id());
-            }
-            else
-            {
-                popup->insertItem(pix, a->title(), m_addToID + a->id());
-            }
-        }
-    }
-
     return popup;
 }
 
@@ -263,8 +235,31 @@
         }
     }
     
+    iterateAndBuildMenu(this, album);
+}
+
+// for qHeapSort
+typedef QPair<QString, Album*> TagsMenuSortType;
+bool operator<(const TagsMenuSortType &lhs, const TagsMenuSortType &rhs)
+{
+    return lhs.first < rhs.first;
+}
+
+void TagsPopupMenu::iterateAndBuildMenu(QPopupMenu *menu, TAlbum *album)
+{
+    QValueVector<TagsMenuSortType> sortedTags;
+
     for (Album* a = album->firstChild(); a; a = a->next())
     {
+        sortedTags.push_back(qMakePair(a->title(), a));
+    }
+
+    qHeapSort(sortedTags);
+    
+    for (QValueVector<TagsMenuSortType>::Iterator i = sortedTags.begin(); i != sortedTags.end(); ++i)
+    {
+        Album *a = i->second;
+        
         if (m_mode == REMOVE)
         {
             IntList::iterator it = qFind(m_assignedTags.begin(),
@@ -277,18 +272,18 @@
         QPixmap pix = SyncJob::getTagThumbnail(((TAlbum*)a)->icon(), KIcon::SizeSmall);
         if (a->firstChild())
         {
-            insertItem(pix, a->title(), buildSubMenu(a->id()));
+            menu->insertItem(pix, a->title(), buildSubMenu(a->id()));
         }
         else
         {
             if ((m_mode == ASSIGN) && (m_assignedTags.contains(a->id())))
             {
-                insertItem(new TagsPopupCheckedMenuItem(this, a->title(), pix),
+                menu->insertItem(new TagsPopupCheckedMenuItem(this, a->title(), pix),
                            m_addToID + a->id());
             }
             else
             {
-                insertItem(pix, a->title(), m_addToID + a->id());
+                menu->insertItem(pix, a->title(), m_addToID + a->id());
             }
         }
     }
--- trunk/extragear/graphics/digikam/digikam/tagspopupmenu.h #489626:489627
@@ -25,6 +25,8 @@
 #include <qvaluelist.h>
 #include <qpixmap.h>
 
+class TAlbum;
+
 class TagsPopupMenu : public QPopupMenu
 {
     Q_OBJECT
@@ -45,6 +47,7 @@
 
     void        clearPopup();
     QPopupMenu* buildSubMenu(int tagid);
+    void        iterateAndBuildMenu(QPopupMenu *menu, TAlbum *album);
     bool        showThisTag(int tagid);
 
     QValueList<Q_LLONG>  m_selectedImageIDs;
Comment 4 Joern Ahrens 2005-12-19 11:55:42 UTC
SVN commit 489628 by jahrens:

Backported the patch from Craig Howard to stable branch.

CCBUG: 113807


 M  +10 -4     tagfolderview.cpp  
 M  +28 -33    tagspopupmenu.cpp  
 M  +3 -0      tagspopupmenu.h  


--- branches/stable/extragear/graphics/digikam/digikam/tagfolderview.cpp #489627:489628
@@ -350,11 +350,17 @@
 
     int counter = 100;
     KABC::AddressBook* ab = KABC::StdAddressBook::self();
-    KABC::AddressBook::Iterator it;
-    for ( it = ab->begin(); it != ab->end(); ++it )
+    QStringList names;
+    for ( KABC::AddressBook::Iterator it = ab->begin(); it != ab->end(); ++it )
     {
-        KABC::Addressee addr = (*it);
-        QString name = addr.formattedName();
+        names.push_back(it->formattedName());
+    }
+
+    qHeapSort(names);
+
+    for ( QStringList::Iterator it = names.begin(); it != names.end(); ++it )
+    {
+        QString name = *it;
         if ( !name.isNull() )
             d->ABCMenu->insertItem( name, ++counter );
     }
--- branches/stable/extragear/graphics/digikam/digikam/tagspopupmenu.cpp #489627:489628
@@ -28,6 +28,7 @@
 #include <qstring.h>
 #include <qpainter.h>
 #include <qstyle.h>
+#include <qvaluevector.h>
 
 #include "albumiconview.h"
 #include "albumiconitem.h"
@@ -169,37 +170,8 @@
         }
     }
     
-    for (Album* a = album->firstChild(); a; a = a->next())
-    {
-        if (m_mode == REMOVE)
-        {
-            IntList::iterator it = qFind(m_assignedTags.begin(),
-                                         m_assignedTags.end(),
-                                         a->id());
-            if (it == m_assignedTags.end())
-                continue;
-        }
+    iterateAndBuildMenu(popup, album);
 
-        QPixmap pix = SyncJob::getTagThumbnail(((TAlbum*)a)->icon(), KIcon::SizeSmall);
-        if (a->firstChild())
-        {
-            popup->insertItem(pix, a->title(), buildSubMenu(a->id()));
-        }
-        else
-        {
-            //popup->insertItem(pix, a->title(), m_addToID + a->id());
-            if ((m_mode == ASSIGN) && (m_assignedTags.contains(a->id())))
-            {
-                popup->insertItem(new TagsPopupCheckedMenuItem(popup, a->title(), pix),
-                                  m_addToID + a->id());
-            }
-            else
-            {
-                popup->insertItem(pix, a->title(), m_addToID + a->id());
-            }
-        }
-    }
-
     return popup;
 }
 
@@ -263,8 +235,31 @@
         }
     }
     
+    iterateAndBuildMenu(this, album);
+}
+
+// for qHeapSort
+typedef QPair<QString, Album*> TagsMenuSortType;
+bool operator<(const TagsMenuSortType &lhs, const TagsMenuSortType &rhs)
+{
+    return lhs.first < rhs.first;
+}
+
+void TagsPopupMenu::iterateAndBuildMenu(QPopupMenu *menu, TAlbum *album)
+{
+    QValueVector<TagsMenuSortType> sortedTags;
+
     for (Album* a = album->firstChild(); a; a = a->next())
     {
+        sortedTags.push_back(qMakePair(a->title(), a));
+    }
+
+    qHeapSort(sortedTags);
+    
+    for (QValueVector<TagsMenuSortType>::Iterator i = sortedTags.begin(); i != sortedTags.end(); ++i)
+    {
+        Album *a = i->second;
+        
         if (m_mode == REMOVE)
         {
             IntList::iterator it = qFind(m_assignedTags.begin(),
@@ -277,18 +272,18 @@
         QPixmap pix = SyncJob::getTagThumbnail(((TAlbum*)a)->icon(), KIcon::SizeSmall);
         if (a->firstChild())
         {
-            insertItem(pix, a->title(), buildSubMenu(a->id()));
+            menu->insertItem(pix, a->title(), buildSubMenu(a->id()));
         }
         else
         {
             if ((m_mode == ASSIGN) && (m_assignedTags.contains(a->id())))
             {
-                insertItem(new TagsPopupCheckedMenuItem(this, a->title(), pix),
+                menu->insertItem(new TagsPopupCheckedMenuItem(this, a->title(), pix),
                            m_addToID + a->id());
             }
             else
             {
-                insertItem(pix, a->title(), m_addToID + a->id());
+                menu->insertItem(pix, a->title(), m_addToID + a->id());
             }
         }
     }
--- branches/stable/extragear/graphics/digikam/digikam/tagspopupmenu.h #489627:489628
@@ -25,6 +25,8 @@
 #include <qvaluelist.h>
 #include <qpixmap.h>
 
+class TAlbum;
+
 class TagsPopupMenu : public QPopupMenu
 {
     Q_OBJECT
@@ -45,6 +47,7 @@
 
     void        clearPopup();
     QPopupMenu* buildSubMenu(int tagid);
+    void        iterateAndBuildMenu(QPopupMenu *menu, TAlbum *album);
     bool        showThisTag(int tagid);
 
     QValueList<Q_LLONG>  m_selectedImageIDs;