[Kde-pim] Review Request: Add message list sort order, aggregation, and theme default settings to configure dialog.

[Thomas McGuire]

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

Ship it!


Thinking about a bit more, I think it is best to keep the sorting out of the configure dialog.
The code is complex, and the options are also complex for the users, especially given that the available modes depend on the aggregation.

What do you think?
I realize that you put much work into that, sorry :(


/trunk/KDE/kdepim/kmail/configuredialog.cpp
<http://reviewboard.kde.org/r/1225/#comment1270>

    Use KPushButton instead



/trunk/KDE/kdepim/kmail/configuredialog.cpp
<http://reviewboard.kde.org/r/1225/#comment1271>

    I think this string looks strange, either only "Appearance" or only "Theme". Same for the View menu.



/trunk/KDE/kdepim/kmail/configuredialog.cpp
<http://reviewboard.kde.org/r/1225/#comment1272>

    KPushButton



/trunk/KDE/kdepim/kmail/configuredialog.cpp
<http://reviewboard.kde.org/r/1225/#comment1273>

    Passing 0 here means the window has no proper parent, although it should have one.
    
    I guess a way around this would be to change the showConfigureAggregationsDialog() function to accept a QWidget * as first parameter, and change all callers.



/trunk/KDE/kdepim/kmail/configuredialog.cpp
<http://reviewboard.kde.org/r/1225/#comment1274>

    Same here: No 0 parent if possible



/trunk/KDE/kdepim/kmail/configuredialog.cpp
<http://reviewboard.kde.org/r/1225/#comment1275>

    can be const, e.g. "const Aggregation *aggregation"



/trunk/KDE/kdepim/kmail/configuredialog.cpp
<http://reviewboard.kde.org/r/1225/#comment1276>

    const here as well.



/trunk/KDE/kdepim/kmail/messagelistview/core/manager.cpp
<http://reviewboard.kde.org/r/1225/#comment1269>

    use foreach instead, easier and more readable


- Thomas


On 2009-08-04 20:59:35, James Bendig wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1225/
> -----------------------------------------------------------
> 
> (Updated 2009-08-04 20:59:35)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> This is a follow up patch to review 1154. (http://reviewboard.kde.org/r/1154/)
> 
> I added settings to Configure->Appearance->Message List for changing the default sort order, aggregation, and theme.
> 
> Is there a more preferred way to forcing message list to refresh itself after configure dialog saves changes?
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/kmail/configuredialog.cpp 1006958 
>   /trunk/KDE/kdepim/kmail/configuredialog_p.h 1006958 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/manager.h 1006958 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/manager.cpp 1006958 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.h 1006958 
>   /trunk/KDE/kdepim/kmail/messagelistview/core/widgetbase.cpp 1006958 
> 
> Diff: http://reviewboard.kde.org/r/1225/diff
> 
> 
> Testing
> -------
> 
> 
> Screenshots
> -----------
> 
> Sort Order, Aggregation, and Theme settings.
>   http://reviewboard.kde.org/r/1225/s/166/
> 
> 
> 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/