Merge lp:~amigadave/gnome-control-center-signon/add-account-order into lp:gnome-control-center-signon

Proposed by David King
Status: Merged
Approved by: Alberto Mardegan
Approved revision: 111
Merged at revision: 110
Proposed branch: lp:~amigadave/gnome-control-center-signon/add-account-order
Merge into: lp:gnome-control-center-signon
Diff against target: 81 lines (+27/-6)
2 files modified
src/cc-credentials-accounts-model.vala (+11/-6)
src/cc-credentials-accounts-page.vala (+16/-0)
To merge this branch: bzr merge lp:~amigadave/gnome-control-center-signon/add-account-order
Reviewer Review Type Date Requested Status
Alberto Mardegan (community) Approve
jenkins (community) continuous-integration Needs Fixing
Review via email: mp+126920@code.launchpad.net

Description of the change

Add new accounts to the bottom of the account list (but above the ‘Add accounts…’ row)

To post a comment you must log in.
Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Mardegan (mardy) wrote :

The code looks good and delivers the described functionality. But please verify with Calum whether this is actually desired (I'm not sure what problem we are trying to solve -- if it's about visibility of the new account, shouldn't we also select it as well)?

Revision history for this message
David King (amigadave) wrote :

The specification already says “The user’s accounts are listed in the order in which they are created.”, so this is just fixing up some previously-unnoticed behaviour, as before this change the account appeared at the top of the list (which was then sorted correctly after closing and opening the panel).

The UI specification indeed says that “[once] the account is created … information is shown about which applications on the desktop will use the account.” which means that the new account should be selected too. Fixing that should be quite straightforward.

111. By David King

Select new accounts automatically

The UI specification says that once “the account is created …
information is shown about which applications on the desktop will use
the account.” which means that the newly-created account should be
selected.

Revision history for this message
David King (amigadave) wrote :

This works well for me.

Revision history for this message
jenkins (martin-mrazik+qa) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alberto Mardegan (mardy) wrote :

And for me too :-)

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/cc-credentials-accounts-model.vala'
--- src/cc-credentials-accounts-model.vala 2012-07-09 11:19:21 +0000
+++ src/cc-credentials-accounts-model.vala 2012-09-28 12:48:19 +0000
@@ -97,10 +97,6 @@
97 set_column_types (types);97 set_column_types (types);
9898
99 accounts_manager = new Ag.Manager ();99 accounts_manager = new Ag.Manager ();
100 var accounts = accounts_manager.list ();
101
102 accounts.foreach (add_account);
103
104 // Add a placeholder row at the end of the list.100 // Add a placeholder row at the end of the list.
105 var add_account_text = _("Add account…");101 var add_account_text = _("Add account…");
106102
@@ -116,7 +112,7 @@
116 message ("Error looking up themed add icon: %s", error.message);112 message ("Error looking up themed add icon: %s", error.message);
117 }113 }
118114
119 insert_with_values (null, int.MAX,115 insert_with_values (null, 0,
120 ModelColumns.ACCOUNT_ID, 0,116 ModelColumns.ACCOUNT_ID, 0,
121 ModelColumns.PROVIDER_ICON, provider_icon,117 ModelColumns.PROVIDER_ICON, provider_icon,
122 ModelColumns.ACCOUNT_DESCRIPTION, add_account_text,118 ModelColumns.ACCOUNT_DESCRIPTION, add_account_text,
@@ -124,6 +120,12 @@
124 ModelColumns.NEEDS_ATTENTION, false,120 ModelColumns.NEEDS_ATTENTION, false,
125 -1);121 -1);
126122
123 var accounts = accounts_manager.list ();
124
125 // Sort by account ID.
126 accounts.sort ((a, b) => { return (int)a - (int)b; });
127 accounts.foreach (add_account);
128
127 try129 try
128 {130 {
129131
@@ -169,8 +171,11 @@
169 return;171 return;
170 }172 }
171173
174 /* Insert the new account at the bottom of the list of accounts, but
175 * before the ‘Add account’ row.
176 */
172 var record = fill_column_record (account_id);177 var record = fill_column_record (account_id);
173 insert_with_values (null, 0,178 insert_with_values (null, this.iter_n_children (null) - 1,
174 ModelColumns.ACCOUNT_ID, record.account_id,179 ModelColumns.ACCOUNT_ID, record.account_id,
175 ModelColumns.ACCOUNT, record.account,180 ModelColumns.ACCOUNT, record.account,
176 ModelColumns.PROVIDER_ICON, record.icon,181 ModelColumns.PROVIDER_ICON, record.icon,
177182
=== modified file 'src/cc-credentials-accounts-page.vala'
--- src/cc-credentials-accounts-page.vala 2012-09-26 14:38:12 +0000
+++ src/cc-credentials-accounts-page.vala 2012-09-28 12:48:19 +0000
@@ -106,6 +106,9 @@
106 var selection = accounts_tree.get_selection ();106 var selection = accounts_tree.get_selection ();
107 on_accounts_selection_changed (selection);107 on_accounts_selection_changed (selection);
108108
109 // Update the selection if a new account was added.
110 accounts_store.row_inserted.connect (on_accounts_store_row_inserted);
111
109 set_size_request (-1, 400);112 set_size_request (-1, 400);
110113
111 show ();114 show ();
@@ -408,4 +411,17 @@
408 {411 {
409 account_edit_options_request (plugin);412 account_edit_options_request (plugin);
410 }413 }
414
415 /**
416 * Select the newly-added row, as required by the UI specification.
417 *
418 * @param path the Gtk.TreePath of the newly-added row
419 * @param iter the Gtk.TreeIter of the newly-added row
420 */
421 private void on_accounts_store_row_inserted (Gtk.TreePath path,
422 Gtk.TreeIter iter)
423 {
424 var selection = accounts_tree.get_selection ();
425 selection.select_iter (iter);
426 }
411}427}

Subscribers

People subscribed via source and target branches

to all changes: