GNOME Bugzilla – Bug 705918
Add gtk_button_new_from_icon_name
Last modified: 2013-08-14 07:54:20 UTC
In many applications in GNOME, need to create button with icon name. For this, always need to create button, create image ans call to gtk_button_set_image with the image. This convenience function for this.
Created attachment 251512 [details] [review] Add gtk_button_new_from_icon_name
Review of attachment 251512 [details] [review]: thanks for the patch! naming issue that I want to raise: should it be new_with_icon_name() or new_from_icon_name()? ::: gtk/gtkbutton.c @@ +1307,3 @@ + * displayed instead. If the current icon theme is changed, the icon + * will be updated appropriately. + * an additional note like this would be helpful: This function is a convenience wrapper around gtk_button_new() and gtk_button_set_image(). @@ +1309,3 @@ + * + * Returns: a new #GtkButton displaying the themed icon + **/ missing "Since: 3.10" annotation
Review of attachment 251512 [details] [review]: Not sure this is entirely right. In other places where image-only buttons are constructed, we do gtk_style_context_add_class (context, "image-button"); The documentation should be clear about the fact that this api is meant to construct image-only buttons. ::: gtk/gtkbutton.c @@ +1309,3 @@ + * + * Returns: a new #GtkButton displaying the themed icon + **/ Since: 3.10
(In reply to comment #2) > Review of attachment 251512 [details] [review]: > > thanks for the patch! > > naming issue that I want to raise: should it be new_with_icon_name() or > new_from_icon_name()? I think should it be new_from_icon_name () for consistency with gtk_image_new_from_icon_name (). > > ::: gtk/gtkbutton.c > @@ +1307,3 @@ > + * displayed instead. If the current icon theme is changed, the icon > + * will be updated appropriately. > + * > > an additional note like this would be helpful: > > This function is a convenience wrapper around gtk_button_new() and > gtk_button_set_image(). > OK. > @@ +1309,3 @@ > + * > + * Returns: a new #GtkButton displaying the themed icon > + **/ > > missing "Since: 3.10" annotation OK.
(In reply to comment #3) > Review of attachment 251512 [details] [review]: > > Not sure this is entirely right. > > In other places where image-only buttons are constructed, we do > gtk_style_context_add_class (context, "image-button"); > > The documentation should be clear about the fact that this api is meant to > construct image-only buttons. > So should also to add gtk_widget_set_valign (button, GTK_ALIGN_CENTER), no? > ::: gtk/gtkbutton.c > @@ +1309,3 @@ > + * > + * Returns: a new #GtkButton displaying the themed icon > + **/ > > Since: 3.10 OK.
(In reply to comment #5) > > So should also to add gtk_widget_set_valign (button, GTK_ALIGN_CENTER), no? > That would align the button inside its parent. I think GtkButton already takes care of centering the image if there is no label.
(In reply to comment #6) > (In reply to comment #5) > > > > So should also to add gtk_widget_set_valign (button, GTK_ALIGN_CENTER), no? > > > > That would align the button inside its parent. I think GtkButton already takes > care of centering the image if there is no label. Not right. I fixed ot the look in some applications of GNOME, and always had need gtk_widget_set_valign. See for example the program an attached.
Created attachment 251530 [details] example of valgtk_widget_set_valign with and without GTK_ALIGN_CENTER
Created attachment 251531 [details] [review] Add gtk_button_new_from_icon_name
Review of attachment 251531 [details] [review]: ::: gtk/gtkbutton.c @@ +1303,3 @@ + * @size: (type int): an icon size + * + * Creates a new #GtkButton containing the an icon from the current icon theme. "the an" @@ +1310,3 @@ + * + * This function is a convenience wrapper around gtk_button_new () and + * gtk_button_set_image (). No space between function name and () here, to make linkification work. @@ +1327,3 @@ + button = g_object_new (GTK_TYPE_BUTTON, + "image", image, + "valign", GTK_ALIGN_CENTER, The valign doesn't belong here and needs to go. @@ +1348,3 @@ * Returns: a new #GtkButton * + * Deprecated: 3.10: Use gtk_button_new_from_icon_name() instead. Depending on stock id, new_from_stock may create a button with icon, with label, or both. In most cases, the label is wanted, so the deprecation notice should stay unchanged, or point to both functions.
Created attachment 251549 [details] [review] Add gtk_button_new_from_icon_name
Review of attachment 251549 [details] [review]: Looks ok, otherwise. Please commit with that correction ::: gtk/gtkbutton.h @@ +81,3 @@ GDK_AVAILABLE_IN_ALL GtkWidget* gtk_button_new_with_label (const gchar *label); +GDK_DEPRECATED_IN_3_10 You mean GDK_AVAILABLE_IN_3_10 here, I assume
Created attachment 251573 [details] [review] Add gtk_button_new_from_icon_name
Review of attachment 251573 [details] [review]: pushed as 2f77a61e61d6eb1d99f74b262c63059726a2ec08 - 'Add gtk_button_new_from_icon_name' and fix a typo in 89f701ab2ef2cfd6f6db15147006415c9743cefa - 'oopx - fix typo' (would need to be 'oops'...).