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 652669 - refactor account creation
refactor account creation
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: Accounts
unspecified
Other Linux
: High critical
: 3.4
Assigned To: empathy-maint
Depends on: 652670 670201 670203
Blocks: 615257 643858
 
 
Reported: 2011-06-15 17:04 UTC by Danielle Madeley
Modified: 2012-02-16 16:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
flowchart of new design (29.76 KB, image/png)
2011-06-15 17:04 UTC, Danielle Madeley
  Details
accounts-dialog: display the import dialog if there is no account (1.71 KB, patch)
2012-02-16 13:13 UTC, Guillaume Desmottes
committed Details | Review
add empathy-local-xmpp-assistant-widget (13.73 KB, patch)
2012-02-16 13:13 UTC, Guillaume Desmottes
needs-work Details | Review
Move should_create_salut_account to local-xmpp-assistant-widget (4.72 KB, patch)
2012-02-16 13:14 UTC, Guillaume Desmottes
committed Details | Review
accounts-dialog: suggest to create a salut account (3.05 KB, patch)
2012-02-16 13:14 UTC, Guillaume Desmottes
committed Details | Review
empathy-accounts: remove --assistant option (2.99 KB, patch)
2012-02-16 13:14 UTC, Guillaume Desmottes
committed Details | Review
Stop using the accounts assistant (3.10 KB, patch)
2012-02-16 13:14 UTC, Guillaume Desmottes
committed Details | Review
Remove empathy-account-assistant (45.57 KB, patch)
2012-02-16 13:14 UTC, Guillaume Desmottes
committed Details | Review
remove empathy-auto-salut-account-helper (8.33 KB, patch)
2012-02-16 13:14 UTC, Guillaume Desmottes
committed Details | Review
add empathy-local-xmpp-assistant-widget (13.73 KB, patch)
2012-02-16 13:35 UTC, Guillaume Desmottes
committed Details | Review
salut-dialog: use 'Skip' instead of 'Ignore' (906 bytes, patch)
2012-02-16 14:38 UTC, Guillaume Desmottes
committed Details | Review
local-xmpp-assistant-widget: inherit from a GtkGrid (3.76 KB, patch)
2012-02-16 14:38 UTC, Guillaume Desmottes
committed Details | Review
widget-local-xmpp: expand the entry fields (2.76 KB, patch)
2012-02-16 14:39 UTC, Guillaume Desmottes
committed Details | Review
move the second part of the message at the end of the widget (2.36 KB, patch)
2012-02-16 14:39 UTC, Guillaume Desmottes
committed Details | Review
use the 48x48 version of the local-xmpp icon (1.14 KB, patch)
2012-02-16 14:39 UTC, Guillaume Desmottes
committed Details | Review

Description Danielle Madeley 2011-06-15 17:04:26 UTC
Created attachment 189991 [details]
flowchart of new design

Empathy currently has two modes for accounts creation:

 - account assistant -- only run if you have no accounts, uses simple dialogs
 - add button in empathy-accounts -- uses advanced dialogs

This is asymmetrical and weird. Account creation should always follow the same path.

Instead we should remove the assistant. And replace it with a number of slideout dialogs:

 - one to import accounts from Pidgin -- if there are no accounts
 - one to set up People Nearby -- if there are no accounts
 - one to add an account

If the user clicks add, then the simple 'Add Account' slideout should come up. Once you've set up the details it returns back to the empathy-accounts dialog to watch it connecting.
Comment 1 Guillaume Desmottes 2011-09-27 10:57:32 UTC
Let's finish bug #652670 first.
Comment 2 Xavier Claessens 2011-09-27 11:02:06 UTC
So creating an account would always go to an assistant, but does editing an account change compared to what we have now?
Comment 3 Xavier Claessens 2011-09-27 11:32:24 UTC
Could you please define "a number of slideout dialogs", that looks like the definition of an assistant to me...

Or do you mean popup an modal GtkDialog and when closing it, popup the next one, until all steps are done? This looks like an original way of doing an assistant, I don't think I would like that myself tbh. It means you won't be able to cancel all steps at once, you must go to through all steps first.

