Together with that I wonder if it makes sense to have a stock icon for it too. I drew one for buzztard and I am totally unhappy with it.
http://buzztard.git.sourceforge.net/git/gitweb.cgi?p=buzztard/buzztard;a=blob_plain;f=pixmaps/popup-menu.png;hb=HEAD

1) Some kind of generic "Popup from" function that positions the popup correctly (just like menu popups, which strive to appear in visible area, which means popping down, up, left or right depending on cursor position, etc), but make sure that it's clear the thing that pops up is positioned in relation to a widget, not to a position on the screen. That is one of the things that have to be done by hand right now, AFAICS.

2) Popup a generic GtkPopupSelectorWindow that might contain just about anything. IMPORTANT: Find a way to hook the hiding code to it, so that it would hide itself automatically when use focuses something outside it (this kind of functionality has been a scourge of W32 GTK+ app developers for years, when it was necessary to write tray popup menus). Hiding it once the user selects something in the "menu" is up to developers (because the window container might have, say, a set of buttons, or a treeview, so the developer will have to call the right API function to hide the menu once user's choice becomes clear). "Menu" contents should be focusable, and keyboard focus changing should only cycle through the menu contents. Hooking up hotkeys for hiding the menu (usually - ESC) is mostly up to developers too, i think.

Created attachment 215211 [details] [review]
gtkmenubutton: Add menu button widget

As used in Totem and gnome-contacts.

First pass at it. TODO:
- make GtkMenuToolButton use the menu button
- review the 3 positioning functions, and test them
- API docs

If the code is on the right path, I'll finish this up.

Review of attachment 215211 [details] [review]:

Initial impressions:

- Would be easier to comment on if there was a testmenubutton thing with a prop-editor

- It might be nice to avoid menus in the api altogether and just have a set_gmenu, or a constructor that takes a GMenuModel, or both

(In reply to comment #5)
> Review of attachment 215211 [details] [review]:
> 
> Initial impressions:
> 
> - Would be easier to comment on if there was a testmenubutton thing with a
> prop-editor

Certainly.

> - It might be nice to avoid menus in the api altogether and just have a
> set_gmenu, or a constructor that takes a GMenuModel, or both

gtk_menu_new_from_model() has one major downside:
"
The created menu items are connected to actions found in the GtkApplicationWindow to which the menu belongs - typically by means of being attached to a widget (see gtk_menu_attach_to_widget()) that is contained within the GtkApplicationWindows widget hierarchy.
"

Given how easy it is to create a GtkMenu, I'm not sure this is a major win, and it would make it impossible for me to use for Totem (the browser-plugin doesn't have a top-level GtkApplicationWindow, and I want the menu attached to an entry: https://github.com/gnome-design-team/gnome-mockups/raw/master/videos/videos-search.png

Internally at least, GtkMenu will be needed as we want to use it for GtkMenuToolButton.

I could make a subclass specifically for application menus though, which would take a GMenuModel and use the appropriate symbolic icon.

Created attachment 215264 [details] [review]
gtkmenubutton: Add menu button widget

As used in Totem and gnome-contacts. The widget
takes either a GtkMenu or a GMenuModel to construct
its menu, and can be given a parent widget to use to
position the drop-down (as used in GtkMenuToolButton).

Created attachment 215265 [details] [review]
gtkmenutoolbutton: Use GtkMenuButton

To implement the drop-down menu.

Now with a test app.

Added the "model" and "parent" properties, to implement GMenuModel support, and GtkMenuToolButton cleanups, respectively.

I also fixed gtk.h, and popup placement for the "up" direction.

Created attachment 215266 [details] [review]
gtkmenubutton: Add menu button widget

As used in Totem and gnome-contacts. The widget
takes either a GtkMenu or a GMenuModel to construct
its menu, and can be given a parent widget to use to
position the drop-down (as used in GtkMenuToolButton).

Forgot the gtkmenubutton.h changes.

Thanks for the test app. Some observations from playing with it:

- "model" and "parent" are both a bit too generic as property names, I think. How about "menu-model", and "menu-parent" ?

- With arrow == up, the menu aligns with the top of the insensitive widget above for me. I don't think that is expected, and it should align with the top of the button instead ? (since parent == NULL)

- I can set button properties, e.g. a stock id to show other content in the menu button. That is good. But when I then change the arrow property, it reverts back to showing just the arrow. I don't think that is intentional ?

(In reply to comment #12)
> Thanks for the test app. Some observations from playing with it:
> 
> - "model" and "parent" are both a bit too generic as property names, I think.
> How about "menu-model", and "menu-parent" ?

Done.

> - With arrow == up, the menu aligns with the top of the insensitive widget
> above for me. I don't think that is expected, and it should align with the top
> of the button instead ? (since parent == NULL)

That was bizarrely broken. I reworked this so the code is almost identical to the down function, and made it call the down function if the menu would have clipped the top of the monitor.

> - I can set button properties, e.g. a stock id to show other content in the
> menu button. That is good. But when I then change the arrow property, it
> reverts back to showing just the arrow. I don't think that is intentional ?

I figured it would only be set once. I've made changes so that we would only destroy widgets we created ourselves.

Created attachment 215324 [details] [review]
gtkmenubutton: Add menu button widget

As used in Totem and gnome-contacts. The widget
takes either a GtkMenu or a GMenuModel to construct
its menu, and can be given a parent widget to use to
position the drop-down (as used in GtkMenuToolButton).

Created attachment 215341 [details] [review]
gtkmenubutton: Add menu button widget

As used in Totem and gnome-contacts. The widget
takes either a GtkMenu or a GMenuModel to construct
its menu, and can be given a parent widget to use to
position the drop-down (as used in GtkMenuToolButton).

I'm happy with that being committed. Post-merge, I'd like to:
- wider coverage for the test app
- alignment support (the GtkMenuToolButton uses left alignment, epiphany's gear menu uses right alignment, see:
http://git.gnome.org/browse/epiphany/tree/src/ephy-page-menu-action.c)
- factor out the 3 positioning functions

I would file bugs for that.

Talking of which. Cosimo, should we do that:
http://git.gnome.org/browse/epiphany/tree/src/ephy-page-menu-action.c#n68
?

Since we're having talks about killing off GtkMenu and replacing it with something nicer it would be interesting if we could avoid having any GtkMenu on the API of this new widget at all; ideally it would always use a GMenuModel.

Is there a reason that direct GtkMenu support is required?

Review of attachment 215341 [details] [review]:

I am not the best person to comment on the use of GtkMenu vs. GMenuModel, but I have some comments on the code below.
As for your theming, I guess we will indeed need to add a style class to this button, but I wouldn't worry too much about it for now, since it can be easily added whenever it's merged.

::: gtk/gtkmenubutton.c
@@ +60,3 @@
+  PROP_MODEL,
+  PROP_PARENT,
+  PROP_DIRECTION

I'm not fully convinced about this...can't we determine it automatically somehow? As far as I can see, we don't to manual positioning of menus for any other widget.

@@ +72,3 @@
+                                const GValue *value,
+                                GParamSpec   *pspec)
+{

Can you cast GtkMenuButton *self = GTK_MENU_BUTTON (object) at the top here instead of doing in every branch?

@@ +103,3 @@
+    {
+      case PROP_MENU:
+        g_value_set_object (value, G_OBJECT (priv->menu));

g_value_set_object() takes a gpointer, so no need to cast here.

@@ +122,3 @@
+gtk_menu_button_state_changed (GtkWidget    *widget,
+                               GtkStateType  previous_state)
+{

This should either use state_flags_changed or notify on the senstive property, since GtkStateType is deprecated. Probably the former, since I'm not sure we'd get a notify if sensitivity changes on a parent widget.

@@ +438,3 @@
+  gtk_container_add (GTK_CONTAINER (menu_button), arrow);
+  gtk_widget_show (arrow);
+  priv->arrow_widget = arrow;

This is also repeated in gtk_menu_button_set_direction(), so is probably better factored out in a helper function.

@@ +445,3 @@
+                    G_CALLBACK (menu_button_toggled_cb), menu_button);
+  g_signal_connect (menu_button, "button-press-event",
+                    G_CALLBACK (menu_button_button_press_event_cb), menu_button);

It seems cleaner to override/chain up the default handlers for these in _class_init rather than setting up signals on self in _init.

@@ +620,3 @@
+    return;
+
+  priv->parent = g_object_ref (G_OBJECT (parent));

This ref is leaked - anyway it looks wrong; the requirement that the parent widget is an ancestor could easily lead to ref cycles, as the child widget would keep the parent alive after destroy.
I think you shouldn't take any refs here, but add a weak ref to the parent widget instead, or explicitly connect to its destroy signal, and clear out priv->parent when it's no longer valid.
The name of the function is also a bit misleading, and potentially conflicting with gtk_widget_set_parent() in bindings.

@@ +680,3 @@
+                                            menu_button_button_press_event_cb,
+                                            object);
+    }

You should add a _dispose implementation and unref priv->menu_model there.

::: gtk/gtkmenubutton.h
@@ +73,3 @@
+void       gtk_menu_button_set_parent     (GtkMenuButton *menu_button,
+                                           GtkWidget     *parent);
+

Why do you have only setters for all these? We usually have both C setters and getters for read-write properties.

(In reply to comment #19)
> Review of attachment 215341 [details] [review]:
> 
> I am not the best person to comment on the use of GtkMenu vs. GMenuModel, but I
> have some comments on the code below.
> As for your theming, I guess we will indeed need to add a style class to this
> button, but I wouldn't worry too much about it for now, since it can be easily
> added whenever it's merged.

It was more for the style class changes that I was asking.

> ::: gtk/gtkmenubutton.c
> @@ +60,3 @@
> +  PROP_MODEL,
> +  PROP_PARENT,
> +  PROP_DIRECTION
> 
> I'm not fully convinced about this...can't we determine it automatically
> somehow? As far as I can see, we don't to manual positioning of menus for any
> other widget.

That's what GtkMenuToolButton does internally. It's what all the apps that want the menu positioned to line up with the button (such as implemented by this widget) have to do (cf. Epiphany, gnome-contacts, Totem, and probably numerous others).

GtkMenuToolButton uses right and left directions for the vertical toolbar, and down for the horizontal toolbar. Totem uses the up direction for its contextual menu in the web browser plugin (the menu is at the bottom of the layout, so it's only normal it popped up, into where the space is).

> @@ +72,3 @@
> +                                const GValue *value,
> +                                GParamSpec   *pspec)
> +{
> 
> Can you cast GtkMenuButton *self = GTK_MENU_BUTTON (object) at the top here
> instead of doing in every branch?

Done.

> @@ +103,3 @@
> +    {
> +      case PROP_MENU:
> +        g_value_set_object (value, G_OBJECT (priv->menu));
> 
> g_value_set_object() takes a gpointer, so no need to cast here.

OK.

> @@ +122,3 @@
> +gtk_menu_button_state_changed (GtkWidget    *widget,
> +                               GtkStateType  previous_state)
> +{
> 
> This should either use state_flags_changed or notify on the senstive property,
> since GtkStateType is deprecated. Probably the former, since I'm not sure we'd
> get a notify if sensitivity changes on a parent widget.

This is copy/paste code from GtkMenuToolButton. I have no idea what to replace it with.

> @@ +438,3 @@
> +  gtk_container_add (GTK_CONTAINER (menu_button), arrow);
> +  gtk_widget_show (arrow);
> +  priv->arrow_widget = arrow;
> 
> This is also repeated in gtk_menu_button_set_direction(), so is probably better
> factored out in a helper function.

Done.

> @@ +445,3 @@
> +                    G_CALLBACK (menu_button_toggled_cb), menu_button);
> +  g_signal_connect (menu_button, "button-press-event",
> +                    G_CALLBACK (menu_button_button_press_event_cb),
> menu_button);
> 
> It seems cleaner to override/chain up the default handlers for these in
> _class_init rather than setting up signals on self in _init.

Done.

> @@ +620,3 @@
> +    return;
> +
> +  priv->parent = g_object_ref (G_OBJECT (parent));
> 
> This ref is leaked - anyway it looks wrong; the requirement that the parent
> widget is an ancestor could easily lead to ref cycles, as the child widget
> would keep the parent alive after destroy.
> I think you shouldn't take any refs here, but add a weak ref to the parent
> widget instead, or explicitly connect to its destroy signal, and clear out
> priv->parent when it's no longer valid.
> The name of the function is also a bit misleading, and potentially conflicting
> with gtk_widget_set_parent() in bindings.

Got another better name?

> @@ +680,3 @@
> +                                            menu_button_button_press_event_cb,
> +                                            object);
> +    }
> 
> You should add a _dispose implementation and unref priv->menu_model there.

I've added it to finalize.

> ::: gtk/gtkmenubutton.h
> @@ +73,3 @@
> +void       gtk_menu_button_set_parent     (GtkMenuButton *menu_button,
> +                                           GtkWidget     *parent);
> +
> 
> Why do you have only setters for all these? We usually have both C setters and
> getters for read-write properties.

Seemed redundant with the properties. I'll add those.

Created attachment 215400 [details] [review]
gtkmenubutton: Add menu button widget

As used in Totem and gnome-contacts. The widget
takes either a GtkMenu or a GMenuModel to construct
its menu, and can be given a parent widget to use to
position the drop-down (as used in GtkMenuToolButton).

(In reply to comment #20)

> It was more for the style class changes that I was asking.

Yeah, my comment had a typo there :)
I would like to just unconditionally add a "menu" style class to the button, but I'm afraid .menu.button already refers to "scroll" buttons in long menus. I will investigate more, but what I meant to say is that since it doesn't touch the API it can be done in a separate step later.

> That's what GtkMenuToolButton does internally. It's what all the apps that want
> the menu positioned to line up with the button (such as implemented by this
> widget) have to do (cf. Epiphany, gnome-contacts, Totem, and probably numerous
> others).
> 
> GtkMenuToolButton uses right and left directions for the vertical toolbar, and
> down for the horizontal toolbar. Totem uses the up direction for its contextual
> menu in the web browser plugin (the menu is at the bottom of the layout, so
> it's only normal it popped up, into where the space is).

Okay.

> > @@ +122,3 @@
> > +gtk_menu_button_state_changed (GtkWidget    *widget,
> > +                               GtkStateType  previous_state)
> > +{
> > 
> > This should either use state_flags_changed or notify on the senstive property,
> > since GtkStateType is deprecated. Probably the former, since I'm not sure we'd
> > get a notify if sensitivity changes on a parent widget.
> 
> This is copy/paste code from GtkMenuToolButton. I have no idea what to replace
> it with.

You can replace it with state_flags_changed; they're emitted at the same time by GtkWidget for compatibility reasons anyway (see [1]), but state_changed is deprecated.

> > The name of the function is also a bit misleading, and potentially conflicting
> > with gtk_widget_set_parent() in bindings.
> 
> Got another better name?

set_align_widget?

[1] http://git.gnome.org/browse/gtk+/tree/gtk/gtkwidget.c#n10887

Review of attachment 215400 [details] [review]:

Some other minor comments

::: gtk/gtkmenubutton.c
@@ +450,3 @@
+
+  g_signal_connect (menu_button, "toggled",
+                    G_CALLBACK (menu_button_toggled_cb), menu_button);

This could be overridden on GtkToggleButtonClass in class_init too.

@@ +548,3 @@
+ * gtk_menu_button_set_menu:
+ * @menu_button: a #GtkMenuButton
+ * @menu: a #GtkMenu

@menu should be (allow-none) (and also @menu_model in the other method below)

@@ +672,3 @@
+ * Returns the parent #GtkWidget to use to line up with menu.
+ *
+ * Returns: (transfer: none): a #GtkWidget value or %NULL.

The correct syntax for the annotation is "Returns: (transfer none): ..."

@@ +687,3 @@
+ * gtk_menu_button_set_direction:
+ * @menu_button: a #GtkMenuButton
+ * @direction: a #GtkWidget

#GtkArrowType

Created attachment 215426 [details] [review]
gtkmenubutton: Add menu button widget

As used in Totem and gnome-contacts. The widget
takes either a GtkMenu or a GMenuModel to construct
its menu, and can be given a parent widget to use to
position the drop-down (as used in GtkMenuToolButton).

Created attachment 215427 [details] [review]
gtkmenutoolbutton: Use GtkMenuButton

To implement the drop-down menu.

Review of attachment 215426 [details] [review]:

This looks almost good to me (just two tiny comments below).
I'll defer to our API and GMenu overlords for the final merge call.

::: gtk/gtkmenubutton.h
@@ +53,3 @@
+  GtkToggleButtonClass parent_class;
+
+  void (*show_menu) (GtkMenuButton *button);

Copy/paste leftover?

@@ +78,3 @@
+
+void         gtk_menu_button_set_align_widget (GtkMenuButton *menu_button,
+                                               GtkWidget     *parent);

Should be GtkWidget *align_widget (or gtk-doc and g-i scanner will complain about the mismatch of arg names between declaration and definition)

Review of attachment 215427 [details] [review]:

::: gtk/gtkmenutoolbutton.c
@@ +222,3 @@
     {
     case PROP_MENU:
+      g_object_get_property (G_OBJECT (button->priv->arrow_button), "menu", value);

I would use g_value_set_object (value, gtk_menu_button_get_menu())

@@ +446,3 @@
   g_return_val_if_fail (GTK_IS_MENU_TOOL_BUTTON (button), NULL);
 
+  g_object_get (button->priv->arrow_button, "menu", &ret, NULL);

Wouldn't this add a reference to |ret|? (I think g_object_get does it automatically for types derived from G_TYPE_OBJECT)
You could use the C getter to avoid it.

Created attachment 215443 [details] [review]
gtkmenubutton: Add menu button widget

As used in Totem and gnome-contacts. The widget
takes either a GtkMenu or a GMenuModel to construct
its menu, and can be given a parent widget to use to
position the drop-down (as used in GtkMenuToolButton).

Created attachment 215444 [details] [review]
gtkmenutoolbutton: Use GtkMenuButton

To implement the drop-down menu.

Created attachment 215446 [details] [review]
gtkmenutoolbutton: Use GtkMenuButton

To implement the drop-down menu.

(In reply to comment #26)
> Review of attachment 215426 [details] [review]:
> 
> This looks almost good to me (just two tiny comments below).
> I'll defer to our API and GMenu overlords for the final merge call.
> 
> ::: gtk/gtkmenubutton.h
> @@ +53,3 @@
> +  GtkToggleButtonClass parent_class;
> +
> +  void (*show_menu) (GtkMenuButton *button);
> 
> Copy/paste leftover?

Removed.

> @@ +78,3 @@
> +
> +void         gtk_menu_button_set_align_widget (GtkMenuButton *menu_button,
> +                                               GtkWidget     *parent);
> 
> Should be GtkWidget *align_widget (or gtk-doc and g-i scanner will complain
> about the mismatch of arg names between declaration and definition)

Yes, my mistake.

(In reply to comment #27)
> Review of attachment 215427 [details] [review]:
> 
> ::: gtk/gtkmenutoolbutton.c
> @@ +222,3 @@
>      {
>      case PROP_MENU:
> +      g_object_get_property (G_OBJECT (button->priv->arrow_button), "menu",
> value);
> 
> I would use g_value_set_object (value, gtk_menu_button_get_menu())

Yep.

> @@ +446,3 @@
>    g_return_val_if_fail (GTK_IS_MENU_TOOL_BUTTON (button), NULL);
> 
> +  g_object_get (button->priv->arrow_button, "menu", &ret, NULL);
> 
> Wouldn't this add a reference to |ret|? (I think g_object_get does it
> automatically for types derived from G_TYPE_OBJECT)
> You could use the C getter to avoid it.

Done.

Review of attachment 215446 [details] [review]:

This part looks fine to me, once we have the menubutton itself

Review of attachment 215443 [details] [review]:

::: gtk/gtkmenubutton.c
@@ +131,3 @@
+    {
+      gtk_menu_shell_deactivate (GTK_MENU_SHELL (priv->menu));
+    }

gtk codingstyle omits the { } in cases like this.

@@ +281,3 @@
+
+  *push_in = FALSE;
+}

It feels like you could unify a lot of the boilerplate (ie all the screen, monitor and workspace handling) here, by having a single position function that looks at priv->arrow_type ?

@@ +308,3 @@
+        func = (GtkMenuPositionFunc) menu_position_down_func;
+        break;
+  }

doing that would also let you drop this switch altogether

@@ +314,3 @@
+                  GTK_WIDGET (menu_button),
+                  event ? event->button : 0,
+                  event ? event->time : gtk_get_current_event_time ());

You should use gtk_menu_popup_for_device, with the device from the event

@@ +372,3 @@
+   * GtkMenuButton:menu:
+   *
+   * The #GtkMenu that will be popped up when the button is clicked.

The documentation should spell out how this interacts with ::menu-model. Which one wins, if both are set ?

@@ +386,3 @@
+   * GtkMenuButton:menu-model:
+   *
+   * The #GMenuModel from which the menu to pop up will be created.

Should a) say how this interacts with ::menu and b) how action references in the model are resolved.

@@ +402,3 @@
+   * The #GtkWidget to use to align the popup menu with.
+   *
+   * Since: 3.6

Should say what the default of NULL means in terms of alignment.

@@ +415,3 @@
+   *
+   * The #GtkArrowType representing the direction in which the
+   * menu will be popped out.

Not sure if you want to add "unless the screen constraints force it top pop out to the other side" ?

@@ +458,3 @@
+ *
+ * Creates a new #GtkMenuButton widget with an arrow pointing
+ * down as the only child.

'downwards-pointing arrow' instead of 'arrow pointing down', maybe ?
Should we say that you can replace the arrow widget with a child of your choice ?

@@ +501,3 @@
+{
+  GtkMenuButtonPrivate *priv;
+

Would be good to have a comment here stating where this function is used.

@@ +552,3 @@
+ *
+ * Sets the #GtkMenu that will be popped up when the button is clicked,
+ * or %NULL to disable the button.

As above, docs should say something about how this interacts with set_menu_model

@@ +588,3 @@
+ * Sets the #GMenuModel from which the "menu" property will be created,
+ * or %NULL to disable the button.
+ *

As above, docs should say something about how actions are resolved.

@@ +642,3 @@
+ *
+ * Sets the #GtkWidget to use to line the menu with when popped up. Note that
+ * the @align_widget must contain the #GtkMenuButton itself.

Should say what the effect of %NULL is

@@ +692,3 @@
+ * Sets the direction in which the menu will be popped up, as
+ * well as changing the arrow's direction. The child will not
+ * be changed to an arrow if it was customized.

What happens for direction == GTK_ARROW_NONE ?

Created attachment 216142 [details] [review]
gtkmenubutton: Add menu button widget

As used in Totem and gnome-contacts. The widget
takes either a GtkMenu or a GMenuModel to construct
its menu, and can be given a parent widget to use to
position the drop-down (as used in GtkMenuToolButton).

(In reply to comment #33)
> Review of attachment 215443 [details] [review]:
> 
> ::: gtk/gtkmenubutton.c
> @@ +131,3 @@
> +    {
> +      gtk_menu_shell_deactivate (GTK_MENU_SHELL (priv->menu));
> +    }
> 
> gtk codingstyle omits the { } in cases like this.

Yep, removed.

> @@ +281,3 @@
> +
> +  *push_in = FALSE;
> +}
> 
> It feels like you could unify a lot of the boilerplate (ie all the screen,
> monitor and workspace handling) here, by having a single position function that
> looks at priv->arrow_type ?

Yes, but I'd rather get the code merged before trying to make optimisations and cleanups. The calculations come from different codebases, and I'm weary of optimising too early.

> @@ +308,3 @@
> +        func = (GtkMenuPositionFunc) menu_position_down_func;
> +        break;
> +  }
> 
> doing that would also let you drop this switch altogether
> 
> @@ +314,3 @@
> +                  GTK_WIDGET (menu_button),
> +                  event ? event->button : 0,
> +                  event ? event->time : gtk_get_current_event_time ());
> 
> You should use gtk_menu_popup_for_device, with the device from the event

Done.

> @@ +372,3 @@
> +   * GtkMenuButton:menu:
> +   *
> +   * The #GtkMenu that will be popped up when the button is clicked.
> 
> The documentation should spell out how this interacts with ::menu-model. Which
> one wins, if both are set ?

Filled in.

> @@ +386,3 @@
> +   * GtkMenuButton:menu-model:
> +   *
> +   * The #GMenuModel from which the menu to pop up will be created.
> 
> Should a) say how this interacts with ::menu and b) how action references in
> the model are resolved.

Done.

> @@ +402,3 @@
> +   * The #GtkWidget to use to align the popup menu with.
> +   *
> +   * Since: 3.6
> 
> Should say what the default of NULL means in terms of alignment.

Done.

> @@ +415,3 @@
> +   *
> +   * The #GtkArrowType representing the direction in which the
> +   * menu will be popped out.
> 
> Not sure if you want to add "unless the screen constraints force it top pop out
> to the other side" ?

Added.

> @@ +458,3 @@
> + *
> + * Creates a new #GtkMenuButton widget with an arrow pointing
> + * down as the only child.
> 
> 'downwards-pointing arrow' instead of 'arrow pointing down', maybe ?
> Should we say that you can replace the arrow widget with a child of your choice
> ?

Done.

> @@ +501,3 @@
> +{
> +  GtkMenuButtonPrivate *priv;
> +
> 
> Would be good to have a comment here stating where this function is used.

Done.

> @@ +552,3 @@
> + *
> + * Sets the #GtkMenu that will be popped up when the button is clicked,
> + * or %NULL to disable the button.
> 
> As above, docs should say something about how this interacts with
> set_menu_model

Done.

> @@ +588,3 @@
> + * Sets the #GMenuModel from which the "menu" property will be created,
> + * or %NULL to disable the button.
> + *
> 
> As above, docs should say something about how actions are resolved.

Done.

> @@ +642,3 @@
> + *
> + * Sets the #GtkWidget to use to line the menu with when popped up. Note that
> + * the @align_widget must contain the #GtkMenuButton itself.
> 
> Should say what the effect of %NULL is

Done.

> @@ +692,3 @@
> + * Sets the direction in which the menu will be popped up, as
> + * well as changing the arrow's direction. The child will not
> + * be changed to an arrow if it was customized.
> 
> What happens for direction == GTK_ARROW_NONE ?

Done.

Also used the GDK_AVAILABLE_IN_3_6 macro.

Created attachment 216404 [details] [review]
gtkmenubutton: Add menu button widget

As used in Totem and gnome-contacts. The widget
takes either a GtkMenu or a GMenuModel to construct
its menu, and can be given a parent widget to use to
position the drop-down (as used in GtkMenuToolButton).

Review of attachment 216404 [details] [review]:

Just  a few cosmetics, otherwise it looks fine to me.

::: gtk/gtkmenubutton.c
@@ +329,3 @@
+    {
+      /* we get here only when the menu is activated by a key
+       * press, so that we can select the first menu item */

Pet peeve of mine: move the */ to the next line and align the *s

@@ +498,3 @@
+/* This function is used in GtkMenuToolButton, the call back will
+ * be called when GtkMenuToolButton would have emitted the "show-menu"
+ * signal. */

Same here

@@ +627,3 @@
+  if (menu_model == NULL)
+    {
+      gtk_menu_button_set_menu (menu_button, NULL);

You forget to notify here

@@ +636,3 @@
+  gtk_menu_button_set_menu (menu_button, menu);
+
+  g_object_notify (G_OBJECT (menu_button), "menu-model");

You probably want to notify ::menu as well here ?

@@ +746,3 @@
+
+  gtk_widget_destroy (child);
+  add_arrow (menu_button);

Why construct an entirely new one ? Wouldn't g_object_set (priv->arrow_widget, "arrow-type", direction, NULL) work ?

(In reply to comment #37)
> Review of attachment 216404 [details] [review]:
> 
> Just  a few cosmetics, otherwise it looks fine to me.
> 
> ::: gtk/gtkmenubutton.c
> @@ +329,3 @@
> +    {
> +      /* we get here only when the menu is activated by a key
> +       * press, so that we can select the first menu item */
> 
> Pet peeve of mine: move the */ to the next line and align the *s

Done.

> @@ +498,3 @@
> +/* This function is used in GtkMenuToolButton, the call back will
> + * be called when GtkMenuToolButton would have emitted the "show-menu"
> + * signal. */
> 
> Same here

Done.

> @@ +627,3 @@
> +  if (menu_model == NULL)
> +    {
> +      gtk_menu_button_set_menu (menu_button, NULL);
> 
> You forget to notify here

Nope. gtk_menu_button_set_menu() calls _gtk_menu_button_set_menu_with_func() which notifies both "menu" and "menu-model".

> @@ +636,3 @@
> +  gtk_menu_button_set_menu (menu_button, menu);
> +
> +  g_object_notify (G_OBJECT (menu_button), "menu-model");
> 
> You probably want to notify ::menu as well here ?

It shouldn't be there actually (see above).

> @@ +746,3 @@
> +
> +  gtk_widget_destroy (child);
> +  add_arrow (menu_button);
> 
> Why construct an entirely new one ? Wouldn't g_object_set (priv->arrow_widget,
> "arrow-type", direction, NULL) work ?

Fixed.

Attachment 215446 [details] pushed as 50cd570 - gtkmenutoolbutton: Use GtkMenuButton
Attachment 216404 [details] pushed as 9fef2dc - gtkmenubutton: Add menu button widget

I was going to make Disks use this widget but it's not really clear to me how it works. How about adding something to gtk3-demo? I thought it was verboten to add new widgets without doing this...

(In reply to comment #40)
> I was going to make Disks use this widget but it's not really clear to me how
> it works. How about adding something to gtk3-demo? I thought it was verboten to
> add new widgets without doing this...

tests/testmenubutton.c ? It's in the same commit as the widget itself.

(In reply to comment #41)
> (In reply to comment #40)
> > I was going to make Disks use this widget but it's not really clear to me how
> > it works. How about adding something to gtk3-demo? I thought it was verboten to
> > add new widgets without doing this...
> 
> tests/testmenubutton.c ? It's in the same commit as the widget itself.

Yeah, I saw that (and am currently building gtk+ with all that it entails), but it's not as easy / streamlined as gtk3-demo...

Patches for gtk-demo very welcome. I wasn't sure where to add it...