Merge lp:~elementary-apps/granite/tab-bar-visibility into lp:~elementary-pantheon/granite/granite

[David Gomes]

Moved all the Pantheon Terminal TabBarBehavior code to Granite.DynamicNotebook,
fixing bug #1264565.

Moved all the Pantheon Terminal TabBarBehavior code to Granite.DynamicNotebook,
fixing bug #1264565.

This now allows any app to set their tab bar visibility behavior.

[Niclas Lockner (niclasl)]

Looks good. I have some minor comments though:

If tab_bar_behavior is changed from never to always, the notebook's visibility isn't updated.
_tab_bar_behavior isn't explicitly initialized, but implicitly initialized to 0, which is more of a readability issue.
Line 659-664 are kind of redundant. When they are executed, _tab_bar_behavior will always be 0 = ALWAYS.

I would suggest you to extend update_tabs_visibility to cover all cases, and then initialize tab_bar_behavior to ALWAYS in the constructor. After that the lines 590-591 and 659-664 can be dropped.

review: Needs Fixing

[Niclas Lockner (niclasl)]

And with lines 590-591 and 659-664 I meant lines 76-77 and 102-108 in the diff.

[David Gomes (davidgomes)]

>If tab_bar_behavior is changed from never to always, the notebook's visibility isn't updated.
Do you mean while the application is running?

[Niclas Lockner (niclasl)]

Exactly. If I were to do a dn.tab_bar_behavior = ALWAYS from an application, there's nothing in the set block that changes the show_tabs property. So if it was hidden before, it is not set to shown.

[David Gomes (davidgomes)]

Pushed according to your suggestions Niclas. Requesting re-review please.

[Niclas Lockner (niclasl)]

review: Approve

[RabbitBot (rabbitbot-a)]

The `tree_dir` option for the target branch is not a lightweight checkout. Please ask a project administrator to resolve the issue, and try again.

