I only translated the undo menu into GtkBuilder's .ui format for now.
The only missing part is that the icon is now shown.
Note that in various parts, I don't rely anymore on a bogus menu action (i.e.
"undo-popup" action in this case) which does nothing but has an associated label
and icon. I simply add the label and icon as submenu attribute directly in the
.ui file, which is translatable and whose strings should be parsed by gettext.
Eventually I'll just get rid of all the various "*-popup" or "*-menu" bogus
actions.
The new GimpMenu is derived from GtkMenu. Both GimpMenu and GimpMenuBar are now
implementing GimpMenuShell which allows to share a lot of the logic for filling
the menus, adding items, etc.
This is less and less a GtkAction. Note that I don't implement
gimp_action_get_name() because once we will drop completely GtkAction and make
GimpObject the parent, we can just use gimp_object_get_name().
The dialogs-menuitems is shared between dockable-menu and image-menu, just as it
was with the old format. For this, we use XSL transformation with XInclude, then
drop the namespace (because GTK doesn't like the unknown namespace being defined
and crash immediately otherwise).
This is a placeholder which is not meant to appear in the menu. Our new system
uses placeholders internally, but plug-ins cannot make use of these.
Not sure anyway if this is so useful in the cases of plug-ins. At least, I don't
feel it is for these particular use cases.
Also getting rid of gimp_action_get_proxies() API which is now unneeded, I
think. Furthermore GimpMenu does not have to connect to label changes. Making
the items proxies of a GimpAction is enough to have them updated.
The tooltip contains the reason for action inactivity, if relevant. And in case
of a GtkMenuItem in particular, it will also contain the "Press F1 for more
help" text at the bottom.
Not sure why gtk_actionable_set_action_name() works fine to activate base
GimpActionImpl, but not subclasses like GimpProcedureAction or GimpEnumAction
(and maybe others subclasses?), such as the "file-open-recent-*" actions. This
will have to be investigated further IMO.
While in the menu, under a contextual "Open Recent" submenu, it is well
understandable, the action is much less understandable without context, e.g. in
the action search. So I prepend an "Open" prefix.
Maybe a future alternative could be to have GimpAction have both contextual and
non-contextual labels (a.k.a. short vs long label)?
The proxy properly shows a color area or viewable (depending on context), and
this is properly updated when the associated properties are changed.
The label change is also properly updated in the label part of the proxy.
Very relevant actions to test the proxy items are dynamically updated are
generated actions such as the "file-open-recent-*" actions (with both label and
viewable updates), or again the Edit > "Fill with (FG|BG Color)|Pattern" (with
color/pattern updates).
These are the menu items such as the recently opened images, or recently used
filters, etc.
Some notes:
- I added back a "placeholder" concept in the GimpMenu logic. This will allow to
place items at specific positions in the menu (either under the placeholder,
which will make the last item be on top or above the placeholder to have the
last item be in the bottom, depending on needs). Technically placeholders are
just menu items with a label (used as placeholder key) and no associated
action, which I will leave invisible.
- I add a logic for submenus so that they are invisible by default and are only
made visible when we add a menu item with an action in there.
- I removed filling the "/Filters/Recently Used/Plug-ins" placeholder. As far as
I could see, it was never filled (neither with old or new code) and the
"/Filters/Recently Used/Filters" actually already takes care of filling the
"Recently Used" submenu with both GEGL operations and plug-in calls.
- The old gimp_ui_manager_add_ui() API is for all types of menus, e.g. including
the ones created by dockables or elsewhere whereas my new API is (for now)
still specific to the top menu. This will have to be further implemented
later. I left a bunch of "TODO GMenu"-s for the time being.
- I see 2 dock-related generated items which seem to never be added, for
recently added and closed docks. It doesn't seem to work in the old API as
well. I'll want to have a closer look too.
Some actions in particular can change their label. E.g. "file-export" and
"file-overwrite" will have customized labels containing the imported or exported
file name. The menu will now reflect such changes, live.
I'm starting to override GtkAction properties, starting with "sensitive" in
order to finally move away from GtkAction. Obviously, this breaks the
sensitivity check of the original menu.
I could reproduce this while closing GIMP immediately after starting it.
Sometimes the idle source would be running and working on now finalized objects.
"windows-show-tabs" and "windows-use-single-window-mode" were set to FALSE by
default, while it is now supposed to be TRUE, same as the config properties. The
discrepancy was only showing with my new GimpMenu which better follows the
action's state.
This demonstrates a first version of our replacing menu, using GAction and
GMenuModel. I had to make our own subclass of GtkMenu to process the model (from
a .ui XML file) for the following reasons:
* gtk_menu_new_from_model() doesn't support tooltips, which is a feature we use
quite extensively in GIMP: with all our filters, being able to give a longer
description is often useful; moreover we use tooltips to give hints about why
a menu item is deactivated as well.
Unfortunately it looks like GTK doesn't consider this lack as a problem and
don't plan on adding tooltip support.
See: https://gitlab.gnome.org/GNOME/gtk/-/issues/785
* I won't to avoid copying action's label and icons in the .ui file. This only
duplicates strings and would be a source of issues each time we change
action's strings (we'd have to do it in 2 places, which someone will
inevitably forget).
Now it still has various issues:
* The syncing between actions and menu items need to be cleaned up. It's still
in early demo code.
* It uses directly some Gtk*Action code because GimpRadioAction and
GimpToggleAction are not directly related right now (only through their
parents).
* gtk_application_set_menubar() might still be necessary on macOS as I think
it's what enables the native menu system on this OS. It means that we'll have
to edit the menu model to add back the labels (as this function does not
extract these from the linked action since GAction has no label or icon
concept).
* Icons are not taken into account right now.
* I'll have to verify if GimpAction with proxy work (but my guess is that right
now, it won't).
* Action's active state is not synced with menu item active state right now.
* Various actions are inserted live, such as opened images, opened views,
recently opened images, and so on. This needs to be implemented back.
* Plug-ins need to be able to create their own menu item into this new menu.
* For all these various reasons, I'm keeping the old menu around, for the sake
of comparison, until the time the new one becomes feature-full.
Part of this commit is inspired by !558 and obsoletes this MR.
Simply with a NULL context, we can't set or get accels from a GimpAction. Having
a mandatory context was a problem for GimpColorButton which could create a
GimpAction even though libgimpwidget has no knowledge of GimpContext.
All the remaining pieces of code where the deprecated concept of accelerator
closure was still used have now been replaced by proper GimpAction API using the
newer GAction API.
It's still not perfect, especially as we might want to get back GimpActionGroup
for this specific dialog (it may be nicer than an overlong list of actions). And
overall, I am not fond of this interface so maybe I'll want to review how we
display and change actions too. Moreover we now have the ability to set several
shortcuts per action, which was a really wanted feature and this dialog doesn't
allow this yet.
I'll get back later at this widget, but for now, it is a step forward for
deprecation.
- New gimp_action_get_display_accels() added to GimpAction API to get directly
the human-visible strings for actions.
- Also adding an "accels-changed" signal to GimpAction. As far as I can see,
even though accelerators are now to be set on the GtkApplication, there seems
to be no signals or properties to connect to at all on this class. So instead,
let's do it on our GimpAction (which means we must absolutely not use
gtk_application_set_accels_for_action()).
- GimpAccelLabel now uses the proper GimpAction API instead of accelerator
closure which is a disappearing concept.
In particular, there is no concept of accel closure for actions anymore in
GAction, which means that eventually gimp_action_get_accel_closure() will have
to disappear too.
Depending on the OS, the display name may contain characters which are not valid
for action names. In particular, on X11, the display name could be ":0.0" and
therefore we get actions named "app.dock-move-to-screen-:0.0" or
"app.view-move-to-screen-:0.0". The ':' is invalid, which will make calling
gtk_application_get_accels_for_action() (or more simply our new function
gimp_action_get_accels()) fail.
See docs of g_action_parse_detailed_name().
The new utils function gimp_make_valid_action_name() will help us get rid of
invalid characters for actions whose name was generated from strings we don't
totally control.
Furthermore gdk_screen_make_display_name() is now deprecated API, and more
generally gdk_display_get_n_screens() is deprecated too and now always returns 1
since GTK+ 3.10 (i.e. now each display only contains a single screen). Instead
we just use gdk_display_get_default_screen() for every GdkDisplay. So this
commit doubles as GTK/GDK deprecation cleanup.
If we want to reuse this in subclasses, we'd need to overwrite finalize() in the
subclasses, call gimp_core_app_finalize() and chain up with the parent class.
Not doing this until now was leading us not to call GApplication or
GtkApplication finalize() function, which in turn, in the later case, was
leaking all the GAction-s which were added to the GActionMap. I realized this as
a leaking GimpContext-s warning was printing when ending GIMP.
Adding a 'Show" before the image identifier, which is now between quotes, and
making this string localizable.
This makes the action more understandable, especially when listed in
less-contextual UI such as the action search.
