Merge lp:~niclasl/granite/fix-padding into lp:~elementary-pantheon/granite/granite

[Niclas Lockner]

Add padding around the new tab button

Add padding around the new tab button

[Niclas Lockner (niclasl)]

With this padding, the button looks a bit weird, but according to #1166389, this bug is worked around in the theme, so it may be due to that.
When I switch to HighContrast, the button looks better.

[Danielle Foré (danrabbit)]

Build fails with lots of output >.<

http://snippi.com/s/wl73xr3

But yes, it's worked around in the theme so it's gonna look funny unless you remove the workaround.

[Niclas Lockner (niclasl)]

Hm I don't know why the build fails for you but it doesn't look like it's caused by a compile error

[Julián Unrrein (junrrein)]

I tested this branch, but I couldn't notice any difference in the distance between the New Tab button and the Previous Tab button while using the High Contrast theme (3.4).

Screenshots:
- In trunk: http://imagebin.org/280253
- In this branch: http://imagebin.org/280254

To test this branch, I first run the following line:

LD_LIBRARY_PATH=/home/julian/elementary-projects/granite/fix-1181657/build/lib ./demo/granite-demo

After not noticing any difference, I make install'd the branch, which made no difference.

Could you share how you tested this and show screenshots?

review: Needs Fixing

[Niclas Lockner (niclasl)]

I simply opened pantheon-terminal and created some tabs so that the arrows appear. I get the same results with Files. (This is with Ubuntu 13.10 + the daily PPA)

With granite trunk:
http://imagebin.org/280264
http://imagebin.org/280263

With this branch (make && sudo make install):
http://imagebin.org/280261
http://imagebin.org/280262

[Julián Unrrein (junrrein)]

I somehow mis-tested your branch. I tested it again and it works just as expected.

Code style review following soon.

[Julián Unrrein (junrrein)]

Change the name of the Gtk.Box to something more descriptive, like "add_button_box".

review: Needs Fixing

[lp:~niclasl/granite/fix-padding]

650. By Niclas Lockner on 2013-12-03

Renamed variables

[Julián Unrrein (junrrein)]

I'm approving, but we still need a review from Dan.

Thanks for your work!

review: Approve

[Danielle Foré (danrabbit)]

Looks good!

review: Approve

=== modified file 'lib/Widgets/DynamicNotebook.vala'
--- lib/Widgets/DynamicNotebook.vala	2013-10-23 21:20:04 +0000
+++ lib/Widgets/DynamicNotebook.vala	2013-12-03 01:03:35 +0000
@@ -560,6 +560,7 @@
         private Gtk.Button add_button;
         private Gtk.Button restore_button; // should be a Gtk.MenuButton when we have Gtk+ 3.6
 
+        private static const int ADD_BUTTON_PADDING = 5; // Padding around the new tab button
         private static const string CLOSE_BUTTON_STYLE = """
         * {
             -GtkButton-default-border : 0;
@@ -626,14 +627,15 @@
                 restore_tab_m.sensitive = false;
             });
 
+            Gtk.Box add_button_box = new Gtk.Box (Gtk.Orientation.HORIZONTAL, 0);
             add_button = new Gtk.Button ();
             add_button.add (new Gtk.Image.from_icon_name ("list-add-symbolic", Gtk.IconSize.MENU));
-            add_button.margin_left = 6;
             add_button.relief = Gtk.ReliefStyle.NONE;
             add_button.tooltip_text = _("New Tab");
-            this.notebook.set_action_widget (add_button, Gtk.PackType.START);
-            add_button.show_all ();
             add_button.get_style_context ().add_provider (button_fix, Gtk.STYLE_PROVIDER_PRIORITY_APPLICATION);
+            add_button_box.pack_start (add_button, false, false, ADD_BUTTON_PADDING);
+            add_button_box.show_all ();
+            this.notebook.set_action_widget (add_button_box, Gtk.PackType.START);
 
             restore_button = new Gtk.Button ();
             restore_button.add (new Gtk.Image.from_icon_name ("user-trash-symbolic", Gtk.IconSize.MENU));