Bug 115157

Summary: digikam usability: Image comments/tags dialog: hard to find/see all already selected tags (and to 'de'select them)
Product: [Applications] digikam Reporter: Achim Bohnet <ach>
Component: Tags-CaptionsAssignee: Digikam Developers <digikam-bugs-null>
Status: RESOLVED FIXED    
Severity: wishlist    
Priority: NOR    
Version: 0.8.0   
Target Milestone: ---   
Platform: unspecified   
OS: Linux   
Latest Commit: Version Fixed In: 0.9.1
Sentry Crash Report:
Attachments: screenshot 1 of new Comments & tags layout
screenshot 2 of new Comments & tags layout
new optimized comments and tags side bar layout
digi1.png
screenshots of the assigned tags misbehaviour
patch to solve assigned tags view problem

Description Achim Bohnet 2005-10-26 23:31:54 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

hard to find/see all already selected tags (and to 'de'select them if necessary).
Similar to #115154 with the 'Tags filter'.

to make the select tags better them visible I one
possibility maybe to show tags below the thumbnail
(and move the filename above the icon or move
icons more to the top)

Achim
Comment 1 Alan Horkan 2006-03-22 10:58:23 UTC
spelling mistake in summary: useability usability
Comment 2 caulier.gilles 2006-09-03 16:49:49 UTC
SVN commit 580439 by cgilles:

digikam from trunk : Tags Filter View : new options to 

- "Select All" tags
- "Deselect" all tags
- "Invert Selection" of tags

BUG: 115154
CCBUGS: 115157

 M  +58 -7     tagfilterview.cpp  


--- trunk/extragear/graphics/digikam/digikam/tagfilterview.cpp #580438:580439
@@ -143,6 +143,8 @@
     QTimer *timer;
 };
 
