[Kde-pim] Review Request: KMail: Added per-folder aggregation and theme message list settings to folder dialog.

[Thomas McGuire]

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


Great, thanks! Looks good already, and the usability for this seems to be straightforward and easy.

One thing that needs adjustment before committing is the code duplication. Basically this code seems to be the same for most parts as in the configure dialog. Duplicate code is something we should always avoid, for several reasons.
So what about sharing the code instead? For example you could create a new class "AggregationComboBox" in the messagelistview/core subdirectory, that handles most of the stuff here, like loading the aggregations into the combobox. Then there should be a ThemeComboBox and maybe even a ThemeConfigButton and a AggregationConfigButton.

I'm wondering if the view settings for the folder should be in a separate tab, as there is now much stuff in the first tab.
Things for the view settings tab would be name, icons, sender/receiver column and the new aggregation/theme option.
But that would be something for later.

Also for later, it might be a good idea to get rid of the "Folder always uses this XYZ" checkbox altogether, as it is confusing, thanks to your settings, there is now a place to configure them.

- Thomas


On 2009-08-14 20:42:04, James Bendig wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1325/
> -----------------------------------------------------------
> 
> (Updated 2009-08-14 20:42:04)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Added folder specific aggregation and theme message list settings to the folder properties dialog.
> 
> The way the folder dialog message list default options are applied, functions differently from the existing context menus. When deselecting "Folder Always Uses This ..." in the context menus, the default option is over written with, what was, that folder's private option. This makes sense because of the limited atomic design of context menus.
> 
> The message list settings that I added to the folder dialog keeps all changes local to that dialog's folder. Checking the "Use default ..." checkbox and clicking the Ok button applies the default options as viewed in Settings->Configure KMail->Appearance->Message List.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/kmail/kmfolderdialog.h 1010998 
>   /trunk/KDE/kdepim/kmail/kmfolderdialog.cpp 1010998 
> 
> Diff: http://reviewboard.kde.org/r/1325/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> Folder Properties Dialog
>   http://reviewboard.kde.org/r/1325/s/180/
> 
> 
> Thanks,
> 
> James
> 
>

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