[Bug 115157] New: digikam useability: Image comments/tags dialog: hard to find/see all already selected tags (and to 'de'select them)

[Gilles Caulier-2]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=115157         




------- Additional Comments From caulier.gilles kdemail net  2007-01-08 20:51 -------
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;
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

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
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=115157         




------- Additional Comments From arnd.baecker web de  2007-01-09 09:10 -------
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
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=115157         




------- Additional Comments From arnd.baecker web de  2007-01-11 12:28 -------
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
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

> 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
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=115157         




------- Additional Comments From arnd.baecker web de  2007-01-11 12:42 -------
> 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
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

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



_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=115157         




------- Additional Comments From arnd.baecker web de  2007-01-11 13:57 -------
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
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

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

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=115157         




------- Additional Comments From arnd.baecker web de  2007-01-12 09:39 -------
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*.
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=115157         




------- Additional Comments From arnd.baecker web de  2007-02-05 09:13 -------
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
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Arnd Baecker]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=115157         




------- Additional Comments From arnd.baecker web de  2007-02-07 15:20 -------
Created an attachment (id=19576)
 --> (http://bugs.kde.org/attachment.cgi?id=19576&action=view)
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).
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-2]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=115157         




------- Additional Comments From caulier.gilles kdemail net  2007-02-07 16:18 -------
Mikolaj,

Can you test this patch and give me a feedback

Thanks in advance

Gilles
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

[Gilles Caulier-2]

------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
         
http://bugs.kde.org/show_bug.cgi?id=115157         




------- Additional Comments From caulier.gilles kdemail net  2007-02-07 22:00 -------
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)
_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel

> 16:18 ------- Mikolaj,
> Can you test this patch and give me a feedback
Yes, it solves #41. Thanks Arnd.

_______________________________________________
Digikam-devel mailing list
[hidden email]
https://mail.kde.org/mailman/listinfo/digikam-devel