Merge lp:~elementary-apps/pantheon-files/props-dialog-perm-button-update into lp:~elementary-apps/pantheon-files/trunk

[Danielle Foré]

Move update_permission_type_buttons in PropertiesWindow.vala to update_buttons in PermissionButton.vala

This duplicates the array vfs_perms, but I'm thinking that's a little less clumsy than trying to access it across the classes. I think maybe a way to move forward more cleanly is to have a utility class instead of having these things in the properties dialog and permissions button

I changed the foreach to a set of if/else statements so that this method doesn't rely on widgets been read in a certain order

Since the permissionbutton widget knows about itself (and its permission type), we only need to pass the new permissions int, which I think really helps with clarity here

[Jeremy Wootten (jeremywootten)]

The update buttons function assumes the children are returned in a certain order. This is probably OK, but you could make use of the data set on the buttons (PermissionType and PermissionValue) to index into the vfs_perm array to avoid this.

[Danielle Foré (danrabbit)]

Yep you're right and this is definitely something I want to address in a future branch. Specifically this breaks when we change the widget from being a Gtk.Box to a Gtk.Grid.

[lp:~elementary-apps/pantheon-files/props-dialog-perm-button-update]

2419. By Danielle Foré on 2016-12-26

move update_button to permsbutton

[Jeremy Wootten (jeremywootten)]

review: Approve (code)

=== modified file 'src/Dialogs/PropertiesWindow.vala'
--- src/Dialogs/PropertiesWindow.vala	2016-12-26 02:09:47 +0000
+++ src/Dialogs/PropertiesWindow.vala	2016-12-26 22:45:29 +0000
@@ -741,19 +741,10 @@
         return vfs_perm;
     }
 
-    private void update_permission_type_buttons (PermissionButton hbox, uint32 permissions, PermissionType pt) {
-        int i=0;
-        foreach (var widget in hbox.get_children ()) {
-            Gtk.ToggleButton btn = (Gtk.ToggleButton) widget;
-            ((permissions & vfs_perms[pt, i]) != 0) ? btn.active = true : btn.active = false;
-            i++;
-        }
-    }
-
     private void update_perm_grid_toggle_states (uint32 permissions) {
-        update_permission_type_buttons (perm_button_user, permissions, PermissionType.USER);
-        update_permission_type_buttons (perm_button_group, permissions, PermissionType.GROUP);
-        update_permission_type_buttons (perm_button_other, permissions, PermissionType.OTHER);
+        perm_button_user.update_buttons (permissions);
+        perm_button_group.update_buttons (permissions);
+        perm_button_other.update_buttons (permissions);
     }
 
     private bool is_chmod_code (string str) {

=== modified file 'src/View/Widgets/PermissionButton.vala'
--- src/View/Widgets/PermissionButton.vala	2016-12-23 19:15:06 +0000
+++ src/View/Widgets/PermissionButton.vala	2016-12-26 22:45:29 +0000
@@ -17,7 +17,7 @@
 * Boston, MA 02111-1307, USA.
 */
 
-public class PermissionButton : Gtk.Box {
+public class PermissionButton : Gtk.Grid {
     public Gtk.ToggleButton btn_read;
     public Gtk.ToggleButton btn_write;
     public Gtk.ToggleButton btn_exe;
@@ -30,6 +30,12 @@
         EXE
     }
 
+    private Posix.mode_t[,] vfs_perms = {
+        { Posix.S_IRUSR, Posix.S_IWUSR, Posix.S_IXUSR },
+        { Posix.S_IRGRP, Posix.S_IWGRP, Posix.S_IXGRP },
+        { Posix.S_IROTH, Posix.S_IWOTH, Posix.S_IXOTH }
+    };
+
     public PermissionButton (Marlin.View.PropertiesWindow.PermissionType permission_type) {
         Object (permission_type: permission_type);
     }
@@ -47,10 +53,30 @@
         btn_exe.set_data ("permissiontype", permission_type);
         btn_exe.set_data ("permissionvalue", Value.EXE);
 
-        homogeneous = true;
+        column_homogeneous = true;
         get_style_context ().add_class (Gtk.STYLE_CLASS_LINKED);
         add (btn_read);
         add (btn_write);
         add (btn_exe);
     }
+
+    public void update_buttons (uint32 permissions) {
+        if ((permissions & vfs_perms[permission_type, 0]) != 0) {
+            btn_read.active = true;
+        } else {
+            btn_read.active = false;
+        }
+
+        if ((permissions & vfs_perms[permission_type, 1]) != 0) {
+            btn_write.active = true;
+        } else {
+            btn_write.active = false;
+        }
+
+        if ((permissions & vfs_perms[permission_type, 2]) != 0) {
+            btn_exe.active = true;
+        } else {
+            btn_exe.active = false;
+        }
+    }
 }