Re: Submenu navigation status?

[Nils Barth <nils_barth post harvard edu>]

Final submenu hysteresis patch! (hopefully)
Attached find a slightly polished version of the already buffed patch.
This gives GTK proper submenu navigation, and solves one TODO list
item.
If no-one has any objections to this, could a maintainer apply the
patch? Or I could upload to the (backlogged?) FTP?

Details on implementation/issues (all resolved/addressed, I hope).

Thus spake Nils Barth:
> Thus spake Mrcooger:
> > Howdy folks. Back in June Nils Barth wrote a patch for the menu code that
> > implemented improved submenu navigation. In early July, I made some
> > modifications according to the email exchanged between Nils and Owen, and
> > submitted a patch. I never got any email about it and I haven't seen any
> > discussion of this feature on the list since then, and so I am wondering
> > if this feature is no longer intended for this version of GTK+? 
> 
> Hi -- just got back and unpacked from travels (more details at
> www.advogato.org, username inri ; sorry for cluttering this list with
> pseudo-diary entries).
> 
> Thanks for the changes/cleanup!
> 
> IIRC, one of the key problems with the patch is that instead of
> grabbing the pointer via gtk_grab_add () when it left a menuitem
> w/active submenu, it uses a kludge of trying to intercept signals at
> other menu items, and this doesn't work reliably (Owen got this to
> fail).
> 
> Thus, we should try to get this working with gtk_grab_add () if
> possible, and once that is done (or proves to be ugly/impossible),
> post/submit a finalized copy.

Okay, I figured out why gtk_grab_add wasn't the right approach:
Basically, the navigation code belongs in gtk+/gtk/gtkmenu.c, since it
deals with GtkMenu's, not MenuShell's or MenuItem's or anything else.
When I suggested using gtk_grab_add (), I had been thinking that the
menu_item should grab the focus, then release it on the proper
motion_event. In retrospect, this is silly and complicates matters.
What we have now is proper: gtkmenu.c creates, destroy, and uses the
navigation region (the triangular region that allows easier
navigation), and nothing else need know of it; this only involves
checking enter/leave/motion/key-pressed events, and 3 of these are
very minimal code (~3 lines).

So gtk_grab_add () was not the right way of going at this (sorry
for the blind alley).

> Are there any other issues to address?

There was some question as to whether the nav_region should be a
static (to gtkmenu.c) or part of the GtkMenu struct.
The nav_region is currently a static, basically to be shared between
a few functions that create/check/destroy it. It's basically a temp
variable, which begs for static. It also has no place in GtkMenu, nor
need to be there, since only one nav_region exists at once. If we had
multiple pointers accessing multiple menus in a given GTK app at the
same time...we'd want to make nav_region part of GtkMenu, but short of
that (radical) development, I see no need nor desire to change it from
being a static.

There was some discussion (between David and Nils Philippsen) about
making the hysteresis timeout be user-definable, which could be
done in an ad-hoc way (patches welcome, I'm sure), but...I agree with
David that it's better to work on a general gtk config system, rather
than causing API bloat with 200 functions of the form:
gtk_..._[get,set]_minor_feature


In the discussion (months ago) there were also some other issues about menu
navigation, but these were basically small, misc. topics. I have a
patch that addresses some of them, but they are mostly of the form:
``I want to customize -this- minor point,'' and while it is easy to
change the code to add the required flexibility, it is harder to have
run-time configuration, and that awaits a general config framework for
GTK, which is a much bigger undertaking (as mentioned above).

So:
I've poured over the code, compiled it, tested it (testgtk->menus),
and it works beautifully and looks rather nice too (I think...at least
now it looks like gtk code, instead of grandchild-of-K&R ;-).
It's vaguely unpleasent to go back to the 1.2 GFoot menu after playing
with the new navigating in testgtk.

If anyone sees anything wrong with the code, please say so/fix it,
else can it be commited?

Thanks all for patience/comments, and thanks for the coding David!

-- 
  -nils
Public key: http://www.fas.harvard.edu/~nbarth/pub-key.txt
Index: gtkmenu.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkmenu.c,v
retrieving revision 1.47
diff -u -r1.47 gtkmenu.c
--- gtkmenu.c	2000/07/26 11:32:45	1.47
+++ gtkmenu.c	2000/09/01 08:15:59
@@ -34,10 +34,12 @@
 #include "gtksignal.h"
 #include "gtkwindow.h"
 
-
 #define MENU_ITEM_CLASS(w)   GTK_MENU_ITEM_GET_CLASS (w)
 #define	MENU_NEEDS_RESIZE(m) GTK_MENU_SHELL (m)->menu_flag
 
+#define SUBMENU_NAV_REGION_PADDING 2
+#define SUBMENU_NAV_HYSTERESIS_TIMEOUT 333
+
 typedef struct _GtkMenuAttachData	GtkMenuAttachData;
 
 struct _GtkMenuAttachData
@@ -47,23 +49,33 @@
 };
 
 
