Merge lp:~aacid/unity-greeter/no_servers_configured into lp:unity-greeter

[Albert Astals Cid]

Add the dialog the spec says we have to show when credentials are right but no servers are configured

[Michael Terry (mterry)]

Ugh, this dialog is very different from anything else unity-greeter can do. Can it be made a separate slide-in list like the SessionChooser?

Also, remote-login-dialog.png needs to be added and referenced correctly from the code. See other png loading in the code.

review: Needs Fixing

[Albert Astals Cid (aacid)]

The spec says we have to show a dialog, speak with mpt/ted about it.

The .png doesn't exist yet (there's a bug about it), but i'll look at how the other loading is done and use it.

[Ted Gould (ted)]

Well, that's a bit untrue. For instance if you do Switch Off.. you see a similar dialog. So there are dialogs in various parts of the greeter, they're just all out of the standard flow. This one is too.

[Michael Terry (mterry)]

Also, the help icon seems out of place (too colorful). Is that a hold-in temporary icon?

[Michael Terry (mterry)]

But yeah, seems sound besides the icons.

[Michael Terry (mterry)]

Oh and two more nits:
1) The cancel button has the default keyboard focus. Should probably be Set Up.
2) Set Up should use a proper ellipses: …

[Albert Astals Cid (aacid)]

About the help icon, it's the stock help icon of the theme, so i assumed it'd be fine. I've asked around to our designers what they think about it, will report back when they answer.

[Albert Astals Cid (aacid)]

Talked to designers and they agree with you we need a new icon, created a bug for tracking it and changed the code to load a custom icon instead of the theme one.

[Albert Astals Cid (aacid)]

Ok, besides the icons missing i think everything should be fine, it'd be cool if you could give it a final review and merge it if there's nothing missing :-)

[Michael Terry (mterry)]

Besides the missing icons, I do see one last thing:

get_badge() is going to keep loading that help icon. We have a cache of those in SessionList for performance reasons. Probably better to add a recognized session in SessionList.get_badge_name() that does "remote-login" -> "remote_login_badge.png" and then call SessionList.get_badge for it. That will automatically cache it as needed.

[lp:~aacid/unity-greeter/no_servers_configured]

525. By Albert Astals Cid on 2012-08-23

Add the "remote-login" help badge to the session list cache so we don't have to load it everytime as suggested by mterry

[Michael Terry (mterry)]

BTW, I've noticed that the original remote-login branch caused an annoying regression. With enable-remote-login enabled, and if you're using an actual installed unity-greeter (i.e. not test mode), as soon as the Remote Login option appears, focus leaves the existing entry. With the effect that right after the greeter appears, you lose focus. Not part of this specific branch review, but wanted to mention it here before I went EOD.

[Albert Astals Cid (aacid)]

The focus thing should be fixed in https://code.launchpad.net/~aacid/unity-greeter/keep_focus_on_adding_entries/+merge/121142 (I also added a timeout for the Remote Login stuff in test mode so i could easily reproduce the error there too)

Was there anything else missing in this review?

[Michael Terry (mterry)]

Still just the two images.

[Albert Astals Cid (aacid)]

Oh, i thought we were commiting this without the images because the Feature Freeze is much before the UI Freeze so we needed to meet the Feature Freeze with code but could wait a few days/weeks for the UI Freeze to get the icons.

[Michael Terry (mterry)]

I changed the badge to the "unknown" badge and commented out the dialog icon for now. Otherwise the badge wouldn't show at all, and we want that to be at least functional until we get icons.

review: Approve

=== modified file 'src/session-list.vala'
--- src/session-list.vala	2012-08-03 13:49:20 +0000
+++ src/session-list.vala	2012-08-23 15:32:20 +0000
@@ -125,6 +125,8 @@
             return "kde_badge.png";
         case "xterm":
             return "recovery_console_badge.png";
+        case "remote-login":
+            return "remote_login_help.png";
         default:
             return null;
         }

=== modified file 'src/user-list.vala'
--- src/user-list.vala	2012-08-22 14:24:18 +0000
+++ src/user-list.vala	2012-08-23 15:32:20 +0000
@@ -192,6 +192,7 @@
                 var e = new PromptBox (list_name);
                 e.label = remote_server.name;
                 e.respond.connect (remote_directory_respond_cb);
+                e.show_options.connect (show_remote_account_dialog);
                 add_entry (e);
 
                 remote_directory_server_list.append (remote_server);
