Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[macOS]: display keyboard accelerators in the macOS menu #18067

Closed
wants to merge 2 commits into from

Conversation

zisoft
Copy link
Collaborator

@zisoft zisoft commented Dec 25, 2024

fixes #14484
fixes #18051

  • The keyboard accelerators are displayed in the application menu
  • Register shortcut CMD+, to open the preferences

This is only for macOS, wrapped in #ifdef MAC_INTEGRATION, so no effects on other OS.

Bildschirmfoto 2024-12-25 um 09 22 45

Bildschirmfoto 2024-12-25 um 11 23 17

- The keyboard accelerators are displayed in the application menu
- Register shortcut CMD+, to open the preferences
@zisoft zisoft added the scope: macos support macos related issues and PR label Dec 25, 2024
According to the Apple Human Interface Guidelines, menu items need to
use title-style-capitalization.
@zisoft zisoft force-pushed the macos-menu-shortcuts branch from 2b3d8a7 to 9a8531c Compare December 25, 2024 10:22
@dterrahe
Copy link
Member

I can't test since I don't have access to the platform, but I assume that adding the accel group to the main window overrides dt's shortcut system for those keys (disabling the ability to reassign or overload, with double click etc). Instead, you could just show the (default) assigned keys, using

static void _osx_add_view_menu_item(GtkWidget* menu,
                                    const char* label,
                                    gpointer mode,
                                    guint key)
{
  GtkWidget *mi = gtk_menu_item_new_with_label(label);
  gtk_accel_label_set_accel(GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(mi))), key, 0);
  gtk_menu_shell_append(GTK_MENU_SHELL (menu), mi);
  gtk_widget_show(mi);
  g_signal_connect(G_OBJECT(mi), "activate",
                   G_CALLBACK(_osx_ctl_switch_mode_to), mode);
}

but have dt handle the actual shortcuts. (Of course this should be updated if the user deletes/reassigns the shortcuts, but that's too much of a hassle for this one case...)

To define a darktable shortcut for preferences, extend the setup for the button in global_toolbox.c
to

  ac = dt_action_define(&darktable.control->actions_global, NULL, N_("preferences"), d->preferences_button, &dt_action_def_button);
  dt_shortcut_register(ac, 0, DT_ACTION_EFFECT_ACTIVATE, GDK_KEY_comma, GDK_META_MASK);

(the second line conditional on #ifdef MAC_INTEGRATION) and add

gtk_accel_label_set_accel(GTK_ACCEL_LABEL(gtk_bin_get_child(GTK_BIN(mi_prefs))), GDK_KEY_comma, GDK_META_MASK);

instead of gtk_widget_add_accelerator in gtk.c

@zisoft
Copy link
Collaborator Author

zisoft commented Dec 27, 2024

Instead, you could just show the (default) assigned keys, using

@dterrahe: I tried that but without an accel group I cannot get the accelerators to show.

According to the documentation for AccelLabel:

A GtkAccelLabel will only display accelerators which have GTK_ACCEL_VISIBLE set (see GtkAccelFlags)

and the AccelFlags are set by gtk_accel_group_connect()

@dterrahe
Copy link
Member

dterrahe commented Dec 28, 2024

But also

The accelerator key to display is typically not set explicitly (although it can be, with gtk_accel_label_set_accel()).

On linux, it is sufficient to do just that, as long as you don't also do gtk_widget_add_accelerator. But maybe on Mac it clears the manual text if an accel widget is set and it doesn't have an accelerator. Again, per doc

Note that creating a GtkMenuItem with gtk_menu_item_new_with_label() [...] automatically adds a GtkAccelLabel to the GtkMenuItem and calls gtk_accel_label_set_accel_widget()

But then clearing the accel widget should work, so I've done that in the POC implementation in a5a4965, which adds a menu to a linux window just for demonstration purposes.
image

The "meta" is just added to show how you would deal with the "preferences" shortcut here (on top of the dt_shortcut_register call I showed before) and that linux doesn't (obviously) use the special Mac squiggly. Again, on linux I don't need to call gtk_accel_label_set_accel_widget(al, NULL); but maybe that makes a difference for Mac.

@zisoft
Copy link
Collaborator Author

zisoft commented Dec 28, 2024

Just spent again some effort and tried all your suggestions, unfortunately without success.
The accelerators are only visible if an accel group is set.

Since this blocks the possibility of reassigning I think it is best to give up on showing the accelerators and better keep the functionality.

So I close this PR and open a new one just to register CMD+, for the preferences on macOS.

Thanks for all your hints @dterrahe

@zisoft zisoft closed this Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: macos support macos related issues and PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce Cmd+, shortcut to open settings on macOS cmd-, does not open preferences on Macosx
2 participants