Merge lp:~azzar1/software-properties/new-auth-dialog into lp:software-properties

[Andrea Azzarone]

Implement new design for authentication dialog according to https://github.com/CanonicalLtd/desktop-design/issues/62

[lp:~azzar1/software-properties/new-auth-dialog]

1038. By Andrea Azzarone on 2018-06-06

Merge trunk.

1039. By Andrea Azzarone on 2018-09-03

Merge with trunk.

[Andrea Azzarone (azzar1)]

Merge with trunk and changelog entry updated.

[lp:~azzar1/software-properties/new-auth-dialog]

1040. By Andrea Azzarone on 2018-12-19

Merge with trunk.

1041. By Andrea Azzarone on 2019-01-07

Refactor dialog-auth.ui and DialogAuth.py

[Andrea Azzarone (azzar1)]

I updated the code to sync it with the one used in gnome-software.

[Sebastien Bacher (seb128)]

Thank you for the work, looks good to me, some small details

* that's not the exact same string but in https://user-images.githubusercontent.com/19801137/38986966-1632a352-43c6-11e8-9f30-4fada71b78ce.png mpt used the string

'Ubuntu One account' where you wrote '*A*ccount' ... is that wanted?

* The dialog is titled "software-properties-gtk" which is not that nice, can we get ride of the title or use something better/more user friendly?

* When I click on the sign in/register button (on disco, testing with an hacked lsb-release to have the UI showing) I get that error

'Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/softwareproperties/gtk/DialogAuth.py", line 237, in _button_continue_clicked_cb
    self._button_add_another_clicked_cb(self, self.button_add_another)
TypeError: _button_add_another_clicked_cb() takes 2 positional arguments but 3 were given'

review: Needs Fixing

[lp:~azzar1/software-properties/new-auth-dialog]

1042. By Andrea Azzarone on 2019-01-17

s/Account/account/g

1043. By Andrea Azzarone on 2019-01-17

DialogAuth: Show an empty titlebar

1044. By Andrea Azzarone on 2019-01-17

DialogAuth: remove extra self argument to _button_add_another_clicked_cb()

[Andrea Azzarone (azzar1)]

> Thank you for the work, looks good to me, some small details
>
> * that's not the exact same string but in https://user-images.githubuserconten
> t.com/19801137/38986966-1632a352-43c6-11e8-9f30-4fada71b78ce.png mpt used the
> string
>
> 'Ubuntu One account' where you wrote '*A*ccount' ... is that wanted?
>
> * The dialog is titled "software-properties-gtk" which is not that nice, can
> we get ride of the title or use something better/more user friendly?
>
> * When I click on the sign in/register button (on disco, testing with an
> hacked lsb-release to have the UI showing) I get that error
>
> 'Traceback (most recent call last):
> File "/usr/lib/python3/dist-packages/softwareproperties/gtk/DialogAuth.py",
> line 237, in _button_continue_clicked_cb
> self._button_add_another_clicked_cb(self, self.button_add_another)
> TypeError: _button_add_another_clicked_cb() takes 2 positional arguments but 3
> were given'

Branch updated!

[Sebastien Bacher (seb128)]

Looks good now!

review: Approve

=== modified file 'data/gtkbuilder/dialog-auth.ui'
--- data/gtkbuilder/dialog-auth.ui	2018-03-21 14:05:30 +0000
+++ data/gtkbuilder/dialog-auth.ui	2019-01-17 18:48:56 +0000
@@ -1,114 +1,130 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<!-- Generated with glade 3.20.4 -->
+<!-- Generated with glade 3.22.1 -->
 <interface>
   <requires lib="gtk+" version="3.10"/>
+  <object class="GtkListStore" id="liststore_account">
+    <columns>
+      <!-- column-name Id -->
+      <column type="gchararray"/>
+      <!-- column-name Email -->
+      <column type="gchararray"/>
+      <!-- column-name Account -->
+      <column type="GObject"/>
+    </columns>
+  </object>
   <object class="GtkDialog" id="dialog_auth">
