Merge lp:~niclasl/granite/new-dn-api into lp:~elementary-pantheon/granite/granite

[Niclas Lockner]

Implemented a new DN API which will introduce better functionality in terms of tab dragging, tab moving and tab organization, but also bring ABI breaks to apps written against the old API.

Replaced the existing tab_added/tab_removed with signals that better match the functionality of Gtk.Notebook's page_added and page_removed. The signals new_tab_requested and close_tab_requested are now used to add and close tabs.

Tab objects are now always created outside DynamicNotebook, i.e. in the application that uses the notebook.

Splitted the tab_moved signal into the signals tab_reordered and tab_moved.

Implemented a settable upper limit of how many tabs to save when the restore tabs feature is enabled. The Granite Demo is extended to demonstrate this feature.

Fixes lp:1066578 lp:1170414 lp:1181657 lp:1198456 lp:1198513 and lp:1226058

[Niclas Lockner (niclasl)]

Mario, Julián Unrrein said that you use DynamicNotebook a lot so I added you as a reviewer

[lp:~niclasl/granite/new-dn-api]

651. By Niclas Lockner on 2013-12-08

Implement a settable upper limit of the size of the set of restorable tabs

652. By Niclas Lockner on 2013-12-09

Don't allow max_restorable_tabs to be <= 0

653. By Niclas Lockner on 2013-12-09

Missing semicolon

654. By Niclas Lockner on 2013-12-10

Renamed and commented the DroppedDelegate in Tab

[David Gomes (davidgomes)]

Line 144 change "( (" to "((".
Reindent diff line 552.
Newline after diff line 567.
287-289 please put some curly braces there to make it more readable.

Change the demo's max_restorable_tabs to 5 so we can give an example of changing DN's default limit.

This is really good stuff.

review: Needs Fixing

[lp:~niclasl/granite/new-dn-api]

655. By Niclas Lockner on 2013-12-10

* Code-style fixes
* Set max_restorable_tabs in granite demo

656. By Niclas Lockner on 2013-12-10

Tab -> Spaces

[David Gomes (davidgomes)]

review: Approve

=== modified file 'demo/GraniteDemo.vala'
--- demo/GraniteDemo.vala	2013-09-16 23:14:20 +0000
+++ demo/GraniteDemo.vala	2013-12-10 01:10:02 +0000
@@ -380,6 +380,7 @@
 
         dynamic_notebook.allow_duplication = true;
         dynamic_notebook.allow_restoring = true;
+        dynamic_notebook.max_restorable_tabs = 5;
         dynamic_notebook.allow_pinning = true;
         dynamic_notebook.show_icons = true;
         dynamic_notebook.add_button_tooltip = "New user tab";
@@ -397,16 +398,22 @@
         tab2.restore_data = "2";
         dynamic_notebook.insert_tab (tab2, -1);
 