-static void gtk_menu_class_init	    (GtkMenuClass      *klass);
-static void gtk_menu_init	    (GtkMenu	       *menu);
-static void gtk_menu_destroy	    (GtkObject	       *object);
-static void gtk_menu_realize	    (GtkWidget	       *widget);
-static void gtk_menu_size_request   (GtkWidget	       *widget,
-				     GtkRequisition    *requisition);
-static void gtk_menu_size_allocate  (GtkWidget	       *widget,
-				     GtkAllocation     *allocation);
-static void gtk_menu_paint	    (GtkWidget	       *widget);
-static void gtk_menu_draw	    (GtkWidget	       *widget,
-				     GdkRectangle      *area);
-static gint gtk_menu_expose	    (GtkWidget	       *widget,
-				     GdkEventExpose    *event);
-static gint gtk_menu_key_press	    (GtkWidget	       *widget,
-				     GdkEventKey       *event);
-static gint gtk_menu_motion_notify  (GtkWidget	       *widget,
-				     GdkEventMotion    *event);
+static void	gtk_menu_class_init    (GtkMenuClass	  *klass);
+static void	gtk_menu_init	       (GtkMenu		  *menu);
+static void	gtk_menu_destroy       (GtkObject	  *object);
+static void	gtk_menu_realize       (GtkWidget	  *widget);
+static void	gtk_menu_size_request  (GtkWidget	  *widget,
+					GtkRequisition    *requisition);
+static void	gtk_menu_size_allocate (GtkWidget	  *widget,
+					GtkAllocation     *allocation);
+static void	gtk_menu_paint	       (GtkWidget	  *widget);
+static void	gtk_menu_draw	       (GtkWidget	  *widget,
+					GdkRectangle      *area);
+static gboolean gtk_menu_expose	       (GtkWidget	  *widget,
+					GdkEventExpose    *event);
+static gboolean gtk_menu_key_press     (GtkWidget	  *widget,
+					GdkEventKey       *event);
+static gboolean gtk_menu_motion_notify (GtkWidget	  *widget,
+					GdkEventMotion    *event);
+static gboolean gtk_menu_enter_notify  (GtkWidget         *widget,
+					GdkEventCrossing  *event); 
+static gboolean gtk_menu_leave_notify  (GtkWidget         *widget,
+					GdkEventCrossing  *event);
+static void	gtk_menu_stop_navigating_submenu    (void);
+static gboolean gtk_menu_stop_navigating_submenu_cb (gpointer user_data); 
+static gboolean gtk_menu_navigating_submenu	    (gint event_x,
+    						     gint event_y);
+static void gtk_menu_set_submenu_navigation_region (GtkMenuItem     *menu_item, 
+						    GdkEventCrossing *event); 
 static void gtk_menu_deactivate	    (GtkMenuShell      *menu_shell);
 static void gtk_menu_show_all       (GtkWidget         *widget);
 static void gtk_menu_hide_all       (GtkWidget         *widget);
@@ -75,8 +87,8 @@
 static GtkMenuShellClass *parent_class = NULL;
 static const gchar	 *attach_data_key = "gtk-menu-attach-data";
 static GQuark             quark_uline_accel_group = 0;
+static GdkRegion         *navigation_region = NULL; 
 