-    <property name="can_focus">False</property>
     <property name="resizable">False</property>
-    <property name="modal">True</property>
-    <property name="destroy_with_parent">True</property>
     <property name="type_hint">dialog</property>
-    <property name="deletable">False</property>
+    <child>
+      <placeholder/>
+    </child>
     <child internal-child="vbox">
-      <object class="GtkBox" id="box_dialog">
-        <property name="can_focus">False</property>
+      <object class="GtkBox">
+        <property name="border_width">6</property>
         <property name="orientation">vertical</property>
         <property name="spacing">2</property>
         <child internal-child="action_area">
-          <object class="GtkButtonBox" id="dialog-action_area1">
-            <property name="can_focus">False</property>
+          <object class="GtkButtonBox">
+            <child>
+              <object class="GtkButton" id="button_add_another">
+                <property name="label" translatable="yes">Add another…</property>
+                <property name="visible">True</property>
+                <property name="can_focus">True</property>
+                <property name="receives_default">True</property>
+                <signal name="clicked" handler="_button_add_another_clicked_cb" swapped="no"/>
+              </object>
+              <packing>
+                <property name="expand">True</property>
+                <property name="secondary">True</property>
+                <property name="non_homogeneous">True</property>
+              </packing>
+            </child>
+            <child>
+              <object class="GtkButton" id="button_cancel">
+                <property name="label" translatable="yes">Cancel</property>
+                <property name="visible">True</property>
+                <property name="can_focus">True</property>
+                <property name="receives_default">True</property>
+                <signal name="clicked" handler="_button_cancel_clicked_cb" swapped="no"/>
+              </object>
+              <packing>
+                <property name="expand">True</property>
+                <property name="non_homogeneous">True</property>
+              </packing>
+            </child>
+            <child>
+              <object class="GtkButton" id="button_continue">
+                <property name="visible">True</property>
+                <property name="can_focus">True</property>
+                <property name="receives_default">True</property>
+                <signal name="clicked" handler="_button_continue_clicked_cb" swapped="no"/>
+              </object>
+              <packing>
+                <property name="expand">True</property>
+                <property name="non_homogeneous">True</property>
+              </packing>
+            </child>
           </object>
           <packing>
-            <property name="expand">False</property>
-            <property name="fill">False</property>
-            <property name="position">0</property>
+            <property name="expand">True</property>
           </packing>
         </child>
         <child>
-          <object class="GtkGrid" id="main_grid">
+          <object class="GtkBox">
             <property name="visible">True</property>
-            <property name="can_focus">False</property>
+            <property name="halign">start</property>
             <property name="border_width">12</property>
-            <property name="row_spacing">12</property>
-            <property name="column_spacing">12</property>
+            <property name="spacing">18</property>
             <child>
-              <object class="GtkLabel" id="label_title">
+              <object class="GtkImage">
                 <property name="visible">True</property>
-                <property name="can_focus">False</property>
-                <property name="hexpand">True</property>
-                <property name="label" translatable="yes">To enable Livepatch choose an Ubuntu Single Sign-on account.</property>
-                <property name="wrap">True</property>
-                <property name="xalign">0</property>
+                <property name="halign">start</property>
+                <property name="valign">start</property>
+                <property name="icon_name">software-properties</property>
+                <property name="icon_size">6</property>
               </object>
-              <packing>
-                <property name="left_attach">0</property>
-                <property name="top_attach">0</property>
-              </packing>
             </child>
             <child>
-              <object class="GtkFrame" id="main_frame">
+              <object class="GtkBox" id="box_auth">
                 <property name="visible">True</property>
-                <property name="can_focus">False</property>
-                <property name="label_xalign">0</property>
+                <property name="halign">start</property>
+                <property name="orientation">vertical</property>
+                <property name="spacing">12</property>
                 <child>
