Review Request: In Konqueror's session restore dialog, allow user to chose which sessions to restore

[Thomas Lübking]

> On Oct. 2, 2012, 8:57 a.m., David Faure wrote:
> > konqueror/src/konqsessionmanager.cpp, line 166
> > <http://git.reviewboard.kde.org/r/106503/diff/4/?file=88056#file88056line166>
> >
> >     Does this really make any difference? Widgets are transparent by default, in Qt4...

To be more aggressive: DO NOT DO THAT! NEVER!

Qt::transparent is Qt::black with zero alpha component, so whenever someone (UI Styles) test the the background color, they will get rubbish for this widget (unless you check the alpha component and conditionally treat this as invisible)

This approach showed up someone in the early KDE4 days and is since then the pestilence in visual KDE code (so eg. bespin during polishment tests widgets for doing this and tries to fix them up as good as possible)

use ::setAutoFillBackground(false) on ::viewport() but please DO NOT FREAK THE PALETTE, seriously ;-)

Another issue in this regard are several QItemViews, because they operate on hardcoded QPalette::Text, so if you want to make such view "transparent" you should not only viewport()->setAutoFillBackground(false) but also (and actually) will unfortunately have to "fix" the palette by aligning palette().color(QPalette::Text) and parentWidget().palette().color(QPalette::Window) - that includes filtering QEvent::Application/PaletteChange and re-fix the colors under this incident (smart approach is to check whether the color is already correct and not trigger another PaletteChange event in that case ;-)

For the very same manner, please do not abuse setStyleSheet() for being lazy to correct colors (now actually changing the palette, maybe even better: set[Back|Fore]groundRole()) - you'll end up with hardcoded colors (even if you select them at runtime) bypassing the style, and in 90% of all cases will break bright on dark palettes.


- Thomas


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


On Oct. 1, 2012, 8:56 p.m., Dawit Alemayehu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106503/
> -----------------------------------------------------------
> 
> (Updated Oct. 1, 2012, 8:56 p.m.)
> 
> 
> Review request for KDE Base Apps and David Faure.
> 
> 
> Description
> -------
> 
> The attached patch fixes one of those pet peeve bugs that infurate me from time to time by allowing me to unselect the sessions I do not want to be restored when Konqueror's restore session dialog pops up.
> 
> 
> This addresses bug 260282.
>     http://bugs.kde.org/show_bug.cgi?id=260282
> 
> 
> Diffs
> -----
> 
>   konqueror/src/konqsessionmanager.cpp 68a003f 
>   konqueror/src/konqsessionmanager.h ee629e4 
> 
> Diff: http://git.reviewboard.kde.org/r/106503/diff/
> 
> 
> Testing
> -------
> 
> * Unselected sessions should not be restored.
> * If all available sessions are selected (the default), the behavior should remain the same as it is today.
> * If all available sessions are unselected, disable the "Restore Session" button.
> 
> 
> Screenshots
> -----------
> 
> old restore dialog
>   http://git.reviewboard.kde.org/r/106503/s/729/
> new restore dialog
>   http://git.reviewboard.kde.org/r/106503/s/731/
> new restore dialog v2
>   http://git.reviewboard.kde.org/r/106503/s/739/
> new restore dialog v3
>   http://git.reviewboard.kde.org/r/106503/s/750/
> 
> 
> Thanks,
> 
> Dawit Alemayehu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121002/36ed2768/attachment.htm>