Merge lp:~jeremywootten/pantheon-files/reload-icon-in-pathbar into lp:~elementary-apps/pantheon-files/trunk

[Jeremy Wootten]

Add a refresh icon to the pathbar

This branch places a refresh icon at the right hand end of the pathbar while in browsing mode. Clicking on the icon refreshes the current view.

To test:

FOR FUNCTION
1) Click on the refresh icon (either button) at the right-hand end of the pathbar. The current view refreshes (reloads). In Miller view, the active slot refreshes.

2) Hover over the icon - a tooltip appears (review wording).

3) Enter edit mode - the icon changes to the Navigate to icon.

4) Enter search mode - the icon disappears

FOR POSSIBLE REGRESSIONS
1) All pathbar functions work as normal.

2) All other ways of refreshing the view work as normal

3) Nothing bad happens if the icon is clicked when the view is not displaying a normal accessible folder.

[Cody Garver (codygarver)]

Icon should probably be view-refresh-symbolic

String should probably be "Reload the current folder" if we want to match the wording of Midori's reload tooltip

[Cody Garver (codygarver)]

UX Team confirms that the icon should be "view-refresh-symbolic"

And the tooltip should be "Reload this folder"

review: Needs Fixing

[lp:~jeremywootten/pantheon-files/reload-icon-in-pathbar]

1683. By Jeremy Wootten on 2015-01-13

Use 'view-refresh-symblic' icon and amend tooltip to 'Reload this folder'

1684. By Jeremy Wootten on 2015-01-13

Correct spelling mistake

[Jeremy Wootten (jeremywootten)]

The icon is now "view-refresh-symbolic" and the tooltip is "Reload this folder".

[Danielle Foré (danrabbit)]

Okay so if you do this:

this.set_icon_from_icon_name (Gtk.EntryIconPosition.SECONDARY, "go-jump-symbolic");

Instead of this:

secondary_icon_pixbuf = arrow_img;

It doesn't get that weird screwed up hover effect

[lp:~jeremywootten/pantheon-files/reload-icon-in-pathbar]

1685. By Jeremy Wootten on 2015-01-18

Do not use pixbufs for secondary icon, fix disappearing tooltip on secondary icon

[Jeremy Wootten (jeremywootten)]

Thanks Dan, I have implemented your fix for both secondary icons and removed unused code. Also fixed a bug I noticed where the tooltip on the secondary icon did not appear if you first hovered over the blank part of the pathbar. It appears calling set_tooltip_text ("") has the effect of turning off ALL the tooltips. Not sure if this is expected or a Gtk.Entry bug.

[Danielle Foré (danrabbit)]

* We shouldn't offer reload while the directory is currently reloading.

* There is a minor regression where the "go" arrow shows immediately instead of only after you've made a change to the path

review: Needs Fixing

[lp:~jeremywootten/pantheon-files/reload-icon-in-pathbar]

1686. By Jeremy Wootten on 2015-01-24

Fix enter key press, hide reload icon if path unchanged, fix keyboard reload while in edit mode, fix regression in completion

1687. By Jeremy Wootten on 2015-01-24

Fix error message re atk_object, remove unneeded entry icon code

[PerfectCarl (name-is-carl)]

Is this branch related/touching code pertaining to https://code.launchpad.net/~jeremywootten/pantheon-files/fix-enter-in-pathbar ?

Also there is MERGE strings in the code: bzr merge failed it seems.

Please add a comment when hiding inherited methods (instead of overriding) as it is unusual and could be viewed as an error.

review: Needs Fixing

[lp:~jeremywootten/pantheon-files/reload-icon-in-pathbar]

1688. By Jeremy Wootten on 2015-01-26

Merge trunk to rev 1723

1689. By Jeremy Wootten on 2015-01-26

Avoid hiding Gtk.Widget.key_press_event

[Jeremy Wootten (jeremywootten)]

1) Trunk merged and conflict resolved
2) Code refactored to avoid hiding Gtk Widget method.

[Danielle Foré (danrabbit)]

review: Approve

=== modified file 'libcore/gof-directory-async.vala'
--- libcore/gof-directory-async.vala	2014-12-26 12:01:20 +0000
+++ libcore/gof-directory-async.vala	2015-01-26 10:59:47 +0000
@@ -129,7 +129,16 @@
         state = State.NOT_LOADED;
     }
 