-        dynamic_notebook.tab_added.connect ((t) => {
-            t.page = new Gtk.Label (@"Page $i");
-            t.label = @"user$i@elementaryos: ~";
+        dynamic_notebook.new_tab_requested.connect (() => {
+            var t = new Granite.Widgets.Tab (@"user$i@elementaryos: ~",
+                                             new ThemedIcon ("empty"),
+                                             new Gtk.Label (@"Page $i"));
             t.restore_data = i.to_string ();
             i++;
+            dynamic_notebook.insert_tab (t, -1);
         });
 
-        dynamic_notebook.tab_restored.connect ((t) => {
-            print ("Restored tab %s\n", t.label);
-            t.page = new Gtk.Label ("Page " + t.restore_data);
+        dynamic_notebook.tab_restored.connect ((label, data, icon) => {
+            var t = new Granite.Widgets.Tab (label,
+                                             icon,
+                                             new Gtk.Label ("Page " + data));
+            t.restore_data = data;
+            dynamic_notebook.insert_tab (t, -1);
+            print ("Restored tab %s\n", label);
         });
 
         dynamic_notebook.tab_duplicated.connect ((t) => {
@@ -429,8 +436,7 @@
         });
 
         dynamic_notebook.tab_removed.connect ((t) => {
-            print ("Going to remove %s\n", t.label);
-            return true;
+            print ("Removed tab %s\n", t.label);
         });
 
         return dynamic_notebook;

=== modified file 'lib/Widgets/DynamicNotebook.vala'
--- lib/Widgets/DynamicNotebook.vala	2013-12-03 01:03:16 +0000
+++ lib/Widgets/DynamicNotebook.vala	2013-12-10 01:10:02 +0000
@@ -25,8 +25,10 @@
                                                      Gdk.ModifierType.CONTROL_MASK |
                                                      Gdk.ModifierType.MOD1_MASK);
 
+    public delegate void DroppedDelegate ();
+
     private class TabPageContainer : Gtk.EventBox {
-        internal weak Tab _tab;
+        private weak Tab _tab;
 
         public Tab tab {
             get { return _tab; }
@@ -59,7 +61,7 @@
                     if (value != _pinned) {
                         if (value) {
                             _label.visible = false;
-                            close.visible = false;
+                            close_button.visible = false;
                             _icon.margin_left = 1;
                             _working.margin_left = 1;
 
@@ -95,6 +97,15 @@
          **/
         public string restore_data { get; set; }
 
+        /**
+         * An optional delegate that is called when the tab is dropped from the set
+         * of restorable tabs in DynamicNotebook.
+         * A tab is dropped either when Clear All is pressed, or when
+         * the tab is the oldest tab in the set of restorable tabs and
+         * the number of restorable tabs has exceeded the upper limit.
+         */
+        public DroppedDelegate dropped_callback = null;
+
         internal TabPageContainer page_container;
         public Gtk.Widget page {
             get {
@@ -136,13 +147,13 @@
             set {
                 if (value != _fixed) {
                     _label.visible = value;
-                    close.visible = value;
+                    close_button.visible = value;
                 }
                 _fixed = value;
             }
         }
 
-        internal Gtk.Button close;
+        internal Gtk.Button close_button;
         public Gtk.Menu menu { get; set; }
 
         //We need to be able to toggle these from the notebook.
@@ -165,13 +176,13 @@
             this._working = new Gtk.Spinner ();
             _working.start();
 
-            this.close = new Gtk.Button ();
-            close.add (new Gtk.Image.from_icon_name ("window-close-symbolic", Gtk.IconSize.MENU));
-            close.tooltip_text = _("Close Tab");
-            close.relief = Gtk.ReliefStyle.NONE;
+            this.close_button = new Gtk.Button ();
+            close_button.add (new Gtk.Image.from_icon_name ("window-close-symbolic", Gtk.IconSize.MENU));
+            close_button.tooltip_text = _("Close Tab");
+            close_button.relief = Gtk.ReliefStyle.NONE;
 
             var tab_box = new Gtk.Box (Gtk.Orientation.HORIZONTAL, 0);
-            tab_box.pack_start (close, false);
+            tab_box.pack_start (close_button, false);
             tab_box.pack_start (_label);
             tab_box.pack_start (_icon, false);
             tab_box.pack_start (_working, false);
@@ -185,7 +196,6 @@
 
             page_container = new TabPageContainer (this);
             this.page = page ?? new Gtk.Label("");
-            page_container.show_all ();
 
             restore_data = "";
 
@@ -236,9 +246,9 @@
             this.button_press_event.connect ((e) => {
                 e.state &= MODIFIER_MASK;
 
-                if (e.button == 2 && e.state == 0 && close.visible) {
+                if (e.button == 2 && e.state == 0 && close_button.visible) {
                     this.closed ();
-                } else if (e.button == 2 && e.state == Gdk.ModifierType.SHIFT_MASK && close.visible) {
+                } else if (e.button == 2 && e.state == Gdk.ModifierType.SHIFT_MASK && close_button.visible) {
                     this.close_others ();
                 } else if (e.button == 1 && e.type == Gdk.EventType.2BUTTON_PRESS && duplicate_m.visible) {
                     this.duplicate ();
@@ -263,68 +273,85 @@
             });*/
 
             page_container.button_press_event.connect (() => { return true; }); //dont let clicks pass through
-            close.clicked.connect ( () => this.closed () );
+            close_button.clicked.connect (() => this.closed ());
             working = false;
         }
+
+        public void close () {
+            closed ();
+        }
     }
 
     private class ClosedTabs : GLib.Object {
 
-        public signal void restored (Tab tab);
+        public signal void restored (string label, string restore_data, GLib.Icon? icon);
         public signal void cleared ();
 
-        private struct Entry {
-            string title;
-            string data;
+        private int _max_restorable_tabs = 10;
+        public int max_restorable_tabs {
+            get { return _max_restorable_tabs; }
+            set {
+                assert (value > 0);
+                _max_restorable_tabs = value;
+            }
+        }
+
+        internal struct Entry {
+            string label;
+            string restore_data;
             GLib.Icon? icon;
+            DroppedDelegate? dropped_callback;
         }
 
-        private Entry[] closed_tabs;
+        private Gee.LinkedList<Entry?> closed_tabs;
 
         public ClosedTabs () {
-            closed_tabs = {};
+            closed_tabs = new Gee.LinkedList<Entry?> ();
         }
 
         public bool empty {
             get {
-                return closed_tabs.length == 0;
+                return closed_tabs.size == 0;
             }
         }
 
         public void push (Tab tab) {
             foreach (var entry in closed_tabs)
-                if (tab.restore_data == entry.data)
+                if (tab.restore_data == entry.restore_data)
                     return;
-            Entry tmp = { tab.label, tab.restore_data, tab.icon };
-            closed_tabs += tmp;
-        }
-
-        public Tab pop () {
-            assert (closed_tabs.length > 0);
-            Entry entry = closed_tabs[closed_tabs.length - 1];
-            var tab = new Tab (entry.title);
-            tab.restore_data = entry.data;
-            tab.icon = entry.icon;
-            closed_tabs.resize (closed_tabs.length - 1);
-            return tab;
-        }
-
-        public Tab? pick (string search) {
-            var tab = (Tab) null;
-            Entry[] copy = {};
-
-            foreach (var entry in closed_tabs) {
-                if (entry.data != search) {
-                    copy += entry;
-                } else {
-                    tab = new Tab (entry.title);
-                    tab.restore_data = entry.data;
-                    tab.icon = entry.icon;
-                }
+
+            // Insert the element at the end of the list.
+            Entry e = { tab.label, tab.restore_data, tab.icon, tab.dropped_callback };
+            closed_tabs.add (e);
+
+            // If the maximum size is exceeded, remove from the beginning of the list.
+            if (closed_tabs.size > max_restorable_tabs) {
+                var elem = closed_tabs.poll_head ();
+                var dropped_callback = elem.dropped_callback;
+
+                if (dropped_callback != null)
+                    dropped_callback ();
             }
-
-            closed_tabs = copy;
-            return tab;
+        }
+
+        public Entry pop () {
+            assert (closed_tabs.size > 0);
+            return closed_tabs.poll_tail ();
+        }
+
+        public Entry pick (string search) {
+            Entry picked = {null, null, null};
+
+            for (int i = 0; i < closed_tabs.size; i++) {
+                var entry = closed_tabs[i];
+
+                if (entry.restore_data == search) {
+                    picked = closed_tabs.remove_at (i);
+                    break;
+                 }
+             }
+
+            return picked;
         }
 
         public Gtk.Menu menu {
@@ -332,7 +359,7 @@
                 var _menu = new Gtk.Menu ();
 
                 foreach (var entry in closed_tabs) {
-                    var item = new Gtk.ImageMenuItem.with_label (entry.title);
+                    var item = new Gtk.ImageMenuItem.with_label (entry.label);
                     item.set_always_show_image (true);
                     if (entry.icon != null) {
                         var icon = new Gtk.Image.from_gicon (entry.icon, Gtk.IconSize.MENU);
@@ -341,7 +368,8 @@
                     _menu.prepend (item);
 
                     item.activate.connect (() => {
-                        this.restored (pick (entry.data));
+                        var e = pick (entry.restore_data);
+                        this.restored (e.label, e.restore_data, e.icon);
                     });
                 }
 
@@ -353,7 +381,13 @@
                     _menu.append (item);
 
                     item.activate.connect (() => {
-                        closed_tabs = {};
+                        foreach (var entry in closed_tabs) {
+                            if (entry.dropped_callback != null) {
+                                entry.dropped_callback ();
+                            }
+                        }
+
+                        closed_tabs.clear ();
                         cleared ();
                     });
                 }
@@ -403,7 +437,7 @@
             set {
                 if (value != _tabs_closable)
                     tabs.foreach ((t) => {
-                            t.close.visible = value;
+                            t.close_button.visible = value;
                         });
                 _tabs_closable = value;
             }
@@ -466,6 +500,15 @@
         }
 
         /**
+         * Set or get the upper limit of the size of the set
+         * of restorable tabs.
+         */
+        public int max_restorable_tabs {
+            get { return closed_tabs.max_restorable_tabs; }
+            set { closed_tabs.max_restorable_tabs = value; }
+        }
+
+        /**
          * Controls the '+' add button visibility
          */
         bool _add_button_visible = true;
@@ -526,7 +569,6 @@
             }
         }
 
-
         public string group_name {
             get { return notebook.group_name; }
             set { notebook.group_name = value; }
@@ -543,16 +585,19 @@
         private Gtk.CssProvider button_fix;
 
         private int tab_width = 150;
-        private int max_tab_width = 150;
-        private int tab_width_pinned = 18;
+        private static const int MAX_TAB_WIDTH = 150;
+        private static const int TAB_WIDTH_PINNED = 18;
 
         public signal void tab_added (Tab tab);
-        public signal bool tab_removed (Tab tab);
+        public signal void tab_removed (Tab tab);
         Tab? old_tab; //stores a reference for tab_switched
         public signal void tab_switched (Tab? old_tab, Tab new_tab);
-        public signal void tab_moved (Tab tab, int new_pos, bool new_window, int x, int y);
+        public signal void tab_reordered (Tab tab, int new_pos);
+        public signal void tab_moved (Tab tab, int x, int y);
         public signal void tab_duplicated (Tab duplicated_tab);
-        public signal void tab_restored (Tab tab);
+        public signal void tab_restored (string label, string data, GLib.Icon? icon);
+        public signal void new_tab_requested ();
+        public signal bool close_tab_requested (Tab tab);
 
         private Gtk.MenuItem new_tab_m;
         private Gtk.MenuItem restore_tab_m;
@@ -605,7 +650,7 @@
             menu.show_all ();
 
             new_tab_m.activate.connect (() => {
-                insert_new_tab_at_end ();
+                new_tab_requested ();
             });
 
             restore_tab_m.activate.connect (() => {
@@ -613,13 +658,12 @@
             });
 
             closed_tabs = new ClosedTabs ();
-            closed_tabs.restored.connect ((tab) => {
+            closed_tabs.restored.connect ((label, restore_data, icon) => {
                 if (!allow_restoring)
                     return;
                 restore_button.sensitive = !closed_tabs.empty;
                 restore_tab_m.sensitive = !closed_tabs.empty;
-                insert_tab (tab, -1);
-                this.tab_restored (tab);
+                this.tab_restored (label, restore_data, icon);
             });
 
             closed_tabs.cleared.connect (() => {
@@ -647,7 +691,7 @@
             restore_button.show_all ();
 
             add_button.clicked.connect (() => {
-                insert_new_tab_at_end ();
+                new_tab_requested ();
             });
 
             restore_button.clicked.connect (() => {
@@ -666,7 +710,7 @@
 
             this.button_press_event.connect ((e) => {
                 if (e.type == Gdk.EventType.2BUTTON_PRESS && e.button == 1) {
-                    insert_new_tab_at_end ();
+                    new_tab_requested ();
                 } else if (e.button == 2 && allow_restoring) {
                     restore_last_tab ();
                     return true;
@@ -695,7 +739,7 @@
                     case Gdk.Key.@t:
                     case Gdk.Key.@T:
                         if (e.state == Gdk.ModifierType.CONTROL_MASK) {
-                            insert_new_tab_at_end ();
+                            new_tab_requested ();
                             return true;
                         } else if (e.state == (Gdk.ModifierType.CONTROL_MASK | Gdk.ModifierType.SHIFT_MASK) && allow_restoring) {
                             restore_last_tab ();
@@ -758,6 +802,8 @@
 
         ~Notebook () {
             notebook.switch_page.disconnect (on_switch_page);
+            notebook.page_added.disconnect (on_page_added);
+            notebook.page_removed.disconnect (on_page_removed);
             notebook.page_reordered.disconnect (on_page_reordered);
             notebook.create_window.disconnect (on_create_window);
         }
@@ -779,21 +825,27 @@
         }
 
         void on_page_added (Gtk.Widget page, uint pagenum) {
-            insert_callbacks ((page as TabPageContainer).tab);
+            var t = (page as TabPageContainer).tab;
+
+            insert_callbacks (t);
+            tab_added (t);
         }
 
         void on_page_removed (Gtk.Widget page, uint pagenum) {
-            remove_callbacks ((page as TabPageContainer).tab);
+            var t = (page as TabPageContainer).tab;
+
+            remove_callbacks (t);
+            tab_removed (t);
         }
 
         void on_page_reordered (Gtk.Widget page, uint pagenum) {
-            tab_moved ((page as TabPageContainer).tab, (int) pagenum, false, -1, -1);
+            tab_reordered ((page as TabPageContainer).tab, (int) pagenum);
             recalc_order ();
         }
 
         unowned Gtk.Notebook on_create_window (Gtk.Widget page, int x, int y) {
             var tab = notebook.get_tab_label (page) as Tab;
-            tab_moved (tab, 0, true, x, y);
+            tab_moved (tab, x, y);
             recalc_order ();
             return (Gtk.Notebook) null;
         }
@@ -843,9 +895,9 @@
             }
 
             var offset = 130;
-            this.tab_width = (this.get_allocated_width () - offset - pinned_tabs * tab_width_pinned) / unpinned_tabs;
-            if (tab_width > max_tab_width)
-                tab_width = max_tab_width;
+            this.tab_width = (this.get_allocated_width () - offset - pinned_tabs * TAB_WIDTH_PINNED) / unpinned_tabs;
+            if (tab_width > MAX_TAB_WIDTH)
+                tab_width = MAX_TAB_WIDTH;
 
             if (tab_width < 0)
                 tab_width = 0;
@@ -854,7 +906,7 @@
                 this.notebook.get_tab_label (this.notebook.get_nth_page (i)).width_request = tab_width;
 
                 if ((this.notebook.get_tab_label (this.notebook.get_nth_page (i)) as Tab).pinned) {
-                    this.notebook.get_tab_label (this.notebook.get_nth_page (i)).width_request = tab_width_pinned;
+                    this.notebook.get_tab_label (this.notebook.get_nth_page (i)).width_request = TAB_WIDTH_PINNED;
                 }
             }
 
@@ -865,11 +917,10 @@
             if (!allow_restoring || closed_tabs.empty)
                 return;
 
-            var tab = closed_tabs.pop ();
+            var restored = closed_tabs.pop ();
             restore_button.sensitive = !closed_tabs.empty;
             restore_tab_m.sensitive = !closed_tabs.empty;
-            insert_tab (tab, -1);
-            this.tab_restored (tab);
+            this.tab_restored (restored.label, restored.restore_data, restored.icon);
         }
 
         private void switch_pin_tab (Tab tab) {
@@ -880,7 +931,7 @@
             var pin_state = !tab.pinned;
             if (pin_state) {
                 tab._icon.visible = !tab.working;
-                tab.close.visible = tabs_closable;
+                tab.close_button.visible = tabs_closable;
             } else {
                 tab._icon.visible = show_icons && !tab.working;
             }
@@ -890,30 +941,8 @@
         }
 
         public void remove_tab (Tab tab) {
-            if (Signal.has_handler_pending (this, Signal.lookup ("tab-removed", typeof (DynamicNotebook)), 0, true)) {
-                var sure = tab_removed (tab);
-                if (!sure)
-                    return;
-            }
-
-            var pos = get_tab_position (tab);
-
-            if (pos != -1) {
-                notebook.remove_page (pos);
-                if (tab.page.get_parent () != null)
-                    tab.page.unparent ();
-            }
-
-            if (tab.label != "" && tab.restore_data != "") {
-                closed_tabs.push (tab);
-                restore_button.sensitive = !closed_tabs.empty;
-                restore_tab_m.sensitive = !closed_tabs.empty;
-            }
-        }
-        
-        [Deprecated (since=0.2)]
-        public void remove_tab_force (Tab tab) {
-            var pos = get_tab_position (tab);
+            var pos = get_tab_position (tab);
+
             if (pos != -1)
                 notebook.remove_page (pos);
         }
@@ -948,7 +977,7 @@
 
         public void set_tab_position (Tab tab, int position) {
             notebook.reorder_child (tab.page_container, position);
-            tab_moved (tab, position, false, -1, -1);
+            tab_reordered (tab, position);
             recalc_order ();
         }
 
@@ -964,12 +993,6 @@
             return notebook.get_nth_page (index);
         }
 
-        private void insert_new_tab_at_end () {
-            var t = new Tab ();
-            notebook.page = (int) this.insert_tab (t, -1);
-            this.tab_added (t);
-        }
-
         public uint insert_tab (Tab tab, int index) {
             return_if_fail (tabs.index (tab) < 0);
 
@@ -990,13 +1013,13 @@
             tab.pinned = false;
 
             tab.width_request = tab_width;
-            tab.close.get_style_context ().add_provider (button_fix,
-                                                         Gtk.STYLE_PROVIDER_PRIORITY_APPLICATION);
+            tab.close_button.get_style_context ().add_provider (button_fix,
+                                                                Gtk.STYLE_PROVIDER_PRIORITY_APPLICATION);
             this.recalc_size ();
             this.recalc_order ();
 
             if (!tabs_closable)
-                tab.close.visible = false;
+                tab.close_button.visible = false;
 
             return i;
         }
@@ -1018,7 +1041,25 @@
         }
 
         private void on_tab_closed (Tab tab) {
+            if (Signal.has_handler_pending (this, Signal.lookup ("close-tab-requested", typeof (DynamicNotebook)), 0, true)) {
+                var sure = close_tab_requested (tab);
+
+                if (!sure)
+                    return;
+            }
+
+            var pos = get_tab_position (tab);
+
             remove_tab (tab);
+
+            if (pos != -1 && tab.page.get_parent () != null)
+                tab.page.unparent ();
+
+            if (tab.label != "" && tab.restore_data != "") {
+                closed_tabs.push (tab);
+                restore_button.sensitive = !closed_tabs.empty;
+                restore_tab_m.sensitive = !closed_tabs.empty;
+            }
         }
 
         private void on_close_others (Tab tab) {