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

[Thomas McGuire]

> On 2009-06-11 09:27:11, Thomas McGuire wrote:
> > 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.
> 
> Vincent Dupont wrote:
>     The combobox is the first step, I will work on move the three other toolbox.
>     
>     I've a problem with the combobox:
>     The status filter was managed by a ToolBox so there was a QActionGroup and a Menu. But the combobox doesn't use Qactions nor menu.
>     Do you know how I can move easily all from the QActionGroup to the combobox ? Because tag filtering doesn't work with the patch.

Hi! You need to refactor the code like fillMessageTagMenu() menu. One possible way would be to pass the combo box as a parameter to fillMessageTagMenu() and let fillMessageTagMenu() add the items itself.

For the aggregations, themes and sort orders is something different: There, we want to display it in a menu ("View" in the main menu) and in a combobox (for the default/global settings that should go into the configure dialog). Maybe you could also write a method that takes a qmenu and adds its entries to a combobox, so the old code can be used for both a combobox and a menu. Pity that a QMenu doesn't support model/view, otherwise we could use that.


> On 2009-06-11 09:27:11, Thomas McGuire wrote:
> > trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.cpp, line 1113
> > <http://reviewboard.kde.org/r/820/diff/1/?file=7077#file7077line1113>
> >
> >     Please remove the old satusSelected. This also makes the diff more readable, since then we can see the actual changes.
> >     
> >     Same for setupStatusFilter()
> 
>  wrote:
>     I think I've corrected problems you pointed out. I hope it's better.

Oh sorry, indeed, I didn't see that it is protected. It should stay protected, not public.


- Thomas


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


On 2009-06-15 08:43:08, Vincent Dupont wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/820/
> -----------------------------------------------------------
> 
> (Updated 2009-06-15 08:43:08)
> 
> 
> 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/