Maybe some mockups would help.
Comment 4 Danielle Madeley 2011-09-27 13:13:29 UTC
I drew mockups in my notebook. I'll scan them in.
Comment 5 Akhil Laddha 2011-11-10 05:28:15 UTC
Danielle, ping
Comment 6 Guillaume Desmottes 2012-02-16 11:34:01 UTC
This branch was getting pretty big so I splitted it a bit: bug #670201 and bug #670203
Comment 7 Guillaume Desmottes 2012-02-16 13:13:54 UTC
Created attachment 207764 [details] [review]
accounts-dialog: display the import dialog if there is no account

This is the first step of merging the accounts-dialog and assistant.
Comment 8 Guillaume Desmottes 2012-02-16 13:13:57 UTC
Created attachment 207765 [details] [review]
add empathy-local-xmpp-assistant-widget

All of this is duplicated code from empathy-account-assistant and
empathy-auto-salut-account-helper but those are going away so I didn't bother
refactoring them.
Comment 9 Guillaume Desmottes 2012-02-16 13:14:00 UTC
Created attachment 207766 [details] [review]
Move should_create_salut_account to local-xmpp-assistant-widget
Comment 10 Guillaume Desmottes 2012-02-16 13:14:04 UTC
Created attachment 207767 [details] [review]
accounts-dialog: suggest to create a salut account
Comment 11 Guillaume Desmottes 2012-02-16 13:14:07 UTC
Created attachment 207768 [details] [review]
empathy-accounts: remove --assistant option

It was only used when hacking on the accounts assistant which is going away.
Comment 12 Guillaume Desmottes 2012-02-16 13:14:10 UTC
Created attachment 207769 [details] [review]
Stop using the accounts assistant

Its features have been re-implemented in empathy-accounts-dialog. Thanks to
Danielle for this new design.
Comment 13 Guillaume Desmottes 2012-02-16 13:14:14 UTC
Created attachment 207770 [details] [review]
Remove empathy-account-assistant

DIE DIE DIE!!!
Comment 14 Guillaume Desmottes 2012-02-16 13:14:17 UTC
Created attachment 207771 [details] [review]
remove empathy-auto-salut-account-helper

It was used only by the accounts assistant
Comment 15 Will Thompson 2012-02-16 13:16:49 UTC
Review of attachment 207764 [details] [review]:

Looks fine.
Comment 16 Will Thompson 2012-02-16 13:23:56 UTC
Review of attachment 207765 [details] [review]:

I'm basically going to believe you when you say that this works. :)

