Add shortcuts list

[rolandlo]

Addresses #5218 (reply in thread)

The shortcuts list is displayed by activating menu Help > Shortcuts. There are three sections:

• menu shortcuts,
• text editor shortcuts
• other shortcuts).

I'm not sure the icons are needed, since they are part of Gtk. However I only see the image missing icon when I don't include the icons on Ubuntu 23.10.

To generate the shortcuts.glade file I took my earlier blueprint file, compiled it to a Gtk 4 .ui file and made the necessary changes so it works with Gtk3. Glade doesn't support ShortcutsWindow, so the shortcuts.glade file cannot actually be edited there.

• Replace <Control> with <Meta> for MacOS for menu Shortcuts. -> Achieved by replacing accelerators by actions

[Febbe]

I think the shortcut list should be generated from the Actions list on runtime.

[rolandlo]

I think the shortcut list should be generated from the Actions list on runtime.

That makes sense for the menu shortcuts, which are associated to actions. There are cons as well. In particular, we can't handtune the titles of the shortcuts and have to take the labels defined in mainmenubar.xml for the menu entries instead. Also the recommended way is to construct them with builder

The recommended way to construct a GtkShortcutsWindow is with GtkBuilder, by using the tag to populate a GtkShortcutsWindow with one or more GtkShortcutsSection objects, which contain one or more GtkShortcutsGroup instances, which, in turn, contain GtkShortcutsShortcut instances.
If you need to add a section programmatically, use gtk_shortcuts_window_add_section() instead of gtk_window_set_child(), as the shortcuts window manages its children directly.

Instead of defining the shortcuts in shortcuts.glade via the accelerator property by hand, it should also be possible to use the action-name property, so that the accelerators are derived from the actions (and change in the shortcuts window if the shortcuts are reassigned by the user in some way). However that somehow didn't work when I tried (no accelerators are displayed). I have checked the GTK code for that and it should be supported, but somehow it doesn't work.

The shortcuts from the "texteditor shortcuts" section and "other shortcuts" section are not associated to actions though.
So do you suggest to just create the menu shortcuts from the Actions list or to try to convert everything into actions or to remove the other two sections?

[bhennion]

Instead of defining the shortcuts in shortcuts.glade via the accelerator property by hand, it should also be possible to use the action-name property, so that the accelerators are derived from the actions (and change in the shortcuts window if the shortcuts are reassigned by the user in some way). However that somehow didn't work when I tried (no accelerators are displayed). I have checked the GTK code for that and it should be supported, but somehow it doesn't work.

This sounds like the best solution to me, if we can manage to get it to work.

The shortcuts from the "texteditor shortcuts" section and "other shortcuts" section are not associated to actions though. So do you suggest to just create the menu shortcuts from the Actions list or to try to convert everything into actions or to remove the other two sections?

I think shortcuts define by hand (e.g. in XournalView::onKeyPressEvent() or TextEditor::onKeyPressEvent()) should be refactored into Actions indeed -- or just straped to the existing actions with same shortcut (e.g. Ctrl++ should zoom in or increase the font size, depending on whether a text box is active or not).

[rolandlo]

Instead of defining the shortcuts in shortcuts.glade via the accelerator property by hand, it should also be possible to use the action-name property, so that the accelerators are derived from the actions (and change in the shortcuts window if the shortcuts are reassigned by the user in some way). However that somehow didn't work when I tried (no accelerators are displayed). I have checked the GTK code for that and it should be supported, but somehow it doesn't work.

This sounds like the best solution to me, if we can manage to get it to work.

The Gtk example app bloatpad actually uses the action-name property and has configurable keyboard shortcuts. It already works in Gtk 3. I will take a look what can learn from there.

[rolandlo]

I got the "action-name" property to work by packing the shortcuts window .ui file as a GResource into the executable.

From the docs

• If there is a resource located at "gtk/help-overlay.ui" which
• defines a #GtkShortcutsWindow with ID "help_overlay" then GtkApplication
• associates an instance of this shortcuts window with each
• #GtkApplicationWindow and sets up keyboard accelerators (Control-F1
• and Control-?) to open it. To create a menu item that displays the
• shortcuts window, associate the item with the action win.show-help-overlay.

See gtk_application_load_resources, gtk_application_window_set_help_overlay and gtk_application_window_added in the Gtk sources for how this works.

Is there a reason why we don't use GResources for other UI elements (glade files, icons)? They are packed into the executable, which makes loading those UI elements more efficient, since it's already in memory.

[Febbe]

We are most likely not using "new" functions of gtk because it simply did not exist at the time the code was written. Xournalpp was originally a gtk2 and pre c++11 application. This is also why we don't have reconfigurable shortcuts. We needed the step @bhennion took to move to GTK. Even though those changes only replaced gtk2 idioms with gtk3 idioms.

Anything that modernises our codebase is welcome.

[rolandlo]

I think shortcuts define by hand (e.g. in XournalView::onKeyPressEvent() or TextEditor::onKeyPressEvent()) should be refactored into Actions indeed -- or just straped to the existing actions with same shortcut (e.g. Ctrl++ should zoom in or increase the font size, depending on whether a text box is active or not).

