Merge lp:~benwaffle/appcenter/buttons into lp:~elementary-apps/appcenter/appcenter

Proposed by Ben
Status: Merged
Approved by: Danielle Foré
Approved revision: 218
Merged at revision: 218
Proposed branch: lp:~benwaffle/appcenter/buttons
Merge into: lp:~elementary-apps/appcenter/appcenter
Diff against target: 189 lines (+75/-42)
2 files modified
src/Views/AppListView.vala (+1/-3)
src/Widgets/PackageRow.vala (+74/-39)
To merge this branch: bzr merge lp:~benwaffle/appcenter/buttons
Reviewer Review Type Date Requested Status
Danielle Foré Needs Fixing
Adam Bieńkowski (community) testing Needs Fixing
Review via email: mp+298054@code.launchpad.net

Commit message

Fix only update buttons showing up in search results

Description of the change

Fix bug 1593609. Now, the search results have the same buttons as the App Info view (most of the code was copied from there). Perhaps we should consider creating a new widget for the install/update/uninstall/progressbar group in the future.

To post a comment you must log in.
lp:~benwaffle/appcenter/buttons updated
211. By Ben

Don't offer to uninstall OS updates

Revision history for this message
Adam Bieńkowski (donadigo) wrote :

It seems good to me, but "Update all" button is not the same width as the other buttons in the list. Also I wouldn't throw criticals in these catch () statements, warning should be enough.

review: Needs Fixing (testing)
lp:~benwaffle/appcenter/buttons updated
212. By Ben

remove uninstall button in search and update views

213. By Ben

Remove suggested-action from individual update button

214. By Ben

fix margins for update button sizegroup

Revision history for this message
Danielle Foré (danrabbit) wrote :

Please merge trunk

I feel like using a symbolic cancel icon is a regression from trunk. Using the label button keeps the UI from shifting and is also a clearer and larger click target

Currently in trunk, after installing the button with change to "uninstall". In your branch it remains as "install"

review: Needs Fixing
lp:~benwaffle/appcenter/buttons updated
215. By Ben

Merge trunk

Revision history for this message
Danielle Foré (danrabbit) wrote :

As mentioned previously, please use the cancel label button instead of process-stop-symbolic

review: Needs Fixing
lp:~benwaffle/appcenter/buttons updated
216. By Ben

Use text cancel button

Revision history for this message
Danielle Foré (danrabbit) wrote :

* Doesn't look like the Cancel button has been added to the sizegroup
* Also looks like there is no column_spacing in progress_grid

review: Needs Fixing
lp:~benwaffle/appcenter/buttons updated
217. By Ben

make cancel button look better

218. By Ben