-    public void load () {
+    public delegate void GOFFileLoadedFunc (GOF.File file);
+
+    /** Views call the following function with null parameter - file_loaded and done_loading
+      * signals are emitted and cause the view and view container to update.
+      * 
+      * LocationBar calls this function, with a callback, on its own Async instances in order
+      * to perform filename completion.- Emitting a done_loaded signal in that case would cause
+      * the premature ending of text entry.
+     **/ 
+    public void load (GOFFileLoadedFunc? file_loaded_func = null) {
         cancellable.reset ();
         longest_file_name = "";
 
@@ -141,15 +150,17 @@
             if (state == State.LOADING)
                 clear_directory_info ();
 
-            list_directory.begin ();
+            list_directory.begin (file_loaded_func);
 
-            try {
-                monitor = location.monitor_directory (0);
-                monitor.rate_limit = 100;
-                monitor.changed.connect (directory_changed);
-            } catch (IOError e) {
-                if (!(e is IOError.NOT_MOUNTED)) {
-                    warning ("directory monitor failed: %s %s", e.message, file.uri);
+            if (file_loaded_func == null) {
+                try {
+                    monitor = location.monitor_directory (0);
+                    monitor.rate_limit = 100;
+                    monitor.changed.connect (directory_changed);
+                } catch (IOError e) {
+                    if (!(e is IOError.NOT_MOUNTED)) {
+                        warning ("directory monitor failed: %s %s", e.message, file.uri);
+                    }
                 }
             }
         } else {
@@ -164,12 +175,15 @@
                         if (track_longest_name)
                             update_longest_file_name (gof);
 
-                        file_loaded (gof);
+                        if (file_loaded_func == null)
+                            file_loaded (gof);
+                        else
+                            file_loaded_func (gof);
                     }
                 }
             }
 
-            if (!cancellable.is_cancelled ())
+            if (file_loaded_func == null && !cancellable.is_cancelled ())
                 done_loading ();
         }
     }
@@ -234,7 +248,7 @@
         yield query_info_async (file, file_info_available);
     }
 
-    private async void list_directory () {
+    private async void list_directory (GOFFileLoadedFunc? file_loaded_func = null) {
         file.exists = true;
         files_count = 0;
         state = State.LOADING;
@@ -267,12 +281,16 @@
                         if (track_longest_name)
                             update_longest_file_name (gof);
 
-                        file_loaded (gof);
+                        if (file_loaded_func == null)
+                            file_loaded (gof);
+                        else
+                            file_loaded_func (gof);
                     }
 
                     files_count++;
                 }
             }
+
             if (state == State.LOADING) {
                 file.exists = true;
                 state = State.LOADED;
@@ -293,8 +311,7 @@
             else if (err is IOError.NOT_MOUNTED)
                 file.is_mounted = false;
         }
-
-        if (!cancellable.is_cancelled ())
+        if (file_loaded_func == null && !cancellable.is_cancelled ())
             done_loading ();
     }
 
 
=== modified file 'libcore/tests/marlincore-tests-file.c'
--- libcore/tests/marlincore-tests-file.c	2014-08-05 22:40:12 +0000
+++ libcore/tests/marlincore-tests-file.c	2015-01-26 10:59:47 +0000
@@ -89,7 +89,7 @@
     GOFDirectoryAsync *dir2;
     dir2 = gof_directory_async_from_file(dir->file);
     g_signal_connect(dir2, "done_loading", (GCallback) second_load_done, NULL);
-    gof_directory_async_load(dir2);
+    gof_directory_async_load(dir2, NULL, NULL);
 
     /* free previously allocated dir */
     g_object_unref (dir);
@@ -107,11 +107,11 @@
 
     dir = gof_directory_async_from_gfile(g_file_new_for_path("/tmp/marlin-test"));
     g_signal_connect(dir, "done_loading", (GCallback) first_load_done, NULL);
-    gof_directory_async_load(dir);
+    gof_directory_async_load(dir, NULL, NULL);
 
     /*dir2 = gof_directory_async_from_gfile(g_file_new_for_path("/tmp/marlin-test"));
     g_signal_connect(dir2, "done_loading", (GCallback) second_load_done, NULL);
-    gof_directory_async_load(dir2);*/
+    gof_directory_async_load(dir2, NULL, NULL);*/
 
     //GOFFile *f1 = gof_file_get (g_file_new_for_path("/tmp/marlin-test/a"));
 
 
=== modified file 'libwidgets/LocationBar.vala'
--- libwidgets/LocationBar.vala	2014-11-21 10:02:45 +0000
+++ libwidgets/LocationBar.vala	2015-01-26 10:59:47 +0000
@@ -57,7 +57,11 @@
 
             _search_mode = value;
 
