Created attachment 222358 [details] [review]
GtkModelMenuItem: first pass at submenu actions

Add support for a stateful action associated with a submenu.  The action
state is set to TRUE when the menu is shown and FALSE when it is
unshown.

This is useful to avoid unnecessary processing for menus that have
frequently-changing content.

A possible future feature is to add support for asynchronously filling
the initial state of the menu by waiting until the action actually emits
its state-change signal to TRUE before showing the menu.

A silly example has been added to Bloatpad to demonstrate the new
feature.

Would be nice to have an g_menu_item_set () here, instead of recreating the entire submenu

We do have g_menu_item_set_attribute(), but that only modifies a GMenuItem.

Once the item is in a GMenuModel, it's immutable...

Review of attachment 222358 [details] [review]:

Looks good.

Should we check whether an action with the right type exists before calling g_action_group_change_action_state?  GSimpleActionGroup's implementation of change_action_state ignores unknown actions, but the interface documentation says "the action must be stateful and must be of the correct type".

::: gtk/gtkmodelmenuitem.c
@@ +153,3 @@
         gtk_actionable_set_action_target_value (GTK_ACTIONABLE (item), value);
 
+      else if (g_str_equal (key, "submenu-action") && g_variant_is_of_type (value, G_VARIANT_TYPE_STRING))

Is there a reason you put this here instead of into the "if ((submenu = ..." block above, where the submenu is actually created?

Mostly because this is the point at which the attributes are being iterated...

I guess I could just do a one-off above, though.  We certainly do it for a few other items...

Review of attachment 222358 [details] [review]:

::: examples/bloatpad.c
@@ +258,3 @@
+  g_menu_append (bloatpad->time, time, NULL);
+  g_date_time_unref (now);
+  g_free (time);

Still think it is a little sad that we recreate the entire menu every second, when we really just want to update the label. Price to pay for immutability, I guess...

(In reply to comment #6)
> Still think it is a little sad that we recreate the entire menu every second,
> when we really just want to update the label. Price to pay for immutability, I
> guess...

The sadness here is only how contrived the example is.  Where else do you know that we actually want to change the entire text of a menuitem once per second?