-                  <object class="GtkListBox" id="listbox_accounts">
+                  <object class="GtkLabel" id="label_header">
+                    <property name="name">label_header</property>
                     <property name="visible">True</property>
-                    <property name="can_focus">False</property>
-                    <property name="selection_mode">none</property>
+                    <property name="halign">start</property>
+                    <property name="valign">start</property>
+                    <property name="use_markup">True</property>
+                    <property name="justify">fill</property>
+                    <property name="wrap">True</property>
+                    <property name="max_width_chars">40</property>
+                  </object>
+                </child>
+                <child>
+                  <object class="GtkComboBox" id="combobox_account">
+                    <property name="halign">start</property>
+                    <property name="model">liststore_account</property>
                     <child>
-                      <object class="GtkListBoxRow" id="listboxrow_new_account">
-                        <property name="visible">True</property>
-                        <property name="can_focus">False</property>
-                        <child>
-                          <object class="GtkLabel" id="label_new_account">
-                            <property name="height_request">48</property>
-                            <property name="visible">True</property>
-                            <property name="can_focus">False</property>
-                            <property name="halign">center</property>
-                            <property name="valign">center</property>
-                            <property name="label" translatable="yes">&lt;b&gt;Use another account...&lt;/b&gt;</property>
-                            <property name="use_markup">True</property>
-                            <property name="justify">center</property>
-                          </object>
-                        </child>
-                      </object>
+                      <object class="GtkCellRendererText"/>
+                      <attributes>
+                        <attribute name="text">1</attribute>
+                      </attributes>
                     </child>
                   </object>
                 </child>
+                <child>
+                  <object class="GtkLabel" id="label_account">
+                    <property name="halign">start</property>
+                  </object>
+                </child>
               </object>
-              <packing>
-                <property name="left_attach">0</property>
-                <property name="top_attach">1</property>
-                <property name="width">2</property>
-              </packing>
             </child>
           </object>
-          <packing>
-            <property name="expand">False</property>
-            <property name="fill">True</property>
-            <property name="position">1</property>
-          </packing>
-        </child>
-      </object>
-    </child>
-    <child type="titlebar">
-      <object class="GtkHeaderBar">
-        <property name="visible">True</property>
-        <property name="can_focus">False</property>
-        <property name="title" translatable="yes">Choose an account</property>
-        <child>
-          <object class="GtkButton" id="button_cancel">
-            <property name="label">gtk-cancel</property>
-            <property name="visible">True</property>
-            <property name="can_focus">True</property>
-            <property name="receives_default">True</property>
-            <property name="use_stock">True</property>
-          </object>
-        </child>
-      </object>
-    </child>
-    <action-widgets>
-      <action-widget response="-6">button_cancel</action-widget>
-    </action-widgets>
+        </child>
+      </object>
+    </child>
   </object>
 </interface>
+

=== modified file 'debian/changelog'
--- debian/changelog	2019-01-16 16:02:03 +0000
+++ debian/changelog	2019-01-17 18:48:56 +0000
@@ -1,3 +1,4 @@
+<<<<<<< TREE
 software-properties (0.96.31) UNRELEASED; urgency=medium
 
   * Remove the migration script, it was online needed until bionic
@@ -12,6 +13,17 @@
 
  -- Sebastien Bacher <[e-mail]>  Tue, 15 Jan 2019 10:48:27 +0100
 
+=======
+software-properties (0.96.30) UNRELEASED; urgency=medium
+
+  * data/gtkbuilder/dialog-auth.ui: Update design of the authentication
+    dialog.
+  * softwareproperties/gtk/DialogAuth.py: Implement new design of the
+    authentication dialog. 
+
+ -- Andrea Azzarone <[e-mail]>  Wed, 19 Dec 2018 15:47:56 +0000
+
+>>>>>>> MERGE-SOURCE
 software-properties (0.96.29) disco; urgency=medium
 
   * SoftwarePropertiesGtk.py: when checking a package's depends for DKMS also