-            primary_icon_name = _search_mode ? "edit-find-symbolic" : null;
+            if (_search_mode) {
+                primary_icon_name = "edit-find-symbolic";
+                show_refresh_icon (false);
+            } else
+                primary_icon_name = null;
 
             grab_focus ();
         }
@@ -66,10 +70,8 @@
     protected string text_completion = "";
     protected bool multiple_completions = false;
     protected bool text_changed = false;
-    protected bool arrow_hovered = false;
     protected bool ignore_focus_in = false;
     protected bool ignore_change = false;
-    private Gdk.Pixbuf arrow_img;
 
     /* if we must display the BreadcrumbsElement which are in  newbreads. */
     bool view_old = false;
@@ -97,10 +99,12 @@
     public signal void path_changed (File file);
     public signal void need_completion ();
     public signal void search_changed (string text);
+    public signal void reload ();
 
     List<IconDirectory?> icons;
 
     string current_path = "";
+    string refresh_tip = _("Reload this folder");
 
     int selected = -1;
     int space_breads = 12;
@@ -123,13 +127,6 @@
     construct {
         icon_factory = Granite.Services.IconFactory.get_default ();
         icons = new List<IconDirectory?> ();
-        
-        /* Load arrow image */
-        try {
-            arrow_img = Gtk.IconTheme.get_default ().load_icon ("go-jump-symbolic", 16, Gtk.IconLookupFlags.GENERIC_FALLBACK);
-        } catch (Error err) {
-            stderr.printf ("Unable to load home icon: %s", err.message);
-        }
 
         button_context = get_style_context ();
         button_context.add_class ("button");
@@ -155,8 +152,10 @@
         secondary_icon_sensitive = true;
         truncate_multiline = true;
         activate.connect (on_activate);
-        icon_press.connect (on_activate);
-        motion_notify_event.connect (on_motion_notify);
+        button_press_event.connect (on_button_press_event);
+        button_release_event.connect (on_button_release_event);
+        icon_press.connect (on_icon_press);
+        motion_notify_event.connect_after (after_motion_notify);
         focus_in_event.connect (on_focus_in);
         focus_out_event.connect (on_focus_out);
         grab_focus.connect_after (on_grab_focus);
@@ -170,8 +169,8 @@
         drag_data_received.connect (on_drag_data_received);
         drag_drop.connect (on_drag_drop);
     }
