[Kde-pim] Review Request: Junior Job complete: "Re-add ability to hide quick search"

[Thomas McGuire]

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

Ship it!


Thank you again for working on the patch, I committed it now. See http://websvn.kde.org/?view=rev&revision=1149628 for details.
I made some minor changes to the patch:
- const correctness here and there
- make the KAction a KToggleAction and change the action name, so that it is shown nicely in the menubar
- remember the visibilty in the settings

About the QWidget and hiding of the children:
Each QWidget has a layout, and the layout can have an arbitrary amount of child widgets in it. If you hide the widget, all child widgets in the layout will also be hidden. Thus my suggestion to use an QWidget as a container widget, which would hold the 3 widgets of the quick search bar in its layout. In the end I didn't implement it, saving 3 lines of code was not worth the effort.

- Thomas


On 2010-07-13 13:17:52, Richard Plangger wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4382/
> -----------------------------------------------------------
> 
> (Updated 2010-07-13 13:17:52)
> 
> 
> Review request for KDE PIM and Thomas McGuire.
> 
> 
> Summary
> -------
> 
> Junior Job "Re-add ability to hide quick search" (http://techbase.kde.org/Projects/PIM/KMail_Junior_Jobs#KMail_Junior_Jobs)
> 
> CTRL + H shows hides the searchbar, combobox and toolbutton!
> 
> This is my fist KDE patch, advice & criticism appreciated!
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepim/messagelist/core/widgetbase.h 1149337 
>   trunk/KDE/kdepim/messagelist/core/widgetbase.cpp 1149337 
>   trunk/KDE/kdepim/messagelist/widget.cpp 1149337 
> 
> Diff: http://reviewboard.kde.org/r/4382/diff
> 
> 
> Testing
> -------
> 
> Tested on local machine
> 
> 
> Screenshots
> -----------
> 
> before hitting ctrl + h
>   http://reviewboard.kde.org/r/4382/s/436/
> after
>   http://reviewboard.kde.org/r/4382/s/437/
> 
> 
> Thanks,
> 
> Richard
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/