Bug 141940

Am Monday 19 February 2007 schrieb Thomas McGuire:
[bugs.kde.org quoted mail]

Bug confirmed
Gerhard

And it's important. It must be fixed before 0.9.1-final release

Gilles

SVN commit 637131 by mwiesweg:

- Do not reset selection in slotUpdate.
  This means that the selection is preserved
	- during any resize operations (hiding sidebar)
	- while filtering out items with the tag filter 
	- when items have been changed from outside (AlbumIconView::refreshItems)
- change the misleading name d->firstVisibleItem to d->storedVisibleItem
  (it does not always point to the current first visible item - it is usually 0)

CCBUG:141940


 M  +21 -22    iconview.cpp  


--- trunk/extragear/graphics/digikam/digikam/iconview.cpp #637130:637131
@@ -65,7 +65,7 @@
         lastGroup           = 0;
         currItem            = 0;
         anchorItem          = 0;
-        firstVisibleItem    = 0;
+        storedVisibleItem   = 0;
         clearing            = false;
         spacing             = 10;
 
@@ -103,7 +103,7 @@
     IconItem*          toolTipItem;
     IconItem*          currItem;
     IconItem*          anchorItem;
-    IconItem*          firstVisibleItem; // store position for slotUpdate
+    IconItem*          storedVisibleItem; // store position for slotUpdate
     
     IconGroupItem*     firstGroup;
     IconGroupItem*     lastGroup;
@@ -403,7 +403,7 @@
         d->lastGroup         = group;
     }
 
-    d->firstVisibleItem = findFirstVisibleItem();
+    d->storedVisibleItem = findFirstVisibleItem();
     startUpdateTimer();
 }
 
@@ -415,7 +415,7 @@
     // this is only to find an alternative visible item if all visible items
     // are removed
     IconGroupItem *alternativeVisibleGroup = 0;
-    d->firstVisibleItem = 0;
+    d->storedVisibleItem = 0;
 
     if (group == d->firstGroup)
     {
@@ -454,11 +454,11 @@
 
     if (!d->clearing)
     {
-        d->firstVisibleItem = findFirstVisibleItem();
-        if (!d->firstVisibleItem && alternativeVisibleGroup)
+        d->storedVisibleItem = findFirstVisibleItem();
+        if (!d->storedVisibleItem && alternativeVisibleGroup)
         {
             // find an alternative visible item
-            d->firstVisibleItem = alternativeVisibleGroup->lastItem();
+            d->storedVisibleItem = alternativeVisibleGroup->lastItem();
         }
         startUpdateTimer();
     }
@@ -469,7 +469,7 @@
     if (!item)
         return;
 
-    d->firstVisibleItem = findFirstVisibleItem();
+    d->storedVisibleItem = findFirstVisibleItem();
     startUpdateTimer();
 }
 
@@ -509,16 +509,16 @@
     
     if (!d->clearing)
     {
-        d->firstVisibleItem = findFirstVisibleItem();
-        if (d->firstVisibleItem == item)
-            d->firstVisibleItem = d->currItem;
+        d->storedVisibleItem = findFirstVisibleItem();
+        if (d->storedVisibleItem == item)
+            d->storedVisibleItem = d->currItem;
         startUpdateTimer();
     }
 }
 
 void IconView::triggerUpdate()
 {
-    d->firstVisibleItem = findFirstVisibleItem();
+    d->storedVisibleItem = findFirstVisibleItem();
     startUpdateTimer();
 }
 
@@ -603,22 +603,21 @@
     }
     d->anchorItem = d->currItem;
 
-    if (d->currItem)
+    // ensure there is a selection
+    if (d->selectedItems.isEmpty())
     {
-        d->currItem->setSelected(true, true);
+        if (d->currItem)
+            d->currItem->setSelected(true, true);
+        else // no selection
+            emit signalSelectionChanged();
     }
-    else
-    {
-        // no selection
-        emit signalSelectionChanged();
-    }
 
     // set first visible item if they where stored before update was triggered
