Re: Submenu arrow patch (Was: Re: Submenu navigation status?)

[Owen Taylor <otaylor redhat com>]

Nils Barth <nils_barth post harvard edu> writes:

> mini-patches/fragments, a remark, and questions:
> 
> * There is an i18n problem with GTK in that submenus default to
>   cascading right, which is wrong in Arabic/Hebrew, so the default
>   direction should be somehow configurable. The patch basically replaces
>   GTK_DIRECTION_RIGHT in necessary places (two) with a macro
>   GTK_DIRECTION_DEFAULT (which is now GTK_DIRECTION_RIGHT).
> 
>   This doesn't much help/solve the problem, but at least localizes it
>   (no pun intended).

Well, actually the support functions for solving the problem are
already there - gtk_widget_get_direction(GtkWidget *widget) will
return either GTK_TEXT_DIR_LTR or GTK_TEXT_DIR_RTL, which you
can use

In order to complete the solution, you need to:

 a) make menubars lay out in the opposite direction 
 b) reverse the side on which the arrow appears and the 
    also the indicator location for GtkChech/RadioMenuItem
     
I held off a bit on doing this for GtkMenu because it wasn't 100%
clear that all speaker's of RTL languages would want their menus
reversed, but I don't think it hurts to add the capability. 
We can always add the option to turn it off for certain widgets
if that turns out to be necessary.
 
Note that applications can set whether particular widgets reverse
or not with gtk_widget_set_direction() [ not this makes a 
lot of sense for menus, but it is important elsewhere ]. So
a macro GTK_DIRECTION_DEFAULT doesn't really help.  

> * The fragment is a tiny function for converting SubmenuDirection's to
>   Arrow directions. I didn't know where to put it -- static/private to
>   gtkmenuitem.c?
> (this is necessary so that one can change, in
> gtkmenuitem.c:467
> GTK_ARROW_RIGHT,
> to
> gtk_submenu_direction_to_arrow (menu_item->submenu_direction),
> so the arrow points the right way;
> however, it could conceivably be a useful lil' function elsewhere)

I would keep it static unless you have a good example of it
being useful elsewhere.
 
> * GTK_DIRECTION_[LEFT,RIGHT] are kinda confusing names for -Submenu-
>   directions; that is, they are confusingly similar to
>   GTK_DIR_[LEFT,RIGHT]. Shouldn't they be renamed something like
>   GTK_SUBMENU_DIR_[LEFT,RIGHT]?
 
I tend to agree its poor idea to have similarly named enumerations
with different values (see GTK_TOPLEVEL, GTK_WINDOW_TOPLEVEL.) 

In normal circumstances, I'd be loath to change an enumeration
just because it is poorly named; but I think this enumeration 
probably was never used in any app, so it may be a good idea.
(It never really appeared in a public API) 

http://cvs.gnome.org/lxr/ bears this out.

If you do change this in your patch, please include the appropriate
diff to docs/Changes-2.0.txt.

[...]

> > > Yup -- I submitted that one before noticing the torn-off menus
> > > problem. Could you elaborate on the expose event strangeness? If you
> > > cover/expose it, it would point in the wrong direction?
> > > 
> > > Now that submenu nav is taken care of, I'll fix this up.
> > > I guess I need to include a re-calculation of the direction for when
> > > a torn-off menu is moved? Or every expose event?
> > > We'll see....
> > 
> > The problem was that the direction _was_ being calculated for every
> > expose event, so if you tore off the menu, moved it so
> > the arrows would pop up in the opposite direction, then did
> > a partial expose of the window, the exposed portions would be
> > drawn pointing one way, the unexposed portions pointing the
> > other way. (Expose half the arrow, get an hourglass shape)
> > 
> > The two solutions I can think of are:
> > 
> >  - Do the calculation when the window is moved. (You can do
> >    this by selecting on GDK_EVENT_CONFIGURE on the toplevel
> >    window), and queue a redraw when the direction changes.
> > 
> >  - When calculating the direction during the expose event,
> >    note if it changed from the previous time, and if so
> >    queue a redraw on the entire menu.
> > 
> > I'm not sure offhand which one is better.
> 
> Okay -- we need to calculate the arrow orientation (call
> gtk_menu_item_position_menu for each submenu) at these times:
> * The menu is first displayed (er, is this called ``mapped'', i.e.,
>   the GDK_MAP event?)

Note that there is a distinction between:

 The ::map signal. This signal corresponds to the virtual function
 that GTK+ uses to show a widget, and not necessarily  

 The GDK_MAP / ::map_event, which is an event received when the
 window appears on the screen.

the ::map_event signal is probably more what you want, since at
the time of ::map, the final position for a torn off window will
not have been determined.

Actually, for the tearoff, you should get both a ::configure_event
and a ::map_event, so I think you don't need to connect to
::map_event if you also connect to ::configure_event. 

For a non-torn off menu, the position is completely determined
by GTK+, so you should be able to do the computation in
the gtk_menu_popup function. Make sure you get the case
right when both the torn off menu and the menu in the
original location are showing. 


(I'll simply point you at the source for the nasty tricks that I
played to get that to work.)


> * The menu is moved (the top window gets a GDK_CONFIGURE event)
> 
> These are the only times the orientation can change (and you need to
> know it at the begining), so they should be the only times the
> computation is done; then if the orientation changes, we need to queue a
> redraw.
> 
> How do I do this??? That is:
>  - how do I ``select'' on a GDK event so a function is called, and

Connect to to the ::configure_event signal. That event even
contains the coordinates of the new upper left, so there
may be some slight efficiency gain from using that, though I 
think the expensive operation is figuring out the size of the
child menus.

>  - how do I queue a redraw to the window?
>    (do I GDK put an expose event for the whole menu item?)

There are multiple ways that are approximately equivalent. 
The old gtk_widget_queue_clear() still works, and would be
fine, but its probably a little more forward-looking to use:

 gdk_window_invalidate_rect (widget->window, NULL, TRUE);
 
> references to code are okay (see how it's done here), but I couldn't
> figure out just from API docs, and didn't want to do it incorrectly.
> 
> > I'd guess maybe
> > the first might look a little less odd, though both will
> > seem a little odd to the user. 
> 
> ? I don't follow. In both cases (and any correct case), when the menu
> is moved so that the submenu will pop up in the opposite direction,
> the arrow is redrawn pointing the opposite way, right ?
> I guess that's a little weird, but should only be surprising the first
> time...

I was just saying that it is a little suprising. Not saying
it isn't correct. 

(Though actually, I'm not sure if other operating systems actually
implement switching the direction of the arrow - the arrow could be
seen as an icon showing that a submenu is going to pop up rather
than an indicator of where.)  

Regards,
                                        Owen