Review Request: Give more meaning to enabling/disabling accounts

[Thomas Richard]

> On Feb. 3, 2011, 10:48 p.m., David Edmundson wrote:
> > src/accounts-list-delegate.cpp, line 31
> > <http://git.reviewboard.kde.org/r/100537/diff/2/?file=8331#file8331line31>
> >
> >     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.

It worked because the tooltip would have been changed in the onCheckBoxToggled slot. I didn't know that updateItemWidgets was called on a check event. Fixed ;)


> On Feb. 3, 2011, 10:48 p.m., David Edmundson wrote:
> > src/accounts-list-delegate.cpp, line 115
> > <http://git.reviewboard.kde.org/r/100537/diff/2/?file=8331#file8331line115>
> >
> >     I think I would rather have setting this message in AccountItem::connectionStatusReason()

Makes sense. Done


> On Feb. 3, 2011, 10:48 p.m., David Edmundson wrote:
> > src/accounts-list-delegate.cpp, line 122
> > <http://git.reviewboard.kde.org/r/100537/diff/2/?file=8331#file8331line122>
> >
> >     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.
> >     
> >     
> >

This makes sense as well after reading this part again after a month :)


- Thomas


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


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/20110324/1aa7cdc1/attachment.htm