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

Vincent Dupont vincent.touffi at gmail.com
Mon Jun 15 16:53:50 BST 2009



> 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.

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.


> 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()

I think I've corrected problems you pointed out. I hope it's better.


> On 2009-06-11 09:27:11, Thomas McGuire wrote:
> > trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.h, line 222
> > <http://reviewboard.kde.org/r/820/diff/1/?file=7076#file7076line222>
> >
> >     Please add some API documentation what this does, as it is now a public method

I added some documentation, but it's a protected method, should I put it public ?


- Vincent


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



More information about the kde-pim mailing list