[Kde-pim] Review Request: Expandable/Collapsible "To" and "Cc" fields in KMail

[Thomas McGuire]

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

Ship it!


Ok, looks good.
I found some minor things still, but no need for an additional review, just fix those.

BTW, I assume you renamed the icon on the file system as well, right? I guess the diff doesn't show that.


/trunk/KDE/kdepim/kmail/headerstyle.cpp
<http://reviewboard.kde.org/r/1492/#comment1560>

    I think this should be QString() instead of "", to make Krazy happy.



/trunk/KDE/kdepim/kmail/kmreaderwin.h
<http://reviewboard.kde.org/r/1492/#comment1562>

    API doc missing



/trunk/KDE/kdepim/kmail/kmreaderwin.h
<http://reviewboard.kde.org/r/1492/#comment1561>

    Spaces missing :)
    And API doc.



/trunk/KDE/kdepim/kmail/kmreaderwin.h
<http://reviewboard.kde.org/r/1492/#comment1563>

    Is this used anywhere?
    Also, from the name it sounds like a bool, but it is an int...



/trunk/KDE/kdepim/kmail/kmreaderwin.cpp
<http://reviewboard.kde.org/r/1492/#comment1564>

    Add a comment to explain why a singleshot timer is used :)



/trunk/KDE/kdepim/kmail/stringutil.cpp
<http://reviewboard.kde.org/r/1492/#comment1565>

    Please make this an entry in kmail.kcfg.cmake, so you can use GlobalSettings() here.
    
    New KMail code should only use KConfigXT, not manual readEntry calls.


- Thomas


On 2009-09-07 16:44:53, Torgny Nyblom wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1492/
> -----------------------------------------------------------
> 
> (Updated 2009-09-07 16:44:53)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Makes the "To" and "Cc" fields collapsible if more then three addresses are present in the respective field.
> Image is borrowed from the same function for attachments, but code is slightly different.
> 
> As of now this is just enabled for "Fancy headers" but the concept is easily expanded into all kinds of header styles.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/kmail/pics/attachmentQuicklistClosed.png UNKNOWN 
>   /trunk/KDE/kdepim/kmail/pics/attachmentQuicklistOpened.png UNKNOWN 
>   /trunk/KDE/kdepim/kmail/stringutil.h 1020776 
>   /trunk/KDE/kdepim/kmail/stringutil.cpp 1020776 
>   /trunk/KDE/kdepim/kmail/urlhandlermanager.cpp 1020776 
>   /trunk/KDE/kdepim/kmail/kmreaderwin.h 1020776 
>   /trunk/KDE/kdepim/kmail/kmreaderwin.cpp 1020776 
>   /trunk/KDE/kdepim/kmail/headerstyle.cpp 1020776 
> 
> Diff: http://reviewboard.kde.org/r/1492/diff
> 
> 
> Testing
> -------
> 
> Built, opened test email in diffent header styles and expanded/collapsed the headers.
> 
> 
> Screenshots
> -----------
> 
> Collapsed
>   http://reviewboard.kde.org/r/1492/s/195/
> One expanded, one collapsed
>   http://reviewboard.kde.org/r/1492/s/196/
> Expanded
>   http://reviewboard.kde.org/r/1492/s/197/
> 
> 
> Thanks,
> 
> Torgny
> 
>

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