Merge lp:~niclasl/midori/granite-fixes into lp:midori

[Niclas Lockner]

Mimic the look of Granite.DynamicNotebook when compiled with --enable-granite.

Mimic the look of Granite.DynamicNotebook when compiled with --enable-granite.

[Niclas Lockner (niclasl)]

Before: http://imgbin.org/images/16459.png
After: http://imgbin.org/images/16460.png

[Cris Dywan (kalikiana)]

The first two hunks should just be without the #if.

> list-add-symbolic
looks very suspicious to me. It's rather clearly for lists isn't it?

> render_activity
What's this meant to do? GTK+ docs say it renders like a spinner or progress. But we have a Spinner, just like DynamicNotebook. I genuinely don't know if this makes sense or not.

review: Needs Information

[David Gomes (davidgomes)]

It works and the code is fine. http://i.imgur.com/MkLlDgY.png

Note that I'm not changing the Status to Approved as I'm not a Midori dev.

review: Approve

[Niclas Lockner (niclasl)]

Granite's DynamicNotebook uses list-add-symbolic for its icon. The add button gets the "wrong" icon (only on Luna and not Isis for some reason) without it.

I'm not sure why render_activity is needed, and it looks hacky, but without it the background isn't drawn (but only if configured for granite). It's this call that makes the background silver and not black.

[Niclas Lockner (niclasl)]

I should add that I copy-pasted render_activity from Granite's Notebook.

[Cris Dywan (kalikiana)]

You say "without [render_activity] the background isn't drawn (but only if configured for granite)". Is this depending on the dynamic-notebook class in this branch? May be a bug in the theme then.

review: Needs Information

[Niclas Lockner (niclasl)]

No it happens regardless of whether dynamic-notebook is used or not, but it seems render_activity is only needed if Elementary's theme is used. If I switch to Ambiance it works just fine without it. So maybe it is a bug in the theme? I don't know.
Sorry for the delay.

[Danielle Foré (danrabbit)]

I've changed the icon in the elementary theme so that we can stick to "tab-new-symbolic". This doesn't need an if.

Also, Gtk.Notebook now draws a background behind the tabbar and eGtk has been updated for this. So I think we can remove code that was intended to duplicate that feature of Dynamic Notebook.

review: Needs Fixing

[lp:~niclasl/midori/granite-fixes]

6544. By Niclas Lockner on 2014-03-21

Center-align the tab label

6545. By Niclas Lockner on 2014-03-21

Do not use list-add-symbolic

6546. By Niclas Lockner on 2014-03-21

Do not call render_activity when drawing

6547. By Niclas Lockner on 2014-03-21

Always add the dynamic-notebook class

[Niclas Lockner (niclasl)]

"The first two hunks should just be without the #if."

Done. Now the label is always center-aligned, and the dynamic-notebook class is always added.

"I've changed the icon in the elementary theme so that we can stick to "tab-new-symbolic". This doesn't need an if."

if removed.

"Also, Gtk.Notebook now draws a background behind the tabbar and eGtk has been updated for this. So I think we can remove code that was intended to duplicate that feature of Dynamic Notebook."

Great! I removed the render_activity call.

The branch is now ready for another review.

[Danielle Foré (danrabbit)]

Looks great :)

review: Approve

[Cris Dywan (kalikiana)]

Thanks a lot!

review: Approve

=== modified file 'midori/midori-notebook.vala'
--- midori/midori-notebook.vala	2014-03-16 11:29:11 +0000
+++ midori/midori-notebook.vala	2014-03-21 18:53:20 +0000
@@ -36,7 +36,7 @@
             spinner.set_size_request (icon_size, icon_size);
             box.pack_start (spinner, false, false, 0);
             label = new Gtk.Label (null);
-            label.set_alignment (0.0f, 0.5f);
+            label.set_alignment (0.5f, 0.5f);
             label.set_padding (0, 0);
             box.pack_start (label, true, true, 0);
             close = new Gtk.Button ();
@@ -207,6 +207,7 @@
             notebook.show_border = false;
             notebook.set ("group-name", PACKAGE_NAME);
             add (notebook);
+            get_style_context ().add_class ("dynamic-notebook");
 
 #if !HAVE_GTK3
             /* Remove the inner border between scrollbars and window border */