+// ---------------------------------------------------------------------
+
 TagFilterView::TagFilterView(QWidget* parent)
              : FolderView(parent)
 {
@@ -330,8 +332,7 @@
     }
     else
     {
-        TagFilterViewItem* parent =
-            (TagFilterViewItem*)(tag->parent()->extraData(this));
+        TagFilterViewItem* parent = (TagFilterViewItem*)(tag->parent()->extraData(this));
         if (!parent)
         {
             kdWarning() << k_funcinfo << " Failed to find parent for Tag "
@@ -354,8 +355,7 @@
     if (!tag)
         return;
 
-    TagFilterViewItem* item =
-            (TagFilterViewItem*)(tag->extraData(this));
+    TagFilterViewItem* item = (TagFilterViewItem*)(tag->extraData(this));
     if (item)
     {
         item->setText(0, tag->title());
@@ -507,10 +507,15 @@
 
     if (item)
     {
-        popmenu.insertItem(SmallIcon("pencil"), i18n("Edit Tag Properties..."), 11);
-        popmenu.insertItem(SmallIcon("reload_page"), i18n("Reset Tag Icon"), 13);
-        popmenu.insertItem(SmallIcon("edittrash"), i18n("Delete Tag"), 12);
+        popmenu.insertItem(SmallIcon("pencil"),      i18n("Edit Tag Properties..."), 11);
+        popmenu.insertItem(SmallIcon("reload_page"), i18n("Reset Tag Icon"),         13);
+        popmenu.insertItem(SmallIcon("edittrash"),   i18n("Delete Tag"),             12);
     }
+ 
+    popmenu.insertSeparator();
+    popmenu.insertItem(i18n("Select All"),       14);
+    popmenu.insertItem(i18n("Deselect"),         15);
+    popmenu.insertItem(i18n("Invert Selection"), 16);
 
     int choice = popmenu.exec((QCursor::pos()));
     switch( choice )
@@ -536,6 +541,52 @@
             AlbumManager::instance()->updateTAlbumIcon(item->m_tag, QString("tag"), 0, errMsg);
             break;
         }        
+        case 14:
+        {
+            QListViewItemIterator it(this, QListViewItemIterator::NotChecked);
+            while (it.current())
+            {
+                TagFilterViewItem* item = (TagFilterViewItem*)it.current();
+                item->setOn(true);
+                ++it;
+            }
+            triggerChange();
+            break;
+        }
+        case 15:
+        {
+            QListViewItemIterator it(this, QListViewItemIterator::Checked);
+            while (it.current())
+            {
+                TagFilterViewItem* item = (TagFilterViewItem*)it.current();
+                item->setOn(false);
+                ++it;
+            }
+            triggerChange();
+            break;
+        }
+        case 16:
+        {
+            QListViewItemIterator it(this);
+            while (it.current())
+            {
+                TagFilterViewItem* item = (TagFilterViewItem*)it.current();
+
+                // Toogle all root tags filter.
+                TAlbum *tag = item->m_tag;
+                if (tag)
+                    if (tag->parent()->isRoot())
+                        item->setOn(!item->isOn());
+
+                // Toogle "Not Tagged" item tag filter.
+                if (item->m_untagged)
+                    item->setOn(!item->isOn());
+
+                ++it;
+            }
+            triggerChange();
+            break;
+        }
         default:
             break;
     }
Comment 3 caulier.gilles 2006-09-03 16:52:03 UTC
Achim, 

Do you that i backport the current patch used to fix #115154 into the tag view of Comments & Tags side bar ?

Gilles
Comment 4 caulier.gilles 2006-09-03 17:47:12 UTC
SVN commit 580457 by cgilles:

digikam from trunk : Tags view from Comments & Tags side bar tab : new options to:

- "Select All" tags
- "Deselect" all tags
- "Invert Selection" of tags

CCBUGS: 115154, 115157

 M  +52 -6     imagedescedittab.cpp  


--- trunk/extragear/graphics/digikam/libs/imageproperties/imagedescedittab.cpp #580456:580457
@@ -508,8 +508,7 @@
     QListViewItemIterator it( d->tagsView);
     while (it.current())
     {
-        TAlbumCheckListItem* tItem =
-                dynamic_cast<TAlbumCheckListItem*>(it.current());
+        TAlbumCheckListItem* tItem = dynamic_cast<TAlbumCheckListItem*>(it.current());
 
         if (tItem)
         {
@@ -569,6 +568,7 @@
     QPopupMenu popmenu(this);
 
     popmenu.insertItem(SmallIcon("tag"), i18n("New Tag..."), 10);
+
     if (!album->isRoot())
     {
         popmenu.insertItem(SmallIcon("pencil"),      i18n("Edit Tag Properties..."), 11);
@@ -576,6 +576,11 @@
         popmenu.insertItem(SmallIcon("edittrash"),   i18n("Delete Tag"),             12);
     }
 
+    popmenu.insertSeparator();
+    popmenu.insertItem(i18n("Select All"),       14);
+    popmenu.insertItem(i18n("Deselect"),         15);
+    popmenu.insertItem(i18n("Invert Selection"), 16);
+
     switch (popmenu.exec(QCursor::pos()))
     {
         case 10:
@@ -601,6 +606,43 @@
             AlbumManager::instance()->updateTAlbumIcon(album, QString("tag"), 0, errMsg);
             break;
         }
+        case 14:
+        {
+            QListViewItemIterator it(d->tagsView, QListViewItemIterator::NotChecked);
+            while (it.current())
+            {
+                TAlbumCheckListItem* item = dynamic_cast<TAlbumCheckListItem*>(it.current());
+                item->setOn(true);
+                ++it;
+            }
+            break;
+        }
+        case 15:
+        {
+            QListViewItemIterator it(d->tagsView, QListViewItemIterator::Checked);
+            while (it.current())
+            {
+                TAlbumCheckListItem* item = dynamic_cast<TAlbumCheckListItem*>(it.current());
+                item->setOn(false);
+                ++it;
+            }
+            break;
+        }
+        case 16:
+        {
+            QListViewItemIterator it(d->tagsView);
+            while (it.current())
+            {
+                TAlbumCheckListItem* item = dynamic_cast<TAlbumCheckListItem*>(it.current());
+                TAlbum *tag = item->m_album;
+                if (tag)
+                    if (!tag->isRoot())
+                        item->setOn(!item->isOn());
+
+                ++it;
+            }
+            break;
+        }
         default:
             break;
     }
@@ -823,7 +865,8 @@
 void ImageDescEditTab::slotImagesChanged(int albumId)
 {
     Album *a = AlbumManager::instance()->findAlbum(albumId);
-    if (!d->ignoreImageAttributesWatch && !d->currInfo || !a || a->isRoot() || a->type() != Album::TAG)
+    if (!d->ignoreImageAttributesWatch && 
+        !d->currInfo || !a || a->isRoot() || a->type() != Album::TAG)
         return;
 
     updateTagsView();
@@ -831,19 +874,22 @@
 
 void ImageDescEditTab::slotImageRatingChanged(Q_LLONG imageId)
 {
-    if (!d->ignoreImageAttributesWatch && d->currInfo && d->currInfo->id() == imageId)
+    if (!d->ignoreImageAttributesWatch && 
+        d->currInfo && d->currInfo->id() == imageId)
         updateRating();
 }
 
 void ImageDescEditTab::slotImageCaptionChanged(Q_LLONG imageId)
 {
-    if (!d->ignoreImageAttributesWatch && d->currInfo && d->currInfo->id() == imageId)
+    if (!d->ignoreImageAttributesWatch && 
+        d->currInfo && d->currInfo->id() == imageId)
         updateComments();
 }
 
 void ImageDescEditTab::slotImageDateChanged(Q_LLONG imageId)
 {
-    if (!d->ignoreImageAttributesWatch && d->currInfo && d->currInfo->id() == imageId)
+    if (!d->ignoreImageAttributesWatch && 
+        d->currInfo && d->currInfo->id() == imageId)
         updateDate();
 }
 
Comment 5 caulier.gilles 2006-09-03 17:50:55 UTC
Achim, 

The option to manage tags selection from Tags Filter Side bar habe been backported to Comments & Tags side bar.

The question now is: since Comments & tags dialog have been moved to a side bar, i think this bug is invalid... right ?

Gilles
Comment 6 Roy 2006-09-03 18:02:47 UTC
On Sunday 03 September 2006 17:50, Gilles Caulier wrote:
> ------- Achim,
>
> The option to manage tags selection from Tags Filter Side bar habe been
> backported to Comments & Tags side bar.
>
> The question now is: since Comments & tags dialog have been moved to a side
> bar, i think this bug is invalid... right ?
>
> Gilles


Maybe I'm missing something but I think it's still hard to get an overview of 
what tags are already assigned to an image. I've got a rather large hierarchy 
of tags and hence need to scroll much until i spotted all selected tags.

It would be nice to have a summary of all selected tags.

Thank you very much for this great piece of software.

Roy
Comment 7 Mikolaj Machowski 2006-09-03 23:38:26 UTC
I don't think this bug can be closed.

Still with huge structure of tags it is hard to see
what tags image has. Especially in Preview mode (F3
in album view) and in Image Editor.
Comment 8 caulier.gilles 2006-12-18 09:32:32 UTC
At url below, you have a patch against svn. This is a first approach to optimize free space to use on the right Comments & tags side bar :

http://digikam3rdparty.free.fr/misc.tarballs/tagsview.patch

This patch use a QToolBox to separate Comments/Date/Rating view of Tags view. You have more space to enter comments and of course more space to display all tags.

Gilles
Comment 9 caulier.gilles 2006-12-18 09:37:57 UTC
Created attachment 18966 [details]
screenshot 1 of new Comments & tags layout
Comment 10 caulier.gilles 2006-12-18 09:38:32 UTC
Created attachment 18967 [details]
screenshot 2 of new Comments & tags layout
Comment 11 Arnd Baecker 2006-12-18 10:38:55 UTC
Can one still have both "Comments/Tags/Ratings" and "Tags"
visible (e.g. by rescaling the size of the "Comments/Tags/Ratings" part)?

Further options to provide more information in the same amount of space:
- allow for free adjustment of the Fontsize of the tags
  (independent of the fonts used at other places of digikam)
- The squares+Icons also take up quite some space.
  For a really condensed view they could be reduced in size
  as well.

In general one might even think of introducing
user-definable viewing profiles for different tasks.
Each profile corresponds to a bunch of settings, e.g.
- tags with very small font visible, but no folders, large icon size
- metadata open, but smallest icon size for the images.

But (all?) this, and how to realize a smooth workflow,
would require more thought and discussion ...
Comment 12 caulier.gilles 2006-12-18 13:45:28 UTC
The patch is now updated to support Drag and Drop in Tags view of Comments & Tags sidebar :

http://digikam3rdparty.free.fr/misc.tarballs/tagsview.patch 

Gilles
Comment 13 caulier.gilles 2006-12-18 18:50:50 UTC
Patch commited to svn with commit #614702

Gilles
Comment 14 Mikolaj Machowski 2006-12-18 20:58:17 UTC
I have mixed feelings.

1. It is always good to have more space for comments.
2. Even full height of monitor is not enough to show complete tag tree. 

   Possible solutions:

   a) don't open tree completely. IMO opening only first
      level of tree should considerably shorten tree length (for me it
      is still not enough...).
   b) use some of gained space to subwindow showing mini-tree of image
      tags

3. Subpanels. I really don't like it. Confusing. Make all panels
   horizontal (at least those on the right side). It would make right
   side unclosable though. Or make it close, only with icons visible.
Comment 15 caulier.gilles 2006-12-19 08:12:52 UTC
SVN commit 614853 by cgilles:

digikam from trunk : Comments & Tags Sidebar tab : using KTabWidget instead QToolBox to render sidebar page content. This will homogenous than others existing sidebar tabs

CCBUGS: 115157

 M  +29 -30    imagedescedittab.cpp  


--- trunk/extragear/graphics/digikam/libs/imageproperties/imagedescedittab.cpp #614852:614853
@@ -29,7 +29,6 @@
 #include <qhgroupbox.h>
 #include <qvgroupbox.h>
 #include <qheader.h>
-#include <qtoolbox.h>
 #include <qtoolbutton.h>
 #include <qiconset.h>
 #include <qwhatsthis.h>
@@ -49,6 +48,7 @@
 #include <kconfig.h>
 #include <klineedit.h>
 #include <kdialogbase.h>
+#include <ktabwidget.h>
 
 // Local includes.
 
@@ -79,7 +79,7 @@
 
     enum SettingsTab
     {
-        COMMENTSPAGE=0,
+        DESCRIPTIONPAGE=0,
         TAGSPAGE
     };
 
@@ -97,14 +97,12 @@
         ratingWidget               = 0;
         navigateBar                = 0;
         ABCMenu                    = 0;
-        toolBox                    = 0;
+        tab                        = 0;
     }
 
     bool               modified;
     bool               ignoreImageAttributesWatch;
 
-    QToolBox          *toolBox;
-
     QToolButton       *recentTagsBtn;
     QToolButton       *tagsSearchClearBtn;
 
@@ -114,6 +112,8 @@
 
     KLineEdit         *tagsSearchEdit;
 
+    KTabWidget        *tab;
+
     KDateTimeEdit     *dateTimeEdit;
 
     ImageInfo         *currInfo;
