Merge lp:~victored/wingpanel/lp-1380392 into lp:~elementary-pantheon/wingpanel/trunk-0.3.x

[Victor Martinez]

Update menu arrow placement after indicator geometry changes. Fixes lp:1380392

Update menu arrow placement after indicator geometry changes. Fixes lp:1380392

[PerfectCarl (name-is-carl)]

1) I tried the test mentioned in the bug report:
  - disable the network
  - click on the indicator
  - re-enable the network
  - click on the indicator

The arrow is not redrawn properly.

2) Also I tried with my ido branch (granted not production ready).
When ido update the main menu margins, the window is not moved properly and is displayed out of the screen.

https://code.launchpad.net/~elementary-apps/+junk/os-patch-ido-trusty
http://i.imgur.com/iE4JtNU.png

This is not related to the bug but I would love this to be fixed: it would fix the asymmetrical margins on indicator once and for all.

review: Needs Fixing

[PerfectCarl (name-is-carl)]

Is the revision used for testing 2)
http://bazaar.launchpad.net/~elementary-apps/+junk/os-patch-ido-trusty/revision/26

[Victor Martinez (victored)]

Thanks for the review Carl.

Addressing your concerns:

1) Without this branch, if you click on the network indicator, disable the network, and then re-enable it, the arrow is permanently pointing to the wrong indicator until wingpanel is restarted. This branch addresses that issue. When the menu shrinks the arrow is properly updated and points in the correct direction.

If by properly you mean that the arrow might be misaligned while the menu resizes, that is correct, and it's actually caused by (2). I'm not addressing that in this branch.

2) Wingpanel only controls the drawing of a popover background beneath a transparent menu background. The positioning of the menus is controlled by GtkMenuBar, and it's glitchy. Since there's no clean way to override that, we'd have to write our own menubar implementation and control Gtk.Menu.popup ourselves in order to override the default positioning. This bug is still reproduceable even if you removed all the popover-related modifications performed by the panel. I might implement that at some point, although this fix makes the situation more bearable.

=== modified file 'src/Indicator/IndicatorObjectEntry.vala'
--- src/Indicator/IndicatorObjectEntry.vala	2015-01-22 06:06:22 +0000
+++ src/Indicator/IndicatorObjectEntry.vala	2015-01-27 16:35:31 +0000
@@ -33,11 +33,13 @@
         private Granite.Drawing.BufferSurface buffer;
         private int w = -1;
         private int h = -1;
-        private int arrow_height = 10;
-        private int arrow_width = 20;
-        private double x = 10.5;
-        private double y = 10.5;
-        private int radius = 5;
+        private int icon_origin_x = -1;
+        private int menu_origin_x = -1;
+        private const int arrow_height = 10;
+        private const int arrow_width = 20;
+        private const double x = 10.5;
+        private const double y = 10.5;
+        private const int radius = 5;
 
         private const string MENU_STYLESHEET = """
             .menu {
@@ -158,12 +160,22 @@
         }
 
         private bool entry_menu_parent_draw_callback (Cairo.Context ctx) {
-            var new_w  = entry.menu.get_parent ().get_allocated_width ();
-            var new_h = entry.menu.get_parent ().get_allocated_height ();
-
-            if (new_w != w || new_h != h) {
-                w = new_w;
-                h = new_h;
+            Gtk.Allocation alloc;
+            entry.menu.get_parent ().get_allocation (out alloc);
+
+            // Get x coordinate where indicator icon starts in panel
+            int icon_x;
+            this.get_window ().get_origin (out icon_x, null);
+
+            // Get x coordinate where menu starts relative to screen area
+            int menu_x;
+            entry.menu.get_window ().get_origin (out menu_x, null);
+
+            if (alloc.width != w || alloc.height != h || icon_origin_x != icon_x || menu_origin_x != menu_x) {
+                w = alloc.width;
+                h = alloc.height;
+                icon_origin_x = icon_x;
+                menu_origin_x = menu_x;
 
                 buffer = new Granite.Drawing.BufferSurface (w, h);
                 cairo_popover (w, h);
@@ -200,34 +212,31 @@
             return false;
         }
 
-        private void cairo_popover (int w, int h) {
-            w = w - 20;
-            h = h - 20;
+        private void cairo_popover (int menu_width, int menu_height) {
+            menu_width -= 20;
+            menu_height -= 20;
+
+            Gtk.Allocation panel_icon_alloc;
+            get_allocation (out panel_icon_alloc);
 
             // Get some nice pos for the arrow
-            var offs = 30;
-            int p_x;
-            int w_x;
-            Gtk.Allocation alloc;
-            this.get_window ().get_origin (out p_x, null);
-            this.get_allocation (out alloc);
-
-            entry.menu.get_window ().get_origin (out w_x, null);
-
-            offs = (p_x + alloc.x) - w_x + this.get_allocated_width () / 4;
-            if (offs + 50 > (w + 20))
-                offs = (w + 20) - 15 - arrow_width;
-            if (offs < 17)
-                offs = 17;
+            int arrow_offset = icon_origin_x + panel_icon_alloc.x;
+            arrow_offset += panel_icon_alloc.width / 4 - menu_origin_x;
+
+            if (arrow_offset + 50 > menu_width + 20)
+                arrow_offset = menu_width + 20 - 15 - arrow_width;
+
+            if (arrow_offset < 17)
+                arrow_offset = 17;
 
             buffer.context.arc (x + radius, y + arrow_height + radius, radius, Math.PI, Math.PI * 1.5);
-            buffer.context.line_to (offs, y + arrow_height);
+            buffer.context.line_to (arrow_offset, y + arrow_height);
             buffer.context.rel_line_to (arrow_width / 2.0, -arrow_height);
             buffer.context.rel_line_to (arrow_width / 2.0, arrow_height);
-            buffer.context.arc (x + w - radius, y + arrow_height + radius, radius, Math.PI * 1.5, Math.PI * 2.0);
+            buffer.context.arc (x + menu_width - radius, y + arrow_height + radius, radius, Math.PI * 1.5, Math.PI * 2.0);
 
-            buffer.context.arc (x + w - radius, y + h - radius, radius, 0, Math.PI * 0.5);
-            buffer.context.arc (x + radius, y + h - radius, radius, Math.PI * 0.5, Math.PI);
+            buffer.context.arc (x + menu_width - radius, y + menu_height - radius, radius, 0, Math.PI * 0.5);
+            buffer.context.arc (x + radius, y + menu_height - radius, radius, Math.PI * 0.5, Math.PI);
 
             buffer.context.close_path ();
         }