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

Proposed by Andrea Azzarone on 2018-05-14
Status: Merged
Approved by: Sebastien Bacher on 2019-01-22
Approved revision: 1044
Merged at revision: 1062
Proposed branch: lp:~azzar1/software-properties/new-auth-dialog
Merge into: lp:software-properties
Diff against target: 569 lines (+243/-202) (has conflicts)
3 files modified
data/gtkbuilder/dialog-auth.ui (+96/-80)
debian/changelog (+12/-0)
softwareproperties/gtk/DialogAuth.py (+135/-122)
Text conflict in debian/changelog
To merge this branch: bzr merge lp:~azzar1/software-properties/new-auth-dialog
Reviewer Review Type Date Requested Status
Sebastien Bacher 2018-05-14 Approve on 2019-01-22
Review via email: mp+345483@code.launchpad.net

Commit message

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

To post a comment you must log in.
1038. By Andrea Azzarone on 2018-06-06

Merge trunk.

1039. By Andrea Azzarone on 2018-09-03

Merge with trunk.

Andrea Azzarone (azzar1) wrote :

Merge with trunk and changelog entry updated.

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) wrote :

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

Sebastien Bacher (seb128) wrote :

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
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) wrote :

> 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) wrote :

Looks good now!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'data/gtkbuilder/dialog-auth.ui'
2--- data/gtkbuilder/dialog-auth.ui 2018-03-21 14:05:30 +0000
3+++ data/gtkbuilder/dialog-auth.ui 2019-01-17 18:48:56 +0000
4@@ -1,114 +1,130 @@
5 <?xml version="1.0" encoding="UTF-8"?>
6-<!-- Generated with glade 3.20.4 -->
7+<!-- Generated with glade 3.22.1 -->
8 <interface>
9 <requires lib="gtk+" version="3.10"/>
10+ <object class="GtkListStore" id="liststore_account">
11+ <columns>
12+ <!-- column-name Id -->
13+ <column type="gchararray"/>
14+ <!-- column-name Email -->
15+ <column type="gchararray"/>
16+ <!-- column-name Account -->
17+ <column type="GObject"/>
18+ </columns>
19+ </object>
20 <object class="GtkDialog" id="dialog_auth">
21- <property name="can_focus">False</property>
22 <property name="resizable">False</property>
23- <property name="modal">True</property>
24- <property name="destroy_with_parent">True</property>
25 <property name="type_hint">dialog</property>
26- <property name="deletable">False</property>
27+ <child>
28+ <placeholder/>
29+ </child>
30 <child internal-child="vbox">
31- <object class="GtkBox" id="box_dialog">
32- <property name="can_focus">False</property>
33+ <object class="GtkBox">
34+ <property name="border_width">6</property>
35 <property name="orientation">vertical</property>
36 <property name="spacing">2</property>
37 <child internal-child="action_area">
38- <object class="GtkButtonBox" id="dialog-action_area1">
39- <property name="can_focus">False</property>
40+ <object class="GtkButtonBox">
41+ <child>
42+ <object class="GtkButton" id="button_add_another">
43+ <property name="label" translatable="yes">Add another…</property>
44+ <property name="visible">True</property>
45+ <property name="can_focus">True</property>
46+ <property name="receives_default">True</property>
47+ <signal name="clicked" handler="_button_add_another_clicked_cb" swapped="no"/>
48+ </object>
49+ <packing>
50+ <property name="expand">True</property>
51+ <property name="secondary">True</property>
52+ <property name="non_homogeneous">True</property>
53+ </packing>
54+ </child>
55+ <child>
56+ <object class="GtkButton" id="button_cancel">
57+ <property name="label" translatable="yes">Cancel</property>
58+ <property name="visible">True</property>
59+ <property name="can_focus">True</property>
60+ <property name="receives_default">True</property>
61+ <signal name="clicked" handler="_button_cancel_clicked_cb" swapped="no"/>
62+ </object>
63+ <packing>
64+ <property name="expand">True</property>
65+ <property name="non_homogeneous">True</property>
66+ </packing>
67+ </child>
68+ <child>
69+ <object class="GtkButton" id="button_continue">
70+ <property name="visible">True</property>
71+ <property name="can_focus">True</property>
72+ <property name="receives_default">True</property>
73+ <signal name="clicked" handler="_button_continue_clicked_cb" swapped="no"/>
74+ </object>
75+ <packing>
76+ <property name="expand">True</property>
77+ <property name="non_homogeneous">True</property>
78+ </packing>
79+ </child>
80 </object>
81 <packing>
82- <property name="expand">False</property>
83- <property name="fill">False</property>
84- <property name="position">0</property>
85+ <property name="expand">True</property>
86 </packing>
87 </child>
88 <child>
89- <object class="GtkGrid" id="main_grid">
90+ <object class="GtkBox">
91 <property name="visible">True</property>
92- <property name="can_focus">False</property>
93+ <property name="halign">start</property>
94 <property name="border_width">12</property>
95- <property name="row_spacing">12</property>
96- <property name="column_spacing">12</property>
97+ <property name="spacing">18</property>
98 <child>
99- <object class="GtkLabel" id="label_title">
100+ <object class="GtkImage">
101 <property name="visible">True</property>
102- <property name="can_focus">False</property>
103- <property name="hexpand">True</property>
104- <property name="label" translatable="yes">To enable Livepatch choose an Ubuntu Single Sign-on account.</property>
105- <property name="wrap">True</property>
106- <property name="xalign">0</property>
107+ <property name="halign">start</property>
108+ <property name="valign">start</property>
109+ <property name="icon_name">software-properties</property>
110+ <property name="icon_size">6</property>
111 </object>
112- <packing>
113- <property name="left_attach">0</property>
114- <property name="top_attach">0</property>
115- </packing>
116 </child>
117 <child>
118- <object class="GtkFrame" id="main_frame">
119+ <object class="GtkBox" id="box_auth">
120 <property name="visible">True</property>
121- <property name="can_focus">False</property>
122- <property name="label_xalign">0</property>
123+ <property name="halign">start</property>
124+ <property name="orientation">vertical</property>
125+ <property name="spacing">12</property>
126 <child>
127- <object class="GtkListBox" id="listbox_accounts">
128+ <object class="GtkLabel" id="label_header">
129+ <property name="name">label_header</property>
130 <property name="visible">True</property>
131- <property name="can_focus">False</property>
132- <property name="selection_mode">none</property>
133+ <property name="halign">start</property>
134+ <property name="valign">start</property>
135+ <property name="use_markup">True</property>
136+ <property name="justify">fill</property>
137+ <property name="wrap">True</property>
138+ <property name="max_width_chars">40</property>
139+ </object>
140+ </child>
141+ <child>
142+ <object class="GtkComboBox" id="combobox_account">
143+ <property name="halign">start</property>
144+ <property name="model">liststore_account</property>
145 <child>
146- <object class="GtkListBoxRow" id="listboxrow_new_account">
147- <property name="visible">True</property>
148- <property name="can_focus">False</property>
149- <child>
150- <object class="GtkLabel" id="label_new_account">
151- <property name="height_request">48</property>
152- <property name="visible">True</property>
153- <property name="can_focus">False</property>
154- <property name="halign">center</property>
155- <property name="valign">center</property>
156- <property name="label" translatable="yes">&lt;b&gt;Use another account...&lt;/b&gt;</property>
157- <property name="use_markup">True</property>
158- <property name="justify">center</property>
159- </object>
160- </child>
161- </object>
162+ <object class="GtkCellRendererText"/>
163+ <attributes>
164+ <attribute name="text">1</attribute>
165+ </attributes>
166 </child>
167 </object>
168 </child>
169+ <child>
170+ <object class="GtkLabel" id="label_account">
171+ <property name="halign">start</property>
172+ </object>
173+ </child>
174 </object>
175- <packing>
176- <property name="left_attach">0</property>
177- <property name="top_attach">1</property>
178- <property name="width">2</property>
179- </packing>
180 </child>
181 </object>
182- <packing>
183- <property name="expand">False</property>
184- <property name="fill">True</property>
185- <property name="position">1</property>
186- </packing>
187- </child>
188- </object>
189- </child>
190- <child type="titlebar">
191- <object class="GtkHeaderBar">
192- <property name="visible">True</property>
193- <property name="can_focus">False</property>
194- <property name="title" translatable="yes">Choose an account</property>
195- <child>
196- <object class="GtkButton" id="button_cancel">
197- <property name="label">gtk-cancel</property>
198- <property name="visible">True</property>
199- <property name="can_focus">True</property>
200- <property name="receives_default">True</property>
201- <property name="use_stock">True</property>
202- </object>
203- </child>
204- </object>
205- </child>
206- <action-widgets>
207- <action-widget response="-6">button_cancel</action-widget>
208- </action-widgets>
209+ </child>
210+ </object>
211+ </child>
212 </object>
213 </interface>
214+
215
216=== modified file 'debian/changelog'
217--- debian/changelog 2019-01-16 16:02:03 +0000
218+++ debian/changelog 2019-01-17 18:48:56 +0000
219@@ -1,3 +1,4 @@
220+<<<<<<< TREE
221 software-properties (0.96.31) UNRELEASED; urgency=medium
222
223 * Remove the migration script, it was online needed until bionic
224@@ -12,6 +13,17 @@
225
226 -- Sebastien Bacher <seb128@ubuntu.com> Tue, 15 Jan 2019 10:48:27 +0100
227
228+=======
229+software-properties (0.96.30) UNRELEASED; urgency=medium
230+
231+ * data/gtkbuilder/dialog-auth.ui: Update design of the authentication
232+ dialog.
233+ * softwareproperties/gtk/DialogAuth.py: Implement new design of the
234+ authentication dialog.
235+
236+ -- Andrea Azzarone <andrea.azzarone@canonical.com> Wed, 19 Dec 2018 15:47:56 +0000
237+
238+>>>>>>> MERGE-SOURCE
239 software-properties (0.96.29) disco; urgency=medium
240
241 * SoftwarePropertiesGtk.py: when checking a package's depends for DKMS also
242
243=== modified file 'softwareproperties/gtk/DialogAuth.py'
244--- softwareproperties/gtk/DialogAuth.py 2018-03-21 15:52:33 +0000
245+++ softwareproperties/gtk/DialogAuth.py 2019-01-17 18:48:56 +0000
246@@ -19,6 +19,7 @@
247 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
248 # USA
249
250+from enum import IntEnum
251 import os
252
253 from gettext import gettext as _
254@@ -28,9 +29,13 @@
255
256 import gi
257 gi.require_version('Goa', '1.0')
258-from gi.repository import Gio, GLib, Goa, GObject, Gtk
259+from gi.repository import Gio, GLib, Goa, Gtk
260 import logging
261
262+class Column(IntEnum):
263+ ID = 0
264+ MAIL = 1
265+ ACCOUNT = 2
266
267 class DialogAuth:
268
269@@ -38,68 +43,126 @@
270 """setup up the gtk dialog"""
271 self.parent = parent
272
273- setup_ui(self, os.path.join(datadir, "gtkbuilder",
274- "dialog-auth.ui"), domain="software-properties")
275- self.label_title.set_max_width_chars(50)
276+ setup_ui(self, os.path.join(datadir, "gtkbuilder", "dialog-auth.ui"),
277+ domain="software-properties")
278
279 self.dialog = self.dialog_auth
280- self.dialog.use_header_bar = True
281+ self.dialog.set_title('')
282+ self.dialog.set_deletable(False)
283 self.dialog.set_transient_for(parent)
284
285- self.listboxrow_new_account.account = None
286+ self.button_continue.grab_focus()
287
288 self.account = None
289 self.dispose_on_new_account = False
290 self.goa_client = Goa.Client.new_sync(None)
291
292- self.listbox_accounts.connect('row-activated', self._listbox_accounts_row_activated_cb)
293+ self._setup_model()
294+ self._check_ui(select=False)
295
296 # Be ready to other accounts
297 self.goa_client.connect('account-added', self._account_added_cb)
298 self.goa_client.connect('account-removed', self._account_removed_cb)
299
300- self._setup_listbox_accounts()
301- self._check_ui()
302-
303 def run(self):
304 res = self.dialog.run()
305 self.dialog.hide()
306 return res
307
308- def _check_ui(self):
309- rows = self.listbox_accounts.get_children()
310- has_accounts = len(rows) > 1
311-
312- if has_accounts:
313- title = _('To continue choose an Ubuntu Single Sign-On account.')
314- new_account = _('Use another account…')
315- else:
316- title = _('To continue you need an Ubuntu Single Sign-On account.')
317- new_account = _('Sign In…')
318-
319- self.label_title.set_text(title)
320- self.label_new_account.set_markup('<b>{}</b>'.format(new_account))
321-
322- def _setup_listbox_accounts(self):
323+ def _setup_model(self):
324 for obj in self.goa_client.get_accounts():
325- account = obj.get_account()
326- if self._is_account_supported(account):
327- self._add_account(account)
328-
329- def _is_account_supported(self, account):
330- return account.props.provider_type == 'ubuntusso'
331-
332- def _add_account(self, account):
333- row = self._create_row(account)
334- self.listbox_accounts.prepend(row)
335- self._check_ui()
336+ self._add_account(obj.get_account(), select=False)
337+
338+ def _set_header(self, label):
339+ self.label_header.set_markup(
340+ "<span size='larger' weight='bold'>%s</span>" % label)
341+
342+ def _check_ui(self, select):
343+ naccounts = len(self.liststore_account)
344+
345+ if naccounts == 0:
346+ self._set_header(
347+ _('To use Livepatch, you need to use an Ubuntu One account.'))
348+ self.combobox_account.set_visible(False)
349+ self.label_account.set_visible(False)
350+ self.button_add_another.set_visible(False)
351+ self.button_continue.set_label(_('Sign In / Register…'))
352+ elif naccounts == 1:
353+ self._set_header(
354+ _('To use Livepatch, you need to use your Ubuntu One account.'))
355+ self.combobox_account.set_visible(False)
356+ self.label_account.set_visible(True)
357+ self.label_account.set_text(self.liststore_account[0][Column.MAIL])
358+ self.button_add_another.set_visible(True)
359+ self.button_continue.set_label(_('Continue'))
360+ else:
361+ self._set_header(
362+ _('To use Livepatch, you need to use an Ubuntu One account.'))
363+ self.button_add_another.set_visible(True)
364+ self.combobox_account.set_visible(True)
365+ self.label_account.set_visible(False)
366+ self.button_continue.set_label(_('Use'))
367+ if select:
368+ self.combobox_account.set_active(naccounts-1)
369+ elif self.combobox_account.get_active() == -1:
370+ self.combobox_account.set_active(0)
371+
372+ def _ignore_account(self, account):
373+ return account.props.provider_type != 'ubuntusso'
374+
375+ def _get_account_iter(self, account):
376+ row = self.liststore_account.get_iter_first()
377+ while row is not None:
378+ account_id = self.liststore_account.get_value(row, Column.ID)
379+ if account_id == account.props.id:
380+ return row
381+ row = self.liststore_account.iter_next(row)
382+ return None
383+
384+ def _add_account(self, account, select):
385+ if self._ignore_account(account):
386+ return
387+
388+ account_iter = self._get_account_iter(account)
389+ if account_iter is not None:
390+ return
391+
392+ account_iter = self.liststore_account.append()
393+ self.liststore_account.set(account_iter,
394+ [Column.ID, Column.MAIL, Column.ACCOUNT],
395+ [account.props.id, account.props.presentation_identity, account])
396+ self._check_ui(select)
397
398 def _remove_account(self, account):
399- for row in self.listbox_accounts.get_children():
400- if row.account == account:
401- row.destroy()
402- self._check_ui()
403- break
404+ if self._ignore_account(account):
405+ return
406+
407+ account_iter = self._get_account_iter(account)
408+ if account_iter is None:
409+ return
410+
411+ self.liststore_account.remove(account_iter)
412+ self._check_ui(select=False)
413+
414+ def _response_if_valid(self, account):
415+ def cb(source, res, data):
416+ try:
417+ source.call_ensure_credentials_finish(res)
418+ valid = True
419+ except GLib.Error as e:
420+ valid = False
421+
422+ if not valid:
423+ try:
424+ self._spawn_goa_with_args(account.props.id, None)
425+ except GLib.Error as e:
426+ logging.warning ('Failed to spawn gnome-control-center: %s',
427+ e.message)
428+ else:
429+ self.account = account
430+ self.dialog.response(Gtk.ResponseType.OK)
431+
432+ account.call_ensure_credentials(None, cb, None)
433
434 def _build_dbus_params(self, action, arg):
435 builder = GLib.VariantBuilder.new(GLib.VariantType.new('av'))
436@@ -118,7 +181,8 @@
437 v = GLib.Variant.new_variant(s)
438 builder.add_value(v)
439
440- array = GLib.Variant.new_tuple(GLib.Variant.new_string('online-accounts'), builder.end())
441+ array = GLib.Variant.new_tuple(
442+ GLib.Variant.new_string('online-accounts'), builder.end())
443 array = GLib.Variant.new_variant(array)
444
445 param = GLib.Variant.new_tuple(
446@@ -135,94 +199,43 @@
447 'org.gtk.Actions', None)
448
449 param = self._build_dbus_params(action, arg)
450- timeout = 10*60*1000 # 10 minutes should be enough to create an account
451- proxy.call_sync('Activate', param, Gio.DBusCallFlags.NONE, timeout, None)
452-
453- def _create_row(self, account):
454- identity = account.props.presentation_identity
455- provider_name = account.props.provider_name
456-
457- row = Gtk.ListBoxRow.new()
458- row.show()
459-
460- hbox = Gtk.Box.new(Gtk.Orientation.HORIZONTAL, 6)
461- hbox.set_hexpand(True)
462- hbox.show()
463-
464- image = Gtk.Image.new_from_icon_name('avatar-default', Gtk.IconSize.DIALOG)
465- image.set_pixel_size(48)
466- image.show()
467- hbox.pack_start(image, False, False, 0)
468-
469- vbox = Gtk.Box.new(Gtk.Orientation.VERTICAL, 2)
470- vbox.set_valign(Gtk.Align.CENTER)
471- vbox.show()
472- hbox.pack_start(vbox, False, False, 0)
473-
474- if identity:
475- ilabel = Gtk.Label.new()
476- ilabel.set_halign(Gtk.Align.START)
477- ilabel.set_markup('<b>{}</b>'.format(identity))
478- ilabel.show()
479- vbox.pack_start(ilabel, True, True, 0)
480-
481- if provider_name:
482- plabel = Gtk.Label.new()
483- plabel.set_halign(Gtk.Align.START)
484- plabel.set_markup('<small><span foreground=\"#555555\">{}</span></small>'.format(provider_name))
485- plabel.show()
486- vbox.pack_start(plabel, True, True, 0)
487-
488- warning_icon = Gtk.Image.new_from_icon_name('dialog-warning-symbolic', Gtk.IconSize.BUTTON)
489- warning_icon.set_no_show_all(True)
490- warning_icon.set_margin_end (15)
491- hbox.pack_end(warning_icon, False, False, 0)
492-
493- row.add(hbox)
494-
495- account.bind_property('attention-needed', warning_icon, 'visible',
496- GObject.BindingFlags.DEFAULT | GObject.BindingFlags.SYNC_CREATE)
497-
498- row.account = account
499- return row
500-
501- # Signals handlers
502- def _listbox_accounts_row_activated_cb(self, listbox, row):
503- account = row.account
504-
505- if account is None:
506- # TODO (azzar1): there is no easy way to put this to false
507- # if the user close the windows without adding an account.
508- # We need to discuss with goa's upstream to support such usercases
509- try:
510- self._spawn_goa_with_args('add', 'ubuntusso')
511- self.dispose_on_new_account = True
512- except GLib.Error as e:
513- logging.warning ('Failed to spawing gnome-control-center: %s', e.message)
514- else:
515- if account.props.attention_needed:
516- try:
517- self._spawn_goa_with_args(account.props.id, None)
518- except GLib.Error as e:
519- logging.warning ('Failed to spawing gnome-control-center: %s', e.message)
520- else:
521- self.account = account
522- self.dialog.response(Gtk.ResponseType.OK)
523+ proxy.call_sync('Activate', param, Gio.DBusCallFlags.NONE, -1, None)
524
525 def _account_added_cb(self, goa_client, goa_object):
526 account = goa_object.get_account()
527- if not self._is_account_supported(account):
528+ if self._ignore_account(account):
529 return
530 if not self.dispose_on_new_account:
531- self._add_account(account)
532+ self._add_account(account, True)
533 else:
534- self.account = account
535- self.dialog.response(Gtk.ResponseType.OK)
536+ self._response_if_valid(account)
537
538 def _account_removed_cb(self, goa_client, goa_object):
539 account = goa_object.get_account()
540- if self._is_account_supported(account):
541+ if not self._ignore_account(account):
542 self._remove_account(account)
543
544-
545-
546+ def _button_add_another_clicked_cb(self, button):
547+ try:
548+ # There is no easy way to put this to false if the user close the
549+ # windows without adding an account.
550+ self._spawn_goa_with_args('add', 'ubuntusso')
551+ self.dispose_on_new_account = True
552+ except GLib.Error as e:
553+ logging.warning ('Failed to spawn control-center: %s', e.message)
554+
555+ def _button_cancel_clicked_cb(self, button):
556+ self.dialog.response(Gtk.ResponseType.CANCEL)
557+
558+ def _button_continue_clicked_cb(self, button):
559+ naccounts = len(self.liststore_account)
560+
561+ account = None
562+ if naccounts >= 1:
563+ active_index = self.combobox_account.get_active()
564+ account = self.liststore_account[active_index][Column.ACCOUNT]
565+
566+ if account is None:
567+ self._button_add_another_clicked_cb(self.button_add_another)
568+ else:
569+ self._response_if_valid(account)

Subscribers

People subscribed via source and target branches

to status/vote changes: