Review Request 109893: Fixed a Dolphin 2.x Regression: no option to select whole group in grouped view

[Emmanuel Pescosta]

> On April 9, 2013, 6:34 a.m., Frank Reininghaus wrote:
> > Thanks for working on this! I appreciate that you work on re-implementing this lost feature. However (I might have said it earlier), I think that any plans to make large changes in the code should be discussed in advance on the mailing list. At least you should add a few explanations why you add new classes and change lots of things. Note that when I submit code to anyone, I always try to explain all design choices to make it easy for people to understand and review it. 
> > 
> > I see that you spent lots of time on this, and I really appreciate it, but I cannot review such a patch (much of which affects code that I'm not familiar with at all) at the moment. After having moved recently, I'm without Internet connection at home for at least another week, and I can't spend hours at work trying to figure out what all this is about, sorry. Maybe others can have a look and say what they think about it.

> I think that any plans to make large changes in the code should be discussed
When I started to implement this feature, I didn't know that this change will end in so much changed/new code.

Sorry for that.

But I hope that my comments help you a little bit ;)


- Emmanuel


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109893/#review30738
-----------------------------------------------------------


On April 7, 2013, 6:04 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109893/
> -----------------------------------------------------------
> 
> (Updated April 7, 2013, 6:04 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Fixed a Dolphin 2.x Regression: no option to select whole group in grouped view
> 
> Added the (de-)selector widget to the item list group-header, to (de-)select all items in the group.
> 
> This feature got lost in Dolphin 2.0
> 
> 
> This addresses bug 292508.
>     http://bugs.kde.org/show_bug.cgi?id=292508
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/private/dolphingraphicswidget.cpp PRE-CREATION 
>   dolphin/src/kitemviews/private/dolphingraphicswidget.h PRE-CREATION 
>   dolphin/src/kitemviews/kstandarditemlistwidget.cpp 6adb546 
>   dolphin/src/kitemviews/kstandarditemlistgroupheader.cpp 1e23c0a 
>   dolphin/src/kitemviews/kitemlistwidget.cpp 6a7111a 
>   dolphin/src/kitemviews/kitemlistwidget.h 55181fa 
>   dolphin/src/kitemviews/kitemlistview.cpp 9ebad7f 
>   dolphin/src/kitemviews/kitemlistview.h cd59ddc 
>   dolphin/src/kitemviews/kitemlistgroupheader.cpp 17c95a9 
>   dolphin/src/kitemviews/kitemlistgroupheader.h 1e8ed2c 
>   dolphin/src/kitemviews/kitemlistcontroller.cpp c6239df 
>   dolphin/src/CMakeLists.txt ffb232c 
>   dolphin/src/kitemviews/kitemlistcontroller.h 4d5fee3 
> 
> Diff: http://git.reviewboard.kde.org/r/109893/diff/
> 
> 
> Testing
> -------
> 
> Works fine. All tests passed.
> 
> * Enable grouping
> * Hover the group header
> * Click on the selector -> All items in this group are selected
> * Click on the selector again -> No item in this group is selected
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130409/e441a248/attachment.htm>