=== modified file 'softwareproperties/gtk/DialogAuth.py'
--- softwareproperties/gtk/DialogAuth.py	2018-03-21 15:52:33 +0000
+++ softwareproperties/gtk/DialogAuth.py	2019-01-17 18:48:56 +0000
@@ -19,6 +19,7 @@
 #  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
 #  USA
 
+from enum import IntEnum
 import os
 
 from gettext import gettext as _
@@ -28,9 +29,13 @@
 
 import gi
 gi.require_version('Goa', '1.0')
-from gi.repository import Gio, GLib, Goa, GObject, Gtk
+from gi.repository import Gio, GLib, Goa, Gtk
 import logging
 
+class Column(IntEnum):
+    ID = 0
+    MAIL = 1
+    ACCOUNT = 2
 
 class DialogAuth:
 
@@ -38,68 +43,126 @@
         """setup up the gtk dialog"""
         self.parent = parent
 
-        setup_ui(self, os.path.join(datadir, "gtkbuilder",
-                                    "dialog-auth.ui"), domain="software-properties")
-        self.label_title.set_max_width_chars(50)
+        setup_ui(self, os.path.join(datadir, "gtkbuilder", "dialog-auth.ui"),
+            domain="software-properties")
 
         self.dialog = self.dialog_auth
-        self.dialog.use_header_bar = True
+        self.dialog.set_title('')
+        self.dialog.set_deletable(False)
         self.dialog.set_transient_for(parent)
 
-        self.listboxrow_new_account.account = None
+        self.button_continue.grab_focus()
 
         self.account = None
         self.dispose_on_new_account = False
         self.goa_client = Goa.Client.new_sync(None)
 
-        self.listbox_accounts.connect('row-activated', self._listbox_accounts_row_activated_cb)
+        self._setup_model()
+        self._check_ui(select=False)
 
         # Be ready to other accounts
         self.goa_client.connect('account-added', self._account_added_cb)
         self.goa_client.connect('account-removed', self._account_removed_cb)
 
-        self._setup_listbox_accounts()
-        self._check_ui()
-
     def run(self):
         res = self.dialog.run()
         self.dialog.hide()
         return res
 
-    def _check_ui(self):
-        rows = self.listbox_accounts.get_children()
-        has_accounts = len(rows) > 1
-
-        if has_accounts:
-           title = _('To continue choose an Ubuntu Single Sign-On account.')
-           new_account = _('Use another account…')
-        else:
-           title = _('To continue you need an Ubuntu Single Sign-On account.')
-           new_account = _('Sign In…')
-
-        self.label_title.set_text(title)
-        self.label_new_account.set_markup('<b>{}</b>'.format(new_account))
-
-    def _setup_listbox_accounts(self):
+    def _setup_model(self):
         for obj in self.goa_client.get_accounts():
