Review Request: Possibility to set presence message per account

[Martin Klapetek]

> On April 2, 2011, 7:32 p.m., David Edmundson wrote:
> > account-button.cpp, line 130
> > <http://git.reviewboard.kde.org/r/101011/diff/1/?file=13699#file13699line130>
> >
> >     should this code not happen in presenceChanged too? 
> >     I'm seeing this as trying to handle not-listed presence states?
> >     
> >

I always thought that it's not a problem because there can't be set a non-listed/non-existing presence state, but now that you pointed it out, yes, it should be there as well. Though in that case there is a bigger issue - shouldn't the account itself be reset to some known presence then? For example offline?


> On April 2, 2011, 7:32 p.m., David Edmundson wrote:
> > account-button.cpp, line 132
> > <http://git.reviewboard.kde.org/r/101011/diff/1/?file=13699#file13699line132>
> >
> >     This icon generation really needs to happen in presenceChanged too.
> >     
> >     Suggestion, change this code to be:
> >     
> >     foreach(QAction *a, actions())  {
> >      setCheckable(true);
> >     }
> >     
> >     onPresenceChnaged(m_account->currentPresence());

Oops, my bad. I forgot to add it there as well. For the suggestion - ok.


> On April 2, 2011, 7:32 p.m., David Edmundson wrote:
> > account-button.cpp, line 140
> > <http://git.reviewboard.kde.org/r/101011/diff/1/?file=13699#file13699line140>
> >
> >     Is this actually true/working?
> >     
> >     actions().last() is now be the presenceMessageAction.
> >     
> >     also I don't really like duplicating code.
> >     
> >     suggestion:
> >     presenceChanged(Presence::offline());
> >     
> >     maybe?

Err, you are correct. I forgot that as well. I'm like decaffeinated today :D


- Martin


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


On April 2, 2011, 6:13 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101011/
> -----------------------------------------------------------
> 
> (Updated April 2, 2011, 6:13 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> Add a possibility to set a different status message per account. If the status message is changed from CL's main status line edit, this gets changed as well. Plus some minor fixes/improvements in AccountButton.
> 
> 
> Diffs
> -----
> 
>   account-button.h c5ac84a5ea53a1ec534efe6580e6cbf321c2e599 
>   account-button.cpp 7a5f27d7ddc459c65d1c97d11c7e946343dc33c2 
> 
> Diff: http://git.reviewboard.kde.org/r/101011/diff
> 
> 
> Testing
> -------
> 
> I set status in the new klineedit, user see it. I overwrite this with the main klineedit presence and users see it again.
> 
> 
> Thanks,
> 
> Martin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110402/4bdbd64a/attachment-0001.htm