[Kde-pim] Review Request: Change the status filter with a QComboBox

[Thomas McGuire]

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


Thanks, this is a good start.

However, this change alone can't go in, as we need the other changes that move the aggragation, theme and sorting buttons away. Otherwise there is not enough space in the layout for a big combobox.

I've added some inline comments about some smaller issues.


trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.h
<http://reviewboard.kde.org/r/820/#comment823>

    This should be removed.



trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.h
<http://reviewboard.kde.org/r/820/#comment826>

    Should use a KComboBox here



trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.h
<http://reviewboard.kde.org/r/820/#comment824>

    Please add some API documentation what this does, as it is now a public method



trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.cpp
<http://reviewboard.kde.org/r/820/#comment827>

    Remove the commented out code, no need to keep it around



trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.cpp
<http://reviewboard.kde.org/r/820/#comment828>

    The combo box can also be enabled or disabled here.



trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.cpp
<http://reviewboard.kde.org/r/820/#comment829>

    Please remove the old satusSelected. This also makes the diff more readable, since then we can see the actual changes.
    
    Same for setupStatusFilter()


- Thomas


On 2009-06-10 12:12:13, Vincent Dupont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/820/
> -----------------------------------------------------------
> 
> (Updated 2009-06-10 12:12:13)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> It replaces the QToolBox status filter by a QComboBox.
> I think improvements are needed but it's a first try.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.h 936945 
>   trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.cpp 954221 
> 
> Diff: http://reviewboard.kde.org/r/820/diff
> 
> 
> Testing
> -------
> 
> Changing status filter.
> 
> 
> Thanks,
> 
> Vincent
> 
>

_______________________________________________
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/