::: libempathy-gtk/empathy-local-xmpp-assistant-widget.c
@@ +36,3 @@
+#include <libempathy/empathy-debug.h>
+
+G_DEFINE_TYPE (EmpathyLocalXmppAssistantWidget,\

You don't need this backslash.

@@ +187,3 @@
+
+  G_OBJECT_CLASS (empathy_local_xmpp_assistant_widget_parent_class)->
+    constructed (object);

You should chain up at the beginning of constructed, not at the end.
Comment 17 Will Thompson 2012-02-16 13:24:54 UTC
Review of attachment 207766 [details] [review]:

Okay.
Comment 18 Guillaume Desmottes 2012-02-16 13:35:08 UTC
Review of attachment 207765 [details] [review]:

::: libempathy-gtk/empathy-local-xmpp-assistant-widget.c
@@ +36,3 @@
+#include <libempathy/empathy-debug.h>
+
+G_DEFINE_TYPE (EmpathyLocalXmppAssistantWidget,\

removed.

@@ +187,3 @@
+
+  G_OBJECT_CLASS (empathy_local_xmpp_assistant_widget_parent_class)->
+    constructed (object);

done.
Comment 19 Guillaume Desmottes 2012-02-16 13:35:47 UTC
Created attachment 207773 [details] [review]
add empathy-local-xmpp-assistant-widget

All of this is duplicated code from empathy-account-assistant and
empathy-auto-salut-account-helper but those are going away so I didn't bother
refactoring them.
Comment 20 Will Thompson 2012-02-16 13:49:55 UTC
Review of attachment 207767 [details] [review]:

The code looks fine… but as discussed on IRC the dialog does not. :)

::: src/empathy-accounts-dialog.c
@@ +2081,3 @@
+  gtk_window_set_modal (GTK_WINDOW (dialog), TRUE);
+
+  gint response;

As on IRC: change this to Skip.
Comment 21 Will Thompson 2012-02-16 13:50:25 UTC
Review of attachment 207768 [details] [review]:

Sure.
Comment 22 Will Thompson 2012-02-16 13:51:16 UTC
Review of attachment 207769 [details] [review]:

Looks fine.
Comment 23 Will Thompson 2012-02-16 14:00:10 UTC
Review of attachment 207770 [details] [review]:

There's not much to review here. If you think it's clear enough how to add a second account after you've created one from the first automatic slide-out dialog, then fine. That's the main thing the assistant did that I think this whole change loses us: it made it very obvious how to set up more than one account (admittedly only on the first run).
Comment 24 Will Thompson 2012-02-16 14:00:52 UTC
Review of attachment 207771 [details] [review]:

++
Comment 25 Will Thompson 2012-02-16 14:02:03 UTC
Review of attachment 207773 [details] [review]:

++
Comment 26 Guillaume Desmottes 2012-02-16 14:38:53 UTC
Created attachment 207777 [details] [review]
salut-dialog: use 'Skip' instead of 'Ignore'
Comment 27 Guillaume Desmottes 2012-02-16 14:38:56 UTC
Created attachment 207778 [details] [review]
local-xmpp-assistant-widget: inherit from a GtkGrid

It simplifies widgets packing.
Comment 28 Guillaume Desmottes 2012-02-16 14:39:00 UTC
Created attachment 207779 [details] [review]
widget-local-xmpp: expand the entry fields

We want them to take all the space available.
Comment 29 Guillaume Desmottes 2012-02-16 14:39:03 UTC
Created attachment 207780 [details] [review]
move the second part of the message at the end of the widget

I rephrased it as well per Will's suggestion.
Comment 30 Guillaume Desmottes 2012-02-16 14:39:06 UTC
Created attachment 207781 [details] [review]
use the 48x48 version of the local-xmpp icon

We don't have to scale up the icon any more and it looks good as it.
Comment 31 Will Thompson 2012-02-16 14:58:43 UTC
Review of attachment 207778 [details] [review]:

Looks fine.
Comment 32 Will Thompson 2012-02-16 14:59:09 UTC
Review of attachment 207779 [details] [review]:

Surely you only want hexpand?
Comment 33 Will Thompson 2012-02-16 14:59:39 UTC
Review of attachment 207780 [details] [review]:

Looks fine.
Comment 34 Will Thompson 2012-02-16 15:00:21 UTC
Review of attachment 207781 [details] [review]:

Looks good.
Comment 35 Guillaume Desmottes 2012-02-16 16:43:11 UTC
(In reply to comment #32)
> Review of attachment 207779 [details] [review]:
> 
> Surely you only want hexpand?

Right; changed.
Comment 36 Guillaume Desmottes 2012-02-16 16:43:53 UTC
Attachment 207764 [details] pushed as 692672a - accounts-dialog: display the import dialog if there is no account
Attachment 207766 [details] pushed as fe9e5a3 - Move should_create_salut_account to local-xmpp-assistant-widget
Attachment 207767 [details] pushed as 1dc8418 - accounts-dialog: suggest to create a salut account
Attachment 207768 [details] pushed as db89425 - empathy-accounts: remove --assistant option
Attachment 207769 [details] pushed as 6fc913d - Stop using the accounts assistant
Attachment 207770 [details] pushed as a266c22 - Remove empathy-account-assistant
Attachment 207771 [details] pushed as 208dcb1 - remove empathy-auto-salut-account-helper
Attachment 207773 [details] pushed as fea6df6 - add empathy-local-xmpp-assistant-widget
Attachment 207777 [details] pushed as 3e85f81 - salut-dialog: use 'Skip' instead of 'Ignore'
Attachment 207778 [details] pushed as 1b0f008 - local-xmpp-assistant-widget: inherit from a GtkGrid
Attachment 207779 [details] pushed as 4e2b90e - widget-local-xmpp: expand the entry fields
Attachment 207780 [details] pushed as cc16ff0 - move the second part of the message at the end of the widget
Attachment 207781 [details] pushed as 7cd8231 - use the 48x48 version of the local-xmpp icon