-    if (d->firstVisibleItem)
+    if (d->storedVisibleItem)
     {
-        ensureItemVisible(d->firstVisibleItem);
+        ensureItemVisible(d->storedVisibleItem);
         // reset to 0
-        d->firstVisibleItem = 0;
+        d->storedVisibleItem = 0;
     }
     else
     {

Point 2. is fixed by the commit.

Point 1 is not a bug IMO:
 - it is consistent with Konqueror's iconview behavior
 - the current item, which is opened in the image editor, is marked with a dashed black line

Thanks for fixing this.

>- while filtering out items with the tag filter 
Introduces a new bug:
Let's say you have two pictues, picture A and B. Picture A has a tag named "Test", picture B has no such tag. Now you select both pictures, then set the tag filter to show only pictures with the "Test" tag. Now, as expected, only picture A is shown. However, the statusbar still shows "2 items selected". 

I think pictures which are filtered away should be deselected. It can only cause problems having pictures which are selected but invisible.
However the other selections should still be preserved.

>Point 1 is not a bug IMO: 
>  - it is consistent with Konqueror's iconview behavior 
>  - the current item, which is opened in the image editor, is marked with a >dashed black line 
Ah, I've never noticed that dashed black line before. 
But think of the following situation: You have two pictures, A and B. A is selected (blue background), B is the current item (dashed black line).
Why does Image->Rotate work on the selected picture (A), while Image->Edit works on the current item (B) ? 

Maybe a solution to this would be that when selecting an image, it should become the current item. This way, it would not be possible that the current item is not selected (except when nothing is selected), eliminating such confusions like above.

SVN commit 637505 by mwiesweg:

Emit signalSelectionChanged if a selected item has been removed

CCBUG: 141940


 M  +9 -5      iconview.cpp  


--- trunk/extragear/graphics/digikam/digikam/iconview.cpp #637504:637505
@@ -490,6 +490,8 @@
 
     // Remove from selected item list
     d->selectedItems.remove(item);
+    if (item->isSelected())
+        d->needEmitSelectionChanged = true;
 
     if (d->toolTipItem == item)
     {
@@ -606,13 +608,15 @@
     d->anchorItem = d->currItem;
 
     // ensure there is a selection
-    if (d->selectedItems.isEmpty())
+    if (d->selectedItems.isEmpty() && d->currItem)
     {
-        if (d->currItem)
-            d->currItem->setSelected(true, true);
-        else // no selection
-            emit signalSelectionChanged();
+        d->currItem->setSelected(true, true);
     }
+    else if (d->needEmitSelectionChanged)
+    {
+        emit signalSelectionChanged();
+    }
+    d->needEmitSelectionChanged = false;
 
     // set first visible item if they where stored before update was triggered
     if (d->storedVisibleItem)

> Why does Image->Rotate work on the selected picture (A), while Image->Edit 
> works on the current item (B) ? 
 
Because Image->Rotate works on multiple images, and 0...n pictures can be selected. Image->Edit can only open one image as the current image, and one and only one image is the current image.

What you experience is only the corner case of rubber-band selecting only one image. I don't really mind to set the one selected image as the current image in this case, but on the other hand, it is not consistent behavior.

> > Why does Image->Rotate work on the selected picture (A), while
> > Image->Edit works on the current item (B) ?
>
> Because Image->Rotate works on multiple images, and 0...n pictures can be
> selected. Image->Edit can only open one image as the current image, and one
> and only one image is the current image.
>
> What you experience is only the corner case of rubber-band selecting only
> one image. I don't really mind to set the one selected image as the current
> image in this case, but on the other hand, it is not consistent behavior.

Well, the real inconsistence is that Edit and Rotate work on different 
pictures in the same situation. I understand the reasons for that (now), but 
the average user does not know that there is a difference between the current 
image and the selected image(s), and that some functions operate on the 
selected images, while others work with the current image. Until you 
explained this here, I found this totally irritating, especially because my 
method for editing a picture was selecting it and then pressing the edit 
toolbar button (which didn't work).

What do you think is not consistent behavior in my proposal?
Anyway, please implement this, it will save a lot of confusion for most users.

Hi Thomas,

I tried to find out the remaining open problem here, but
the present behaviour seems ok and intuitive to me.
Could you maybe summarize the open issue (with respect to current svn)
and your proposed solution?

Many thanks,

Arnd

P.S.: the only issue I see is that when F4 (edit) is pressed,
the current selection is lost.

>Could you maybe summarize the open issue (with respect to current svn) and your proposed solution? 
Ok, the open issue is the following: If I use the "click whitespace and then drag the selection" method to select one image, that image is NOT the current one, which is confusing for me (see description above).

Basically I want the following: If the user selects only one image with the drag method, please make that image also the current one (with a dashed rectangle around it).

I can not the test the current KDE4 SVN version, as it always crashes for me on startup (I'll write a bug report about that later).
I'm still using the 0.9.1 version.
 

Thomas,

Do not file a report about digiKam for KDE4. it still in alpha stage. Please wait until 0.10.0-beta1 is out. Thanks in advance

Gilles Caulier

Thomas,

What's news about this report ? Are you tested 0.9.2 or 0.9.3-beta3 ? Problem still exist ?

Gilles Caulier

>What's news about this report ? Are you tested 0.9.2 or 0.9.3-beta3 ? Problem still exist ? 
Sorry, but for some reason my SVN build fails at the moment, so I am still using 0.9.1

But unless someone changed it, problem problem 1) is still there:

Suggestions:
a) When using the mouse to select exactly one image, make that image also the current image (with the dotted rectangle). See the above discussion why.

b) When selecting multiple (>1) images, disable the actions which operate only on one image, as the user excepts it to work on all selected images. An example is the "Edit" action.

Not sure if I'm experiencing this problem as well, but I'll describe it anyway.

When I rotate one or a selection of images (walking through an album using cursor keys, selecting multiple using shift press with cursor keys), the ability to use the cursor keys sometimes disappears after the rotation is finished. Usually a simple mouseclick on the right thumbnail fixes the problem, which causes me to believe it is a focus setting problem, but one that isn't consistent.

If necessary, I can file a separate bugreport...

/Simon

What's news about this report ? it still valid using digiKam 0.9.4 or 0.10.0 ?

Gilles Caulier

Andi,

A lots of patches have been apply with this entry. Is the original problem is fixed now ?

Gilles

I can confirm bug 1 for 0.9.5, although it doesn't seem to be a real bug. But I also think the rubberband behavior feels wrong, if I highlight an image I want this to be active, not the one I previously clicked on.

Andi

1 exists for 0.9.5svn and it *is* real bug. Not operating on image not pointed by user is grave bug.

It also exists for selecting images by clicking: click on image A and B holding Ctrl pressed to select both and click once more on B to deselect it. Edit will open B in IE.

> What's news about this report ? it still valid using digiKam 0.9.4 or 0.10.0 ?

It is still valid in 0.10.0 beta 7. At least point 1 of this report.

Any news with digiKam 1.0.0-betaX here?
We switched to Qt4 model view, so it might be solved...

Should we close this as WAITINGFORINFO as there is no feedback since one year?

Thomas,

We need some fresh feedback with digiKam 2.x serie...

Gilles Caulier

New digiKam 4.11.0 is available.

https://www.digikam.org/node/740

Can you reproduce the problem with this release ?

Gilles Caulier

With digiKam 5.0.0  this problem is not reproducible.
I close this file now. Don't hesitate to re-open if necessary.
Gilles Caulier