Review Request: Bugfix for Bug 303133 - wrong text color in places

[Emmanuel Pescosta]

> On Aug. 4, 2012, 6:56 a.m., Frank Reininghaus wrote:
> > Thanks for the patch! Looks very good overall! Just two little ideas how you could make it even better:
> > 
> > 1. Maybe it might be better to derive your new class FoldersItemListWidget from KFileItemListWidget rather than KStandardItemListWidget? I think that symbolic links won't be shown in italics otherwise.
> > 
> > 2. A possibility to reduce code duplication and make the code more future-proof: rather than making KStandardItemListWidget::textColor() virtual and overriding it in derived classes, one could make a small change in KStandardItemListWidget::textColor(): replace QPalette::Text by a call to a new virtual function normalTextColor(), make KStandardItemListWidget::normalTextColor() return QPalette::Text and let it return QPalette::WindowText in the derived classes. This could have two small advantages:
> > 
> > a) If any change in the 3 code lines that determine the color is required, we only have to change it in KStandardItemListWidget::textColor(), rather than in three different places.
> > b) We might decide at some point that setting a custom text color for some items in the panels might be necessary.
> > 
> > But these are just ideas - if you think that there are good reasons to not do it this way, just let me know.

1. Oh okay. I will change it.
2. Your idea with KStandardItemListWidget::normalTextColor() is a much better solution, I will update my patch in the afternoon ;) 

Thank you for your suggestions :)  


- Emmanuel


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


On Aug. 2, 2012, 9:05 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105832/
> -----------------------------------------------------------
> 
> (Updated Aug. 2, 2012, 9:05 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Fixes wrong text color in places and in folders panel.
> 
> 
> This addresses bug 303133.
>     http://bugs.kde.org/show_bug.cgi?id=303133
> 
> 
> Diffs
> -----
> 
>   dolphin/src/panels/folders/foldersitemlistwidget.cpp PRE-CREATION 
>   dolphin/src/panels/folders/folderspanel.cpp 78e437a 
>   dolphin/src/panels/places/placesitemlistwidget.h d7a4f3d 
>   dolphin/src/panels/places/placesitemlistwidget.cpp 24c2b3f 
>   dolphin/src/CMakeLists.txt 5c1a6da 
>   dolphin/src/kitemviews/kstandarditemlistwidget.h 82b68ea 
>   dolphin/src/panels/folders/foldersitemlistwidget.h PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105832/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

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