Merge lp:~jeremywootten/pantheon-files/fix-select-when-scrolled-regression into lp:~elementary-apps/pantheon-files/trunk

[Jeremy Wootten]

Implement separate is_on_icon () method for IconView

This branch fixes a fairly serious regression in the selection of items in Icon View using the selection helper, which no longer works properly when the view is scrolled.

This was due to the same code being used to determine whether the pointer was on the icon or helper for all the views, but there are subtle differences in the way IconView renders the items and the coordinates it supplies to the renderer meaning that a separate slightly different method has to be used.

[David Hewitt (davidmhewitt)]

Can confirm that it fixes the bug and selection in the other two views remains working as intended.

review: Approve (code, function)

=== modified file 'src/View/AbstractDirectoryView.vala'
--- src/View/AbstractDirectoryView.vala	2017-02-15 18:54:50 +0000
+++ src/View/AbstractDirectoryView.vala	2017-03-01 17:49:39 +0000
@@ -3482,31 +3482,6 @@
             unselect_all ();
         }
 
-        protected bool is_on_icon (int x, int y, int orig_x, int orig_y, ref bool on_helper) {
-            /* orig_x and orig_y must be top left hand corner of icon (excluding helper) */
-            int x_offset = x - orig_x;
-            int y_offset = y - orig_y;
-
-
-            bool on_icon =  (x_offset >= 0 &&
-                             x_offset <= icon_size &&
-                             y_offset >= 0 &&
-                             y_offset <= icon_size);
-
-            on_helper = false;
-            if (icon_renderer.selection_helpers) {
-                int x_helper_offset = x - icon_renderer.helper_x;
-                int y_helper_offset = y - icon_renderer.helper_y;
-
-                on_helper =  (x_helper_offset >= 0 &&
-                             x_helper_offset <= icon_renderer.helper_size &&
-                             y_helper_offset >= 0 &&
-                             y_helper_offset <= icon_renderer.helper_size);
-            }
-
-            return on_icon;
-        }
-
         protected void invert_selection () {
             GLib.List<Gtk.TreeRowReference> selected_row_refs = null;
 
@@ -3560,6 +3535,7 @@
         protected abstract void thaw_tree ();
         protected new abstract void freeze_child_notify ();
         protected new abstract void thaw_child_notify ();
+        protected abstract bool is_on_icon (int x, int y, int orig_x, int orig_y, ref bool on_helper);
 
 /** Unimplemented methods
  *  fm_directory_view_parent_set ()  - purpose unclear

=== modified file 'src/View/AbstractTreeView.vala'
--- src/View/AbstractTreeView.vala	2017-02-07 19:38:46 +0000
+++ src/View/AbstractTreeView.vala	2017-03-01 17:49:39 +0000
@@ -321,5 +321,29 @@
         protected override void thaw_child_notify () {
             tree.thaw_child_notify ();
         }
+
+        protected override bool is_on_icon (int x, int y, int orig_x, int orig_y, ref bool on_helper) {
+            /* orig_x and orig_y must be top left hand corner of icon (excluding helper) */
+            int x_offset = x - orig_x;
+            int y_offset = y - orig_y;
+
+            bool on_icon =  (x_offset >= 0 &&
+                             x_offset <= icon_size &&
+                             y_offset >= 0 &&
+                             y_offset <= icon_size);
+
+            on_helper = false;
+            if (icon_renderer.selection_helpers) {
+                int x_helper_offset = x - icon_renderer.helper_x;
+                int y_helper_offset = y - icon_renderer.helper_y;
+
+                on_helper =  (x_helper_offset >= 0 &&
+                             x_helper_offset <= icon_renderer.helper_size &&
+                             y_helper_offset >= 0 &&
+                             y_helper_offset <= icon_renderer.helper_size);
+            }
+
+            return on_icon;
+        }
     }
 }

=== modified file 'src/View/IconView.vala'
--- src/View/IconView.vala	2017-02-08 12:48:45 +0000
+++ src/View/IconView.vala	2017-03-01 17:49:39 +0000
@@ -225,10 +225,10 @@
                     if (x >= rect.x &&
                         x <= rect.x + rect.width &&
                         y >= rect.y &&
-                        y <= rect.y + (r as Marlin.TextRenderer).text_height)
+                        y <= rect.y + (r as Marlin.TextRenderer).text_height) {
 
                         zone = ClickZone.NAME;
-                    else if (rubberband) {
+                    } else if (rubberband) {
                         /* Fake location outside centre bottom of item for rubberbanding */
                         event.x = rect.x + rect.width / 2;
                         event.y = rect.y + rect.height + 10 + (int)(get_vadjustment ().value);
@@ -238,11 +238,11 @@
                     bool on_helper = false;
                     bool on_icon = is_on_icon (x, y, area.x, area.y, ref on_helper);
 
-                    if (on_helper)
+                    if (on_helper) {
                         zone = ClickZone.HELPER;
-                    else if (on_icon)
+                    } else if (on_icon) {
                         zone = ClickZone.ICON;
-                    else if (rubberband) {
+                    } else if (rubberband) {
                         /* Fake location outside centre top of item for rubberbanding */
                         event.x = rect.x + rect.width / 2;
                         event.y = rect.y - 10 + (int)(get_vadjustment ().value);
@@ -440,6 +440,34 @@
             }
         }
 
+        protected override bool is_on_icon (int x, int y, int orig_x, int orig_y, ref bool on_helper) {
+            /* orig_x and orig_y must be top left hand corner of icon (excluding helper) */
+            int x_offset = x - orig_x;
+            int y_offset = y - orig_y;
+
+            bool on_icon =  (x_offset >= 0 &&
+                             x_offset <= icon_size &&
+                             y_offset >= 0 &&
+                             y_offset <= icon_size);
+
+            on_helper = false;
+            if (icon_renderer.selection_helpers) {
+                int x_helper_offset = x - icon_renderer.helper_x;
+                /* IconView provide IconRenderer with bin coords not widget coords (unlike TreeView) so we have to
+                 * correct for scrolling */ 
+                int y_helper_offset = y - icon_renderer.helper_y + (int)(get_vadjustment ().value);
+
+                on_helper =  (x_helper_offset >= 0 &&
+                             x_helper_offset <= icon_renderer.helper_size &&
+                             y_helper_offset >= 0 &&
+                             y_helper_offset <= icon_renderer.helper_size);
+
+            }
+
+            return on_icon;
+        }
+
+
         /* When Icon View is automatically adjusting column number it does not expose the actual number of
          * columns (get_columns () returns -1). So we have to write our own method. This is the only way
          * (I can think of) that works on row 0.