Review: Approve The only thing that I'd say is that for the bug, I think we need to handle bitmap items as well. That can be done with using dbusmenu_menuitem_property_set_image(). I'm not sure if we want all that in one merge request or not, I'll let you choose :)
review approve On Sat, 2010-07-03 at 13:31 +0000, Cody Russell wrote: > Cody Russell has proposed merging lp:~bratsche/appmenu-gtk/icons into > lp:appmenu-gtk. > > Requested reviews: > Canonical Desktop Experience Team (canonical-dx-team) > Related bugs: > #598528 doesn't display icons > https://bugs.launchpad.net/bugs/598528 > > differences between files attachment (review-diff.txt) > === modified file 'src/bridge.c' > --- src/bridge.c 2010-07-02 18:03:46 +0000 > +++ src/bridge.c 2010-07-03 13:31:27 +0000 > @@ -372,6 +372,93 @@ > } > } > > +static gboolean > +should_show_image (GtkImage *image) > +{ > + GtkWidget *item; > + > + item = gtk_widget_get_ancestor (GTK_WIDGET (image), > + GTK_TYPE_IMAGE_MENU_ITEM); > + > + if (item) > + { > + GtkSettings *settings; > + gboolean gtk_menu_images; > + > + settings = gtk_widget_get_settings (item); > + > + g_object_get (settings, "gtk-menu-images", >k_menu_images, NULL); > + > + if (gtk_menu_images) > + return TRUE; > + > + return gtk_image_menu_item_get_always_show_image (GTK_IMAGE_MENU_ITEM > (item)); > + } > + > + return FALSE; > +} > + > +static gboolean > +update_stock_item (DbusmenuMenuitem *menuitem, > + GtkWidget *widget) > +{ > + GtkStockItem stock; > + GtkImage *image; > + > + g_return_val_if_fail (GTK_IS_IMAGE (widget), FALSE); > + > + image = GTK_IMAGE (widget); > + > + if (gtk_image_get_storage_type (image) != GTK_IMAGE_STOCK) > + return FALSE; > + > + gtk_stock_lookup (image->data.stock.stock_id, &stock); > + > + if (should_show_image (image)) > + dbusmenu_menuitem_property_set (menuitem, > + DBUSMENU_MENUITEM_PROP_ICON_NAME, > + image->data.stock.stock_id); > + else > + dbusmenu_menuitem_property_remove (menuitem, > + DBUSMENU_MENUITEM_PROP_ICON_NAME); > + > + const gchar * label = dbusmenu_menuitem_property_get (menuitem, > + > DBUSMENU_MENUITEM_PROP_LABEL); > + > + if (stock.label != NULL && label != NULL) > + { > + dbusmenu_menuitem_property_set (menuitem, > + DBUSMENU_MENUITEM_PROP_LABEL, > + stock.label); > + > + return TRUE; > + } > + > + return FALSE; > +} > + > +static void > +update_icon_name (DbusmenuMenuitem *menuitem, > + GtkWidget *widget) > +{ > + GtkImage *image; > + > + g_return_if_fail (GTK_IS_IMAGE (widget)); > + > + image = GTK_IMAGE (widget); > + > + if (gtk_image_get_storage_type (image) != GTK_IMAGE_ICON_NAME) > + return; > + > + if (should_show_image (image)) > + dbusmenu_menuitem_property_set (menuitem, > + DBUSMENU_MENUITEM_PROP_ICON_NAME, > + image->data.name.icon_name); > + else > + dbusmenu_menuitem_property_remove (menuitem, > + DBUSMENU_MENUITEM_PROP_ICON_NAME); > +} > + > static void > widget_notify_cb (GtkWidget *widget, > GParamSpec *pspec, > @@ -397,6 +484,14 @@ > DBUSMENU_MENUITEM_PROP_VISIBLE, > gtk_widget_get_visible (widget)); > } > + else if (pspec->name == g_intern_static_string ("stock")) > + { > + update_stock_item (child, widget); > + } > + else if (pspec->name == g_intern_static_string ("icon-name")) > + { > + update_icon_name (child, widget); > + } > } > > static void > @@ -484,6 +579,7 @@ > else > { > gboolean visible = FALSE; > + gboolean label_set = FALSE; > > g_signal_connect (widget, > "accel-closures-changed", > @@ -506,6 +602,24 @@ > mi); > } > > + if (GTK_IS_IMAGE_MENU_ITEM (widget)) > + { > + GtkWidget *image; > + GtkImageType image_type; > + > + image = gtk_image_menu_item_get_image (GTK_IMAGE_MENU_ITEM > (widget)); > + image_type = gtk_image_get_storage_type (GTK_IMAGE (image)); > + > + if (image_type == GTK_IMAGE_STOCK) > + { > + label_set = update_stock_item (mi, image); > + } > + else if (image_type == GTK_IMAGE_ICON_NAME) > + { > + update_icon_name (mi, image); > + } > + } > + > dbusmenu_menuitem_property_set (mi, > "label", > get_menu_label_text (widget)); > -- https://code.launchpad.net/~bratsche/appmenu-gtk/icons/+merge/29147 Your team ayatana-commits is subscribed to branch lp:appmenu-gtk. _______________________________________________ Mailing list: https://launchpad.net/~ayatana-commits Post to : [email protected] Unsubscribe : https://launchpad.net/~ayatana-commits More help : https://help.launchpad.net/ListHelp
