Add keyboard shortcuts to context menus

luke-jr commented at 7:42 pm on June 12, 2021: member

Various keyboard shortcuts were lost in #263; this restores them, and also adds new ones for other context menus.

Note that with a context menu open, simply the shortcut by itself (no Alt) is used.

GUI: Restore keyboard shortcuts for context menu entries

This partially reverts f385ad765174afb02e60900581612a19c143cf83.

02b5263cd4

GUI: Add keyboard shortcuts for other context menus 94e7cdd7e0

hebasto commented at 7:47 pm on June 12, 2021: member

Note that with a context menu open, simply the shortcut by itself (no Alt) is used.

As pointed by @luke-jr on IRC:

<luke-jr> no idea, it’s an OS behaviour, not Qt-specific

hebasto requested review from jarolrod on Jun 12, 2021

hebasto requested review from promag on Jun 12, 2021

hebasto added the label Bug on Jun 12, 2021

hebasto added the label UI on Jun 12, 2021

hebasto added this to the milestone 22.0 on Jun 12, 2021

in src/qt/rpcconsole.cpp:680 in 94e7cdd7e0 outdated

680-        peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 day"), [this] { banSelectedNode(60 * 60 * 24); });
681-        peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 week"), [this] { banSelectedNode(60 * 60 * 24 * 7); });
682-        peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 year"), [this] { banSelectedNode(60 * 60 * 24 * 365); });
683+        peersTableContextMenu->addAction(tr("&Disconnect"), this, &RPCConsole::disconnectSelectedNode);
684+        peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 &hour"), [this] { banSelectedNode(60 * 60); });
685+        peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 &day"), [this] { banSelectedNode(60 * 60 * 24); });

jarolrod commented at 7:56 pm on June 13, 2021:

This causes a collision with &Disconnect on line 678, lets avoid collisions:

0        peersTableContextMenu->addAction(ts.ban_for + " " + tr("1 d&ay"), [this] { banSelectedNode(60 * 60 * 24); });

hebasto commented at 6:00 am on June 14, 2021:

It reminds me #129.

jarolrod commented at 8:53 pm on June 13, 2021: member

Concept ACK

This introduces mnemonic shortcuts for the context menu actions: QShortcut Documentation

On certain widgets, using ‘&’ in front of a character will automatically create a mnemonic (a shortcut) for that character, e.g. “E&xit” will create the shortcut Alt+X (use ‘&&’ to display an actual ampersand). The widget might consume and perform an action on a given shortcut.

This is a UX improvement and improves the accessibility of the GUI as well as convenience.

Tested functionality on Arch Linux Qt 5.15.2, Windows 7,8,10 cross-compiled from Linux, and macOS 11.3 Qt 5.15.2.

Linux

Mnemonic shortcuts work well. I need to hit ALT in order to learn about what the shortcuts are. Because of this, the OP is misleading in saying:

Note that with a context menu open, simply the shortcut by itself (no Alt) is used. A user needs to know about a shortcut before they can use it, and to know about it ALT is required.

Master	PR

Windows 7-10

The mnemonics work on Windows 7,8, and 10. But again, ALT is required to learn about the shortcuts.

Master	PR

macOS

mnemonics aren’t really supported on macOS, so it is ok that they do not work well on macOS:

However, because mnemonic shortcuts do not fit in with Aqua’s guidelines, Qt will not show the shortcut character underlined.

If there is no mechanism to highlight the shortcut, then it is useless as a user would need to dig through the source code to learn about them.

Testing on macOS, the mnemonics work if you know the correct key, press it, then press enter. This is surprising as Qt docs states:

On Mac, shortcuts are disabled by default. Call qt_set_sequence_auto_mnemonic() to enable them.

I see no difference in behavior applying the following patch, so i think this is a poorly documented part of Qt and must have changed:

 0diff --git a/src/qt/bitcoingui.cpp b/src/qt/bitcoingui.cpp
 1index 3d632ec70..3266ca13c 100644
 2--- a/src/qt/bitcoingui.cpp
 3+++ b/src/qt/bitcoingui.cpp
 4@@ -28,6 +28,7 @@
 5 
 6 #ifdef Q_OS_MAC
 7 #include <qt/macdockiconhandler.h>
 8+    extern void qt_set_sequence_auto_mnemonic(bool b);
 9 #endif
10 
11 #include <functional>
12@@ -214,6 +215,7 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty
13 
14 #ifdef Q_OS_MAC
15     m_app_nap_inhibitor = new CAppNapInhibitor;
16+    qt_set_sequence_auto_mnemonic(true);
17 #endif

jarolrod commented at 8:54 pm on June 13, 2021: member

@hebasto

hebasto added this to the 22.0 milestone yesterday

What about the translation freeze?

hebasto commented at 6:05 am on June 14, 2021: member

@jarolrod

Testing on macOS, the mnemonics work if you know the correct key, press it, then press enter. This is surprising as Qt docs states:

On Mac, shortcuts are disabled by default. Call qt_set_sequence_auto_mnemonic() to enable them.

Sound like a Qt bug :)

hebasto added this to the 22.0 milestone yesterday

What about the translation freeze?

From https://github.com/bitcoin/bitcoin/issues/20851:

2021-06-01 heavy_check_mark

* Open Transifex translations for 22

* Soft translation string freeze (no large or unnecessary string changes until release)

* Finalize and close translations for 0.20

2021-06-15 construction

* Feature freeze (bug fixes only until release)

* Translation string freeze (no more source language changes until release)

As this PR fixes the behavior regression introduced in #263, it could be considered as “necessary string changes”.

hebasto commented at 6:23 am on June 14, 2021: member

Approach ACK 94e7cdd7e04ca79f89474df1400e1a189e068939, modulo #362 (review).

Bugfix: GUI: Use a different shortcut for "1 d&ay" banning, due to conflict with "&Disconnect" e4c916a0ea

hebasto approved

hebasto commented at 7:44 am on June 14, 2021: member

ACK e4c916a0ea0637f4a765b1cb57ee10abef6fb4d6, tested on Linux Mint 20.1 (Qt 5.12.8).

Also checked that 02b5263cd4e02aa540cab35c2bf6cf9eda3522ae reverts 79311750b58d650d49a3f0edd59d31dd132ab8c0.

jarolrod commented at 6:02 pm on June 14, 2021: member

Code Review ACK e4c916a

hebasto merged this on Jun 14, 2021

hebasto closed this on Jun 14, 2021

sidhujag referenced this in commit 69e3599eaf on Jun 16, 2021

hebasto referenced this in commit d3203a99d8 on Aug 23, 2021

gwillen referenced this in commit a92037772b on Jun 1, 2022

bitcoin-core locked this on Aug 16, 2022

Add keyboard shortcuts to context menus #362

Linux

Windows 7-10

macOS

2021-06-01 heavy_check_mark

2021-06-15 construction