=== modified file 'lib/Widgets/DynamicNotebook.vala'
--- lib/Widgets/DynamicNotebook.vala	2013-12-27 16:31:25 +0000
+++ lib/Widgets/DynamicNotebook.vala	2014-01-23 12:56:09 +0000
@@ -1,6 +1,6 @@
 /***
     Copyright (C) 2011-2013 Tom Beckmann <[e-mail]>
-
+    
     This program or library is free software; you can redistribute it
     and/or modify it under the terms of the GNU Lesser General Public
     License as published by the Free Software Foundation; either
@@ -43,10 +43,10 @@
      * This is a standard tab which can be used in a notebook to form a tabbed UI.
      */
     public class Tab : Gtk.EventBox {
-
         Gtk.Label _label;
         public string label {
             get { return _label.label;  }
+
             set {
                 _label.label = value;
                 _label.set_tooltip_text (value);
@@ -56,6 +56,7 @@
         private bool _pinned;
         public bool pinned {
             get { return _pinned; }
+
             set {
                 if (pinnable) {
                     if (value != _pinned) {
@@ -64,7 +65,6 @@
                             close_button.visible = false;
                             _icon.margin_left = 1;
                             _working.margin_left = 1;
-
                         } else {
                             _label.visible = true;
                             _icon.margin_left = 0;
@@ -191,7 +191,7 @@
             _icon.set_size_request (16, 16);
             _working.set_size_request (16, 16);
             this.visible_window = false;
-            
+
             this.add (tab_box);
 
             page_container = new TabPageContainer (this);
@@ -398,7 +398,6 @@
     }
 
     public class DynamicNotebook : Gtk.EventBox {
-
         /**
          * number of pages
          */
@@ -517,7 +516,7 @@
             set {
                 if (value != _add_button_visible) {
                     if (_add_button_visible) {
-                        this.notebook.set_action_widget (null, Gtk.PackType.START);                 
+                        this.notebook.set_action_widget (null, Gtk.PackType.START);
                     } else {
                         this.notebook.set_action_widget (add_button, Gtk.PackType.START);
                     }
@@ -574,6 +573,26 @@
             set { notebook.group_name = value; }
         }
 
+        public enum TabBarBehavior {
+            ALWAYS = 0,
+            SINGLE = 1,
+            NEVER = 2
+        }
+
+        /**
+         * The behavior of the tab bar and its visibility
+        */
+        public TabBarBehavior tab_bar_behavior {
+            set {
+                _tab_bar_behavior = value;
+                update_tabs_visibility ();
+            }
+
+            get { return _tab_bar_behavior; }
+        }
+
+        private TabBarBehavior _tab_bar_behavior;
+
         /**
          * The menu appearing when the notebook is clicked on a blank space
          */
@@ -621,8 +640,8 @@
          * Create a new dynamic notebook
          */
         public DynamicNotebook () {
-
             this.button_fix = new Gtk.CssProvider ();
+
             try {
                 this.button_fix.load_from_data (CLOSE_BUTTON_STYLE, -1);
             } catch (Error e) { warning (e.message); }
@@ -633,6 +652,7 @@
 
             this.notebook.scrollable = true;
             this.notebook.show_border = false;
+            _tab_bar_behavior = TabBarBehavior.ALWAYS;
 
             this.draw.connect ( (ctx) => {
                 this.get_style_context ().render_activity (ctx, 0, 0, this.get_allocated_width (), 27);
@@ -727,7 +747,6 @@
             });
 
             this.key_press_event.connect ((e) => {
-
                 e.state &= MODIFIER_MASK;
 
                 switch (e.keyval) {
@@ -834,6 +853,7 @@
 
             insert_callbacks (t);
             tab_added (t);
+            update_tabs_visibility ();
         }
 
         void on_page_removed (Gtk.Widget page, uint pagenum) {
@@ -841,6 +861,7 @@
 
             remove_callbacks (t);
             tab_removed (t);
+            update_tabs_visibility ();
         }
 
         void on_page_reordered (Gtk.Widget page, uint pagenum) {
@@ -866,7 +887,7 @@
                 }
             }
 
-            for (var p = 0; p < pinned_tabs; p++) {         
+            for (var p = 0; p < pinned_tabs; p++) {
                 int sel = p;
                 for (var i = p; i < this.notebook.get_n_pages (); i++) {
                     if ((this.notebook.get_tab_label (this.notebook.get_nth_page (i)) as Tab).pinned) {
@@ -1092,5 +1113,14 @@
         private void on_pin_switch (Tab tab) {
             switch_pin_tab (tab);
         }
+
+        private void update_tabs_visibility () {
+            if (_tab_bar_behavior == TabBarBehavior.SINGLE)
+                notebook.show_tabs = n_tabs > 1;
+            else if (_tab_bar_behavior == TabBarBehavior.NEVER)
+                notebook.show_tabs = false;
+            else if (_tab_bar_behavior == TabBarBehavior.ALWAYS)
+                notebook.show_tabs = true;
+        }
     }
 }

=== modified file 'lib/Widgets/SourceList.vala'
--- lib/Widgets/SourceList.vala	2013-12-15 12:40:31 +0000
+++ lib/Widgets/SourceList.vala	2014-01-23 12:56:09 +0000
@@ -10,7 +10,7 @@
     but WITHOUT ANY WARRANTY; without even the implied warranty of
     MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
     Lesser General Public License for more details.
- 
+
     You should have received a copy of the GNU Lesser General
     Public License along with this library; if not, write to the
     Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
@@ -2206,7 +2206,7 @@
             if (data_model.iter_previous (ref new_iter))
                 return data_model.get_item (new_iter);
         }
-        
+
         return null;
     }
 
@@ -2225,7 +2225,7 @@
             if (data_model.iter_next (ref new_iter))
                 return data_model.get_item (new_iter);
         }
-        
+
         return null;
     }