elementaryart team mailing list archive

[xapantu <xapantu@xxxxxxxxx>]

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.