make sure cancel button is exactly the same as the install

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/Views/AppListView.vala'
--- src/Views/AppListView.vala 2016-06-25 15:34:25 +0000
+++ src/Views/AppListView.vala 2016-06-26 23:17:14 +0000
@@ -71,7 +71,7 @@
7171
72 public void add_package (AppCenterCore.Package package) {72 public void add_package (AppCenterCore.Package package) {
73 var row = new Widgets.PackageRow (package);73 var row = new Widgets.PackageRow (package);
74 update_button_group.add_widget (row.update_button);74 update_button_group.add_widget (row.action_button);
75 update_button_group.add_widget (row.cancel_button);75 update_button_group.add_widget (row.cancel_button);
76 row.show_all ();76 row.show_all ();
77 list_box.add (row);77 list_box.add (row);
@@ -126,7 +126,6 @@
126 [CCode (instance_pos = -1)]126 [CCode (instance_pos = -1)]
127 private void package_row_update_header (Widgets.PackageRow row, Widgets.PackageRow? before) {127 private void package_row_update_header (Widgets.PackageRow row, Widgets.PackageRow? before) {
128 bool update_available = row.package.update_available;128 bool update_available = row.package.update_available;
129 row.action_stack.visible = row.package.update_available;
130 if (before == null && update_available) {129 if (before == null && update_available) {
131 var updates_grid = get_updates_grid ();130 var updates_grid = get_updates_grid ();
132 row.set_header (updates_grid);131 row.set_header (updates_grid);
@@ -142,7 +141,6 @@
142 var updated_grid = new Gtk.Grid ();141 var updated_grid = new Gtk.Grid ();
143 updated_grid.orientation = Gtk.Orientation.HORIZONTAL;142 updated_grid.orientation = Gtk.Orientation.HORIZONTAL;
144 updated_grid.column_spacing = 12;143 updated_grid.column_spacing = 12;
145 updated_grid.margin = 6;
146 if (updating_cache) {144 if (updating_cache) {
147 updated_grid.halign = Gtk.Align.CENTER;145 updated_grid.halign = Gtk.Align.CENTER;
148 var updating_label = new Gtk.Label (_("Searching for updates…"));146 var updating_label = new Gtk.Label (_("Searching for updates…"));
149147
=== modified file 'src/Widgets/PackageRow.vala'
--- src/Widgets/PackageRow.vala 2016-06-21 18:28:52 +0000
+++ src/Widgets/PackageRow.vala 2016-06-26 23:17:14 +0000
@@ -27,12 +27,12 @@
27 Gtk.Label package_name;27 Gtk.Label package_name;
28 Gtk.Label package_summary;28 Gtk.Label package_summary;
2929
30 public Gtk.Button update_button;30 // The action button covers Install and Update
31 public Gtk.Button action_button;
32 Gtk.ProgressBar progress_bar;
33 Gtk.Label progress_label;
31 public Gtk.Button cancel_button;34 public Gtk.Button cancel_button;
32 public Gtk.Stack action_stack;35 Gtk.Stack action_stack;
33 Gtk.Grid update_grid;
34 Gtk.ProgressBar update_progressbar;
35 Gtk.Label update_label;
3636
37 public PackageRow (Package package) {37 public PackageRow (Package package) {
38 this.package = package;38 this.package = package;
@@ -41,11 +41,38 @@
41 package_summary.ellipsize = Pango.EllipsizeMode.END;41 package_summary.ellipsize = Pango.EllipsizeMode.END;
42 image.gicon = package.get_icon ();42 image.gicon = package.get_icon ();
4343
44 if (package.update_available) {
45 action_button.label = _("Update");
46 } else if (package.installed) {
47 action_stack.no_show_all = true;
48 action_stack.hide ();
49 }
50
44 package.notify["installed"].connect (() => {51 package.notify["installed"].connect (() => {
52 if (package.installed && package.update_available) {
53 action_button.label = _("Update");
54 action_stack.no_show_all = false;
55 action_stack.show ();
56 } else if (package.installed) {
57 action_stack.no_show_all = true;
58 action_stack.hide ();
59 } else {
60 action_button.label = _("Install");
61 action_stack.no_show_all = false;
62 action_stack.show ();
63 }
45 changed ();64 changed ();
46 });65 });
4766
48 package.notify["update-available"].connect (() => {67 package.notify["update-available"].connect (() => {
68 if (package.update_available) {
69 action_button.label = _("Update");
70 action_stack.no_show_all = false;
71 action_stack.show_all ();
72 } else {
73 action_stack.no_show_all = true;
74 action_stack.hide ();
75 }
49 changed ();76 changed ();
50 });77 });
5178
@@ -81,28 +108,32 @@
81 ((Gtk.Misc) package_summary).xalign = 0;108 ((Gtk.Misc) package_summary).xalign = 0;
82109
83 action_stack = new Gtk.Stack ();110 action_stack = new Gtk.Stack ();
84111 action_stack.transition_type = Gtk.StackTransitionType.CROSSFADE;
85 update_button = new Gtk.Button.with_label (_("Update"));112 action_stack.hhomogeneous = false;
86 update_button.halign = Gtk.Align.END;113
87 update_button.valign = Gtk.Align.CENTER;114 progress_bar = new Gtk.ProgressBar ();
88 update_button.margin_end = 6;115
89 update_button.clicked.connect (() => update_package.begin ());116 progress_label = new Gtk.Label (null);
90
91 update_label = new Gtk.Label (null);
92 update_progressbar = new Gtk.ProgressBar ();
93 update_progressbar.margin_end = 12;
94117
95 cancel_button = new Gtk.Button.with_label (_("Cancel"));118 cancel_button = new Gtk.Button.with_label (_("Cancel"));
119 cancel_button.margin_end = 6;
96 cancel_button.valign = Gtk.Align.CENTER;120 cancel_button.valign = Gtk.Align.CENTER;
97 cancel_button.margin_end = 6;121
98122 action_button = new Gtk.Button.with_label (_("Install"));
99 update_grid = new Gtk.Grid ();123 action_button.margin_end = 6;
100 update_grid.attach (update_label, 0, 0, 1, 1);124 action_button.valign = Gtk.Align.CENTER;
101 update_grid.attach (update_progressbar, 0, 1, 1, 1);125 action_button.clicked.connect (() => action_clicked.begin ());
102 update_grid.attach (cancel_button, 1, 0, 1, 2);126
103127 var progress_grid = new Gtk.Grid ();
104 action_stack.add (update_button);128 progress_grid.valign = Gtk.Align.CENTER;
105 action_stack.add (update_grid);129 progress_grid.row_spacing = 6;
130 progress_grid.column_spacing = 6;
131 progress_grid.attach (progress_label, 0, 0, 1, 1);
132 progress_grid.attach (progress_bar, 0, 1, 1, 1);
133 progress_grid.attach (cancel_button, 1, 0, 1, 2);
134
135 action_stack.add_named (action_button, "buttons");
136 action_stack.add_named (progress_grid, "progress");
106137
107 grid.attach (image, 0, 0, 1, 2);138 grid.attach (image, 0, 0, 1, 2);
108 grid.attach (package_name, 1, 0, 1, 1);139 grid.attach (package_name, 1, 0, 1, 1);
@@ -115,28 +146,32 @@
115 });146 });
116 }147 }
117148
118 private async void update_package () {
119 try {
120 update_button.sensitive = false;
121 yield package.update ();
122 } catch (Error e) {
123 critical (e.message);
124 }
125
126 update_button.sensitive = true;
127 }
128
129 private void update_status () {149 private void update_status () {
130 update_label.label = package.change_information.get_status ();150 progress_label.label = package.change_information.get_status ();
131 }151 }
132152
133 private void update_progress () {153 private void update_progress () {
134 var progress = package.change_information.get_progress ();154 var progress = package.change_information.get_progress ();
135 if (progress < 1.0f) {155 if (progress < 1.0f) {
136 action_stack.set_visible_child (update_grid);156 action_stack.set_visible_child_name ("progress");
137 update_progressbar.fraction = progress;157 progress_bar.fraction = progress;
138 } else {158 } else {
139 action_stack.set_visible_child (update_button);159 action_stack.set_visible_child_name ("buttons");
160 }
161 }
162
163 private async void action_clicked () {
164 try {
165 if (package.update_available) {
166 yield package.update ();
167 } else {
168 yield package.install ();
169 }
170 action_stack.no_show_all = true;
171 action_stack.hide ();
172 } catch (Error e) {
173 critical (e.message);
174 action_stack.set_visible_child_name ("buttons");
140 }175 }
141 }176 }
142}177}

Subscribers

People subscribed via source and target branches

to all changes: