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

gui: Add CMD+W shortcut in macOS #15768

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@IPGlider
Copy link
Contributor

IPGlider commented Apr 7, 2019

CMD+W is the standard shortcut in macOS to close a window without
exiting the program.

This adds support to use the shortcut in both main and debug windows.

@fanquake fanquake added GUI macOS labels Apr 7, 2019

@jonasschnelli

This comment has been minimized.

Copy link
Member

jonasschnelli commented Apr 8, 2019

@promag
Copy link
Member

promag left a comment

Pressing ESC also closes the dialogs (except the main window). Is this something to keep in macos? (not suggesting it though, just want to mention the redundancy)

@@ -409,6 +409,10 @@ void BitcoinGUI::createActions()

connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_C), this), &QShortcut::activated, this, &BitcoinGUI::showDebugWindowActivateConsole);
connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_D), this), &QShortcut::activated, this, &BitcoinGUI::showDebugWindow);

#ifdef Q_OS_MAC

This comment has been minimized.

Copy link
@promag

promag Apr 8, 2019

Member

Do you mind moving this to something like GUIUtil::handleHideShortcut(QWdiget*)? There are dialogs, like coin control dialog, transaction details, that could use the same feature.

@@ -409,6 +409,10 @@ void BitcoinGUI::createActions()

connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_C), this), &QShortcut::activated, this, &BitcoinGUI::showDebugWindowActivateConsole);
connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_D), this), &QShortcut::activated, this, &BitcoinGUI::showDebugWindow);

#ifdef Q_OS_MAC
connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::Key_W), this), &QShortcut::activated, this, [this]() { this->hide(); });

This comment has been minimized.

Copy link
@promag

promag Apr 8, 2019

Member

No need to lambda, could just be ... , &QShortcut::activated, this, &QWidget::hide)?

@practicalswift

This comment has been minimized.

Copy link
Member

practicalswift commented Apr 9, 2019

FWIW: I've verified that a disassembly of the bitcoind binary built on a Ubuntu 18.04 machine with this patch applied is identical to a disassembly of the bitcoind binary built against master (as expected).

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Apr 10, 2019

utACK, ideally this kind of stuff would be handled by the cross-platform windowing system, so it's a bit disappointing that it's not, but also, this is only a few lines and increases user friendliness.

BTW: CTRL-W is also quite a common shortcut for closing windows in applications outside of OS-X, for example in browsers it closes the tab.

@IPGlider

This comment has been minimized.

Copy link
Contributor Author

IPGlider commented Apr 10, 2019

I will do the suggested changes and add the functionality to all the windows.

Thanks for the review.

@laanwj

This comment has been minimized.

Copy link
Member

laanwj commented Apr 10, 2019

QShortcut has a Qt::ApplicationShortcut for application-global shortcuts (see https://doc.qt.io/Qt-5/qt.html#ShortcutContext-enum )—would this be useful here?

@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Apr 18, 2019

Any reason not to enable this on all platforms?

gui: Add CMD+W shortcut in macOS
CMD+W is the standard shortcut in macOS to close a window without
exiting the program.

@IPGlider IPGlider force-pushed the IPGlider:add_cmd_w_support_in_macos branch from 20859ae to 263c383 Apr 19, 2019

@IPGlider

This comment has been minimized.

Copy link
Contributor Author

IPGlider commented Apr 19, 2019

I have updated the PR with the given suggestions and added the shortcut to most windows.

The only one without this shortcut is the 'About Qt' as it is part of Qt and I have not been able to add the shortcut to it.

If I have missed any other window please let me know.

@laanwj Qt::ApplicationShortcut does not work for the intended purpose. It makes the shortcut works even if it is not focused.

@luke-jr CMD+W is usually only used on macOS, more information can be found in the macOS HID posted by @jonasschnelli and in Wikepedia's Table of keyboard shortcuts.

@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Apr 19, 2019

I use Ctrl+W on Linux all the time to close browser tabs. My IRC client seems to use it for closing tabs too.

Furthermore, if macOS is using it, we can't use it for anything else (eg, closing wallets?).

So overall, I'd say it makes sense to just support it on all platforms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.