-            account = obj.get_account()
-            if self._is_account_supported(account):
-                self._add_account(account)
-
-    def _is_account_supported(self, account):
-        return account.props.provider_type == 'ubuntusso'
-
-    def _add_account(self, account):
-        row = self._create_row(account)
-        self.listbox_accounts.prepend(row)
-        self._check_ui()
+            self._add_account(obj.get_account(), select=False)
+
+    def _set_header(self, label):
+        self.label_header.set_markup(
+            "<span size='larger' weight='bold'>%s</span>" % label)
+
+    def _check_ui(self, select):
+        naccounts = len(self.liststore_account)
+
+        if naccounts == 0:
+            self._set_header(
+                _('To use Livepatch, you need to use an Ubuntu One account.'))
+            self.combobox_account.set_visible(False)
+            self.label_account.set_visible(False)
+            self.button_add_another.set_visible(False)
+            self.button_continue.set_label(_('Sign In / Register…'))
+        elif naccounts == 1:
+            self._set_header(
+                _('To use Livepatch, you need to use your Ubuntu One account.'))
+            self.combobox_account.set_visible(False)
+            self.label_account.set_visible(True)
+            self.label_account.set_text(self.liststore_account[0][Column.MAIL])
+            self.button_add_another.set_visible(True)
+            self.button_continue.set_label(_('Continue'))
+        else:
+            self._set_header(
+                _('To use Livepatch, you need to use an Ubuntu One account.'))
+            self.button_add_another.set_visible(True)
+            self.combobox_account.set_visible(True)
+            self.label_account.set_visible(False)
+            self.button_continue.set_label(_('Use'))
+            if select:
+                self.combobox_account.set_active(naccounts-1)
+            elif self.combobox_account.get_active() == -1:
+                self.combobox_account.set_active(0)
+
+    def _ignore_account(self, account):
+        return account.props.provider_type != 'ubuntusso'
+
+    def _get_account_iter(self, account):
+        row = self.liststore_account.get_iter_first()
+        while row is not None:
+            account_id =  self.liststore_account.get_value(row, Column.ID)
+            if account_id == account.props.id:
+                return row
+            row = self.liststore_account.iter_next(row)
+        return None
+
+    def _add_account(self, account, select):
+        if self._ignore_account(account):
+            return
+
+        account_iter = self._get_account_iter(account)
+        if account_iter is not None:
+            return
+
+        account_iter = self.liststore_account.append()
+        self.liststore_account.set(account_iter,
+            [Column.ID, Column.MAIL, Column.ACCOUNT],
+            [account.props.id, account.props.presentation_identity, account])
+        self._check_ui(select)
 
     def _remove_account(self, account):
-        for row in self.listbox_accounts.get_children():
-            if row.account == account:
-                row.destroy()
-                self._check_ui()
-                break
+        if self._ignore_account(account):
+            return
+
+        account_iter = self._get_account_iter(account)
+        if account_iter is None:
+            return
+
+        self.liststore_account.remove(account_iter)
+        self._check_ui(select=False)
+
+    def _response_if_valid(self, account):
+        def cb(source, res, data):
+            try:
+                source.call_ensure_credentials_finish(res)
+                valid = True
+            except GLib.Error as e:
+                valid = False
+            
+            if not valid:
+                try:
+                    self._spawn_goa_with_args(account.props.id, None)
+                except GLib.Error as e:
+                    logging.warning ('Failed to spawn gnome-control-center: %s',
+                        e.message)
+            else:
+                self.account = account
+                self.dialog.response(Gtk.ResponseType.OK)
+
+        account.call_ensure_credentials(None, cb, None)
 
     def _build_dbus_params(self, action, arg):
         builder = GLib.VariantBuilder.new(GLib.VariantType.new('av'))
@@ -118,7 +181,8 @@
                 v = GLib.Variant.new_variant(s)
                 builder.add_value(v)
 
-        array = GLib.Variant.new_tuple(GLib.Variant.new_string('online-accounts'), builder.end())
+        array = GLib.Variant.new_tuple(
+            GLib.Variant.new_string('online-accounts'), builder.end())
         array = GLib.Variant.new_variant(array)
 
         param = GLib.Variant.new_tuple(
@@ -135,94 +199,43 @@
             'org.gtk.Actions', None)
 
         param = self._build_dbus_params(action, arg)
