Review Request: Fix Bug 304643 - selected place looks ugly and incomplete

[Frank Reininghaus]

> On Oct. 15, 2012, 3:43 p.m., Frank Reininghaus wrote:
> > First of all, thanks for the patch and the screenshots!
> > 
> > It should be added that the "new" look would match the look of KFilePlacesView, i.e., the look of Dolphin's Places Panel in KDE <= 4.8. In particular, it makes sure that the red color of the "Root" place is preserved (the user who reported this considered the color change distracting, and I agree that it can be considered a bit odd).
> > 
> > However, when we change this, the behaviour of the Places Panel would be different from the one of the DolphinView. This is most obvious when using Details View. I'm not really sure what the best solution is. There are several options:
> > 
> > a) Apply the patch as it is.
> > b) Only remove the color change, and do not extend the selection rectangle.
> > c) Don't change anything.
> > d) Like a), but do the same change in the DolphinView.
> > e) Like b), but do the same change in the DolphinView.
> > 
> > I'm not entirely sure what the 'right' approach is, but considering that the wish report did not get much feedback, it's not clear if it's really worth adding extra complexity to implement this.
> > 
> > Any other opinions? Do we know how other file managers do it (I don't have any others to test here right now)?
> >
> 
> Emmanuel Pescosta wrote:
>     I implemented option d.) So you can decide what looks better, the old or the new Dolphin look. ;) 
>     
>     I hope you and the other Dolphin users like it.
> 
> Frank Reininghaus wrote:
>     Thanks for the new patch, Emmanuel!
>     
>     I'm still not quite sure what look is best :-/
> 
> Emmanuel Pescosta wrote:
>     Should I push or discard my patch? ;) (What if we ask the other people on planetkde or on the mailing-list?)
> 
> Frank Reininghaus wrote:
>     Well, the people who read the mailing list read this anyway :-) I'm not sure if asking on Planet KDE would really lead to a productive discussion. What I would do first is to check how other file managers handle it and then think about the pro and con arguments for the different possibilites. But I don't have any non-Dolphin file managers here at the moment, and I have lots of other things to worry about :-(
>     
>     Moreover, it seems that there is no really big demand to change it at the moment (wishlist report by a single user with 1 vote), so I'd prefer to not make the code more complex in order to change it at the moment, sorry! In any case, you should add a link to this review request to the bug report, such that it does not get forgotten if the issue pops up again at some later point.
> 
> Emmanuel Pescosta wrote:
>     Finder, Nautilus, PCManFM, ... do it this way ;)
>     
>     http://lxde.org/sites/default/files/images/pcmanfm.png
>     http://osxhelp.com/wp-content/uploads/2008/03/finder-default-sidebar.png
>     
>     > Moreover, it seems that there is no really big demand to change it at the moment
>     Yes that is true.
> 
> Christoph Feck wrote:
>     Since you asked for feedback: I have locally patched Dolphin to always highlight the complete bounding rect (icon and text), for all modes, to get rid of the ugly coloring :P
>     
>     So whatever can be done "officially" to resolve the issue is welcomed. I understand that for regular Icon Mode, there would be issues with my patch.

Thanks for the screenshots, Emmanuel, and for your feedback, Christoph!

To be honest, before Peter stepped down, I mostly worked on code in the 'background' and never thought much about painting-related things. Therefore, I usually do not have strong opinions about these issues. But I know that Peter put a lot of work and thought into these things and discussed some aspects with designers, so I'm never quite sure if some particular thing has been done on purpose because there was a good reason for it. This is why I try to avoid changing design-related things, unless I see that there is a very good reason to do it. But I see that this is not a good long-term solution.

Maybe we should add some people with design knowledge to this review request (Nuno comes to my mind)?


- Frank


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


On Oct. 17, 2012, 7:42 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106827/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2012, 7:42 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Fix Bug 304643 - selected place looks ugly and incomplete
> 
> 
> This addresses bug 304643.
>     http://bugs.kde.org/show_bug.cgi?id=304643
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kitemlistwidget.h 55181fa 
>   dolphin/src/kitemviews/kitemlistwidget.cpp 6a7111a 
>   dolphin/src/kitemviews/kstandarditemlistwidget.h 787722d 
>   dolphin/src/kitemviews/kstandarditemlistwidget.cpp 72d10cf 
>   dolphin/src/panels/folders/foldersitemlistwidget.cpp b4f9a5b 
>   dolphin/src/panels/places/placesitemlistwidget.h a2a88c1 
>   dolphin/src/panels/places/placesitemlistwidget.cpp e33d1da 
> 
> Diff: http://git.reviewboard.kde.org/r/106827/diff/
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> Old and new Dolphin Places look
>   http://git.reviewboard.kde.org/r/106827/s/779/
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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