Merge lp:~junrrein/granite/fix-1196721 into lp:~elementary-pantheon/granite/granite

[Julián Unrrein]

[DynamicNotebook] Let tabs handle clicks over their icons and spinners to fix bug #1196721.

[DynamicNotebook] Let tabs handle clicks over their icons and spinners to fix bug #1196721.

[lp:~junrrein/granite/fix-1196721]

596. By Julián Unrrein on 2013-07-15

Don't let the icon or the spinner take more space than it should.

597. By Julián Unrrein on 2013-07-16

Force icons and spinners to have the same minimum size to avoid padding issues in tabs.

[Julián Unrrein (junrrein)]

Rev597 increments the spinner size a bit to make it the same size as the icon (in Midori, at least).

I did this to prevent a padding problem when the tab has a short label (opening Google in Midori, for example). The size of the default spinner is 14px, while the size of the icon is 16px. This causes text to move a little to the sides when changing from an icon to a spinner and viceversa.

This can also be worked around incrementing the padding around the spinner. I don't know which way is preferable.

[David Gomes (davidgomes)]

s/lblbox/label_box

Don't be lazy when it comes to variable names.

It does seem to work pretty well, I'll test more before approving and it will be merged if you fix the above.

review: Needs Fixing

[David Gomes (davidgomes)]

Also, double clicking slightly below the close button still causes a new tab to be opened.

[David Gomes (davidgomes)]

Actually, not just below, to the left too. Quite interesting this is.

[lp:~junrrein/granite/fix-1196721]

598. By Julián Unrrein on 2013-07-20

Base Granite.Widgets.Tab on Gtk.EventBox instead of Gtk.Box.

[Julián Unrrein (junrrein)]

I didn't notice the bug about clicks around the close button. After testing some more, it seems it also happens when clicking under the label and the icon (tested with right-click).

I turned the entire thing into an EventBox to try to catch all mouse events and prevent this, but the situation is the same.

I then discovered that those mouse events don't even reach the EventBox (added debug output that is not present here to check that).

Also notice that I commented out a section of code that referred to the original Gtk.Box. What's the intention of that piece of code?

[Julián Unrrein (junrrein)]

It seems it isn't possible to even switch tabs if clicking in the lower border of a tab.

[David Gomes (davidgomes)]

I just know it's doable because Gtk.Notebook works that way.

[Julián Unrrein (junrrein)]

I know what you mean, but that is a different issue from the one at hand.

[David Gomes (davidgomes)]

review: Approve

=== modified file 'lib/Widgets/DynamicNotebook.vala'
--- lib/Widgets/DynamicNotebook.vala	2013-07-13 22:40:24 +0000
+++ lib/Widgets/DynamicNotebook.vala	2013-07-20 22:06:26 +0000
@@ -27,7 +27,7 @@
     /**
      * This is a standard tab which can be used in a notebook to form a tabbed UI.
      */
-    public class Tab : Gtk.Box {
+    public class Tab : Gtk.EventBox {
 
         Gtk.Label _label;
         public string label {
@@ -94,7 +94,7 @@
         internal Gtk.Button close;
         public Gtk.Menu menu { get; set; }
 
-        //we need to be able to toggle those from the notebook
+        //We need to be able to toggle these from the notebook.
         internal Gtk.MenuItem new_window_m;
         internal Gtk.MenuItem duplicate_m;
 
@@ -117,16 +117,18 @@
             close.tooltip_text = _("Close Tab");
             close.relief = Gtk.ReliefStyle.NONE;
 
-            var lbl = new Gtk.EventBox ();
+            var tab_box = new Gtk.Box (Gtk.Orientation.HORIZONTAL, 0);
+            tab_box.pack_start (close, false);
+            tab_box.pack_start (_label);
+            tab_box.pack_start (_icon, false);
+            tab_box.pack_start (_working, false);
             _label.set_tooltip_text (label);
-            lbl.add (_label);
             _label.ellipsize = Pango.EllipsizeMode.END;
-            lbl.visible_window = false;
-
-            this.pack_start (this.close, false);
-            this.pack_start (lbl);
-            this.pack_start (this._icon, false);
-            this.pack_start (this._working, false);
+            _icon.set_size_request (16, 16);
+            _working.set_size_request (16, 16);
+            this.visible_window = false;
+            
+            this.add (tab_box);
 
             page_container = new Gtk.EventBox ();
             this.page = page ?? new Gtk.Label("");
@@ -152,7 +154,7 @@
             new_window_m.activate.connect (() => new_window () );
             duplicate_m.activate.connect (() => duplicate () );
 
-            lbl.scroll_event.connect ((e) => {
+            this.scroll_event.connect ((e) => {
                 var notebook = (this.get_parent () as Gtk.Notebook);
                 switch (e.direction) {
                     case Gdk.ScrollDirection.UP:
@@ -175,8 +177,7 @@
                 return false;
             });
 
-            lbl.button_press_event.connect ((e) => {
-
+            this.button_press_event.connect ((e) => {
                 e.state &= MODIFIER_MASK;
 
                 if (e.button == 2 && e.state == 0 && close.visible) {
@@ -197,9 +198,9 @@
                 return true;
             });
 
-            this.button_press_event.connect ((e) => {
+            /*this.button_press_event.connect ((e) => {
                 return (e.type == Gdk.EventType.2BUTTON_PRESS || e.button != 1);
-            });
+            });*/
 
             page_container.button_press_event.connect (() => { return true; }); //dont let clicks pass through
             close.clicked.connect ( () => this.closed () );