-    
-    public override bool key_press_event (Gdk.EventKey event) {
+
+    public bool on_key_press_event (Gdk.EventKey event) {
         switch (event.keyval) {
             case Gdk.Key.KP_Tab:
             case Gdk.Key.Tab:
@@ -195,7 +194,12 @@
         return base.key_press_event (event);
     }
 
-    public override bool button_press_event (Gdk.EventButton event) {
+    public bool on_button_press_event (Gdk.EventButton event) {
+        /* We need to distinguish whether the event comes from one of the icons.
+         * There doesn't seem to be a way of doing this directly so we check the window width */
+        if (event.window.get_width () < 24)
+            return false;
+
         if (is_focus)    
             return base.button_press_event (event);
 
@@ -233,7 +237,12 @@
         return true;
     }
 
-    public override bool button_release_event (Gdk.EventButton event) {
+    public bool on_button_release_event (Gdk.EventButton event) {
+        /* We need to distinguish whether the event comes from one of the icons.
+         * There doesn't seem to be a way of doing this directly so we check the window width */
+        if (event.window.get_width () < 24)
+            return false;
+
         reset_elements_states ();
 
         if (timeout != -1) {
@@ -244,6 +253,7 @@
         if (is_focus)
             return base.button_release_event (event);
 
+
         if (event.button == 1) {
             var el = get_element_from_coordinates ((int) event.x, (int) event.y);
             if (el != null) {
@@ -256,7 +266,16 @@
         
         return base.button_release_event (event);
     }
-    
+
+    public void on_icon_press (Gtk.EntryIconPosition pos, Gdk.Event event) {
+        if (pos == Gtk.EntryIconPosition.SECONDARY) {
+            if (is_focus)
+                on_activate ();
+            else
+                reload ();
+        }
+    }
+
     void on_change () {
         if (search_mode) {
             search_changed (text);
@@ -268,19 +287,24 @@
             return;
         }
 
-        set_entry_icon (true, (text.length > 0) ? "Navigate to: " + text : "");
+        show_navigate_icon ();
         text_completion = "";
         need_completion ();
     }
     
-    bool on_motion_notify (Gdk.EventMotion event) {
+    bool after_motion_notify (Gdk.EventMotion event) {
+
+        if (is_focus)
+            return false;
+
         int x = (int) event.x;
         double x_render = 0;
         double x_previous = -10;
         set_tooltip_text ("");
-        
-        if (is_focus)
-            return base.motion_notify_event (event);
+        /* We must reset the icon tooltip as the above line turns all tooltips off */
+        set_icon_tooltip_text (Gtk.EntryIconPosition.SECONDARY, refresh_tip);
+
+
 
         foreach (BreadcrumbsElement element in elements) {
             if (element.display) {
@@ -299,22 +323,25 @@
             set_entry_cursor (new Gdk.Cursor (Gdk.CursorType.ARROW));
         else
             set_entry_cursor (null);
-            
-        return base.motion_notify_event (event);
+
+        return false;
     }
 
     bool on_focus_out (Gdk.EventFocus event) {
-        if (is_focus) {
+        if (is_focus)
             ignore_focus_in = true;
-            return base.focus_out_event (event);
+        else {
+            reset ();
+            show_refresh_icon (true);
         }
-    
+
+        return base.focus_out_event (event);
+    }
+
+    void reset () {
         ignore_focus_in = false;
-        set_entry_icon (false);
         set_entry_text ("");
         search_mode = false;
-        
-        return base.focus_out_event (event);
     }
 
     bool on_focus_in (Gdk.EventFocus event) {
@@ -323,10 +350,11 @@
 
         if (search_mode)
             set_entry_text ("");
-        else
-            set_entry_text (sanitise_path (GLib.Uri.unescape_string (get_elements_path ())));
-                    
-                
+        else {
+            current_path = sanitise_path (GLib.Uri.unescape_string (get_elements_path ()));
+            set_entry_text (current_path);
+            show_navigate_icon ();
+        }
         return base.focus_in_event (event);
     }
 
@@ -343,10 +371,25 @@
     }
 
     void on_activate () {
-        path_changed (get_file_for_path (text + text_completion));
+        string path = text + text_completion;
+        path_changed (get_file_for_path (path));
         text_completion = "";
     }
 
+    public void show_refresh_icon (bool show = true) {
+        /* Cancel any editing or search if refresh icon is to be shown */
+        if (show) {
+            reset ();
+            escape ();
+        }
+
+        set_entry_secondary_icon (false, show);
+    }
+
+    public void show_navigate_icon (bool show = true) {
+        set_entry_secondary_icon (true, show && text != current_path);
+    }
+
     protected abstract void on_drag_leave (Gdk.DragContext drag_context, uint time);
 
     protected abstract void on_drag_data_received (Gdk.DragContext context,
@@ -406,12 +449,16 @@
         get_window ().get_children ().nth_data (13).set_cursor (cursor ?? new Gdk.Cursor (Gdk.CursorType.XTERM));
     }
     
-    public void set_entry_icon (bool active, string? tooltip = null) {
-        if (!active)
+    public void set_entry_secondary_icon (bool active, bool visible) {
+        if (!visible)
             secondary_icon_pixbuf = null;
-        else {
-            secondary_icon_pixbuf = arrow_img;
-            secondary_icon_tooltip_text = tooltip;
+        else if (!active) {
+            set_icon_from_icon_name (Gtk.EntryIconPosition.SECONDARY, "view-refresh-symbolic");
+            set_icon_tooltip_text (Gtk.EntryIconPosition.SECONDARY, refresh_tip);
+        } else {
+            set_icon_from_icon_name (Gtk.EntryIconPosition.SECONDARY, "go-jump-symbolic");
+            var tooltip = (text.length > 0) ? (_("Navigate to %s")).printf (text) : "";
+            set_icon_tooltip_text (Gtk.EntryIconPosition.SECONDARY, tooltip);
         }
     }
 
@@ -809,7 +856,7 @@
                 Pango.cairo_show_layout (cr, layout);
             }
         }
-        
+
         return true;
     }
 
 
=== modified file 'src/View/LocationBar.vala'
--- src/View/LocationBar.vala	2014-12-01 12:20:37 +0000
+++ src/View/LocationBar.vala	2015-01-26 10:59:47 +0000
@@ -62,8 +62,12 @@
             this.win = win;
             bread = new Breadcrumbs (win);
             bread.escape.connect (() => { escape(); });
-
             bread.path_changed.connect (on_path_changed);
+
+            bread.reload.connect (() => {
+                win.win_actions.activate_action ("refresh", null);
+            });
+
             bread.activate_alternate.connect ((file) => { activate_alternate(file); });
             bread.notify["search-mode"].connect (() => {
                 if (!bread.search_mode) {
@@ -78,6 +82,7 @@
             margin_bottom = 4;
             margin_left = 3;
 
+            bread.set_entry_secondary_icon (false, true);
             pack_start (bread, true, true, 0);
         }
 
@@ -331,17 +336,10 @@
                 file = file.get_parent ();
             else
                 return;
-
-            var directory = file;
-            var files_cache = files;
             
-            files = GOF.Directory.Async.from_gfile (directory);
+            files = GOF.Directory.Async.from_gfile (file);
             if (files.file.exists) {
-                /* Verify that we got a new instance of files so we do not double up events */
-                if (files_cache != files)
-                    files.file_loaded.connect (on_file_loaded);
-                
-                files.load ();
+                files.load (on_file_loaded);
             }
         }
 
@@ -406,7 +404,6 @@
             menu_open_with.set_submenu (submenu_open_with);
             menu.append (new Gtk.SeparatorMenuItem ());
 
-
             unowned List<GOF.File>? sorted_dirs = files_menu.get_sorted_dirs ();
             foreach (var gof in sorted_dirs) {
                 var menuitem = new Gtk.MenuItem.with_label(gof.get_display_name ());
@@ -418,6 +415,9 @@
                 });
             }
             menu.show_all ();
+            /* Release the Async directory as soon as possible */
+            files_menu.done_loading.disconnect (on_files_loaded_menu);
+            files_menu = null;
         }
 
         private void launch_uri_with_app (AppInfo app, string uri) {
@@ -437,9 +437,8 @@
             menu = new Gtk.Menu ();
             menu.cancel.connect (() => { reset_elements_states (); });
             menu.deactivate.connect (() => { reset_elements_states (); });
+            /* current_right_click_root is parent of the directory named on the breadcrumb. */ 
             var directory = File.new_for_uri (current_right_click_root);
-            if (files_menu != null)
-                files_menu.done_loading.disconnect (on_files_loaded_menu);
             files_menu = GOF.Directory.Async.from_gfile (directory);
             files_menu.done_loading.connect (on_files_loaded_menu);
             files_menu.load ();
 
=== modified file 'src/View/ViewContainer.vala'
--- src/View/ViewContainer.vala	2014-12-28 09:23:52 +0000
+++ src/View/ViewContainer.vala	2015-01-26 10:59:47 +0000
@@ -331,6 +331,10 @@
             if (file == null || location.equal (file))
                 return;
 
+            var filetype = file.query_file_type (0);
+            if (filetype == FileType.UNKNOWN)
+                return;
+
             GLib.File? loc = null;
             File? parent = file.get_parent ();
             if (parent != null && location.equal (file.get_parent ())) {
@@ -341,7 +345,7 @@
                 } else
                     loc = file;
             } else if (!select_in_current_only) {
-                if (file.query_file_type (0) == FileType.DIRECTORY)
+                if (filetype == FileType.DIRECTORY)
                     loc = file;
                 else if (parent != null) {
                     loc = parent;
 
=== modified file 'src/View/Window.vala'
--- src/View/Window.vala	2015-01-17 18:42:21 +0000
+++ src/View/Window.vala	2015-01-26 10:59:47 +0000
@@ -214,13 +214,6 @@
 
             undo_manager.request_menu_update.connect (undo_redo_menu_update_callback);
 
-            key_press_event.connect ((event) => {
-                if (top_menu.location_bar.bread.is_focus)
-                    return top_menu.location_bar.bread.key_press_event (event);
-
-                return false;
-            });
-
             button_press_event.connect (on_button_press_event);
 
             window_state_event.connect ((event) => {
@@ -375,6 +368,14 @@
 
             content.loading.connect ((is_loading) => {
                 tab.working = is_loading;
+                top_menu.location_bar.bread.show_refresh_icon (!is_loading);
+            });
+
+            key_press_event.connect ((event) => {
+                if (top_menu.location_bar.bread.is_focus)
+                    return top_menu.location_bar.bread.on_key_press_event (event);
+
+                return false;
             });
 
             change_tab ((int)tabs.insert_tab (tab, -1));