> 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 > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