-
 GtkType
 gtk_menu_get_type (void)
 {
@@ -129,6 +141,8 @@
   widget_class->motion_notify_event = gtk_menu_motion_notify;
   widget_class->show_all = gtk_menu_show_all;
   widget_class->hide_all = gtk_menu_hide_all;
+  widget_class->enter_notify_event = gtk_menu_enter_notify;
+  widget_class->leave_notify_event = gtk_menu_leave_notify;
   
   menu_shell_class->submenu_placement = GTK_LEFT_RIGHT;
   menu_shell_class->deactivate = gtk_menu_deactivate;
@@ -156,7 +170,7 @@
 				GTK_MENU_DIR_CHILD);
 }
 
-static gint
+static gboolean
 gtk_menu_window_event (GtkWidget *window,
 		       GdkEvent  *event,
 		       GtkWidget *menu)
@@ -978,7 +992,7 @@
     }
 }
 
-static gint
+static gboolean
 gtk_menu_expose (GtkWidget	*widget,
 		 GdkEventExpose *event)
 {
@@ -1016,7 +1030,7 @@
   return FALSE;
 }
 
-static gint
+static gboolean
 gtk_menu_key_press (GtkWidget	*widget,
 		    GdkEventKey *event)
 {
@@ -1029,6 +1043,8 @@
       
   menu_shell = GTK_MENU_SHELL (widget);
 
+  gtk_menu_stop_navigating_submenu (); 
+
   if (GTK_WIDGET_CLASS (parent_class)->key_press_event (widget, event))
     return TRUE;
 
@@ -1102,15 +1118,22 @@
   return TRUE;
 }
 