@@ -130,24 +130,29 @@
 {
     d = new ImageDescEditTabPriv;
 
-    QVBoxLayout *vLayout        = new QVBoxLayout(this);
-    d->navigateBar              = new NavigateBarWidget(this, navBar);
-    d->toolBox                  = new QToolBox(this);
-    QWidget *propertiesBox      = new QWidget(d->toolBox);
-    QGridLayout *settingsLayout = new QGridLayout(propertiesBox, 2, 1, 
-                                      KDialog::marginHint(), KDialog::spacingHint());
+    QVBoxLayout *vLayout = new QVBoxLayout(this);
+    d->navigateBar       = new NavigateBarWidget(this, navBar);
+    d->tab               = new KTabWidget(this);
     
+    vLayout->addWidget(d->navigateBar);
+    vLayout->addSpacing(KDialog::spacingHint());
+    vLayout->addWidget(d->tab);
+
     // Comments/Date/Rating view -----------------------------------
+
+    QWidget *descriptionPage    = new QWidget(d->tab);
+    QGridLayout *settingsLayout = new QGridLayout(descriptionPage, 2, 1, 
+                                      KDialog::marginHint(), KDialog::spacingHint());
     
-    QVGroupBox* commentsBox = new QVGroupBox(i18n("&Comments"), propertiesBox);
+    QVGroupBox* commentsBox = new QVGroupBox(i18n("&Comments"), descriptionPage);
     d->commentsEdit         = new KTextEdit(commentsBox);
     d->commentsEdit->setTextFormat(QTextEdit::PlainText);
     d->commentsEdit->setCheckSpellingEnabled(true);
 
-    QHGroupBox* dateTimeBox = new QHGroupBox(i18n("&Date && Time"), propertiesBox);
+    QHGroupBox* dateTimeBox = new QHGroupBox(i18n("&Date && Time"), descriptionPage);
     d->dateTimeEdit         = new KDateTimeEdit( dateTimeBox, "datepicker");
 
-    QHGroupBox* ratingBox = new QHGroupBox(i18n("Rating"), propertiesBox);
+    QHGroupBox* ratingBox = new QHGroupBox(i18n("Rating"), descriptionPage);
     ratingBox->layout()->setAlignment(Qt::AlignCenter);
     d->ratingWidget = new RatingWidget(ratingBox);
 
@@ -156,14 +161,13 @@
     settingsLayout->addMultiCellWidget(ratingBox, 2, 2, 0, 1);
     settingsLayout->setRowStretch(0, 10);
 
-    d->toolBox->insertItem(ImageDescEditTabPriv::COMMENTSPAGE, propertiesBox, 
-                           SmallIconSet("imagecomment"), i18n("Comments/Date/Rating"));
+    d->tab->insertTab(descriptionPage, i18n("Description"), ImageDescEditTabPriv::DESCRIPTIONPAGE);
 
     // Tags view ---------------------------------------------------
 
-    QWidget *tagsBox      = new QWidget(d->toolBox);
-    QVBoxLayout *vLayout2 = new QVBoxLayout(tagsBox, KDialog::marginHint(), KDialog::spacingHint());
-    QHBox* tagsSearch     = new QHBox(tagsBox);
+    QWidget *tagsPage     = new QWidget(d->tab);
+    QVBoxLayout *vLayout2 = new QVBoxLayout(tagsPage, KDialog::marginHint(), KDialog::spacingHint());
+    QHBox* tagsSearch     = new QHBox(tagsPage);
     tagsSearch->setSpacing(KDialog::spacingHint());
 
     d->tagsSearchClearBtn = new QToolButton(tagsSearch);
@@ -184,7 +188,7 @@
     d->recentTagsBtn->setPopup(popupMenu);
     d->recentTagsBtn->setPopupDelay(1);
 
-    d->tagsView = new TAlbumListView(tagsBox);
+    d->tagsView = new TAlbumListView(tagsPage);
     d->tagsView->addColumn(i18n("Tags"));
     d->tagsView->header()->hide();
     d->tagsView->setSelectionMode(QListView::Single);
@@ -193,19 +197,14 @@
     vLayout2->addWidget(tagsSearch);
     vLayout2->addWidget(d->tagsView);
 
-    d->toolBox->insertItem(ImageDescEditTabPriv::TAGSPAGE, tagsBox, 
-                           kapp->iconLoader()->loadIcon("tag", KIcon::NoGroup, KIcon::SizeSmall,
-                           KIcon::DefaultState, 0, true), i18n("Tags"));
-
+    d->tab->insertTab(tagsPage, i18n("Tags"), ImageDescEditTabPriv::TAGSPAGE);
+    
     // --------------------------------------------------
     
-    vLayout->addWidget(d->navigateBar);
-    vLayout->addWidget(d->toolBox);    
-
     KConfig* config = kapp->config();
     config->setGroup("Image Properties SideBar");
-    d->toolBox->setCurrentIndex(config->readNumEntry("Comments And Tags Tab",
-                                ImageDescEditTabPriv::COMMENTSPAGE));
+    d->tab->setCurrentPage(config->readNumEntry("Comments And Tags Tab",
+                           ImageDescEditTabPriv::DESCRIPTIONPAGE));
 
     // --------------------------------------------------
 
@@ -308,7 +307,7 @@
  
     KConfig* config = kapp->config();
     config->setGroup("Image Properties SideBar");
-    config->writeEntry("Comments And Tags Tab", d->toolBox->currentIndex());
+    config->writeEntry("Comments And Tags Tab", d->tab->currentPageIndex());
     config->sync();
    
     /*
Comment 16 caulier.gilles 2006-12-19 13:49:32 UTC
SVN commit 614902 by cgilles:

digikam from trunk : Comments & Tags Sidebar tab : add a new small push button to only display the currently assigned tags of the image.

BUG: 115157

 M  +75 -9     imagedescedittab.cpp  
 M  +1 -0      imagedescedittab.h  


--- trunk/extragear/graphics/digikam/libs/imageproperties/imagedescedittab.cpp #614901:614902
@@ -30,6 +30,7 @@
 #include <qvgroupbox.h>
 #include <qheader.h>
 #include <qtoolbutton.h>
+#include <qpushbutton.h>
 #include <qiconset.h>
 #include <qwhatsthis.h>
 #include <qtooltip.h>
@@ -98,6 +99,7 @@
         navigateBar                = 0;
         ABCMenu                    = 0;
         tab                        = 0;
+        assignedTagsBtn            = 0;
     }
 
     bool               modified;
@@ -108,6 +110,8 @@
 
     QPopupMenu        *ABCMenu;
 
+    QPushButton       *assignedTagsBtn;
+
     KTextEdit         *commentsEdit;
 
     KLineEdit         *tagsSearchEdit;
@@ -179,11 +183,19 @@
     d->tagsSearchEdit = new KLineEdit(tagsSearch);
     d->tagsSearchEdit->setSizePolicy(QSizePolicy(QSizePolicy::Expanding, QSizePolicy::Minimum));
 
+    d->assignedTagsBtn = new QPushButton(tagsSearch);
+    QToolTip::add(d->assignedTagsBtn, i18n("Already Assigned Tags"));
+    d->assignedTagsBtn->setIconSet(kapp->iconLoader()->loadIcon("tag-assigned",
+                                   KIcon::NoGroup, KIcon::SizeSmall, 
+                                   KIcon::DefaultState, 0, true));
+    d->assignedTagsBtn->setToggleButton(true);
+
     d->recentTagsBtn      = new QToolButton(tagsSearch);
     QPopupMenu *popupMenu = new QPopupMenu(d->recentTagsBtn);
     QToolTip::add(d->recentTagsBtn, i18n("Recent Tags"));
-    d->recentTagsBtn->setIconSet(kapp->iconLoader()->loadIcon("tag-recents", KIcon::NoGroup,
-                                 KIcon::SizeSmall, KIcon::DefaultState, 0, true));
+    d->recentTagsBtn->setIconSet(kapp->iconLoader()->loadIcon("tag-recents", 
+                                 KIcon::NoGroup, KIcon::SizeSmall, 
+                                 KIcon::DefaultState, 0, true));
     d->recentTagsBtn->setUsesBigPixmap(false);
     d->recentTagsBtn->setPopup(popupMenu);
     d->recentTagsBtn->setPopupDelay(1);
@@ -243,6 +255,9 @@
             
     connect(d->tagsSearchEdit, SIGNAL(textChanged(const QString&)),
             this, SLOT(slotTagsSearchChanged()));
+
+    connect(d->assignedTagsBtn, SIGNAL(toggled(bool)),
+            this, SLOT(slotAssignedTagsToggled(bool)));
             
     // Initalize ---------------------------------------------
 
@@ -627,7 +642,8 @@
             while (it.current())
             {
                 TAlbumCheckListItem* item = dynamic_cast<TAlbumCheckListItem*>(it.current());
-                item->setOn(true);
+                if (item->isVisible())
+                    item->setOn(true);
                 ++it;
             }
             break;
@@ -638,7 +654,8 @@
             while (it.current())
             {
                 TAlbumCheckListItem* item = dynamic_cast<TAlbumCheckListItem*>(it.current());
-                item->setOn(false);
+                if (item->isVisible())
+                    item->setOn(false);
                 ++it;
             }
             break;
@@ -649,11 +666,13 @@
             while (it.current())
             {
                 TAlbumCheckListItem* item = dynamic_cast<TAlbumCheckListItem*>(it.current());
-                TAlbum *tag = item->m_album;
-                if (tag)
-                    if (!tag->isRoot())
-                        item->setOn(!item->isOn());
-
+                if (item->isVisible())
+                {
+                    TAlbum *tag = item->m_album;
+                    if (tag)
+                        if (!tag->isRoot())
+                            item->setOn(!item->isOn());
+                }
                 ++it;
             }
             break;
@@ -1133,5 +1152,52 @@
     }
 }
 
+void ImageDescEditTab::slotAssignedTagsToggled(bool t)
+{
+    QListViewItemIterator it(d->tagsView);
+    while (it.current())
+    {
+        TAlbumCheckListItem* item = dynamic_cast<TAlbumCheckListItem*>(it.current());
+        TAlbum *tag               = item->m_album;
+        if (tag)
+        {
+            if (!tag->isRoot())
+            {
+                if (t)
+                {
+                    bool isOn = item->isOn();
+                    item->setVisible(isOn);
+
+                    if (isOn)
+                    {
+                        Album* parent = tag->parent();
+                        while (parent && !parent->isRoot())
+                        {
+                            QCheckListItem *pitem = (QCheckListItem*)parent->extraData(this);
+                            pitem->setVisible(true);
+                            parent = parent->parent();
+                        }
+                    }
+                }
+                else
+                {
+                    item->setVisible(true);
+                }
+            }
+        }
+        ++it;
+    }
+
+    TAlbum *root                  = AlbumManager::instance()->findTAlbum(0);
+    TAlbumCheckListItem *rootItem = (TAlbumCheckListItem*)(root->extraData(this));
+    if (rootItem)
+    {
+        if (t)
+            rootItem->setText(0, i18n("Assigned Tags"));
+        else
+            rootItem->setText(0, root->title());
+    }
+}
+
 }  // NameSpace Digikam
 
--- trunk/extragear/graphics/digikam/libs/imageproperties/imagedescedittab.h #614901:614902
@@ -108,6 +108,7 @@
     void slotImageCaptionChanged(Q_LLONG imageId);
 
     void slotRecentTagsMenuActivated(int);
+    void slotAssignedTagsToggled(bool);
 
 private:
 
Comment 17 Fabien 2006-12-19 16:52:38 UTC
Well... I must confess I don't like the new look with 2 tabs. I fully understand that one would like to have more space to display tags and/or comments.
But, I agree with Arnd, it would be great to be able to still view comments tags and rating at the same time.
I often use right/left arrow keys to browse the different pictures within an album and it's quite convenient to check comments *and* tags for each without having to go from one tab to the 2nd.

The best would be to choose between both views in an elegant way. I have no idea how to do that properly, but I hope others will have :)
Comment 18 Fabien 2006-12-20 10:15:32 UTC
Finally, I've got a simple idea : there could just be 2 small buttons on the top of the panel to choose between tabs view and standard view.
What's your opinion ?
Comment 19 caulier.gilles 2006-12-20 10:22:51 UTC
It could be not simple to implement, but not impossible to do. I can also revert the tab view implementation if necessary...

The discussion still open. I have chosen a tab view to add more free space (try to display all tags of an huge Tags collection with the old implementation : it's not really suitable). Personnally i'm happy with it, but yes, it change a little bit the comments & tags editing workflow.

To resume, the users will have the last word about this subject. Let's me hear your viewpoints. Is tab view is the right way to display Comments & Tags sidebar content, and especially to separate the Tags view ?

Gilles
Comment 20 Gerhard Kulzer 2006-12-20 10:43:16 UTC
Am Mittwoch, 20. Dezember 2006 10:22 schrieb Gilles Caulier:
[bugs.kde.org quoted mail]
In my opinion the original comment space (upper half) was enough, what was 
really needed is the summary view of the applied tags.

So my suggestion would be to go back to the center split version (comments, 
dates, rating, tags) and keep the little button of the tag summary.

Gerhard
Comment 21 Fabien 2006-12-20 11:22:21 UTC
I agree with Gerhard.
For me, it would really be a problem to not be able to see all informations (description, comment, rate and tags) at the same time.
Or if you want to keep space for tags, we could use 3 tabs :
- view (mixed view of all elements)
- edit description (full height edit box)
- edit tags (full height tags list)


BTW, I can't find it but I'm sure I read it somewhere in archives... What is the max size for comments (ie to be stored in IPTC, EXIF and JFIF) ?
Comment 22 caulier.gilles 2006-12-20 11:29:50 UTC
IPTC caption : limited to 2000 ASCII char.
JFIF comments : it's a dedicaced JPEG section typically limited to 65535 bytes (not especially ASCII char)
EXIF comments : it's a tag from the Exif JPEG section. Limitation is 65535 bytes - size of others Exif tags from this section (not especially ASCII char). This limitation is true for JPEG only. TIFF and PNG can store more than 65535 bytes.

Gilles
Comment 23 Achim Bohnet 2006-12-20 11:44:20 UTC
I also prefer a version that shows desc comment rate and only assigned tags
together.  (Regardsless how big a tags area is it can't show my tag hierarchy completely;) )

Howto switch to 'show all tags' to use it for assigning new tags I'm not sure.
I assume it's important that switching via keyboard is important. Otherwise it
interupts the workflow.

Brainstorming:
Maybe a widget with  'line edit with autocompletition'  plus a list of recently
used tags below  that can be expanded [Edit >>] and collapsed [Edit <<] like
the option button in KDE's printer dialog?

Achim
Comment 24 caulier.gilles 2006-12-20 12:48:37 UTC
ok... I try again before my hollidays (:=)))...

At url below, there is a screenshot of the new layout of Comment & Tags sidebar. Look like the space used verticaly is very optimized. I have removed all QGroupBox and oriented horizontally label and widget when it's possible (Date and Rating)

http://digikam3rdparty.free.fr/Screenshots/newcommentsandtagssidebarlayout.png

Tags search bar is under Tags view to separate these widgets of others ("My tags" root text is on the top of Tags stuff like a title).

Achim, i have add an "Apply Changes" button like you talk to me recently by IRC. This will be less confuse for user i think...

The screen size is 1024*3 / 768 (xinerama)

I attach to this thread a patch to test on your computer.

Let's me hear your viewpoints...

Gilles
Comment 25 caulier.gilles 2006-12-20 12:54:15 UTC
Created attachment 18986 [details]
new optimized comments and tags side bar layout
Comment 26 Arnd Baecker 2006-12-20 14:26:01 UTC
This looks rather good to me.
Would it technically be possible that
a user can resize the vertical space used for the comment
(e.g. down to one line ... ;-)?

Another point is also nicely seen in this screenshot:
Sometimes I would like to make the Comments/Tags part
as small as possible (horizontally).
However, there is always a minium width.
I would not care, if  part of the date, time and rating become
invisible. I.e. in the screenshot I would like to reduce
the width so that only the first "d" of "ddd" stays visible.
This is enough to assign tags while having a maximum
amount of space in the middle for as many thumbs as possible.

Arnd
Comment 27 caulier.gilles 2006-12-20 14:36:05 UTC
> Would it technically be possible that a user can resize the vertical space used for the comment (e.g. down to one line ... ;-)? 

Well, no, because i suspect a bug in KDE libs about this point. You not the first guy to ask me this feature. We using a KTextEdit widget (witch support Spell Checking) and i suspect a problem in this widget witch set a miminal height by default.

Note than in the patch, i always set the maximum height to Tags view.

Other little optimization is to remove the "Comments:" label on the top to get more vertical free space. We can set a tool tip message like "Enter here your comments" witch be appear when the user move mouse under the KTextEdit.

Gilles
Comment 28 Arnd Baecker 2006-12-20 14:44:42 UTC
Honestly, I would not care about spell checking,
if there is a another text widget which allows vertical resizing ;-).
But maybe others got used to spell-checking ...

I am not sure about the mouse-over variant - seems a bit too hidden
in my opinion (thinking of new users. Personally I would not mind).
Comment 29 caulier.gilles 2006-12-20 14:47:26 UTC
We have used KTextEdit instead QTextEdit especially for Spell Check. I remember a wish in B.K.O about...

Gilles
Comment 30 caulier.gilles 2006-12-20 15:58:43 UTC
Fabien, Achim, Gerhard, etc...

...Your viewpoints about new layout proposal ?

Gilles
Comment 31 Fabien 2006-12-20 16:00:54 UTC
I just recompiled and tried. I like it !
Comment 32 Mikolaj Machowski 2006-12-20 17:28:21 UTC
Agree with Gerhard, plus possibility to drag separator between comments
and tags increasing/decreasing appropriate area.
Comment 33 Mikolaj Machowski 2006-12-20 17:32:39 UTC
> ------- Honestly, I would not care about spell checking,

Spell checking in text widgets is one of major KDE "selling points",
please don't remove that feature.
Comment 34 Gerhard Kulzer 2006-12-20 17:52:58 UTC
Am Mittwoch, 20. Dezember 2006 15:58 schrieb Gilles Caulier:
[bugs.kde.org quoted mail]

It took me a while to realise that there was a patch and not a svn commit...

But now I compiled it and I like it, very efficient :-))

A+

> ...Your viewpoints about new layout proposal ?
>
> Gilles
> _______________________________________________
> Digikam-devel mailing list
> Digikam-devel@kde.org
> https://mail.kde.org/mailman/listinfo/digikam-devel

Comment 35 Gerhard Kulzer 2006-12-20 17:54:13 UTC
Am Mittwoch, 20. Dezember 2006 17:32 schrieb Mikolaj Machowski:
[bugs.kde.org quoted mail]

I agree, every time somebody sees this feature on my machine, they're 
impressed.
> _______________________________________________
> Digikam-devel mailing list
> Digikam-devel@kde.org
> https://mail.kde.org/mailman/listinfo/digikam-devel

Comment 36 Mikolaj Machowski 2006-12-20 18:32:50 UTC
> Let's me hear your viewpoints...

I think you hit the best solution :) Dragging of border would be an
icing on cake but it there is KDElibs bug, pity - but, please don't
remove spellchecking.

Two bugs with filtering of tags view:

- when switching between photos initial selection of tags isn't changed,
  boxes are unchecked and checked but only in limits of selection for
  first photo; for example if I have two photos: 
  	1.jpg with tags - a,b,c
        2.jpg with tags - a,c,d
  select 1.jpg I have all tags limited to a,b,c, OK. Switch to 2.jpg
  I still see 3 boxes: a,b,c. b is unchecked but I don't see d anywhere.
  To update view I have click 'already assigned tags' button twice.
- when I have long list of tags on some branch of tags tree after
  filtering I see all boxes *before* assigned tag.

Attached image illustrates both those bugs. First image (not shown on
this screenshot) has tags (Witkacy,1911), second (visible) has tags
(Witkacy, 1906).

m.


Created an attachment (id=18990)
digi1.png
Comment 37 caulier.gilles 2006-12-20 20:25:33 UTC
Mik,

I understand the problem. I will fix it tomorrow...

Gilles
Comment 38 caulier.gilles 2006-12-20 20:30:32 UTC
SVN commit 615238 by cgilles:

digikam from trunk : second stage to optimize layout of Comments & Tags sidebar. Free space optimization, removing Tabs, and add "Apply" button.

CCBUGS: 115157

 M  +45 -62    imagedescedittab.cpp  
 M  +1 -1      imagedescedittab.h  


--- trunk/extragear/graphics/digikam/libs/imageproperties/imagedescedittab.cpp #615237:615238
@@ -24,10 +24,9 @@
 // Qt includes.
 
 #include <qhbox.h>
+#include <qvbox.h>
 #include <qlabel.h>
 #include <qlayout.h>
-#include <qhgroupbox.h>
-#include <qvgroupbox.h>
 #include <qheader.h>
 #include <qtoolbutton.h>
 #include <qpushbutton.h>
@@ -49,7 +48,6 @@
 #include <kconfig.h>
 #include <klineedit.h>
 #include <kdialogbase.h>
-#include <ktabwidget.h>
 
 // Local includes.
 
@@ -78,12 +76,6 @@
 {
 public:
 
-    enum SettingsTab
-    {
-        DESCRIPTIONPAGE=0,
-        TAGSPAGE
-    };
-
     ImageDescEditTabPriv()
     {
         modified                   = false;
@@ -98,8 +90,8 @@
         ratingWidget               = 0;
         navigateBar                = 0;
         ABCMenu                    = 0;
-        tab                        = 0;
         assignedTagsBtn            = 0;
+        applyBtn                   = 0;
     }
 
     bool               modified;
@@ -107,17 +99,16 @@
 
     QToolButton       *recentTagsBtn;
     QToolButton       *tagsSearchClearBtn;
+    QToolButton       *assignedTagsBtn;
 
     QPopupMenu        *ABCMenu;
 
-    QPushButton       *assignedTagsBtn;
+    QPushButton       *applyBtn;
 
     KTextEdit         *commentsEdit;
 
     KLineEdit         *tagsSearchEdit;
 
-    KTabWidget        *tab;
-
     KDateTimeEdit     *dateTimeEdit;
 
     ImageInfo         *currInfo;
@@ -134,44 +125,35 @@
 {
     d = new ImageDescEditTabPriv;
 
-    QVBoxLayout *vLayout = new QVBoxLayout(this);
-    d->navigateBar       = new NavigateBarWidget(this, navBar);
-    d->tab               = new KTabWidget(this);
+    QVBoxLayout *vLayout  = new QVBoxLayout(this);
+    d->navigateBar        = new NavigateBarWidget(this, navBar);
+    QWidget *settingsArea = new QWidget(this);
     
     vLayout->addWidget(d->navigateBar);
-    vLayout->addSpacing(KDialog::spacingHint());
-    vLayout->addWidget(d->tab);
+    vLayout->addWidget(settingsArea);
 
-    // Comments/Date/Rating view -----------------------------------
-
-    QWidget *descriptionPage    = new QWidget(d->tab);
-    QGridLayout *settingsLayout = new QGridLayout(descriptionPage, 2, 1, 
+    QGridLayout *settingsLayout = new QGridLayout(settingsArea, 5, 1, 
                                       KDialog::marginHint(), KDialog::spacingHint());
+
+    // Comments/Date/Rating view -----------------------------------
     
-    QVGroupBox* commentsBox = new QVGroupBox(i18n("&Comments"), descriptionPage);
-    d->commentsEdit         = new KTextEdit(commentsBox);
+    QVBox *commentsBox = new QVBox(settingsArea);
+    new QLabel(i18n("Comments:"), commentsBox);
+    d->commentsEdit    = new KTextEdit(commentsBox);
     d->commentsEdit->setTextFormat(QTextEdit::PlainText);
     d->commentsEdit->setCheckSpellingEnabled(true);
 
-    QHGroupBox* dateTimeBox = new QHGroupBox(i18n("&Date && Time"), descriptionPage);
-    d->dateTimeEdit         = new KDateTimeEdit( dateTimeBox, "datepicker");
+    QHBox *dateBox  = new QHBox(settingsArea);
+    new QLabel(i18n("Date:"), dateBox);
+    d->dateTimeEdit = new KDateTimeEdit(dateBox, "datepicker");
 
-    QHGroupBox* ratingBox = new QHGroupBox(i18n("Rating"), descriptionPage);
-    ratingBox->layout()->setAlignment(Qt::AlignCenter);
-    d->ratingWidget = new RatingWidget(ratingBox);
+    QHBox *ratingBox = new QHBox(settingsArea);
+    new QLabel(i18n("Rating:"), ratingBox);
+    d->ratingWidget  = new RatingWidget(ratingBox);
 
-    settingsLayout->addMultiCellWidget(commentsBox, 0, 0, 0, 1);
-    settingsLayout->addMultiCellWidget(dateTimeBox, 1, 1, 0, 1);
-    settingsLayout->addMultiCellWidget(ratingBox, 2, 2, 0, 1);
-    settingsLayout->setRowStretch(0, 10);
-
-    d->tab->insertTab(descriptionPage, i18n("Description"), ImageDescEditTabPriv::DESCRIPTIONPAGE);
-
     // Tags view ---------------------------------------------------
 
-    QWidget *tagsPage     = new QWidget(d->tab);
-    QVBoxLayout *vLayout2 = new QVBoxLayout(tagsPage, KDialog::marginHint(), KDialog::spacingHint());
-    QHBox* tagsSearch     = new QHBox(tagsPage);
+    QHBox *tagsSearch = new QHBox(settingsArea);
     tagsSearch->setSpacing(KDialog::spacingHint());
 
     d->tagsSearchClearBtn = new QToolButton(tagsSearch);
@@ -183,7 +165,7 @@
     d->tagsSearchEdit = new KLineEdit(tagsSearch);
     d->tagsSearchEdit->setSizePolicy(QSizePolicy(QSizePolicy::Expanding, QSizePolicy::Minimum));
 
-    d->assignedTagsBtn = new QPushButton(tagsSearch);
+    d->assignedTagsBtn = new QToolButton(tagsSearch);
     QToolTip::add(d->assignedTagsBtn, i18n("Already Assigned Tags"));
     d->assignedTagsBtn->setIconSet(kapp->iconLoader()->loadIcon("tag-assigned",
                                    KIcon::NoGroup, KIcon::SizeSmall, 
@@ -200,24 +182,25 @@
     d->recentTagsBtn->setPopup(popupMenu);
     d->recentTagsBtn->setPopupDelay(1);
 
-    d->tagsView = new TAlbumListView(tagsPage);
+    d->tagsView = new TAlbumListView(settingsArea);
     d->tagsView->addColumn(i18n("Tags"));
     d->tagsView->header()->hide();
     d->tagsView->setSelectionMode(QListView::Single);
     d->tagsView->setResizeMode(QListView::LastColumn);
 
-    vLayout2->addWidget(tagsSearch);
-    vLayout2->addWidget(d->tagsView);
-
-    d->tab->insertTab(tagsPage, i18n("Tags"), ImageDescEditTabPriv::TAGSPAGE);
-    
     // --------------------------------------------------
-    
-    KConfig* config = kapp->config();
-    config->setGroup("Image Properties SideBar");
-    d->tab->setCurrentPage(config->readNumEntry("Comments And Tags Tab",
-                           ImageDescEditTabPriv::DESCRIPTIONPAGE));
 
+    d->applyBtn = new QPushButton(i18n("Apply Changes"), settingsArea);
+    d->applyBtn->setEnabled(false);
+
+    settingsLayout->addMultiCellWidget(commentsBox, 0, 0, 0, 1);
+    settingsLayout->addMultiCellWidget(dateBox, 1, 1, 0, 1);
+    settingsLayout->addMultiCellWidget(ratingBox, 2, 2, 0, 1);
+    settingsLayout->addMultiCellWidget(d->tagsView, 3, 3, 0, 1);
+    settingsLayout->addMultiCellWidget(tagsSearch, 4, 4, 0, 1);
+    settingsLayout->addMultiCellWidget(d->applyBtn, 5, 5, 0, 1);
+    settingsLayout->setRowStretch(3, 10);
+
     // --------------------------------------------------
 
     connect(popupMenu, SIGNAL(activated(int)),
@@ -258,7 +241,10 @@
 
     connect(d->assignedTagsBtn, SIGNAL(toggled(bool)),
             this, SLOT(slotAssignedTagsToggled(bool)));
-            
+
+    connect(d->applyBtn, SIGNAL(clicked()),
+            this, SLOT(slotApplyAllChanges()));
+           
     // Initalize ---------------------------------------------
 
     d->commentsEdit->installEventFilter(this);
@@ -318,12 +304,7 @@
 
 ImageDescEditTab::~ImageDescEditTab()
 {
-    applyAllChanges();
- 
-    KConfig* config = kapp->config();
-    config->setGroup("Image Properties SideBar");
-    config->writeEntry("Comments And Tags Tab", d->tab->currentPageIndex());
-    config->sync();
+    slotApplyAllChanges();
    
     /*
     AlbumList tList = AlbumManager::instance()->allTAlbums();
@@ -367,8 +348,7 @@
     AlbumList tList = AlbumManager::instance()->allTAlbums();
     for (AlbumList::iterator it = tList.begin(); it != tList.end(); ++it)
     {
-        TAlbum* tag  = (TAlbum*)(*it);
-
+        TAlbum *tag = (TAlbum*)(*it);
         slotAlbumAdded(tag);
     }
 }
@@ -376,6 +356,7 @@
 void ImageDescEditTab::slotModified()
 {
     d->modified = true;
+    d->applyBtn->setEnabled(true);
 }
 
 void ImageDescEditTab::assignRating(int rating)
@@ -383,7 +364,7 @@
     d->ratingWidget->setRating(rating);
 }
 
-void ImageDescEditTab::applyAllChanges()
+void ImageDescEditTab::slotApplyAllChanges()
 {
     if (!d->modified)
         return;
@@ -480,13 +461,14 @@
     }
 
     d->modified = false;
+    d->applyBtn->setEnabled(false);
 
     updateRecentTags();
 }
 
 void ImageDescEditTab::setItem(ImageInfo *info, int itemType)
 {
-    applyAllChanges();
+    slotApplyAllChanges();
 
     if (!info)
     {
@@ -500,6 +482,7 @@
     setEnabled(true);
     d->currInfo = info;
     d->modified = false;
+    d->applyBtn->setEnabled(false);
 
     KURL fileURL;
     fileURL.setPath(d->currInfo->filePath());
--- trunk/extragear/graphics/digikam/libs/imageproperties/imagedescedittab.h #615237:615238
@@ -70,7 +70,6 @@
 
 private:
 
-    void applyAllChanges();
     void updateTagsView();
     void updateComments();
     void updateRating();
@@ -86,6 +85,7 @@
 
 private slots:
 
+    void slotApplyAllChanges();
     void slotModified();
     void slotRightButtonClicked(QListViewItem *, const QPoint &, int);
     void slotTagsSearchChanged();
Comment 39 caulier.gilles 2006-12-20 21:42:41 UTC
SVN commit 615252 by cgilles:

digikam from trunk : Comments & Tags sidebar : Tags View : check if "Already Assigned Tags" button is on when user change current picture focus to refresh the assigned tags list properly.
CCBUGS: 115157

 M  +2 -0      imagedescedittab.cpp  


--- trunk/extragear/graphics/digikam/libs/imageproperties/imagedescedittab.cpp #615251:615252
@@ -525,6 +525,8 @@
         ++it;
     }
 
+    slotAssignedTagsToggled(d->assignedTagsBtn->isOn());
+
     d->tagsView->blockSignals(false);
 }
 
Comment 40 caulier.gilles 2006-12-20 21:53:24 UTC
Mik,

"Et voilĂ ". Let's me hear if 2 bugs are fixed now... (before that i go to hollidays...)

Gilles
Comment 41 Mikolaj Machowski 2006-12-21 00:23:11 UTC
> "Et voil?". Let's me hear if 2 bugs are fixed now... (before that i go
> to hollidays...)

There is still bug two:

- when I have long list of tags on some branch of tags tree after
  filtering I see all boxes *before* assigned tag.

This is not so painful but looks strange.

I though about another solution. Maybe filtering should be done not to
the exact tag but the *branch* where tag is located (with exclusion of
root level)? In that way it would be possible to see context of that tag
and allow for some manipulation on limited tree.

What other users think?
Comment 42 Arnd Baecker 2006-12-21 09:14:20 UTC
Filtering to the branch does sound like a cool idea to me.
Would there be space for another button to toggle this behaviour?
Comment 43 caulier.gilles 2006-12-21 09:16:51 UTC
free-space is limited to have a new button in this bar. Sound better to have an option in pop-up menu.

Gilles
Comment 44 Achim Bohnet 2006-12-21 20:08:27 UTC
Hi Gilles,

I like the I really like the 'Assigned Task' list.  Great work!
Thanks a lot!

Small problem I found when playing with the new feature:

a) Select all images (ctrl-a) and  'Assigned tags' no longer worked.
I always see the complete expaned tree

b) I assiged two tags and when I switched to recent tags nothing was
listed

c) Feature: Multiselect in tags view.  When e.g. assigning seerver tags,
like place + several people, often the same set of tags could be assigned
to the next (and next) picture too.  Maybe module +/- some persons.  So
it would be very nice to be able to select all 'assigned' tags of the photo
shown and drop them on the following picture too.

d) When selecting several thumbmails, IMHO, the comment, rating, date
fields should be cleared and disabled.  Tags can still be useful:
show all tags assigned to all photos.  So via selecting a tag in all
photos get be tagged with this tag.  Or the tag is removed from all
pics when the tag is deselected.  So assigning/removing is like do/undo.

Achim
Comment 45 caulier.gilles 2006-12-21 20:31:08 UTC
Achim,

I know all these problem. All your remarks depand of multiple items selection support of Comments & Tags side bar... witch is not yet implemented. there is another file in B.K.O about.

I fact only the first item of the selection is drived by Comments & Tags.

Gilles
Comment 46 Arnd Baecker 2007-01-03 17:12:15 UTC
I am not sure if this is the same as #41 above:
If one has:

- Tag0
- Tag1
  - Tag2                     # this should not be shown
    - Tag2a                  # this should not be shown
  - Tag3     
    - Tag3a                  # this should not be shown
    - Tag3b
      - x Tagged Tag         # <--- this is tagged
    - Tag5
    - Tag6

When using "Already assigned TABs",  also Tag2, Tag2a, Tag3a are visible
while I thought that they should not be displayed
(Tag0, Tag5, Tag6 are not shown, as expected).
Comment 47 caulier.gilles 2007-01-04 07:55:14 UTC
Arnd,

Strange. I will tested indeep...

Somebody can reproduce this problem with current implemntation from svn ?

Gilles
Comment 48 Arnd Baecker 2007-01-05 18:42:09 UTC
Created attachment 19113 [details]
screenshots of the assigned tags misbehaviour

#46 is not yet fixed (svn version 620314).
Interestingly, for a situation as shown in the attachment 
it depends on the parent (U1) being tagged or not whether its (not tagged)
child (AA) is shown (which it should not).
Comment 49 caulier.gilles 2007-01-05 19:03:36 UTC
ok, Thanks for the screenshot.

You have a complex tag hierarchy. I will try to reproduce the problem.

Gilles
Comment 50 caulier.gilles 2007-01-06 12:09:58 UTC
Arnd,

I cannot reproduce the problem here. Please, can you try to hack the implementation in this method with your tags hierarchy:

void ImageDescEditTab::slotAssignedTagsToggled(bool t)

It's at end of the the file /libs/imageproperties/imagedescedittab.cpp.

Thanks in advance

Gilles
Comment 51 Arnd Baecker 2007-01-08 14:45:13 UTC
> I cannot reproduce the problem here.


I see - if I remove my digikam3.db  and start from scratch
I cannot reproduce it anymore. Needs further investigation
(I am prettyy swamped during the next 4 weeks...)

There are three more issues with current svn (revision 621162):
- moving tags around does not work anymore
  for Comments/Tags and Tag Filters on the right hand side?
- Comments/Tags: there is no way to ``un-toggle'' Childs/Parents/Both
- Tag Filters: there is no way to ``un-toggle'' Childs/Parents/Both
  Also, sometimes clicking does active the whole tree, but
  for other tags nothing happens at alll?
  (this might be another difficult to reproduce one)
Hmm, should these points be better mentioned in
http://bugs.kde.org/show_bug.cgi?id=139547
?
Comment 52 caulier.gilles 2007-01-08 15:15:03 UTC
>There are three more issues with current svn (revision 621162): 
> - moving tags around does not work anymore 
>   for Comments/Tags and Tag Filters on the right hand side? 

I suspect this problem is relevant of an indeep change into digikam core about multiple selection of item support into Comments & Tags side bar...

>Hmm, should these points be better mentioned in 
> http://bugs.kde.org/show_bug.cgi?id=139547 ? 

yes

Gilles
Comment 53 caulier.gilles 2007-01-08 18:42:21 UTC
Arnd,

About comment #48, I think that i have fixed this point in svn. Please try again.

Gilles
Comment 54 caulier.gilles 2007-01-08 20:51:31 UTC
SVN commit 621422 by cgilles:

digikam from trunk : Tags Filter View : do not handle 'Not Tagget' filter when Select/Deselect All Tags option  is used
CCBUGS: 115157

 M  +16 -10    tagfilterview.cpp  


--- trunk/extragear/graphics/digikam/digikam/tagfilterview.cpp #621421:621422
@@ -683,26 +683,38 @@
         }        
         case 14:    // Select All Tags.
         {
+            d->toggleAutoTags = TagFilterView::NoToggleAuto;
             QListViewItemIterator it(this, QListViewItemIterator::NotChecked);
             while (it.current())
             {
                 TagFilterViewItem* item = (TagFilterViewItem*)it.current();
-                item->setOn(true);
+
+                // Ignore "Not Tagged" tag filter.
+                if (!item->m_untagged)
+                    item->setOn(true);
+
                 ++it;
             }
             triggerChange();
+            d->toggleAutoTags = oldAutoTags;
             break;
         }
         case 15:    // Deselect All Tags.
         {
+            d->toggleAutoTags = TagFilterView::NoToggleAuto;
             QListViewItemIterator it(this, QListViewItemIterator::Checked);
             while (it.current())
             {
                 TagFilterViewItem* item = (TagFilterViewItem*)it.current();
-                item->setOn(false);
+
+                // Ignore "Not Tagged" tag filter.
+                if (!item->m_untagged)
+                    item->setOn(false);
+
                 ++it;
             }
             triggerChange();
+            d->toggleAutoTags = oldAutoTags;
             break;
         }
         case 16:       // Invert All Tags Selection.
@@ -713,14 +725,8 @@
             {
                 TagFilterViewItem* item = (TagFilterViewItem*)it.current();
 
-                // Toggle all root tags filter.
-                TAlbum *tag = item->m_tag;
-                if (tag)
-                    if (tag->parent()->isRoot())
-                        item->setOn(!item->isOn());
-
-                // Toggle "Not Tagged" item tag filter.
-                if (item->m_untagged)
+                // Ignore "Not Tagged" tag filter.
+                if (!item->m_untagged)
                     item->setOn(!item->isOn());
 
                 ++it;
Comment 55 Arnd Baecker 2007-01-09 09:10:52 UTC
I just tried, but #48/#46  is still not working correctly
in my case. Unfortunately, I can't look into this at the moment
due to severe time constraints (really  sorry, this problem seems
like one which I could maybe track down).

The only thing I could do right now is to privately send you a zip
(2.8 MB) of the Pictures directory structure with the database.
The other option is to leave this aside for the moment and I will
have a look in about 4/5 weeks?

Many thanks, Arnd
Comment 56 Arnd Baecker 2007-01-11 12:28:01 UTC
Gilles, I started to have a quick look at what you suggest in #50
(I really should not ;-).

I cooked it down to the example
  - Hallo
    - U2
      - Aa
      x TestAdd      # this one is tagged

Here "Aa" should not be displayed.
I splattered loads of 
  DWarning() << k_funcinfo << "       parent:" << parent->title() << endl;
into ImageDescEditTab::slotAssignedTagsToggled.
and checked at the end that the visibility of "Aa" is false.
Still it gets displayed!?
Very weird ...

Just wait: when you do the ``pitem->setVisible(true);``
for all parents to make them visible, does this also set
the visibility to true of the children? That could explain
why part of the tree becomes visible, while children further down
in the tree are hidden again (because they are processed separately again?
Checking the visibility at the end shows, that it got changed for the "Aa"
element!

However, what I do not understand is, why this problem does not appear, 
when "U2" is tagged as well. Maybe it has to do with the order
in which the iteration of the tree is done, which could also explain
why you were not able to reproduce the problem?

Best, Arnd

Comment 57 Arnd Baecker 2007-01-11 12:42:52 UTC
> and checked at the end that the visibility of "Aa" is false.


To be precise here: I first confirmed at the end (=after running
through the iterations) that
  titem->isOn();
is still False. However, it was still displayed.

When iterating over the elements,
  item->setVisible(isOn);
the visibility is set to false, as expected.

However, at the end (after running through the iterations),
the visibility  of the "Aa" element changes, i.e.
  item->isVisible()
is True instead of false.

Hope this is clearer ...

Arnd
Comment 58 Arnd Baecker 2007-01-11 13:57:24 UTC
Ok, so what I suspected seems to be indeed correct:
  // The logic used here seems to be awkward. The problem is, that
  // QListViewItem::setVisible works recursively on all it's children
  // and grand-children.
from
http://www.koders.com/cpp/fidBE871C6AFC00CA7D5CFC9A1F1CFCFE88768EDD51.aspx

Surely, there should be better places to find an information like
this (but google gave me this one ;-).

Best, Arnd
Comment 59 Arnd Baecker 2007-01-12 09:39:13 UTC
To solve this issue - would this work:
after the current code, go through the full tree again.
for each *visible* item check if
 - itself is selected  (i.e,  item->isOn() gives True)
 - or there is any child which is selected.
If none of these two is the case, make the item
under consideration *invisible*.
Comment 60 Arnd Baecker 2007-02-05 09:13:58 UTC
I would like to try to fix this, but need a little bit of help by one of you experts here, as I don't speak C++/Qt:

My plan is to implement what is suggested in #59, just
after L. 1468  of digikam/libs/imageproperties/imagedescedittab.cpp

I would start with an iterator:
  QListViewItemIterator ittT(d->tagsView);
  while (ittT.current())
over the whole tree and then for each visible tag I want to iterate 
over its children.

Question: How can I construct an iterator over the children of a tag?
I tried something like
  QListViewItemIterator itchild(tag->child());
  //Album* child = tag->child();
but that did not work ...

Many thanks,

Arnd





Comment 61 Arnd Baecker 2007-02-07 15:20:43 UTC
Created attachment 19576 [details]
patch to solve assigned tags view problem

With the kind help of Frank Siegert concerning the logic for
iterators in C++ the attached patch was created. It does solve #59 for me
(and I think #41 From Mikolaj Machowski).
Comment 62 caulier.gilles 2007-02-07 16:18:25 UTC
Mikolaj,

Can you test this patch and give me a feedback

Thanks in advance

Gilles
Comment 63 caulier.gilles 2007-02-07 22:00:45 UTC
SVN commit 631371 by cgilles:

digikam from trunk : patch from Arnd Baecker to solve assigned tags view problem

CCBUGS: 115157   

 M  +43 -1     imagedescedittab.cpp  


--- trunk/extragear/graphics/digikam/libs/imageproperties/imagedescedittab.cpp #631370:631371
@@ -2,7 +2,7 @@
  * Authors: Renchi Raju <renchi@pooh.tam.uiuc.edu>
  *          Caulier Gilles <caulier dot gilles at kdemail dot net>
  *          Marcel Wiesweg <marcel.wiesweg@gmx.de>
- * Date  : 2003-03-09
+ * Date   : 2003-03-09
  * Description : Comments, Tags, and Rating properties editor
  *
  * Copyright 2003-2005 by Renchi Raju & Gilles Caulier
@@ -1466,6 +1466,48 @@
         ++it;
     }
 
+    // correct visibilities afterwards:
+    // As QListViewItem::setVisible works recursively on all it's children
+    // we have to correct this
+    if (t)
+    {
+        it = d->tagsView;
+        while (it.current())
+        {
+            TAlbumCheckListItem* item = dynamic_cast<TAlbumCheckListItem*>(it.current());
+            TAlbum *tag               = item->m_album;
+            if (tag)
+            {
+                if (!tag->isRoot())
+                {
+                    // only if the current item is not marked as tagged, check all children 
+                    if (!item->isOn())
+                    {
+                        bool somethingIsSet         = false;
+                        QListViewItem* nextSibling  = (*it)->nextSibling();
+                        QListViewItemIterator tmpIt = it;
+                        ++tmpIt;
+                        while (*tmpIt != nextSibling )
+                        {
+                            TAlbumCheckListItem* tmpItem = dynamic_cast<TAlbumCheckListItem*>(tmpIt.current());
+                            TAlbum *tmpTag = tmpItem->m_album;
+                            if(tmpItem->isOn())
+                            {
+                                somethingIsSet = true;
+                            }
+                            ++tmpIt;
+                        }
+                        if (!somethingIsSet) 
+                        {
+                            item->setVisible(false);
+                        }
+                    }
+                }
+            }
+            ++it;
+        }
+    }
+
     TAlbum *root                  = AlbumManager::instance()->findTAlbum(0);
     TAlbumCheckListItem *rootItem = (TAlbumCheckListItem*)(root->extraData(this));
     if (rootItem)