Review Request: Give more meaning to enabling/disabling accounts

David Edmundson kde at davidedmundson.co.uk
Thu Feb 3 23:48:20 CET 2011


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



src/accounts-list-delegate.cpp
<http://git.reviewboard.kde.org/r/100537/#comment990>

    What if it starts off enabled?
    
    setTooltip should be done in updateItemWidgets. This is called before they are shown and gives you access to whether it is checked or not.



src/accounts-list-delegate.cpp
<http://git.reviewboard.kde.org/r/100537/#comment989>

    I think I would rather have setting this message in AccountItem::connectionStatusReason()



src/accounts-list-delegate.cpp
<http://git.reviewboard.kde.org/r/100537/#comment991>

    It took me 3 attempts to read through this section. It all appears to work, but it might have been clearer to have:
    
    QFont accountNameFont
    QFont statusTextFont
    
    and to always 
    painter->setFont before drawing each part.
    
    
    


- David


On Feb. 3, 2011, 9:09 p.m., Thomas Richard wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100537/
> -----------------------------------------------------------
> 
> (Updated Feb. 3, 2011, 9:09 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> Adds a tooltip to the checkbox and makes the account text italic/gray when disabled
> 
> 
> Diffs
> -----
> 
>   src/accounts-list-delegate.cpp fb0afbd872f928118fb71598476e34f664f22e80 
> 
> Diff: http://git.reviewboard.kde.org/r/100537/diff
> 
> 
> Testing
> -------
> 
> Enabling/disabling accounts
> 
> 
> Screenshots
> -----------
> 
> 
>   http://git.reviewboard.kde.org/r/100537/s/67/
> 
> 
> Thanks,
> 
> Thomas
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110203/3bc7f245/attachment-0001.htm 


More information about the KDE-Telepathy mailing list