Merge lp:~donadigo/appcenter/sorting-headers into lp:~elementary-apps/appcenter/appcenter

[Adam Bieńkowski]

Simplify sorting headers and comparing rows

This branch simplifies header sorting and comparing rows to sort them also. The branch is a part of drivers-2 branch.

Header assigning is now done within Gtk's native header_func function, rather than having it in sorting rows.

[Jeremy Wootten (jeremywootten)]

I notice that, when running "Update all", "Searching for updates" and its spinner appear before installation of updates has completed, on a line below the applications being updated. Is this intended? It looks a bit odd.

[Felipe Escoto (philip.scott)]

The searching for updates still shows on master, so i'm merging it since it just shouldn't appear

review: Approve

[RabbitBot (rabbitbot-a)]

Attempt to merge into lp:appcenter failed due to conflicts:

text conflict in src/Views/AppListView.vala
text conflict in src/Widgets/UpdateHeaderRow.vala

[David Hewitt (davidmhewitt)]

This functionality has been merged into the GitHub repository of AppCenter. Closing.

Unmerged revisions

406. By Adam Bieńkowski on 2017-02-20

Do not remove the update button from the size group

405. By Adam Bieńkowski on 2017-02-20

Simplify comparing rows and assigning headers for them

=== modified file 'src/Views/AppListView.vala'
--- src/Views/AppListView.vala	2017-02-15 18:02:21 +0000
+++ src/Views/AppListView.vala	2017-02-20 17:20:45 +0000
@@ -86,9 +86,7 @@
       * Does not show Uninstall Button **/
     public class AppListUpdateView : AbstractAppList {
         private bool updates_on_top;
-        private Widgets.UpdateHeaderRow updates_header;
-        private Widgets.UpdateHeaderRow updated_header;
-        private Widgets.AppActionButton update_all_button;
+        private Widgets.AppActionButton? update_all_button;
         private bool updating_all_apps = false;
         private bool apps_remaining_started = false;
         private GLib.Mutex update_mutex;
@@ -105,7 +103,7 @@
             set {
                 if (_updating_cache != value) {
                     _updating_cache = value;
-                    update_headers ();
+                    list_box.invalidate_headers ();
                 }
             }
         }
@@ -113,19 +111,7 @@
         construct {
             updates_on_top = true;
 
-            update_all_button = new Widgets.AppActionButton (_("Update All"));
-            update_all_button.set_suggested_action_header ();
-            update_all_button.clicked.connect (on_update_all);
-            update_all_button.no_show_all = true;
-            action_button_group.add_widget (update_all_button);
-
-            updates_header = new Widgets.UpdateHeaderRow.updates ();
-            updates_header.add_widget (update_all_button);
-
-            updated_header = new Widgets.UpdateHeaderRow.updated ();
-
-            list_box.add (updates_header);
-            list_box.add (updated_header);
+            list_box.set_header_func ((Gtk.ListBoxUpdateHeaderFunc) row_update_header);
 
             update_mutex = GLib.Mutex ();
             apps_to_update = new Gee.LinkedList<AppCenterCore.Package> ();
@@ -137,7 +123,7 @@
             _updating_cache = true;
         }
 
