GtkImageMenutItem

[Tim Janik <timj gtk org>]

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.

+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):

+static void
+gtk_image_menu_item_forall (GtkContainer   *container,
+                            gboolean        include_internals,
+                            GtkCallback     callback,
+                            gpointer        callback_data)
+{
+  GtkImageMenuItem *image_menu_item;
+
+  g_return_if_fail (GTK_IS_IMAGE_MENU_ITEM (container));
+
+  image_menu_item = GTK_IMAGE_MENU_ITEM (container);
+  
+  (* GTK_CONTAINER_CLASS (parent_class)->forall) (container,
+                                                  include_internals,
+                                                  callback,
+                                                  callback_data);
+
+  if (image_menu_item->image)
+    (* callback) (image_menu_item->image, callback_data);
+}


but since it's not internal, it has to be destroyed
in ::destroy.

+void
+gtk_image_menu_item_add_image (GtkImageMenuItem *image_menu_item,
+                               GtkWidget        *child)

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.

+void
+gtk_image_menu_item_remove (GtkContainer *container,
+                            GtkWidget    *child)
+{
+  GtkImageMenuItem *image_menu_item;
+
+  image_menu_item = GTK_IMAGE_MENU_ITEM (container);
+
+  if (child == image_menu_item->image)
+    {
+      gboolean widget_was_visible;
+      
+      widget_was_visible = GTK_WIDGET_VISIBLE (child);
+      
+      gtk_widget_unparent (child);
+      image_menu_item->image = NULL;
+      
+      /* queue resize regardless of GTK_WIDGET_VISIBLE (container),
+       * since that's what is needed by toplevels, which derive from GtkBin.
+       */
+      if (widget_was_visible)
+        gtk_widget_queue_resize (GTK_WIDGET (container));
+    }
+  else
+    {
+      (* GTK_CONTAINER_CLASS (parent_class)->remove) (container, child);
+    }
+}

this is _way_ hackish. first, we don't overload public API in
derived classes (here, gtk_container_remove), 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().

+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.


---
ciaoTJ