Re: another patch for gtkoptionmenu

[Tim Janik <timj gtk org>]

On Tue, 25 Jan 2000, George wrote:

> Problem:  There is no signal to catch changes occuring to an option menu.
> The only way to do it currently is to catch 'activate' signal, and if by any
> chance you do gtk_option_menu_set_history you need to properly update the UI
> as well.
> 
> So this patch adds a 'history_set' signal which is emitted when the current
> item changes to something else.  This makes it much easier to for example
> update the UI on certain dialogs or to catch the changes.

hum, i think i'd prefer "history_changed" as a signal name here.
why did you create the signal as a GTK_RUN_FIRST signal?
signals usually should by default be GTK_RUN_LAST with some very few
exceptions where internal widget state maintenance is critical.

> @@ -548,8 +564,17 @@
>    if (option_menu->menu)
>      {
>        gtk_option_menu_remove_contents (option_menu);
> +
> +      active_item = gtk_menu_get_active (GTK_MENU (option_menu->menu));
> +
> +      /* if a new item was selected emit 'history_set' */
> +      if (option_menu->menu_item != active_item)
> +        gtk_signal_emit (GTK_OBJECT (option_menu),
> +                      option_menu_signals[HISTORY_SET],
> +                      gtk_option_menu_get_history (option_menu));
> +
> +      option_menu->menu_item = active_item;
>
> -      option_menu->menu_item = gtk_menu_get_active (GTK_MENU (option_menu->menu));
>        if (option_menu->menu_item)
>       {
>         gtk_widget_ref (option_menu->menu_item);

you should do option_menu->menu_item = active_item; *before* emiting the
signal, so the option_menu staructure has consistent state when
the signal handlers get called (this is an important point that many people
seem to miss when emitting signals).
also the history index (the second arg of the signal hanlders) is somewhat
superfluous since it can be queried from the option_menu with
gtk_option_menu_get_history(), there reason we have such an argument for
other notify signals such as state_changed or parent_set, is to preserve
the old value where user code needs it, since that can not simply be queried
from the option_menu. so i'd suggest you change the index field to
old_history.

other than that, i really like the idea of the notification signal ;)

> 
> George
> 

---
ciaoTJ