Review Request 113824: Refactor of Contact plasmoid

[Martin Klapetek]

> On Nov. 15, 2013, 10:14 a.m., Martin Klapetek wrote:
> > contact/src/contact-wrapper.cpp, line 184
> > <http://git.reviewboard.kde.org/r/113824/diff/3/?file=213859#file213859line184>
> >
> >     I don't think that's correct - account can be offline, in which case contact has presence status offline
> 
> Alexandr Akulich wrote:
>     You read my mind, but I just tested it again. Contacts presence remains the same after account disconnection. To test it added Text item to main.qml and does
>     text: TelepathyContact.presenceStatus + ", but isAccountOnline is " + TelepathyContact.isAccountOnline
>     It show, e.g. "away, but isAccountOnline is false". May be there is some bug in underlying components (I use git-master versions of ktp components, but telepathy-qt is only 0.9.3).
> 
> Alexandr Akulich wrote:
>     Just find http://community.kde.org/KTp/Tasks/KPeople . There is info about "bug in which some contacts seem to show as online, even though they are quite clearly not". May be it is the same issue.

I don't think so, KPeople models are slightly different, we're doing a wrapper around KTp::Contact here (KPeople model has a ton more layers above that). I haven't looked at the code but afair when account goes offline, KTp::Contact gets invalidated and receives no more updates, but I may be misremembering things. So basically we have to watch for KTp::Contact::isValid(). 

I'll do proper investigation later, but feel free to look into that.


> On Nov. 15, 2013, 10:14 a.m., Martin Klapetek wrote:
> > contact/src/contact-wrapper.cpp, lines 241-245
> > <http://git.reviewboard.kde.org/r/113824/diff/3/?file=213859#file213859line241>
> >
> >     Didn't you fix this in some previous patch?
> 
> Alexandr Akulich wrote:
>     This patch is refactor and next patch is implementation (https://git.reviewboard.kde.org/r/113832/ I'm surpriced, why there is "removed: Depends On: Refactor of Contact plasmoid" changed after diff r2 uploading).

Yeah, I got confused a bit :) All good.


- Martin


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


On Nov. 25, 2013, 11:34 a.m., Alexandr Akulich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113824/
> -----------------------------------------------------------
> 
> (Updated Nov. 25, 2013, 11:34 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-desktop-applets
> 
> 
> Description
> -------
> 
> TelepathyContact:
>     Removed qmlObject related stuff.
>     m_contact renamed to m_contactWrapper as it should be.
>     Setup context property before loading qml file.
> 
> ContactWrapper:
>     Properties placed there them should be.
>     Added property isContactOnline;
>     Added notifier-signals information for properties.
>     Property canStartVideo renamed to canStartVideoCall (like canStartAudioCall).
> 
>     undoAccountConnects() reimplemented to don't break connections possible maked outside.
>     Same about undoContactConnects().
> 
>     Methods reordered in cpp file to group simular methods together.
>     Implemented updateProperties(). In future it's possible to implement granulated properties updating.
>     Added checks for isAccountOnline in presenceStatus().
>     Fixed coding-style.
> 
> QML part:
>     Get rid a lot of functions, that anyway doesn't work properly.
>     Get rid a lot of useless properties.
>     Don't check avatarPresenceStatus for empty string and don't replace it with default one, because it's already done in c++ part.
>     Get rid a lot of "anchors.fill: parent" and such, because it's bad practice to let children position itself on parent.
>     Comminication buttons reordered like in KDE IM Contacts application.
> 
> Side effects:
>     Fixed losted after few went offline/online presenceStatus.
>     Fixed communication buttons remaining available after account becomes offline.
> 
> 
> Diffs
> -----
> 
>   contact/src/contact-wrapper.h 7b37907 
>   contact/src/contact-wrapper.cpp 5ef5919 
>   contact/src/declarative/contents/ui/Avatar.qml 6133563 
>   contact/src/declarative/contents/ui/Contact.qml ab434c8 
>   contact/src/declarative/contents/ui/DropDownMenu.qml 7177c70 
>   contact/src/declarative/contents/ui/main.qml debb0ed 
>   contact/src/telepathy-contact.h fe95c83 
>   contact/src/telepathy-contact.cpp 67c8a32 
> 
> Diff: http://git.reviewboard.kde.org/r/113824/diff/
> 
> 
> Testing
> -------
> 
> Plasmoid state properly response on account and/or contact online/offline changing. Contact capabilities showed correctly.
> 
> 
> Thanks,
> 
> Alexandr Akulich
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20131125/2373773a/attachment-0001.html>