Review: Needs Information
So, I have just tested your branch, but I have some issues:
- There is a build error in PopOver.vala, at line 99. Here is a fix, let me
know if you think there is a problem with it (I hope vala didn't break the api
in recent versions about that?):
=== modified file 'lib/Widgets/PopOver.vala'
--- lib/Widgets/PopOver.vala 2012-01-16 22:00:46 +0000
+++ lib/Widgets/PopOver.vala 2012-01-21 09:00:39 +0000
@@ -396,7 +396,7 @@
// Background
main_buffer.context.clip ();
- menu.get_style_context().render_background (main_buffer.context,
SHADOW_SIZE, SHADOW_SIZE, w - 2 * SHADOW_SIZE, h - 2 * SHADOW_SIZE);
+ Gtk.render_background (menu.get_style_context (), main_buffer.context,
SHADOW_SIZE, SHADOW_SIZE, w - 2 * SHADOW_SIZE, h - 2 * SHADOW_SIZE);
if(is_composited) {
h -= 2* (PADDINGS.top + SHADOW_SIZE) + ARROW_HEIGHT;
w -= 2*(PADDINGS.right + SHADOW_SIZE);
- Bug #732168: Click-and-holding AppMenu buttons is inconsistent: this bug
isn't fixed here, and I don't understand this line:
- private int long_press_time =
Gtk.Settings.get_default().gtk_double_click_time * 2;
+ private int long_press_time =
Gtk.Settings.get_default().gtk_double_click_time / 2;
This is not the appropriate fix, because we would like to be able to use two
different actions on these buttons (see the next/back buttons in marlin), and
only if there is no action with a single click, we want to trigger the menu
immediately.
You also removed menu_orientation, then it is an API break, maybe we could keep
menu_orientation and don't add menu_position?
Otherwise, the code cleanup you did in this area is nice!
- Bug #873811: more padding for buttons in About: Perfect!
- Bug #873813: larger primary text in About: I'm not sure to see the
difference, but my eyes may be bad. However, the code cleanup there is a very
good thing :)
- Bug #889551: StaticNotebook should allow to hide the ModeButton: it works
fine, but I had to make a quick changes, because if switcher_box wasn't
visible, it will be shown, even if switcher_hidden is true. (Just test with the
demo app, you'll see what I mean).
=== modified file 'lib/Widgets/StaticNotebook.vala'
--- lib/Widgets/StaticNotebook.vala 2012-01-18 07:40:45 +0000
+++ lib/Widgets/StaticNotebook.vala 2012-01-21 09:22:59 +0000
@@ -93,7 +93,7 @@
}
private void update_switcher_visibility() {
- if (switcher_hidden && switcher_box.visible) {
+ if (switcher_hidden) {
switcher_box.hide();
return;
}
If you could take a look at these issues, it would be very cool!
Thanks for all this work :)
--
https://code.launchpad.net/~victored/granite/fixes/+merge/88979
Your team elementaryart (old) is subscribed to branch lp:granite.
--
Mailing list: https://launchpad.net/~elementaryart
Post to : [email protected]
Unsubscribe : https://launchpad.net/~elementaryart
More help : https://help.launchpad.net/ListHelp