-        protected override void after_add_remove_change_row () {update_headers ();}
+        protected override void after_add_remove_change_row () { list_box.invalidate_headers (); }
 
         protected override Widgets.AppListRow make_row (AppCenterCore.Package package) {
             return (Widgets.AppListRow)(new Widgets.PackageRow.installed (package, action_button_group, false));
@@ -145,34 +131,72 @@
 
         protected override void on_package_changing (AppCenterCore.Package package, bool is_changing) {
             base.on_package_changing (package, is_changing);
-            update_all_button.sensitive = packages_changing == 0;
+            if (update_all_button != null) {
+                update_all_button.sensitive = packages_changing == 0;
+            }
         }
 
         [CCode (instance_pos = -1)]
         protected override int package_row_compare (Widgets.AppListRow row1, Widgets.AppListRow row2) {
-            bool a_is_header = !row1.has_package ();
-            bool b_is_header = !row2.has_package ();
-            bool a_has_updates = row1.get_update_available ();
-            bool b_has_updates = row2.get_update_available ();
-
-            if (a_is_header) {
-                return (a_has_updates || !b_has_updates) ? -1 : 1;
-            } else if (b_is_header) {
-                return (b_has_updates || !a_has_updates) ? 1 : -1;
-            }
-
             bool a_is_os = row1.get_is_os_updates ();
             bool b_is_os = row2.get_is_os_updates ();
 
             if (a_is_os || b_is_os) { /* OS update row sorts ahead of other update rows */
                 return a_is_os ? -1 : 1;
-            } else if ((a_has_updates && !b_has_updates) || (!a_has_updates && b_has_updates)) { /* Updates rows sort ahead of updated rows */
+            }
+
+            bool a_has_updates = row1.get_update_available ();
+            bool b_has_updates = row2.get_update_available ();
+
+            if ((a_has_updates && !b_has_updates) || (!a_has_updates && b_has_updates)) { /* Updates rows sort ahead of updated rows */
                 return a_has_updates ? -1 : 1;
             }
 
             return row1.get_name_label ().collate (row2.get_name_label ()); /* Else sort in name order */
         }
 
+        [CCode (instance_pos = -1)]
+        private void row_update_header (Widgets.AppListRow row, Widgets.AppListRow? before) {
+            bool update_available = row.get_update_available ();            
+            if (before != null && update_available == before.get_update_available ()) {
+                row.set_header (null);
+                return;
+            }
+
+            if (update_available) {
+                var header = new Widgets.UpdatesGrid ();
+
+                uint update_numbers = 0U;
+                uint64 update_real_size = 0ULL;
+                foreach (var package in get_packages ()) {
+                    if (package.update_available) {
+                        update_numbers++;
+                        update_real_size += package.change_information.get_size ();
+                    }
+                }
+
+                header.update (update_numbers, update_real_size, updating_cache);
+
+                // Unfortunately the update all button needs to be recreated everytime the header needs to be updated
+                if (!updating_cache && update_numbers > 0) {
+                    update_all_button = new Widgets.AppActionButton (_("Update All"));
+                    update_all_button.set_suggested_action_header ();
+                    update_all_button.clicked.connect (on_update_all);
+                    action_button_group.add_widget (update_all_button);
+
+                    header.add_widget (update_all_button);   
+                }
+
+                header.show_all ();
+                row.set_header (header);                
+            } else {
+                var header = new Widgets.UpdatedGrid ();
+                header.update (0, 0, updating_cache);
+                header.show_all ();
+                row.set_header (header);              
+            }
+        }
+
         private void on_update_all () {
             perform_all_updates.begin ();
         }
@@ -278,42 +302,5 @@
                 return GLib.Source.REMOVE;
             });
         }
-
-        private void update_headers () {
-            /* Do not repeatedly update headers while rapidly adding packages to list */
-            schedule_header_update ();
-        }
-
-        uint grid_timeout_id = 0;
-        private void schedule_header_update () {
-            if (grid_timeout_id > 0) {
-                return;
-            }
-
-            grid_timeout_id = GLib.Timeout.add (20, () => {
-                update_updates_grid ();
-                update_updated_grid ();
-                grid_timeout_id = 0;
-                return false;
-            });
-        }
-
-        private void update_updates_grid () {
-            uint update_numbers = 0U;
-            uint64 update_real_size = 0ULL;
-            foreach (var package in get_packages ()) {
-                if (package.update_available) {
-                    update_numbers++;
-                    update_real_size += package.change_information.get_size ();
-                }
-            }
-
-            updates_header.update (update_numbers, update_real_size, updating_cache);
-            update_all_button.visible = !updating_cache && update_numbers > 0;
-        }
-
-        private void update_updated_grid () {
-            updated_header.update (0, 0, updating_cache);
-        }
     }
 }

