Re: GtkImageMenutItem
- From: Havoc Pennington <hp redhat com>
- To: Tim Janik <timj gtk org>
- Cc: Gtk+ Developers <gtk-devel-list gnome org>
- Subject: Re: GtkImageMenutItem
- Date: 14 Feb 2001 20:49:25 -0500
Tim Janik <timj gtk org> writes:
> here are a couple comments regarding the newly added
> image menuitem. first, i think the name is a bit odd
> as that menu item can hold any kind of extra widget,
> not only an image.
Yes, but GtkWidgetMenuItem is blatantly a bad name (overlaps with
GtkWidget namespace if nothing else), and it will always contain an
image in practice. I'm open to better ideas.
> +static void
> +gtk_image_menu_item_destroy (GtkObject *object)
> +{
> + GtkImageMenuItem *image_menu_item;
> +
> + image_menu_item = GTK_IMAGE_MENU_ITEM (object);
> +
> + /* If you change forall to treat the image widget as
> + * an internal, then you have to destroy the image widget
> + * here.
> + */
> +
> + (* GTK_OBJECT_CLASS (parent_class)->destroy) (object);
> +}
>
> that comment is wrong. currently the image is treated as
> a non-internal widget (which is the right thing to do as
> it's supplied by the user):
>
The comment is correct, though perhaps confusing. The point of the
comment is that the GtkContainer default implementation of destroy
(which we chain to) destroys all non-internal widgets. So we don't
need to do anything in destroy. But if you changed the image widget to
be internal, you would need to destroy it in destroy. I thought people
might suggest this change, so I put the comment as a reminder.
Basically this is an empty destroy, which always annoys Owen too, I
should take it out. ;-) But it's correct for it to be empty.
> but since it's not internal, it has to be destroyed
> in ::destroy.
It is, in GtkContainer::destroy which I chain up to.
> this shut be menu_item_set_image() since the menu item
> cannot handle multiple images. it should also handle NULL
> to take over the current remove functionality.
You just call gtk_container_remove() to remove. I thought this way
would be more consistent with e.g. gtk_box_pack_start(), which does
not accept NULL. I had set_image() originally.
> this is _way_ hackish. first, we don't overload public API in
> derived classes (here, gtk_container_remove),
I'm not overloading it, I'm overriding it, look again. It does appear
I left off the "static," oops. Anyway, gtk_container_remove() should
work on the image widget, that is the purpose of this override.
> second, it breaks
> pair-ness of gtk_container_add/gtk_container_remove.
> get rid of this and use just gtk_image_menu_item_set_image().
I don't understand what you mean by "breaks pair-ness".
> +struct _GtkImageMenuItem
> +{
> + GtkMenuItem menu_item;
> +
> + /*< private >*/
> + GtkWidget *image;
> +};
>
> what's the point in /*<private>*/ here? there's no reason
> to make the image widget anymore private that say GtkBin.child.
>
*shrug* no strong feelings. There's a get_image() accessor, so there's
no reason to make it public, private is my default setting.
Havoc
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]