Merge lp:~mefrio-g/pantheon-files/split-view into lp:~elementary-apps/pantheon-files/trunk

[Mario Guerriero]

Implement Split View which can be accessed by hitting F3 key fix bug #1078973.

This branch contains an implementation of a Split View which can be used by hitting F3 key.

[Julián Unrrein (junrrein)]

Wow Mario! This is a great contribution!

The Split View isn't added when I hit F3. Nothing happens.

Notice that, since Files doesn't have an appmenu anymore, it would be necessary to also use the F3 key to close the current view. Skimming through you changes, that doesn't seem to be implemented.

[Julián Unrrein (junrrein)]

Wow Mario! This is a great contribution!

The Split View isn't added when I hit F3. Nothing happens.

Notice that, since Files doesn't have an appmenu anymore, it would be necessary to also use the F3 key to close the current view. Skimming through you changes, that doesn't seem to be implemented.

[Julián Unrrein (junrrein)]

Also, be aware of this: https://code.launchpad.net/~junrrein/pantheon-files/dn-refactor/+merge/169673

[Julián Unrrein (junrrein)]

Disregard my last comment. I haven't tested that branch before submitting for merging, and it didn't work. A slip up.

[Cody Garver (codygarver)]

Design team Approves this new feature.

review: Approve

[Cody Garver (codygarver)]

When the code is deemed ready, this will be merged AFTER final Luna release of Files, to avoid any bugs it could introduce.

[Cody Garver (codygarver)]

Changing to Needs Fixing because I stupidly thought Launchpad would report me as clicking the design team's review link...

review: Needs Fixing

[Danielle Foré (danrabbit)]

I have a few issues:

* View switching seems to only work for the right pane
* There's a duplicate overlay statusbar
* Should we use ThinPane here?
* Close the last tab on the left view then hit F3 again: crash.
* Source List affects the right pane instead of last selected pane
* Select an item inside each pane and copy, then cd. pasting doesn't seem to work correctly.
* Hitting F3 again creates a new tab instead of closing the right pane

review: Needs Fixing

[Mario Guerriero (mefrio-g)]

Julian you have to install this branch so that you will install the pantheon-files.xml file containing the F3 action.

Hey Daniel I think ThinPaned is not necessary there. It's better to be able to re-size the views. SourceList affects the last used pane e.g. the last pane which switched folder. I don't think that hitting F3 again should close the right pane as it would be a contradiction that F3 creates and destroys a pane. The rest are bugs :)

[Danielle Foré (danrabbit)]

Hey Mario, you know that thin pane has an invisible resize area that's the same size as hpane right? :p

[Mario Guerriero (mefrio-g)]

Yes but in hpane it is more visible :)

[Julián Unrrein (junrrein)]

When closing the split view and hovering over items in the view that is left, console output throws this http://pastebin.com/D4TLzAuv every time you hover over an item.

This can devolve in a crash: http://pastebin.com/9T48hGs8. I got this crash when hovering over an item, but none in particular. I can't reproduce this with a definite series of steps, but if I flail the mouse some time (not much) I can get it to crash.

[Julián Unrrein (junrrein)]