@@ -381,18 +382,25 @@
             }
             if (login_success)
             {
-                string last_used_server_list_name = "";
-                foreach (RemoteServer remote_server in server_list)
-                {
-                    var e = create_prompt_for_login_server (remote_server);
-                    if (remote_server.last_used_server)
-                        last_used_server_list_name = e.id;
-                }
-                if (last_used_server_list_name != "")
-                    set_active_entry (last_used_server_list_name);
-                else
-                    set_active_first_entry_with_prefix ("*remote_login");
                 password_field.did_respond = false;
+                if (server_list.length == 0)
+                {
+                    show_remote_account_dialog();
+                }
+                else
+                {
+                    string last_used_server_list_name = "";
+                    foreach (RemoteServer remote_server in server_list)
+                    {
+                        var e = create_prompt_for_login_server (remote_server);
+                        if (remote_server.last_used_server)
+                            last_used_server_list_name = e.id;
+                    }
+                    if (last_used_server_list_name != "")
+                        set_active_entry (last_used_server_list_name);
+                    else
+                        set_active_first_entry_with_prefix ("*remote_login");
+                }
             }
             else
             {
@@ -426,6 +434,58 @@
         }
     }
 
+    private void show_remote_account_dialog ()
+    {
+        Gtk.Dialog d = new Gtk.Dialog();
+        d.set_position(Gtk.WindowPosition.CENTER_ALWAYS);
+        string label1_text;
+        if (offer_guest)
+        {
+            d.add_button(_("Cancel"), 0);
+            Gtk.Widget b = d.add_button(_("Set Up…"), 1);
+            b.grab_focus();
+            label1_text = _("<b>You need an Ubuntu Remote Login\naccount to use this service. Would you\nlike to set up an account now?</b>");
+        }
+        else
+        {
+            d.add_button(_("OK"), 0);
+            label1_text = _("<b>You need an Ubuntu Remote Login\naccount to use this service. Visit\nuccs.canonical.com to set up an account.</b>");
+        }
+
+        Gtk.HBox hb = new Gtk.HBox(false, 10);
+        Gtk.Image im = new Gtk.Image.from_file(Path.build_filename (Config.PKGDATADIR, "remote_login_dialog.png", null));
+        im.set_alignment(0.5f, 0);
+        Gtk.VBox vb = new Gtk.VBox(false, 10);
+        hb.pack_start(im, true, true, 5);
+        hb.pack_end(vb, true, true, 5);
+
+        Gtk.Label l1 = new Gtk.Label(null);
+        l1.set_markup(label1_text);
+        l1.set_alignment(0, 0.5f);
+        Gtk.Label l2 = new Gtk.Label(_("If you have an account on an RDP or\nCitrix server, Remote Login lets you run\napplications from that server."));
+        l2.set_alignment(0, 0.5f);
+        vb.pack_start(l1);
+        vb.pack_end(l2);
+
+        d.get_content_area().pack_start(hb);
+
+        d.show_all();
+        d.response.connect((id) => {
+            if (id == 1)
+            {
+                var config_session = "uccsconfigure";
+                if (is_supported_remote_session(config_session))
+                {
+#if HAVE_REMOTE_LIGHTDM
+                    UnityGreeter.greeter.authenticate_remote (config_session, "");
+#endif
+                }
+            }
+            d.destroy();
+        });
+        d.run();
+    }
+
     private bool change_background_timeout_cb ()
     {
         string new_background_file;
@@ -750,15 +810,33 @@
         }
         else
         {
-            return null;
-        }
+            if (selected_entry.id.has_prefix ("*remote_directory"))
+                return SessionList.get_badge ("remote-login");
+            else
+                return null;
+        }
+    }
+
+    private bool is_supported_remote_session (string session_name)
+    {
+        bool found = false;
+#if HAVE_REMOTE_LIGHTDM
+        foreach (var session in LightDM.get_remote_sessions ())
+        {
+            if (session.name == session_name)
+            {
+                found = true;
+                break;
+            }
+        }
+#endif
+        return found;
     }
 
     protected override string get_lightdm_session ()
     {
         if (selected_entry.id.has_prefix ("*remote_login"))
         {
-#if HAVE_REMOTE_LIGHTDM
             var url = url_from_remote_loding_server_list_name (selected_entry.id);
             unowned List<RemoteServer?> it = remote_login_server_list;
             var answer = "";
@@ -771,25 +849,10 @@
                 }
                 it = it.next;
             }
-            // Check answer is one of the supported types
-            if (answer != "")
-            {
-                bool found = false;
-                foreach (var session in LightDM.get_remote_sessions ())
-                {
-                    if (session.name == answer)
-                    {
-                        found = true;
-                        break;
-                    }
-                }
-                if (!found)
-                    answer = "";
-            }
-            return answer;
-#else
-            return "";
-#endif
+            if (is_supported_remote_session(answer))
+                return answer;
+            else
+                return "";
         }
         else
             return session;