After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 661981 - new message/call dialog: use EmpathyContactChooser
new message/call dialog: use EmpathyContactChooser
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: General
2.33.x
Other Linux
: Normal enhancement
: ---
Assigned To: Danielle Madeley
empathy-maint
Depends on: 661993
Blocks: 650861 661728
 
 
Reported: 2011-10-17 11:12 UTC by Guillaume Desmottes
Modified: 2011-10-20 22:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
new-call-dialog: fix typo in comment (951 bytes, patch)
2011-10-17 12:42 UTC, Guillaume Desmottes
committed Details | Review
factor out empathy_individual_can_audio_video_call() (5.95 KB, patch)
2011-10-17 12:42 UTC, Guillaume Desmottes
committed Details | Review
new-call-dialog: port to EmpathyContactChooser (10.99 KB, patch)
2011-10-17 12:42 UTC, Guillaume Desmottes
committed Details | Review
new-message-dialog: port to EmpathyContactChooser (12.26 KB, patch)
2011-10-18 09:46 UTC, Guillaume Desmottes
committed Details | Review
new-call-dialog: use the self->priv pattern (7.29 KB, patch)
2011-10-18 09:54 UTC, Guillaume Desmottes
committed Details | Review
fixes (28.89 KB, patch)
2011-10-19 22:48 UTC, Danielle Madeley
committed Details | Review

Description Guillaume Desmottes 2011-10-17 11:12:49 UTC
It would provide a better and more coherent UI.
Comment 1 Guillaume Desmottes 2011-10-17 12:42:48 UTC
Created attachment 199193 [details] [review]
new-call-dialog: fix typo in comment
Comment 2 Guillaume Desmottes 2011-10-17 12:42:51 UTC
Created attachment 199194 [details] [review]
factor out empathy_individual_can_audio_video_call()

Also allow caller to get a ref on the EmpathyContact supporting audio/video.
Comment 3 Guillaume Desmottes 2011-10-17 12:42:53 UTC
Created attachment 199195 [details] [review]
new-call-dialog: port to EmpathyContactChooser

Replace the 'send video' checkbox by an extra button

It's more coherent with the other places where we allow user to start calls.
It also gives better feedback as we can unsensitive this button if the
selected contact doesn't support video.
Comment 4 Guillaume Desmottes 2011-10-17 12:47:21 UTC
This is just the new call dialog. I'll do the text one now
Comment 5 Danielle Madeley 2011-10-18 02:19:34 UTC
Review of attachment 199193 [details] [review]:

++
Comment 6 Danielle Madeley 2011-10-18 02:23:06 UTC
Review of attachment 199194 [details] [review]:

::: libempathy/empathy-utils.c
@@ +1132,3 @@
+
+          g_object_unref (contact);
+

Empty space after braces.

@@ +1138,3 @@
+      if (can_audio && can_video)
+        break;
+

Here too.

@@ +1145,3 @@
+
+  if (can_video_call != NULL)
+        {

Suspect indenting.
Comment 7 Danielle Madeley 2011-10-18 02:27:59 UTC
Review of attachment 199195 [details] [review]:

++
Comment 8 Guillaume Desmottes 2011-10-18 08:56:15 UTC
(In reply to comment #6)
> Review of attachment 199194 [details] [review]:
> 
> ::: libempathy/empathy-utils.c
> @@ +1132,3 @@
> +
> +          g_object_unref (contact);
> +
> 
> Empty space after braces.

fixed.

> @@ +1138,3 @@
> +      if (can_audio && can_video)
> +        break;
> +
> 
> Here too.

fixed.

> @@ +1145,3 @@
> +
> +  if (can_video_call != NULL)
> +        {
> 
> Suspect indenting.

fixed.
Comment 9 Guillaume Desmottes 2011-10-18 08:57:16 UTC
Attachment 199193 [details] pushed as 5af3f21 - new-call-dialog: fix typo in comment
Attachment 199194 [details] pushed as 588247c - factor out empathy_individual_can_audio_video_call()
Attachment 199195 [details] pushed as 625867b - new-call-dialog: port to EmpathyContactChooser
Comment 10 Guillaume Desmottes 2011-10-18 09:46:21 UTC
Created attachment 199314 [details] [review]
new-message-dialog: port to EmpathyContactChooser
Comment 11 Guillaume Desmottes 2011-10-18 09:54:47 UTC
Created attachment 199315 [details] [review]
new-call-dialog: use the self->priv pattern
Comment 12 Danielle Madeley 2011-10-18 10:48:34 UTC
Review of attachment 199314 [details] [review]:

::: libempathy-gtk/empathy-new-message-dialog.c
@@ +168,3 @@
+        contact = empathy_contact_dup_best_for_action (individual,
+            EMPATHY_ACTION_CHAT);
+        g_return_if_fail (contact != NULL);

Leaks @individual.

@@ +180,3 @@
+        contact = empathy_contact_dup_best_for_action (individual,
+            EMPATHY_ACTION_SMS);
+        g_return_if_fail (contact != NULL);

Here too.
Comment 13 Danielle Madeley 2011-10-18 10:55:17 UTC
Review of attachment 199315 [details] [review]:

++
Comment 14 Guillaume Desmottes 2011-10-18 12:32:15 UTC
Review of attachment 199314 [details] [review]:

::: libempathy-gtk/empathy-new-message-dialog.c
@@ +168,3 @@
+        contact = empathy_contact_dup_best_for_action (individual,
+            EMPATHY_ACTION_CHAT);
+        g_return_if_fail (contact != NULL);

How so? See the code after out:

  tp_clear_object (&individual);
  tp_clear_object (&contact);
Comment 15 Guillaume Desmottes 2011-10-18 12:36:23 UTC
Attachment 199314 [details] pushed as 7e68d58 - new-message-dialog: port to EmpathyContactChooser
Attachment 199315 [details] pushed as d895fd8 - new-call-dialog: use the self->priv pattern
Comment 16 Danielle Madeley 2011-10-19 22:48:56 UTC
Created attachment 199489 [details] [review]
fixes

http://cgit.collabora.com/git/user/danni/empathy.git/log/?h=dialog-cleanup

This patch removes the extra buttons (caused by a missing parameter) and swaps the button ordering. It also removes the old contact-selector-dialog.

[Random thought: I wonder how much other unused code there is in Empathy... worth checking maybe.]
Comment 17 Danielle Madeley 2011-10-20 03:01:11 UTC
(In reply to comment #16)

> [Random thought: I wonder how much other unused code there is in Empathy...
> worth checking maybe.]

It seems that every class is used somewhere. Well done us.
Comment 18 Guillaume Desmottes 2011-10-20 09:21:47 UTC
++