Merge lp:~elementary-pantheon/switchboard-plug-security-privacy/abstract-service-panel into lp:~elementary-apps/switchboard-plug-security-privacy/trunk

[Danielle Foré]

Create AbstractServicePanel and use it for each page

This branch creates an abstract service panel and uses it. The main purpose here is to make sure that we keep the layout and design of each panel consistent and to make it easier to build future panels.

The abstract service panel comes with three areas: header_area, content_area, and action_area. The first one is private and you only interact with it by setting the icon_name, title, description?, and activatable properties of the panel. The latter are public and the naming should be familiar and predictable if you've used Gtk.Dialog

[lp:~elementary-pantheon/switchboard-plug-security-privacy/abstract-service-panel]

316. By Danielle Foré on 2017-03-02

get rid of a weird method

317. By Danielle Foré on 2017-03-02

get rid of some extra margins

[David Hewitt (davidmhewitt)]

This is good. Definitely simplifies the code in the panels. I think we should maybe look at making the AlertView when a service is disabled and the enabled/disabled status indicators a bit more generic too, so there's not as much repeated code there.

But this is a really good start.

review: Approve (code, function)

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2017-02-27 23:35:21 +0000
+++ src/CMakeLists.txt	2017-03-02 17:18:33 +0000
@@ -18,6 +18,7 @@
     UFWHelpers.vala
     ZGUtilities.vala
 
+    Views/AbstractServicePanel.vala
     Views/FirewallPanel.vala
     Views/LockPanel.vala
     Views/TrackPanel.vala
 
=== modified file 'src/Plug.vala'
--- src/Plug.vala	2017-03-01 23:31:31 +0000
+++ src/Plug.vala	2017-03-02 17:18:33 +0000
@@ -168,7 +168,6 @@
                 case "locking<sep>privacy-mode":
                     stack.set_visible_child_name ("locking");
                     service_list.select_service_name ("locking");
-                    tracking.focus_privacy_switch ();
                     break;
                 case "firewall":
                     stack.set_visible_child_name ("firewall");
 
=== added file 'src/Views/AbstractServicePanel.vala'
--- src/Views/AbstractServicePanel.vala	1970-01-01 00:00:00 +0000
+++ src/Views/AbstractServicePanel.vala	2017-03-02 17:18:33 +0000
@@ -0,0 +1,81 @@
+/*
+* Copyright (c) 2017 elementary LLC. (http://launchpad.net/switchboard-plug-security-privacy)
+*
+* This program is free software; you can redistribute it and/or
+* modify it under the terms of the GNU General Public
+* License as published by the Free Software Foundation; either
+* version 2 of the License, or (at your option) any later version.
+*
+* This program is distributed in the hope that it will be useful,
+* but WITHOUT ANY WARRANTY; without even the implied warranty of
+* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+* General Public License for more details.
+*
+* You should have received a copy of the GNU General Public
+* License along with this program; if not, write to the
+* Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+* Boston, MA 02110-1301 USA
+*
+*/
+
+public abstract class ServicePanel : Gtk.Grid {
+    public Gtk.ButtonBox action_area;
+    public Gtk.Grid content_area;
+    public Gtk.Switch? status_switch;
+
+    public bool activatable { get; construct; }
+
+    public string description { get; construct; }
+    public string icon_name { get; construct; }
+    public string title { get; construct; }
+
+    public ServicePanel () {
+        Object (activatable: activatable,
+                icon_name: icon_name,
+                description: description,
+                title: title);
+    }
+
+    construct {
+        var header_icon = new Gtk.Image.from_icon_name (icon_name, Gtk.IconSize.DIALOG);
+
+        var header_label = new Gtk.Label (title);
+        header_label.get_style_context ().add_class ("h2");
+
+        var header_area = new Gtk.Grid ();
+        header_area.column_spacing = 12;
+        header_area.add (header_icon);
+        header_area.add (header_label);
+
+        if (description != null) {
+            var description_icon = new Gtk.Image.from_icon_name ("help-info-symbolic", Gtk.IconSize.MENU);
+            description_icon.xalign = 0;
+            description_icon.tooltip_text = description;
+
+            header_area.add (description_icon);
+        }
+
+        if (activatable) {
+            status_switch = new Gtk.Switch ();
+            status_switch.hexpand = true;
+            status_switch.halign = Gtk.Align.END;
+            status_switch.valign = Gtk.Align.CENTER;
+            header_area.add (status_switch);
+        }
+
+        content_area = new Gtk.Grid ();
+        content_area.column_spacing = 12;
+        content_area.row_spacing = 12;
+
+        action_area = new Gtk.ButtonBox (Gtk.Orientation.HORIZONTAL);
+        action_area.set_layout (Gtk.ButtonBoxStyle.END);
+        action_area.set_spacing (6);
+
+        margin = 12;
+        orientation = Gtk.Orientation.VERTICAL;
+        row_spacing = 24;
+        add (header_area);
+        add (content_area);
+        add (action_area);
+    }
+}
 
