Review Request: Make it possible to start conversations from the Quick Chat plasmoid

[Aleix Pol Gonzalez]

> On Jan. 3, 2013, 9:47 a.m., David Edmundson wrote:
> > plasmoid/declarative-plugin/pinnedcontactsmodel.cpp, line 81
> > <http://git.reviewboard.kde.org/r/107811/diff/3/?file=103934#file103934line81>
> >
> >     Argh!
> >     
> >     We now have two account factories in the same application. One from here, and one in the client registrar.
> >     
> >     If you have two account factories, you can end up with different Tp::Account* objects representing the same account, such that account1 != account2 even though account1->uniqueIdentifier() == account2->uniqueIdentifier();
> >     
> >     I guess you may have figured this out because you have the line:
> >     "if p.account->uniqueIdentifer == account->uni..." rather than the more obvious  "if p.account=account"
> >     
> >     This isn't a situation I want to be in. Is there a way to share the factories (or the AccountManager) between here and the TelepathyTextObserver?
> >     It may need a bit more moving code about, but it'll be worth it.
> >     
> >

Yes, I'll do that. I didn't do it yet because I'll be changing code because to me it sounded like a big "let's use a singleton" use-case, but probably it's something that has to happen in the declarative plugin side.


> On Jan. 3, 2013, 9:47 a.m., David Edmundson wrote:
> > plasmoid/declarative-plugin/pinnedcontactsmodel.cpp, line 99
> > <http://git.reviewboard.kde.org/r/107811/diff/3/?file=103934#file103934line99>
> >
> >     what file does this write into?
> >     
> >     AFAIK plasma has a special config group you're meant to use.

Yes, it's something to be fixed. I didn't do it because it is harder and I wanted to know what people thought about the new UI.


> On Jan. 3, 2013, 9:47 a.m., David Edmundson wrote:
> > plasmoid/declarative-plugin/pinnedcontactsmodel.cpp, line 126
> > <http://git.reviewboard.kde.org/r/107811/diff/3/?file=103934#file103934line126>
> >
> >     if the pending operation fails to find the contact (like if you've pinned a contact you've now removed) you get a crash.
> >

Yes, I didn't remove it because I have some odd behaviors there, where sometimes I get an error that I don't understand.

But yes, definitely.


- Aleix


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


On Jan. 2, 2013, 5:34 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107811/
> -----------------------------------------------------------
> 
> (Updated Jan. 2, 2013, 5:34 p.m.)
> 
> 
> Review request for Telepathy and David Edmundson.
> 
> 
> Description
> -------
> 
> I've wanted to use this plasmoid for longtime, I fear that the reason why I'm not using it yet is because I can't start chats from there. The plan is that this will improve the situation, to some extent.
> 
> The important part about the patch is that the root element is refactored into a grid that can have different elements. Now I added a button with a contactlist that probably should go, but eventually i'd like to have non-conversation buttons so that one can pin contacts.
> 
> 
> Diffs
> -----
> 
>   plasmoid/declarative-plugin/CMakeLists.txt 48ba8a7 
>   plasmoid/declarative-plugin/contactpin.h PRE-CREATION 
>   plasmoid/declarative-plugin/contactpin.cpp PRE-CREATION 
>   plasmoid/declarative-plugin/conversation-target.h cd45f2d 
>   plasmoid/declarative-plugin/conversation-target.cpp f9c285d 
>   plasmoid/declarative-plugin/conversation.h 6eeab86 
>   plasmoid/declarative-plugin/conversation.cpp 152d940 
>   plasmoid/declarative-plugin/conversations-model.h f9dc047 
>   plasmoid/declarative-plugin/conversations-model.cpp faaa60b 
>   plasmoid/declarative-plugin/ktp-metatypes.h PRE-CREATION 
>   plasmoid/declarative-plugin/pinnedcontactsmodel.h PRE-CREATION 
>   plasmoid/declarative-plugin/pinnedcontactsmodel.cpp PRE-CREATION 
>   plasmoid/declarative-plugin/qml-plugins.cpp 23a4291 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ChatWidget.qml ea68f41 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ContactList.qml PRE-CREATION 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ConversationDelegate.qml 8a8d851 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/ConversationDelegateButton.qml PRE-CREATION 
>   plasmoid/org.kde.ktp-chatplasmoid/contents/ui/main.qml feb766b 
> 
> Diff: http://git.reviewboard.kde.org/r/107811/diff/
> 
> 
> Testing
> -------
> 
> Very little, mostly sending this review for starting a discussion on where we'd like to go with this plasmoid.
> 
> 
> Screenshots
> -----------
> 
> i am pinned
>   http://git.reviewboard.kde.org/r/107811/s/952/
> pin me
>   http://git.reviewboard.kde.org/r/107811/s/953/
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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