Bug 292882

We should add features because they're useful and you find yourself wanting to have this feature NEVER simply because Kopete has them.

I definitely don't think we need everything in that Kopete screenshot. 

With regards to the two items explicitly mentioned, I like the idea of doing them from directly. The plasma contact list /may/ be the more logical place once that gets going. I'm not sure. 

It might be worth doing them as individual bug reports. This current bug is too vague to ever be closed as "Fixed", because there's always more features that could be added.

I think it is worth to have them in the presence applet, since (at least for the 2 items mentioned) doesn't completely fit the contact list, since they are not actions on the contacts (it is probably worth to have them in both places)

If you want to join a chat room it might not have sense to start the contact list, same for adding a contact (for example, I receive an e-mail with a contact that I just want to add, but I'm not interested in chatting with him right now).

Anyway I'm changing this bug to Add "Add Contact" button to the presence applet,
and I'll clone this for the "Join chat room"

Marking as a junior job since this can be easily done, we just need to move  the add contact dialog in ktp-common-internals/Widgets and then use it in contact list and in the presence applet...

Created attachment 69824 [details]
first patch

Created attachment 69825 [details]
second patch

Begin work on this bug.

Patch one move the add contact dialog from ktp-contact-list to ktp-common-internals\Widgets.
Patch two rename the add contact dialog as add contact widget. (not sure that it should be)

Can you say me the right name for this class?

Mikhail

I don't understand why you've renamed it from Dialog to Widget. 

Especially when the class inherits from KDialog.

Oops. I'm confused that contact grid widget is placed in ktp-common-internals/Widgets. He is named so because he inherits from QWidget, not because he placed in widgets library. I understood my mistake. 

Just ignore second patch.

Mikhail

Created attachment 69830 [details]
ktp-contact-list patch 1

Updating ktp-contact-list main widget to use ktp-common-internals/Widgets add contact dialog instead of his own add contact dialog.

Created attachment 69831 [details]
ktp-contact-list patch 2

Removing ktp-contact-list/dialogs add contact dialog since ktp-contact-list isn't used it.

Created attachment 69832 [details]
ktp-presence-applet patch

Adding ktp-common-internals/Widgets add contact dialod button.

P.S.: It's okey how I'm posting patches or I should use git post-review command in future instead?

(In reply to comment #11)
> P.S.: It's okey how I'm posting patches or I should use git post-review
> command in future instead?

Reviewing patch on bugzilla is not easy and boring, we have to download the patch, apply it, make a lot of copy and paste to point you at the problems with the patch. And programmers are usually lazy.

If you want your patch reviewed you should use reviewboard (read this blog post by david http://www.sharpley.org.uk/node/30 )

For complicated patches another option to allow us to review the single commits is to have you git repository clone, push your changes there and add to the reviewboard your git repository url (if you have a kde identity account see http://community.kde.org/Sysadmin/GitKdeOrgManual#Personal_clones_of_project_repositories )

If you are working on this bug you should also assign this to yourself, and mark it as assigned, so that the others know that you are doing this and won't do it :)
Actually we usually forget about this, but we shouldn't ;)

in the presence applet you've called the variable "m_addContactAction"

we use "m_" to say "member variable". This isn't one. 

In your patch the presence applet now has an accounts model, which means the applet keeps track of every contacts name, avatar, capabilities etc.. all the time and it doesn't ever show them.

Create it only when you show the add-contact dialog, and that way it won't be in memory and updating constantly.

You've changed the AccountFactory, ContactFactory etc? Why? I don't mind if there's a good reason, but doing it without one adds a lot of dbus overhead and I'm pretty sure there's no need for it.

(Long term discussion for everyone else, we should make a new model just for accounts (i.e the one the accounts kcm uses) and use that in these dialogs. It'll need doing for the port to nepomuk models anyway)

Since this comment is useless for the patch I'll use this to assign the bug to Mikhail :)

(In reply to comment #13)
> (Long term discussion for everyone else, we should make a new model just for
> accounts (i.e the one the accounts kcm uses) and use that in these dialogs.
> It'll need doing for the port to nepomuk models anyway)

Just a reminder unrelated to the patch: it would be even better to have just one model that can work in account/contact/metacontact mode and handles all the factory/features stuff depending on what you need

Git commit f40ae1128e1e12b229aa7c1f8d85b687abac8345 by David Edmundson, on behalf of Mikhail Krishtop.
Committed on 28/03/2012 at 23:23.
Pushed by davidedmundson into branch 'master'.

Add "Add Contact" action

REVIEW:104430

M  +2    -0    CMakeLists.txt
M  +50   -0    src/presenceapplet.cpp
M  +3    -0    src/presenceapplet.h

http://commits.kde.org/telepathy-presence-applet/f40ae1128e1e12b229aa7c1f8d85b687abac8345