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

_______________________________________________
KDE-Telepathy mailing list
[email protected]
https://mail.kde.org/mailman/listinfo/kde-telepathy

Reply via email to