=== modified file 'src/Views/FirewallPanel.vala'
--- src/Views/FirewallPanel.vala	2017-03-01 22:38:37 +0000
+++ src/Views/FirewallPanel.vala	2017-03-02 17:18:33 +0000
@@ -20,14 +20,13 @@
  * Authored by: Corentin Noël <[e-mail]>
  */
 
-public class SecurityPrivacy.FirewallPanel : Gtk.Grid {
+public class SecurityPrivacy.FirewallPanel : ServicePanel {
     private Gtk.ListStore list_store;
     private Gtk.TreeView view;
     private Gtk.Toolbar list_toolbar;
     private bool loading = false;
     private Gtk.Popover add_popover;
     private Gtk.ToolButton remove_button;
-    public Gtk.Switch status_switch;
 
     private enum Columns {
         ACTION,
@@ -40,22 +39,12 @@
     }
 
     public FirewallPanel () {
-        Object (column_spacing: 12,
-                margin: 12,
-                row_spacing: 6);
+        Object (activatable: true,
+                icon_name: "network-firewall",
+                title: _("Firewall"));
     }
 
     construct {
-        var status_icon = new Gtk.Image.from_icon_name ("network-firewall", Gtk.IconSize.DIALOG);
-
-        var status_label = new Gtk.Label (_("Firewall"));
-        status_label.get_style_context ().add_class ("h2");
-        status_label.hexpand = true;
-        status_label.xalign = 0;
-
-        status_switch = new Gtk.Switch ();
-        status_switch.valign = Gtk.Align.CENTER;
-
         status_switch.notify["active"].connect (() => {
             if (loading == false) {
                 view.sensitive = status_switch.active;
@@ -64,10 +53,6 @@
             show_rules ();
         });
 
-        attach (status_icon, 0, 0, 1, 1);
-        attach (status_label, 1, 0, 1, 1);
-        attach (status_switch, 2, 0, 1, 1);
-
         create_treeview ();
 
         sensitive = false;
@@ -264,9 +249,8 @@
         view_grid.attach (list_toolbar, 0, 1, 1, 1);
 
         var frame = new Gtk.Frame (null);
-        frame.margin_top = 12;
         frame.add (view_grid);
 
-        attach (frame, 0, 1, 3, 1);
+        content_area.attach (frame, 0, 1, 3, 1);
     }
 }
 
=== modified file 'src/Views/LocationPanel.vala'
--- src/Views/LocationPanel.vala	2017-03-01 22:38:37 +0000
+++ src/Views/LocationPanel.vala	2017-03-02 17:18:33 +0000
@@ -1,6 +1,6 @@
 // -*- Mode: vala; indent-tabs-mode: nil; tab-width: 4 -*-
 /*-
- * Copyright (c) 2017 elementary LLC.
+ * Copyright (c) 2017 elementary LLC. (http://launchpad.net/switchboard-plug-security-privacy)
  * Copyright (C) 2017 David Hewitt <[e-mail]>   
  *
  * This program is free software; you can redistribute it and/or modify
@@ -20,7 +20,7 @@
  * Authored by: David Hewitt <[e-mail]>
  */
 
-public class SecurityPrivacy.LocationPanel : Gtk.Grid {
+public class SecurityPrivacy.LocationPanel : ServicePanel {
 
     private GLib.Settings location_settings;
     private Variant remembered_apps;
@@ -29,7 +29,6 @@
     private Gtk.TreeView tree_view;
     private Gtk.Grid treeview_grid;
     private Gtk.Stack disabled_stack;
-    public Gtk.Switch status_switch;
 
     private enum Columns {
         AUTHORIZED,
@@ -39,28 +38,17 @@
         N_COLUMNS
     }
 
+    public LocationPanel () {
+        Object (activatable: true,
+                icon_name: "find-location",
+                title: _("Location Services"));
+    }
+
     construct {
-        column_spacing = 12;
-        row_spacing = 6;
-        margin = 12;
-
         location_settings = new GLib.Settings ("org.pantheon.agent-geoclue2");
         disabled_stack = new Gtk.Stack ();
         
-        var status_icon = new Gtk.Image.from_icon_name ("find-location", Gtk.IconSize.DIALOG);
-
-        var status_label = new Gtk.Label (_("Location Services"));
-        status_label.get_style_context ().add_class ("h2");
-        status_label.hexpand = true;
-        status_label.xalign = 0;
-
-        status_switch = new Gtk.Switch ();
-        status_switch.valign = Gtk.Align.CENTER;        
-
-        attach (status_icon, 0, 0, 1, 1);
-        attach (status_label, 1, 0, 1, 1);
-        attach (status_switch, 2, 0, 1, 1);
-        attach (disabled_stack, 0, 1, 3, 1);        
+        content_area.attach (disabled_stack, 0, 1, 3, 1);        
         
         create_treeview ();
         create_disabled_panel ();
@@ -144,7 +132,6 @@
         scrolled.add (tree_view);
 
         treeview_grid = new Gtk.Grid ();
-        treeview_grid.margin_top = 12;
         treeview_grid.row_spacing = 6;
         treeview_grid.attach (locations_label, 0, 0, 1, 1);
         treeview_grid.attach (scrolled, 0, 1, 1, 1);
 
=== modified file 'src/Views/LockPanel.vala'
--- src/Views/LockPanel.vala	2017-03-01 22:38:37 +0000
+++ src/Views/LockPanel.vala	2017-03-02 17:18:33 +0000
@@ -20,15 +20,16 @@
  * Authored by: Corentin Noël <[e-mail]>
  */
 
-public class SecurityPrivacy.LockPanel : Gtk.Grid {
+public class SecurityPrivacy.LockPanel : ServicePanel {
 
     Settings locker;
 
     public LockPanel () {
-        column_spacing = 12;
-        row_spacing = 6;
-        margin = 12;
+        Object (icon_name: "system-lock-screen",
+                title: _("Locking"));
+    }
 
+    construct {
         locker = new Settings ("apps.light-locker");
 
         var lock_suspend_label = new Gtk.Label (_("Lock on sleep:"));
@@ -66,15 +67,11 @@
         lock_suspend_switch.halign = Gtk.Align.START;
         lock_sleep_switch.halign = Gtk.Align.START;
 
-        var expander_grid = new Gtk.Grid ();
-        expander_grid.hexpand = true;
-
-        attach (expander_grid, 0, 0, 4, 1);
-        attach (lock_suspend_label, 1, 0, 1, 1);
-        attach (lock_sleep_label, 1, 1, 1, 1);
-        attach (lock_suspend_switch, 2, 0, 1, 1);
-        attach (lock_sleep_switch, 2, 1, 1, 1);
-
-        hexpand = true;
+        content_area.hexpand = true;
+        content_area.halign = Gtk.Align.CENTER;
+        content_area.attach (lock_suspend_label, 0, 0, 1, 1);
+        content_area.attach (lock_sleep_label, 0, 1, 1, 1);
+        content_area.attach (lock_suspend_switch, 1, 0, 1, 1);
+        content_area.attach (lock_sleep_switch, 1, 1, 1, 1);
     }
 }
 
=== modified file 'src/Views/TrackPanel.vala'
--- src/Views/TrackPanel.vala	2017-03-01 23:31:31 +0000
+++ src/Views/TrackPanel.vala	2017-03-02 17:18:33 +0000
@@ -1,6 +1,6 @@
 // -*- Mode: vala; indent-tabs-mode: nil; tab-width: 4 -*-
 /*-
- * Copyright (c) 2014 elementary LLC. ((http://launchpad.net/switchboard-plug-security-privacy)
+ * Copyright (c) 2014 elementary LLC. (http://launchpad.net/switchboard-plug-security-privacy)
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -20,14 +20,14 @@
  * Authored by: Corentin Noël <[e-mail]>
  */
 
-public class SecurityPrivacy.TrackPanel : Gtk.Grid {
+public class SecurityPrivacy.TrackPanel : ServicePanel {
     private Widgets.ClearUsagePopover remove_popover;
-    public Gtk.Switch record_switch;
 
     public TrackPanel () {
-        Object (column_spacing: 12,
-                margin: 12,
-                row_spacing: 12);
+        Object (activatable: true,
+                description: _("This operating system can gather useful statistics about file and app usage to provide extra functionality. If other people can see or access your account, you may wish to limit which items are recorded."),
+                icon_name: "document-open-recent",
+                title: _("History"));
     }
 
     construct {
@@ -43,30 +43,9 @@
         description_frame.no_show_all = true;
         description_frame.add (alert);
 
-        var header_image = new Gtk.Image.from_icon_name ("document-open-recent", Gtk.IconSize.DIALOG);
-
-        var record_label = new Gtk.Label (_("History"));
-        record_label.get_style_context ().add_class ("h2");
-
-        record_switch = new Gtk.Switch ();
-        record_switch.active = true;
-        record_switch.valign = Gtk.Align.CENTER;
-
-        var info_button = new Gtk.Image.from_icon_name ("help-info-symbolic", Gtk.IconSize.MENU);
-        info_button.hexpand = true;
-        info_button.xalign = 0;
-        info_button.tooltip_text = _("This operating system can gather useful statistics about file and app usage to provide extra functionality. If other people can see or access your account, you may wish to limit which items are recorded.");
-
-        var header_grid = new Gtk.Grid ();
-        header_grid.column_spacing = 12;
-        header_grid.margin_bottom = 12;
-        header_grid.add (header_image);
-        header_grid.add (record_label);
-        header_grid.add (info_button);
-        header_grid.add (record_switch);
+        status_switch.active = true;
 
         var clear_data = new Gtk.ToggleButton.with_label (_("Clear History…"));
-        clear_data.halign = Gtk.Align.END;
         clear_data.notify["active"].connect (() => {
             if (clear_data.active == false) {
                 remove_popover.hide ();
@@ -83,14 +62,14 @@
         var include_treeview = new IncludeTreeView ();
         var exclude_treeview = new ExcludeTreeView ();
 
-        attach (header_grid, 0, 0, 2, 1);
-        attach (description_frame, 0, 1, 2, 1);
-        attach (include_treeview, 0, 1, 1, 1);
-        attach (exclude_treeview, 1, 1, 1, 1);
-        attach (clear_data, 1, 2, 1, 1);
-
-        record_switch.notify["active"].connect (() => {
-            bool privacy_mode = !record_switch.active;
+        content_area.attach (description_frame, 0, 1, 2, 1);
+        content_area.attach (include_treeview, 0, 1, 1, 1);
+        content_area.attach (exclude_treeview, 1, 1, 1, 1);
+
+        action_area.add (clear_data);
+
+        status_switch.notify["active"].connect (() => {
+            bool privacy_mode = !status_switch.active;
             include_treeview.visible = !privacy_mode;
             exclude_treeview.visible = !privacy_mode;
             description_frame.visible = privacy_mode;
@@ -104,11 +83,7 @@
             }
         });
 
-        record_switch.active = !blacklist.get_incognito ();
-    }
-    
-    public void focus_privacy_switch () {
-        record_switch.grab_focus ();
+        status_switch.active = !blacklist.get_incognito ();
     }
 
     private string get_operating_system_name () {
 
=== modified file 'src/Widgets/ServiceList.vala'
--- src/Widgets/ServiceList.vala	2017-03-01 23:31:31 +0000
+++ src/Widgets/ServiceList.vala	2017-03-02 17:18:33 +0000
@@ -39,10 +39,10 @@
             update_service_status (firewall_item, SecurityPrivacy.firewall.status_switch.active);
         });
 
-        update_service_status (privacy_item, SecurityPrivacy.tracking.record_switch.active);
+        update_service_status (privacy_item, SecurityPrivacy.tracking.status_switch.active);
 
-        SecurityPrivacy.tracking.record_switch.notify["active"].connect (() => {
-            update_service_status (privacy_item, SecurityPrivacy.tracking.record_switch.active);
+        SecurityPrivacy.tracking.status_switch.notify["active"].connect (() => {
+            update_service_status (privacy_item, SecurityPrivacy.tracking.status_switch.active);
         });
 
         if (SecurityPrivacy.LocationPanel.location_agent_installed ()) {