The sidebar (which is not a SourceList by the way, it is just styled as one) should really affect the last focused view. You can even open some files or view the properties in the other view (the one that didn't change its directory last) and still the sidebar will affect the last changed view. I don't think this is desirable.

[Julián Unrrein (junrrein)]

After opening the split view, right clicking on items in the left view works erratically. Most of the times options pertaining to the item don't show up or are disabled (contracts, open, cut/copy/paste). Sometimes it works correctly. Sometimes a menu doesn't show up.

Console output when pressing right-click: http://pastebin.com/d1RpQbu6.

[Julián Unrrein (junrrein)]

Related to right-click: Right-clicking on blank space in the left panel works. However, the action is executed on the active tab in the right view.

[Julián Unrrein (junrrein)]

Window.vala has a member called current_tab that holds the current active tab. It seems that we need this variable to hold the current active ViewContainer, but its valued is never updated on your branch (only when creating a new tab).

My understanding of how that works is limited though, so you may want to ask Victor about it.

[Jeremy Wootten (jeremywootten)]

This branch is now incompatible with Files so will have to be rejected. Sorry.

Unmerged revisions

1226. By Mario Guerriero on 2013-06-16

SplitView works fine

1225. By Mario Guerriero on 2013-06-14

pushed missing files

1224. By Mario Guerriero on 2013-06-14

press F3 to have a split view, set a minimum size for the path bar

=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt	2013-06-01 19:39:36 +0000
+++ src/CMakeLists.txt	2013-06-16 14:19:28 +0000
@@ -47,6 +47,8 @@
     View/Browser.vala
     View/ViewMode.vala
     View/LocationBar.vala
+    View/SplitView.vala
+    View/FilesView.vala
     View/Chrome/TopMenu.vala
     View/Chrome/ViewSwicher.vala
     View/Chrome/ColorWidget.vala

=== modified file 'src/View/Chrome/TopMenu.vala'
--- src/View/Chrome/TopMenu.vala	2013-06-14 02:10:41 +0000
+++ src/View/Chrome/TopMenu.vala	2013-06-16 14:19:28 +0000
@@ -65,11 +65,12 @@
                 if (name == "LocationEntry")
                 {
                     location_bar = new LocationBar (win.ui, win);
+                    location_bar.set_size_request (13, 26);
                     location_bar.halign = Gtk.Align.FILL;
                     location_bar.valign = Gtk.Align.FILL;
                     location_bar.margin_left = 6;
                     location_bar.margin_right = 12;
-
+                                       
                     /* init the path if we got a curent tab with a valid slot
                        and a valid directory loaded */
                     if (win.current_tab != null && win.current_tab.slot != null
@@ -85,7 +86,7 @@
                             win.current_tab.slot.view_box.grab_focus ();
                     });
                     location_bar.activate.connect(() => { win.current_tab.path_changed(File.new_for_commandline_arg(location_bar.path)); });
-                    location_bar.activate_alternate.connect((a) => { win.add_tab(File.new_for_commandline_arg(a)); });
+                    location_bar.activate_alternate.connect((a) => { win.get_current_view ().add_tab(File.new_for_commandline_arg(a)); });
                     location_bar.show_all();
                     insert(location_bar, -1);
                     continue;

=== added file 'src/View/FilesView.vala'
--- src/View/FilesView.vala	1970-01-01 00:00:00 +0000
+++ src/View/FilesView.vala	2013-06-16 14:19:28 +0000
@@ -0,0 +1,67 @@
+// -*- Mode: vala; indent-tabs-mode: nil; tab-width: 4 -*-
+/***
+  BEGIN LICENSE
+
+  Copyright (C) 2013 Mario Guerriero <[e-mail]>
+  This program is free software: you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License version 3, as published
+  by the Free Software Foundation.
+
+  This program is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranties of
+  MERCHANTABILITY, SATISFACTORY QUALITY, 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, see <http://www.gnu.org/licenses/>
+
+  END LICENSE
+***/
+
+using Gtk;
+using Granite.Widgets;
+
+namespace Marlin.View {
+    public class FilesView : DynamicNotebook {
+        public FilesView () {
+            // Connect to some signals
+            this.tab_added.connect ((tab) => {
+                if (tab != null)
+                    this.remove_tab (tab);
+                add_tab ();
+            });
+                
+            this.tab_removed.connect ((tab) => {
+                //if (this.n_tabs == 1)
+                //    add_tab ();
+                return true;
+            });
+        }
+        
+        public void add_tab (File location = File.new_for_commandline_arg (Environment.get_home_dir ())) {
+            var toplevel = this.get_toplevel () as Marlin.View.Window;
+            ViewContainer content = new View.ViewContainer(toplevel, location,
+                                    toplevel.current_tab != null ? toplevel.current_tab.view_mode : Preferences.settings.get_enum("default-viewmode"));
+
+            var new_tab = new Granite.Widgets.Tab (content.tab_name, null, content);
+            
+            content.tab_name_changed.connect ((tab_name) => {
+                toplevel.current_tab = content;
+                new_tab.label = tab_name;
+                content.content.grab_focus ();
+            });
+            content.path_changed.connect (() => {
+                toplevel.current_tab = content;
+                content.content.grab_focus ();
+                content.refresh_slot_info ();
+            });
+            
+            this.insert_tab (new_tab, -1);
+            
+            this.current = new_tab;
+            toplevel.current_tab = content;
+            content.refresh_slot_info ();
+            content.content.grab_focus ();
+        }
+    }
+}

=== modified file 'src/View/LocationBar.vala'
--- src/View/LocationBar.vala	2013-06-08 23:27:45 +0000
+++ src/View/LocationBar.vala	2013-06-16 14:19:28 +0000
@@ -302,7 +302,7 @@
             var menuitem_newtab = new Gtk.MenuItem.with_label (_("Open in New Tab"));
             menu.append (menuitem_newtab);
             menuitem_newtab.activate.connect (() => {
-                win.add_tab (File.new_for_uri (current_right_click_path));
+                win.get_current_view ().add_tab (File.new_for_uri (current_right_click_path));
             });
             
             menu.append (new Gtk.SeparatorMenuItem ());

=== added file 'src/View/SplitView.vala'
--- src/View/SplitView.vala	1970-01-01 00:00:00 +0000
+++ src/View/SplitView.vala	2013-06-16 14:19:28 +0000
@@ -0,0 +1,108 @@
+// -*- Mode: vala; indent-tabs-mode: nil; tab-width: 4 -*-
+/***
+  BEGIN LICENSE
+
+  Copyright (C) 2013 Mario Guerriero <[e-mail]>
+  This program is free software: you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License version 3, as published
+  by the Free Software Foundation.
+
+  This program is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranties of
+  MERCHANTABILITY, SATISFACTORY QUALITY, 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, see <http://www.gnu.org/licenses/>
+
+  END LICENSE
+***/
+
+using Gtk;
+using Granite.Widgets;
+
+namespace Marlin.View {
+    
+    public class SplitView : CollapsiblePaned {
+        
+        public FilesView? current_view = null;
+        public GLib.List<FilesView> views;
+        
+        // Signals
+        public signal void view_added (FilesView view);
+        
+        public SplitView () {
+            base (Gtk.Orientation.HORIZONTAL);
+            
+            views = new GLib.List<FilesView> ();
+        }
+        
+        public FilesView? add_view () {
+            if (views.length () >= 2) {
+                warning ("Maximum view number was already reached!");
+                return null;
+            }
+            
+            var view = new FilesView ();
+            view.tab_removed.connect (() => {
+                if (view.n_tabs == 1) 
+                    if (views.length () >= 2)
+                        remove_view (view);
+                    else
+                        view.add_tab ();
+                return true;
+            });
+            
+            if (views.length () < 1)
+                this.pack1 (view, true, true);
+            else
+                this.pack2 (view, true, true);
+            
+            view.show_all ();
+            
+            views.append (view);
+            this.current_view = view;
+            debug ("View added succefully");
+            
+            this.show_all ();
+            
+            this.view_added (view);
+            
+            return view;
+        }
+        
+        public void remove_view (FilesView? view = null) {
+            // If no specific view is required to be removed, just remove the current one
+            if (view == null)
+                view = get_focus_child () as FilesView;
+            if (view == null) {
+                warning ("The is no focused view to remove!");
+                return;                
+            }
+            
+            this.remove (view);
+            this.views.remove (view);
+            view.destroy ();
+            debug ("View removed succefully");
+            
+            // Move the focus on the other view
+            if (views.nth_data (0) != null) {
+                views.nth_data (0).current.grab_focus ();
+            }
+        }
+        
+        public FilesView? get_current_view () {
+            views.foreach ((v) => {
+                if (v.current.has_focus)
+                    current_view = v;
+            });
+            
+            return current_view;
+        }
+        
+        public bool is_empty () {
+            return (views.length () == 0);
+        }
+        
+    }
+}

=== modified file 'src/View/ViewContainer.vala'
--- src/View/ViewContainer.vala	2013-06-14 23:37:29 +0000
+++ src/View/ViewContainer.vala	2013-06-16 14:19:28 +0000
@@ -159,6 +159,7 @@
             /* update window title */
             if (window.current_tab == this) {
                 window.set_title (tab_name);
+                //print("\n\n\n");
                 if (window.top_menu.location_bar != null)
                     window.top_menu.location_bar.path = aslot.directory.file.location.get_parse_name ();
             }

=== modified file 'src/View/Window.vala'
--- src/View/Window.vala	2013-06-14 02:10:41 +0000
+++ src/View/Window.vala	2013-06-16 14:19:28 +0000
@@ -36,7 +36,8 @@
         public Gtk.InfoBar info_bar;
         public Granite.Widgets.DynamicNotebook tabs;
         public Marlin.Places.Sidebar sidebar;
-
+        
+        public SplitView split_view;
         public ViewContainer? current_tab;
 
         public Gtk.ActionGroup main_actions;
@@ -152,10 +153,8 @@
             show_infobar (!is_marlin_mydefault_fm ());
 
             /* Contents */
-            tabs = new Granite.Widgets.DynamicNotebook ();
-            tabs.show_tabs = true;
-            tabs.show ();
-
+            split_view = new SplitView ();
+            
             /* Sidebar */
             sidebar = new Marlin.Places.Sidebar (this);
             Preferences.settings.bind("sidebar-zoom-level", sidebar, "zoom-level", SettingsBindFlags.DEFAULT);
@@ -164,7 +163,7 @@
             lside_pane.show();
 
             lside_pane.pack1(sidebar, false, false);
-            lside_pane.pack2(tabs, true, true);
+            lside_pane.pack2(split_view, true, true);
 
             sidebar.show ();
 
@@ -221,23 +220,12 @@
             	return false;
             });
             
