Re: [Gnash-dev] [PATCH] Image icon in menu

[Hiroyuki Ikezoe]

Hello Bastiaan,

Thank you for your advices.

2006-11-09 (木) の 12:59 +0100 に Bastiaan Jacques さんは書きました:

> In general, I like your patch. You haven't added icons to all menu 
> items; can you find suitable icons for the missing ones too?

I have nerver been found other suitable icons. I hope someone create new
nice icons.

> Instead of defining each menu item twice, I think it would be easier to 
> read this way:
> 
>   GtkMenuItem *menuitem_play =
>     GTK_MENU_ITEM(gtk_menu_item_new_with_label("Play Movie"));
> ...
> 
> #if GTK_CHECK_VERSION(2,6,0)
>   GtkWidget *play_image =
>      GTK_WIDGET(gtk_image_new_from_stock(GTK_STOCK_REFRESH,
>                 GTK_ICON_SIZE_MENU));
>   gtk_image_menu_item_set_image (GTK_IMAGE_MENU_ITEM(menuitem_play),
>                                  play_image);
> ...
> #endif
> 
> So that's like your implementation for menuitem_restart, for all menu 
> items.

Actually, GTK+ can't to do such thing because GtkImageMenuItem is a sub
object of GtkMenuItem.

gtk_image_menu_new_from_stock() do gtk_image_menu_item_new_with_label()
and gtk_image_menu_set_image() with label and image owned by GTK+
itself, and moreover, the label is gettextized. That is very important
for non-English users like me.
The reason why I do not use gtk_image_menu_new_from_stock() for "Restart
Movie" menu is that the label of GTK_STOCK_REFRESH is "Refresh". It is
not suitable for I think.

> > +    // Menu for sound
> >      if (get_sound_handler()) {
> > +        GtkMenuItem *menuitem_sound =
> > +       GTK_MENU_ITEM(gtk_check_menu_item_new_with_label("Toggle
> > Sound")); gtk_menu_append(_popup_menu, GTK_WIDGET(menuitem_sound));
> > +   gtk_check_menu_item_set_active(GTK_CHECK_MENU_ITEM(menuitem_sound),
> > +                                  get_sound_handler()->is_muted() ?
> 
> You call get_sound_handler() twice; you should store the return value of 
> this function to a temporary pointer to use in your call to 
> gtk_check_menu_item_set_active().

Yes, you are right. It is redundant.
I've fixed it in a new patch attaching in this mail.

Thank you,
Hiroyuki Ikezoe

gnash_menu_with_image.diff
Description: Text Data