I don't think that different actions should be merged with the action being executed depending on whether some widget is active or not.
In future we might also allow activating actions through dbus (like inkscape does, see https://gitlab.com/inklinea/ink-dbus) as requested in #4193. In that context it doesn't make sense to merge several actions into one. An action should really be a (separate) piece of high-level functionality that the app provides.

[Febbe]

In that context it doesn't make sense to merge several actions into one.

Why?

An action should really be a (separate) piece of high-level functionality that the app provides.

An GioAction is a low level abstraction which splits the functionality from the actual implementation.
It also allows for a more general handling of actions, independently how and were they are used.

Therefore, I would suggest, to actually handle all (general) actions we have with GioActions. This might even mean, that several Actions might trigger the same action handler.

For example, we have an insert-text action, which is only valid for specific text insertions, but the strg+c action triggers the same when a text field is selected: strg-action -> insert-text -> handler.
In terms of extensibility, we might even get around of actions which require the context of their execution. For example, when we want to add workspaces, multiple windows (mac related) and tabs. Any document related action must be executed in the context of the current document, for example.

[bhennion]

I don't think that different actions should be merged with the action being executed depending on whether some widget is active or not.

Agreed, my proposal was naive. The point is that two GAction's can have the same accelerator as long as they are not in the same GActionMap. In this Ctrl++ example, there is a widget W catching every keyboard input on behalf of the TextEditor. A GAction added to a GActionMap added to this widget W would only be triggered by its accelerator Ctrl++ if the widget W (or one of its descendents) has the focus (I think). Otherwise, the Ctrl++ sequence should be passed up all the way to the GtkApplicationWindow's GActionMap, triggering a zoom in sequence.

[rolandlo]

I don't think that different actions should be merged with the action being executed depending on whether some widget is active or not.

Agreed, my proposal was naive. The point is that two GAction's can have the same accelerator as long as they are not in the same GActionMap. In this Ctrl++ example, there is a widget W catching every keyboard input on behalf of the TextEditor. A GAction added to a GActionMap added to this widget W would only be triggered by its accelerator Ctrl++ if the widget W (or one of its descendents) has the focus (I think). Otherwise, the Ctrl++ sequence should be passed up all the way to the GtkApplicationWindow's GActionMap, triggering a zoom in sequence.

How is this implemented in Gtk 3?

Looks like in Gtk 4, one can

• insert a G_ACTION_GROUP containing some action with name action-name on a widget via gtk_widget_insert_action_group using some prefix
• Create a GtkShortcutController and add a shortcut via

gtk_shortcut_controller_add_shortcut (GTK_SHORTCUT_CONTROLLER (controller),
           gtk_shortcut_new (gtk_keyval_trigger_new (key, modifier),
                             gtk_named_action_new ("prefix.action-name")));

• Set the scope of the GtkShortcutController to GTK_SHORTCUT_SCOPE_LOCAL and add it to the widget via gtk_widget_add_controller.
• Then according to the docs

With GTK_SHORTCUT_SCOPE_LOCAL, shortcuts will only be activated when the widget has focus.

Compare gtk-demo/builder.c in the gtk sources.

In Gtk 3 it seems that gtk_widget_insert_action_group only helps to trigger actions that are specified in some .ui file via action-name of a GtkActionable. From the docs:

Inserts group into widget. Children of widget that implement GtkActionable can then be associated with actions in group by setting their “action-name” to prefix.action-name.

[rolandlo]

I don't think that different actions should be merged with the action being executed depending on whether some widget is active or not.

Agreed, my proposal was naive. The point is that two GAction's can have the same accelerator as long as they are not in the same GActionMap. In this Ctrl++ example, there is a widget W catching every keyboard input on behalf of the TextEditor. A GAction added to a GActionMap added to this widget W would only be triggered by its accelerator Ctrl++ if the widget W (or one of its descendents) has the focus (I think). Otherwise, the Ctrl++ sequence should be passed up all the way to the GtkApplicationWindow's GActionMap, triggering a zoom in sequence.

How is this implemented in Gtk 3?

Looks like in Gtk 4, one can

* insert a G_ACTION_GROUP containing some action with name `action-name` on a widget via `gtk_widget_insert_action_group` using some `prefix`

* Create a `GtkShortcutController` and add a shortcut via

gtk_shortcut_controller_add_shortcut (GTK_SHORTCUT_CONTROLLER (controller),
           gtk_shortcut_new (gtk_keyval_trigger_new (key, modifier),
                             gtk_named_action_new ("prefix.action-name")));

* Set the scope of the `GtkShortcutController` to GTK_SHORTCUT_SCOPE_LOCAL and add it to the widget via `gtk_widget_add_controller`.

* Then according to the docs

With GTK_SHORTCUT_SCOPE_LOCAL, shortcuts will only be activated when the widget has focus.

Compare gtk-demo/builder.c in the gtk sources.

In Gtk 3 it seems that gtk_widget_insert_action_group only helps to trigger actions that are specified in some .ui file via action-name of a GtkActionable. From the docs:

Inserts group into widget. Children of widget that implement GtkActionable can then be associated with actions in group by setting their “action-name” to prefix.action-name.

I tested this in python in Gtk4: app.zip

Yes, the same shortcut can be used for different actions when they are added to different widgets. In case one of the widgets is a child of the other one, the action for the child is activated.
In the python code I have a window with four buttons. I added different actions to the window, to the first button and to the second button and used the <Ctrl>+v accelerator for all of them. Now when I press that accelerator:

• In case the first button is focused, the action for the first button is activated
• In case the second button is focused, the action for the second button is activated
• In case one of the other buttons is focused, the action for the window is activated

Accelerators added via gtk_application_set_accels_for_action overwrite any shortcuts added to widgets, as far as I observed.

In the case of plus it should be possible to activate different actions depending on which widget is focused. If text is entered, the dummy text widget should have focus, otherwise a different widget contained in the main window.