=== modified file 'src/Widgets/UpdateHeaderRow.vala'
--- src/Widgets/UpdateHeaderRow.vala	2016-09-17 19:28:03 +0000
+++ src/Widgets/UpdateHeaderRow.vala	2017-02-20 17:20:45 +0000
@@ -19,153 +19,99 @@
  */
 
 namespace AppCenter.Widgets {
-    public class UpdateHeaderRow : Gtk.ListBoxRow, AppListRow {
-        AbstractHeaderGrid grid;
-        public bool is_updates_header {get; private set;}
+    /** Base class for Grids in Header Rows **/
+    public abstract class AbstractHeaderGrid : Gtk.Grid {
+        public uint update_numbers {get; protected set; default = 0;}
+        public uint64 update_real_size {get; protected set; default = 0;}
+        public bool is_updating {get; protected set; default = false;}
 
         construct {
-            set_activatable (false);
-            set_selectable (false);
-        }
-
-        public UpdateHeaderRow.updates () {
-            grid = new UpdatesGrid ();
-            add (grid);
-            is_updates_header = true;
-        }
-
-        public UpdateHeaderRow.updated () {
-            grid = new UpdatedGrid ();
-            add (grid);
-            is_updates_header = false;
-        }
-
-        /** AppListRow Interface methods **/
-
-        /** Updates header row reports it has updates in order to sort first in list **/
-        /** Updated header reports it has updates when updating in order to sort first in list **/
-        public bool get_update_available () {
-            return is_updates_header || grid.is_updating;
-        }
-
-        public bool get_is_os_updates () {
-            critical ("Must not attempt to get is_os_update from header row");
-            assert_not_reached ();
-        }
-
-        /* This indicates it is a header row, not a package row */
-        public bool has_package () {return false;}
-
-        public AppCenterCore.Package? get_package () {
-            critical ("Must not attempt to get package from header row");
-            assert_not_reached ();
-        }
-
-        public string get_name_label () {
-            critical ("Must not attempt to get package name from header row");
-            assert_not_reached ();
+            margin = 12;
+            column_spacing = 12;
+        }
+
+        protected void store_data (uint _update_numbers, uint64 _update_real_size, bool _is_updating) {
+            update_numbers = _update_numbers;
+            update_real_size = _update_real_size;
+            is_updating = _is_updating;
         }
 
         public void add_widget (Gtk.Widget widget) {
-            grid.add (widget);
-        }
-
-        public void update (uint _update_numbers, uint64 _update_real_size, bool _is_updating) {
-            grid.update (_update_numbers, _update_real_size, _is_updating);
-            changed (); /* Triggers resort */
-        }
-        /** ---------------- **/
-
-        /** Base class for Grids in Header Rows **/
-        private abstract class AbstractHeaderGrid : Gtk.Grid {
-            public uint update_numbers {get; protected set; default = 0;}
-            public uint64 update_real_size {get; protected set; default = 0;}
-            public bool is_updating {get; protected set; default = false;}
-
-            construct {
-                margin = 12;
-                column_spacing = 12;
-            }
-
-            protected void store_data (uint _update_numbers, uint64 _update_real_size, bool _is_updating) {
-                update_numbers = _update_numbers;
-                update_real_size = _update_real_size;
-                is_updating = _is_updating;
-            }
-
-            public abstract void update (uint _update_numbers, uint64 _update_real_size, bool _is_updating);
-        }
-
-        /** Header to show at top of list if there are updates available **/
-        private class UpdatesGrid : AbstractHeaderGrid {
-            private Gtk.Label update_size_label;
-            private Gtk.Label updates_label;
-
-            construct {
-                margin_top = 18;
-                updates_label = new Gtk.Label (null);
-                ((Gtk.Misc) updates_label).xalign = 0;
-                updates_label.get_style_context ().add_class ("h4");
-                updates_label.hexpand = true;
-
-                update_size_label = new Gtk.Label (null);
-
-                add (updates_label);
-                add (update_size_label);
-            }
-
-            public override void update (uint _update_numbers, uint64 _update_real_size, bool _is_updating) {
-                store_data (_update_numbers,  _update_real_size, _is_updating);
-
-                if (!is_updating) {
-                    updates_label.label = ngettext ("%u Update Available", "%u Updates Available", update_numbers).printf (update_numbers);
-                    update_size_label.label = _("Size: %s").printf (GLib.format_size (update_real_size));
-                    if (update_numbers > 0) {
-                        show_all ();
-                    } else {
-                        hide ();
-                    }
-                } else {
-                    hide (); /* Updated header shows updating spinner and message */
-                }
-            }
-        }
-
-        /** Header to show above first package that is up to date, or if the cache is updating **/
-        private class UpdatedGrid : AbstractHeaderGrid {
-            private Gtk.Label label;
-            private Gtk.Spinner spinner;
-
-            construct {
-                label = new Gtk.Label (""); /* Should not be displayed before being updated */
-                label.hexpand = true;
-                ((Gtk.Misc)label).xalign = 0;
-
-                spinner = new Gtk.Spinner ();
-
-                add (label);
-                add (spinner);
-            }
-
-            public override void update (uint _update_numbers, uint64 _update_real_size, bool _is_updating) {
-                store_data (_update_numbers,  _update_real_size, _is_updating);
-
-                if (is_updating) {
-                    halign = Gtk.Align.CENTER;
-                    spinner.start ();
-                    spinner.no_show_all = false;
-                    spinner.show ();
-                    label.label = _("Searching for updates…");
-                    label.get_style_context ().remove_class ("h4");
-                } else {
-                    halign = Gtk.Align.START;
-                    spinner.stop ();
-                    spinner.no_show_all = true;
-                    spinner.hide ();
-                    label.label = _("Up to Date");
-                    label.get_style_context ().add_class ("h4");
-                }
-            }
-        }
-    }
-}
+            add (widget);
+        }
+
+        public abstract void update (uint _update_numbers, uint64 _update_real_size, bool _is_updating);
+    }
+
+    /** Header to show at top of list if there are updates available **/
+    public class UpdatesGrid : AbstractHeaderGrid {
+        private Gtk.Label update_size_label;
+        private Gtk.Label updates_label;
+
+        construct {
+            margin_top = 18;
+            updates_label = new Gtk.Label (null);
+            ((Gtk.Misc) updates_label).xalign = 0;
+            updates_label.get_style_context ().add_class ("h4");
+            updates_label.hexpand = true;
+
+            update_size_label = new Gtk.Label (null);
+
+            add (updates_label);
+            add (update_size_label);
+        }
+
+        public override void update (uint _update_numbers, uint64 _update_real_size, bool _is_updating) {
+            store_data (_update_numbers,  _update_real_size, _is_updating);
+
+            if (!is_updating) {
+                updates_label.label = ngettext ("%u Update Available", "%u Updates Available", update_numbers).printf (update_numbers);
+                update_size_label.label = _("Size: %s").printf (GLib.format_size (update_real_size));
+                if (update_numbers > 0) {
+                    show_all ();
+                } else {
+                    hide ();
+                }
+            } else {
+                hide (); /* Updated header shows updating spinner and message */
+            }
+        }
+    }
+
+    /** Header to show above first package that is up to date, or if the cache is updating **/
+    public class UpdatedGrid : AbstractHeaderGrid {
+        private Gtk.Label label;
+        private Gtk.Spinner spinner;
+
+        construct {
+            label = new Gtk.Label (""); /* Should not be displayed before being updated */
+            label.hexpand = true;
+            ((Gtk.Misc)label).xalign = 0;
+
+            spinner = new Gtk.Spinner ();
+
+            add (label);
+            add (spinner);
+        }
+
+        public override void update (uint _update_numbers, uint64 _update_real_size, bool _is_updating) {
+            store_data (_update_numbers,  _update_real_size, _is_updating);
+
+            if (is_updating) {
+                halign = Gtk.Align.CENTER;
+                spinner.start ();
+                spinner.no_show_all = false;
+                spinner.show ();
+                label.label = _("Searching for updates…");
+                label.get_style_context ().remove_class ("h4");
+            } else {
+                halign = Gtk.Align.START;
+                spinner.stop ();
+                spinner.no_show_all = true;
+                spinner.hide ();
+                label.label = _("Up to Date");
+                label.get_style_context ().add_class ("h4");
+            }
+        }
+    }
+}
\ No newline at end of file