-            tabs.tab_added.connect ((tab) => {
-                if (tab != null)
-                    tabs.remove_tab (tab);
-                add_tab ();
-            });
-            
-            tabs.tab_removed.connect ((tab) => {
-                if (tabs.n_tabs == 1) {
-                    add_tab ();
-                }
-                return true;
-            });
-            
-            tabs.tab_switched.connect ((old_tab, new_tab) => {
-                change_tab (tabs.get_tab_position (new_tab));
-            });
-
+            this.split_view.view_added.connect ((view) => {
+                view.tab_switched.connect ((old_tab, new_tab) => {
+                    change_tab (view.get_tab_position (new_tab));
+                });
+            });
+            
             Gtk.Allocation win_alloc;
             get_allocation (out win_alloc);
 
@@ -252,7 +240,9 @@
             undo_manager = Marlin.UndoManager.instance ();
             undo_manager.request_menu_update.connect (undo_redo_menu_update_callback);
             undo_actions_set_insensitive ();
-
+            
+            // Add a view
+            split_view.add_view ();
         }
 
         private void show_infobar (bool val) {
@@ -290,7 +280,7 @@
 
         public void change_tab (int offset) {
             ViewContainer old_tab = current_tab;
-            current_tab = (tabs.get_tab_by_index (offset)).page as ViewContainer;
+            current_tab = (get_current_view ().get_tab_by_index (offset)).page as ViewContainer;
             if (old_tab == current_tab) {
                 return;
             }
@@ -317,26 +307,13 @@
         }
 
         public void add_tab (File location = File.new_for_commandline_arg (Environment.get_home_dir ())) {
-        
-            ViewContainer content = new View.ViewContainer(this, location,
-                                    current_tab != null ? current_tab.view_mode : Preferences.settings.get_enum("default-viewmode"));
-
-            var new_tab = new Granite.Widgets.Tab ("", null, content);
-            
-            content.tab_name_changed.connect ((tab_name) => {
-                new_tab.label = tab_name;
-            });
-            
-            tabs.insert_tab (new_tab, -1);
-
-            tabs.current = new_tab;
-            //current_tab = content;
+            this.get_current_view ().add_tab (location);
         }
 
         public void remove_tab (ViewContainer view_container){
-            var tab = tabs.get_tab_by_widget (view_container as Gtk.Widget);
+            var tab = get_current_view ().get_tab_by_widget (view_container as Gtk.Widget);
             if (tab != null)
-                tabs.remove_tab (tab);
+                get_current_view ().remove_tab (tab);
         }
 
         public void add_window(File location){
@@ -386,11 +363,11 @@
         }
 
         private void action_new_tab (Gtk.Action action) {
-            add_tab ();
+            get_current_view ().add_tab ();
         }
 
         private void action_remove_tab (Gtk.Action action) {
-            tabs.remove_tab (tabs.current);
+            get_current_view ().remove_tab (get_current_view ().current);
         }
 
     	private void save_geometries () {
@@ -438,26 +415,17 @@
             return false;
         }
 
-        private bool is_close_first () {
-            string path = "/apps/metacity/general/button_layout";
-            GConf.Client cl = GConf.Client.get_default ();
-            string key;
-
-            try {
-                if (cl.get (path) != null)
-                    key = cl.get_string (path);
-                else
-                    return false;
-            } catch (GLib.Error err) {
-                warning ("Unable to read metacity settings: %s", err.message);
+        // Get current view
+        public FilesView? get_current_view () {
+            FilesView? view = null;
+
+            view = split_view.get_current_view ();
+
+            if (view == null && !split_view.is_empty ()) {
+                view = (split_view.get_child2 () ?? split_view.get_child2 ()) as FilesView;
             }
 
-            string[] keys = key.split (":");
-            if ("close" in keys[0])
-                return true;
-            else
-                return false;
-
+            return view;
         }
 
         private bool is_marlin_mydefault_fm ()
@@ -529,13 +497,17 @@
 
         void action_next_tab ()
         {
-            tabs.next_page ();
-        }
-        void action_previous_tab ()
-        {
-            tabs.previous_page ();
-        }
-
+            get_current_view ().next_page ();
+        }
+        void action_previous_tab () {
+            get_current_view ().previous_page ();
+        }
+        
+        void action_new_view () {
+            this.split_view.add_view ();
+            get_current_view ().add_tab ();
+        }
+        
         private void action_zoom_normal_callback (Gtk.Action action) {
             if (current_tab != null && current_tab.slot != null)
                 ((FM.Directory.View) current_tab.slot.view_box).zoom_normal ();
@@ -665,6 +637,10 @@
   /* label, accelerator */       N_("Previous Tab"), "<control>Page_Up",
   /* tooltip */                  "",
                                  action_previous_tab },
+  /* name, stock id */         { "New View", null,
+  /* label, accelerator */       N_("New View"), "F3",
+  /* tooltip */                  "",
+                                 action_new_view },
   /* name, stock id */         { "Connect to Server", null,
   /* label, accelerator */       N_("Connect to _Server..."), null,
   /* tooltip */                  N_("Connect to a remote computer or shared disk"),

=== modified file 'src/pantheon-files-ui.xml'
--- src/pantheon-files-ui.xml	2013-05-28 08:04:18 +0000
+++ src/pantheon-files-ui.xml	2013-06-16 14:19:28 +0000
@@ -7,6 +7,7 @@
 <menubar name="MenuBar">
 	<menu action="File">
         <placeholder name="New Items Placeholder">
+            <menuitem name="New View" action="New View"/>
             <menuitem name="New Tab" action="New Tab"/>
 	    	<menuitem name="New Window" action="New Window"/>
         </placeholder>