-static gint 
+static gboolean
 gtk_menu_motion_notify  (GtkWidget	   *widget,
 			 GdkEventMotion    *event)
 {
+  GtkMenuShell *menu_shell; 
+
   g_return_val_if_fail (widget != NULL, FALSE);
   g_return_val_if_fail (GTK_IS_MENU (widget), FALSE);
+  
+  if (gtk_menu_navigating_submenu (event->x_root, event->y_root))
+    return TRUE; 
+
+  menu_shell = GTK_MENU_SHELL (widget); 
   
-  if (GTK_MENU_SHELL (widget)->ignore_enter)
-    GTK_MENU_SHELL (widget)->ignore_enter = FALSE;
+  if (menu_shell->ignore_enter)
+    menu_shell->ignore_enter = FALSE; 
   else 
     {
       gint width, height;
@@ -1133,7 +1156,163 @@
   return FALSE;
 }
 
+static gboolean
+gtk_menu_enter_notify (GtkWidget        *widget,
+		       GdkEventCrossing *event)
+{
+  if (gtk_menu_navigating_submenu (event->x_root, event->y_root))
+	return TRUE; 
+
+  return GTK_WIDGET_CLASS (parent_class)->enter_notify_event (widget, event); 
+}
+
+static gboolean
+gtk_menu_leave_notify (GtkWidget        *widget,
+		       GdkEventCrossing *event)
+{
+  GtkMenuShell *menu_shell;
+  GtkMenuItem *menu_item;
+  GtkWidget *event_widget; 
+
+  if (gtk_menu_navigating_submenu (event->x_root, event->y_root))
+    return TRUE; 
+  
+  menu_shell = GTK_MENU_SHELL (widget); 
+  event_widget = gtk_get_event_widget ((GdkEvent*) event); 
+  if (!event_widget || !GTK_IS_MENU_ITEM (event_widget))
+    return TRUE;
+  menu_item = GTK_MENU_ITEM (event_widget); 
+  
+  /* Here we check to see if we're leaving an active menu item with a submenu, 
+   * in which case we enter submenu navigation mode. 
+   */
+  if (menu_shell->active_menu_item != NULL
+      && menu_item->submenu != NULL
+      && menu_item->submenu_placement == GTK_LEFT_RIGHT)
+    {
+      if (menu_item->submenu->window != NULL) 
+	{
+	  gtk_menu_set_submenu_navigation_region (menu_item, event);
+	  return TRUE;
+	}
+    }
+  
+  return GTK_WIDGET_CLASS (parent_class)->leave_notify_event (widget, event); 
+}
+
+static void 
+gtk_menu_stop_navigating_submenu (void)
+{
+  if (navigation_region) 
+    {
+      gdk_region_destroy (navigation_region);
+      navigation_region = NULL;
+    }
+}
+
+/* When the timeout is elapsed, the navigation region is destroyed
+ * and the menuitem under the pointer (if any) is selected.
+ */
+static gboolean
+gtk_menu_stop_navigating_submenu_cb (gpointer user_data)
+{
+  GdkEvent send_event; 
+
+  gtk_menu_stop_navigating_submenu (); 
+
+  send_event.crossing.window = gdk_window_at_pointer (NULL, NULL);
+  send_event.crossing.type = GDK_ENTER_NOTIFY;
+  send_event.crossing.time = GDK_CURRENT_TIME;
+  send_event.crossing.send_event = TRUE;
+
+  gtk_widget_event (GTK_WIDGET (user_data), &send_event);
+
+  return FALSE; 
+}
+
+static gboolean
+gtk_menu_navigating_submenu (gint event_x, gint event_y)
+{
+  if (navigation_region)
+    {
+      if (gdk_region_point_in (navigation_region, event_x, event_y))
+	return TRUE;
+      else
+	{
+	  gtk_menu_stop_navigating_submenu ();
+	  return FALSE;
+	}
+    }
+  return FALSE;
+}
+
 static void
+gtk_menu_set_submenu_navigation_region (GtkMenuItem      *menu_item,
+					GdkEventCrossing *event)
+{
+  gint submenu_left = 0;
+  gint submenu_right = 0;
+  gint submenu_top = 0;
+  gint submenu_bottom = 0;
+  gint width = 0;
+  gint height = 0;
+  GdkPoint point[3];
+  GtkWidget *event_widget;
+
+  g_return_if_fail (menu_item->submenu != NULL);
+  g_return_if_fail (event != NULL);
+  
+  event_widget = gtk_get_event_widget ((GdkEvent*) event);
+  
+  gdk_window_get_root_origin (menu_item->submenu->window,
+      &submenu_left, &submenu_top);
+  gdk_window_get_size (menu_item->submenu->window, &width, &height);
+  submenu_right = submenu_left + width;
+  submenu_bottom = submenu_top + height;
+  gdk_window_get_size (event_widget->window, &width, &height);
+  
+  if ((event->x >= 0) && (event->x <= width)
+      && (((event->y < 0) && (event->y_root >= submenu_top))
+	  || ((event->y >= 0) && (event->y_root <= submenu_bottom))))
+    {
+      /* Set navigation region */
+      /* We fudge/give a little padding in case the user
+       * ``misses the vertex'' of the triangle/is off by a pixel or two.
+       */ 
+      if (menu_item->submenu_direction == GTK_DIRECTION_RIGHT)
+	point[0].x = event->x_root - SUBMENU_NAV_REGION_PADDING; 
+      else                             
+	point[0].x = event->x_root + SUBMENU_NAV_REGION_PADDING;  
+      
+      /* Exiting the top or bottom? */ 
+      if (event->y < 0)
+        { /* top */
+	  point[0].y = event->y_root + SUBMENU_NAV_REGION_PADDING;
+	  point[1].y = submenu_top;
+	}
+      else
+        { /* bottom */
+	  point[0].y = event->y_root - SUBMENU_NAV_REGION_PADDING; 
+	  point[1].y = submenu_bottom;
+	}
+      
+      /* Submenu is to the left or right? */ 
+      if (menu_item->submenu_direction == GTK_DIRECTION_RIGHT)
+	point[1].x = submenu_left;  /* right */
+      else
+	point[1].x = submenu_right; /* left */
+      
+      point[2].x = point[1].x;
+      point[2].y = point[0].y;
+      
+      navigation_region = gdk_region_polygon (point, 3, GDK_WINDING_RULE);
+
+      gtk_timeout_add (SUBMENU_NAV_HYSTERESIS_TIMEOUT, 
+		       gtk_menu_stop_navigating_submenu_cb, menu_item); 
+    }
+}
+
+static void
 gtk_menu_deactivate (GtkMenuShell *menu_shell)
 {
   GtkWidget *parent;
@@ -1149,7 +1328,6 @@
   if (parent)
     gtk_menu_shell_deactivate (GTK_MENU_SHELL (parent));
 }
-
 
 static void
 gtk_menu_position (GtkMenu *menu)