Created attachment 234627 [details] [review]
online-accounts: New "Other" providers page

Created attachment 237368 [details]
Screenshot of work in progress - 1

Created attachment 237369 [details]
Screenshot of work in progress - 2

Review of attachment 234627 [details] [review]:

::: panels/online-accounts/cc-online-accounts-add-account-dialog.c
@@ +102,3 @@
+  if (provider == NULL)
+    {
+      gtk_notebook_next_page (GTK_NOTEBOOK (add_account->priv->notebook));

That looks fragile.

@@ +140,3 @@
+  gtk_container_add (GTK_CONTAINER (page_grid), *group_grid);
+
+  markup = g_strconcat ("<b>", name, "</b>", NULL);

markup = g_strdup_printf ("<b>%s</b>", name);

@@ +296,3 @@
   priv->list_store = gtk_list_store_new (N_COLUMNS, GOA_TYPE_PROVIDER, G_TYPE_ICON, G_TYPE_STRING);
 
+  priv->notebook = gtk_notebook_new ();

I don't think we want to introduce new uses of GtkNotebook. Can you please try a patch using GdStack?

Created attachment 237459 [details] [review]
online-accounts: New "Other" providers page

(In reply to comment #4)
> Review of attachment 234627 [details] [review]:

Thanks for the review.

> ::: panels/online-accounts/cc-online-accounts-add-account-dialog.c
> @@ +102,3 @@
> +  if (provider == NULL)
> +    {
> +      gtk_notebook_next_page (GTK_NOTEBOOK (add_account->priv->notebook));
> 
> That looks fragile.

I added a "provider != NULL" guard to goa_panel_add_account_dialog_add_provider. It will ensure that a NULL provider can never be added by the user and uniquely identifies the "Other" entry.

I agree that this is a bit hacky, but since the designs only have 2 pages and, as far as I can see, there is only a transition from the 1st to the 2nd page, I decided to keep it simple.

If you don't like it then we can think of something else.

> @@ +140,3 @@
> +  gtk_container_add (GTK_CONTAINER (page_grid), *group_grid);
> +
> +  markup = g_strconcat ("<b>", name, "</b>", NULL);
> 
> markup = g_strdup_printf ("<b>%s</b>", name);

Done.

> @@ +296,3 @@
>    priv->list_store = gtk_list_store_new (N_COLUMNS, GOA_TYPE_PROVIDER,
> G_TYPE_ICON, G_TYPE_STRING);
> 
> +  priv->notebook = gtk_notebook_new ();
> 
> I don't think we want to introduce new uses of GtkNotebook. Can you please try
> a patch using GdStack?

Done.

(In reply to comment #6)
<snip>
> I agree that this is a bit hacky, but since the designs only have 2 pages and,
> as far as I can see, there is only a transition from the 1st to the 2nd page, I
> decided to keep it simple.
> 
> If you don't like it then we can think of something else.

It's the "gtk_notebook_next_page" that I didn't like. Setting a page name or page number is fine. I just don't like the uncertainty of using gtk_notebook_next_page() which relies on a particular ordering. This is the sort of thing that breaks when we extend programs.

<snip>
> > I don't think we want to introduce new uses of GtkNotebook. Can you please try
> > a patch using GdStack?
> 
> Done.

As asked on IRC, can you check whether it's appropriate to use GdStack's animations here? See gd_stack_set_transition_type() (might need to update the libgd submodule to get that).

Looks good other than that (but you'll need to ask for UI freeze break)

From #gnome-design:
<aday> rishi, looks ok from what i can tell