-        timeout = 10*60*1000 # 10 minutes should be enough to create an account
-        proxy.call_sync('Activate', param, Gio.DBusCallFlags.NONE, timeout, None)
-
-    def _create_row(self, account):
-        identity = account.props.presentation_identity
-        provider_name = account.props.provider_name
-
-        row = Gtk.ListBoxRow.new()
-        row.show()
-
-        hbox = Gtk.Box.new(Gtk.Orientation.HORIZONTAL, 6)
-        hbox.set_hexpand(True)
-        hbox.show()
-
-        image = Gtk.Image.new_from_icon_name('avatar-default', Gtk.IconSize.DIALOG)
-        image.set_pixel_size(48)
-        image.show()
-        hbox.pack_start(image, False, False, 0)
-
-        vbox = Gtk.Box.new(Gtk.Orientation.VERTICAL, 2)
-        vbox.set_valign(Gtk.Align.CENTER)
-        vbox.show()
-        hbox.pack_start(vbox, False, False, 0)
-
-        if identity:
-            ilabel = Gtk.Label.new()
-            ilabel.set_halign(Gtk.Align.START)
-            ilabel.set_markup('<b>{}</b>'.format(identity))
-            ilabel.show()
-            vbox.pack_start(ilabel, True, True, 0)
-
-        if provider_name:
-            plabel = Gtk.Label.new()
-            plabel.set_halign(Gtk.Align.START)
-            plabel.set_markup('<small><span foreground=\"#555555\">{}</span></small>'.format(provider_name))
-            plabel.show()
-            vbox.pack_start(plabel, True, True, 0)
-
-        warning_icon = Gtk.Image.new_from_icon_name('dialog-warning-symbolic', Gtk.IconSize.BUTTON)
-        warning_icon.set_no_show_all(True)
-        warning_icon.set_margin_end (15)
-        hbox.pack_end(warning_icon, False, False, 0)
-
-        row.add(hbox)
-
-        account.bind_property('attention-needed', warning_icon, 'visible',
-            GObject.BindingFlags.DEFAULT | GObject.BindingFlags.SYNC_CREATE)
-
-        row.account = account
-        return row
-
-    # Signals handlers
-    def _listbox_accounts_row_activated_cb(self, listbox, row):
-        account = row.account
-
-        if account is None:
-            # TODO (azzar1): there is no easy way to put this to false
-            # if the user close the windows without adding an account.
-            # We need to discuss with goa's upstream to support such usercases
-            try:
-                self._spawn_goa_with_args('add', 'ubuntusso')
-                self.dispose_on_new_account = True
-            except GLib.Error as e:
-                logging.warning ('Failed to spawing gnome-control-center: %s', e.message)
-        else:
-            if account.props.attention_needed:
-                try:
-                    self._spawn_goa_with_args(account.props.id, None)
-                except GLib.Error as e:
-                    logging.warning ('Failed to spawing gnome-control-center: %s', e.message)
-            else:
-                self.account = account
-                self.dialog.response(Gtk.ResponseType.OK)
+        proxy.call_sync('Activate', param, Gio.DBusCallFlags.NONE, -1, None)
 
     def _account_added_cb(self, goa_client, goa_object):
         account = goa_object.get_account()
-        if not self._is_account_supported(account):
+        if self._ignore_account(account):
             return
         if not self.dispose_on_new_account:
-            self._add_account(account)
+            self._add_account(account, True)
         else:
-            self.account = account
-            self.dialog.response(Gtk.ResponseType.OK)
+            self._response_if_valid(account)
 
     def _account_removed_cb(self, goa_client, goa_object):
         account = goa_object.get_account()
-        if self._is_account_supported(account):
+        if not self._ignore_account(account):
             self._remove_account(account)
 
-
-
+    def _button_add_another_clicked_cb(self, button):
+        try:
+            # There is no easy way to put this to false if the user close the
+            # windows without adding an account.
+            self._spawn_goa_with_args('add', 'ubuntusso')
+            self.dispose_on_new_account = True
+        except GLib.Error as e:
+            logging.warning ('Failed to spawn control-center: %s', e.message)
+
+    def _button_cancel_clicked_cb(self, button):
+        self.dialog.response(Gtk.ResponseType.CANCEL)
+
+    def _button_continue_clicked_cb(self, button):
+        naccounts = len(self.liststore_account)
+
+        account = None
+        if naccounts >= 1:
+            active_index = self.combobox_account.get_active()
+            account = self.liststore_account[active_index][Column.ACCOUNT]
+
+        if account is None:
+            self._button_add_another_clicked_cb(self.button_add_another)
+        else:
+            self._response_if_valid(account)