gui: Add close window shortcut #15768

pull IPGlider wants to merge 1 commits into bitcoin:master from IPGlider:add_cmd_w_support_in_macos changing 15 files +41 −0
  1. IPGlider commented at 7:54 PM on April 7, 2019: contributor

    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.

  2. fanquake added the label GUI on Apr 7, 2019
  3. fanquake added the label macOS on Apr 7, 2019
  4. jonasschnelli commented at 9:20 AM on April 8, 2019: contributor

    Thanks! Tested ACK 20859aef8b571542772a42b4c413c4a71a6576c5

    The macOS HIG about Command+W: https://developer.apple.com/design/human-interface-guidelines/macos/user-interaction/keyboard/

  5. in src/qt/bitcoingui.cpp:413 in 20859aef8b outdated 
     408 | @@ -409,6 +409,10 @@ void BitcoinGUI::createActions()
 409 |  
 410 |      connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_C), this), &QShortcut::activated, this, &BitcoinGUI::showDebugWindowActivateConsole);
 411 |      connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_D), this), &QShortcut::activated, this, &BitcoinGUI::showDebugWindow);
 412 | +
 413 | +#ifdef Q_OS_MAC
    promag commented at 1:58 PM on April 8, 2019:

    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.

  6. in src/qt/bitcoingui.cpp:414 in 20859aef8b outdated 
     408 | @@ -409,6 +409,10 @@ void BitcoinGUI::createActions()
 409 |  
 410 |      connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_C), this), &QShortcut::activated, this, &BitcoinGUI::showDebugWindowActivateConsole);
 411 |      connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::SHIFT + Qt::Key_D), this), &QShortcut::activated, this, &BitcoinGUI::showDebugWindow);
 412 | +
 413 | +#ifdef Q_OS_MAC
 414 | +    connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::Key_W), this), &QShortcut::activated, this, [this]() { this->hide(); });
    promag commented at 2:00 PM on April 8, 2019:

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

  7. promag commented at 2:02 PM on April 8, 2019: member

    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)

  8. practicalswift commented at 2:29 PM on April 9, 2019: contributor

    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).

  9. laanwj commented at 10:41 AM on April 10, 2019: member

    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.

  10. IPGlider commented at 10:54 AM on April 10, 2019: contributor

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

    Thanks for the review.

  11. laanwj commented at 11:43 AM on April 10, 2019: member

    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?

  12. luke-jr commented at 6:45 PM on April 18, 2019: member

    Any reason not to enable this on all platforms?

  13. IPGlider force-pushed on Apr 19, 2019
  14. IPGlider commented at 10:26 AM on April 19, 2019: contributor

    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.

  15. luke-jr commented at 4:20 PM on April 19, 2019: member

    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.

  16. luke-jr commented at 8:59 PM on April 20, 2019: member

    Note: When extending it to support other platforms, care needs to be taken that the main window is not actually hidden if there's no system tray icon. Probably should minimise in that case? Not sure...

  17. in src/qt/utilitydialog.cpp:164 in 263c3830f6 outdated 
     159 | @@ -158,6 +160,8 @@ ShutdownWindow::ShutdownWindow(QWidget *parent, Qt::WindowFlags f):
 160 |          tr("%1 is shutting down...").arg(tr(PACKAGE_NAME)) + "<br /><br />" +
 161 |          tr("Do not shut down the computer until this window disappears.")));
 162 |      setLayout(layout);
 163 | +
 164 | +    GUIUtil::handleHideShortcut(this);
    luke-jr commented at 9:03 PM on April 20, 2019:

    Do we want this to be hide-able??

    IPGlider commented at 7:54 PM on August 5, 2019:

    The criteria I followed to make windows hide-able has been what I understand is the common usage in macOS, meaning that every window that has a close button can also be closed using the CMD+W shortcut.

  18. luke-jr commented at 9:03 PM on April 20, 2019: member

    Probably a number of windows ought to be closed normally, not merely hidden?

  19. in src/qt/guiutil.cpp:377 in 263c3830f6 outdated 
     370 | @@ -370,6 +371,13 @@ void bringToFront(QWidget* w)
 371 |      }
 372 |  }
 373 |  
 374 | +void handleHideShortcut(QWidget* w)
 375 | +{
 376 | +#ifdef Q_OS_MAC
 377 | +    w->connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::Key_W), w), &QShortcut::activated, w, &QWidget::hide);
    jonasschnelli commented at 6:55 AM on April 29, 2019:

    I haven't looked closely at this,... but is there a proper deallocation of that QShortcut?

    IPGlider commented at 7:56 PM on August 5, 2019:

    I have followed the official documentation in this regard[0]. If you consider that this is not a correct application please let me know a more appropriate way.

    [0] https://doc.qt.io/qt-5/qobject.html

  20. hebasto commented at 9:59 AM on August 1, 2019: member

    @IPGlider Could you address reviewers' comments?

  21. fanquake added the label Up for grabs on Aug 2, 2019
  22. fanquake closed this on Aug 2, 2019
  23. IPGlider commented at 6:37 AM on August 2, 2019: contributor

    Is working in this PR still worth it? I would like to finish it. @hebasto I will address the comments in a few days.

  24. fanquake removed the label Up for grabs on Aug 2, 2019
  25. fanquake reopened this on Aug 2, 2019
  26. hebasto commented at 11:25 AM on August 3, 2019: member

    @IPGlider

    Is working in this PR still worth it?

    Yes, IMO.

  27. IPGlider commented at 7:58 PM on August 5, 2019: contributor

    @luke-jr

    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.

    I wasn't aware that this shortcut was also commonly used in other OSs other than macOS. Considering this I will remove the ifdef to support all OSs.

    Probably a number of windows ought to be closed normally, not merely hidden?

    I agree and after reading the QT documentation I will replace the hide call with a close call.

    Should I continue working on this PR considering that the name of the branch is add_cmd_w_support_in_macos or would it be preferable to create another PR?

  28. fanquake commented at 1:15 AM on August 6, 2019: member

    @IPGlider No need for a new PR. Just update the PR title, body etc if required.

  29. IPGlider renamed this:
    gui: Add CMD+W shortcut in macOS
    gui: Add close window shortcut
    on Aug 6, 2019
  30. IPGlider force-pushed on Aug 6, 2019
  31. IPGlider force-pushed on Aug 6, 2019
  32. IPGlider commented at 5:50 PM on August 6, 2019: contributor

    Two jobs failed in Travis CI during the set up:

    I have not found a way to force a re-run. Nevertheless the other jobs completed successfully.

  33. hebasto commented at 12:13 PM on August 10, 2019: member

    @IPGlider

    Two jobs failed in Travis CI...

    Restarted...

  34. in src/qt/bitcoingui.cpp:215 in ab589f0db4 outdated 
     209 | @@ -210,6 +210,8 @@ BitcoinGUI::BitcoinGUI(interfaces::Node& node, const PlatformStyle *_platformSty
 210 |  #ifdef Q_OS_MAC
 211 |      m_app_nap_inhibitor = new CAppNapInhibitor;
 212 |  #endif
 213 | +
 214 | +    GUIUtil::handleCloseWindowShortcut(this);
    hebasto commented at 1:50 PM on August 10, 2019:

    Ctrl+W should close a subwindow within an app but not the main window.

    jonasschnelli commented at 10:10 AM on October 9, 2019:

    I think it should also close the main window (at least on macOS it is the case for almost all apps).

    IPGlider commented at 8:18 AM on October 13, 2019:

    The CTRL+W/CMD+W behaviour in macOS is usually to close the focused window.

  35. in src/qt/splashscreen.cpp:11 in ab589f0db4 outdated 
       7 | @@ -8,6 +8,7 @@
   8 |  
   9 |  #include <qt/splashscreen.h>
  10 |  
  11 | +#include <qt/guiutil.h>
    hebasto commented at 1:53 PM on August 10, 2019:

    Could splash screen be untouched?

    IPGlider commented at 8:18 AM on October 13, 2019:

    The rationale for adding the shortcut here is that on every window with an enabled close button the shortcut should be applicable.

    It could be worth it taking into consideration disabling the close button or removing the bar all together.

  36. in src/qt/utilitydialog.cpp:108 in ab589f0db4 outdated 
     114 | @@ -114,6 +115,8 @@ HelpMessageDialog::HelpMessageDialog(interfaces::Node& node, QWidget *parent, bo
 115 |          ui->scrollArea->setVisible(false);
 116 |          ui->aboutLogo->setVisible(false);
 117 |      }
 118 | +
 119 | +    GUIUtil::handleCloseWindowShortcut(this);
    hebasto commented at 1:57 PM on August 10, 2019:

    The "About Qt" dialog does not accept Ctrl+W shortcut. So the "About Bitcoin Core" dialog should do for consistency.

    IPGlider commented at 8:18 AM on October 13, 2019:

    Which of the following cases is preferable?

    Two windows with their close behaviour consistent among them but inconsistent with the behaviour of the rest of them or keeping the inconsistency only on the QT window?

  37. in src/qt/utilitydialog.cpp:149 in ab589f0db4 outdated 
     155 | @@ -153,6 +156,8 @@ ShutdownWindow::ShutdownWindow(QWidget *parent, Qt::WindowFlags f):
 156 |          tr("%1 is shutting down...").arg(PACKAGE_NAME) + "<br /><br />" +
 157 |          tr("Do not shut down the computer until this window disappears.")));
 158 |      setLayout(layout);
 159 | +
 160 | +    GUIUtil::handleCloseWindowShortcut(this);
    hebasto commented at 1:59 PM on August 10, 2019:

    Could shutdown window be untouched?

    IPGlider commented at 8:19 AM on October 13, 2019:

    Same comment as for the splash screen.

  38. in src/qt/guiutil.cpp:384 in ab589f0db4 outdated 
     378 | @@ -378,6 +379,11 @@ void bringToFront(QWidget* w)
 379 |      }
 380 |  }
 381 |  
 382 | +void handleCloseWindowShortcut(QWidget* w)
 383 | +{
 384 | +    w->connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::Key_W), w), &QShortcut::activated, w, &QWidget::close);
    hebasto commented at 2:16 PM on August 10, 2019:

    QObject::connect() is a static function of QObject:

        QObject::connect(new QShortcut(QKeySequence(Qt::CTRL + Qt::Key_W), w), &QShortcut::activated, w, &QWidget::close);
  39. hebasto changes_requested
  40. DrahtBot commented at 7:32 PM on August 23, 2019: member

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #17955 (gui: Paste button in Open URI dialog by emilengler)
    • #17597 (qt: Fix height of QR-less ReceiveRequestDialog by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  41. DrahtBot added the label Needs rebase on Aug 29, 2019
  42. laanwj added the label Feature on Sep 30, 2019
  43. GChuf commented at 2:25 PM on October 4, 2019: contributor

    @IPGlider are you still working on this?

  44. jonasschnelli commented at 10:12 AM on October 9, 2019: contributor

    Needs rebase. I think this is almost done. Let's finish it @IPGlider.

  45. GChuf commented at 11:00 AM on October 9, 2019: contributor

    IPglider hasn't been active since April I think - (when) is it okay if I try to pick it up and include hebasto's suggestions?

  46. IPGlider commented at 3:29 PM on October 9, 2019: contributor

    Sorry for the delay, I will rebase and continue working on this PR this weekend.

  47. fanquake added the label Waiting for author on Oct 9, 2019
  48. GChuf commented at 3:52 PM on October 9, 2019: contributor

    Excellent, good luck!

  49. IPGlider force-pushed on Oct 13, 2019
  50. DrahtBot removed the label Needs rebase on Oct 13, 2019
  51. DrahtBot added the label Needs rebase on Oct 26, 2019
  52. IPGlider force-pushed on Nov 10, 2019
  53. DrahtBot removed the label Needs rebase on Nov 10, 2019
  54. luke-jr referenced this in commit 186e2003e8 on Nov 15, 2019
  55. MarkLTZ referenced this in commit 2b39e987ad on Nov 17, 2019
  56. fanquake removed the label Waiting for author on Dec 9, 2019
  57. DrahtBot added the label Needs rebase on Dec 9, 2019
  58. IPGlider force-pushed on Dec 22, 2019
  59. DrahtBot removed the label Needs rebase on Dec 22, 2019
  60. luke-jr referenced this in commit c29a4ec823 on Jan 5, 2020
  61. DrahtBot added the label Needs rebase on Jan 8, 2020
  62. IPGlider force-pushed on Jan 24, 2020
  63. DrahtBot removed the label Needs rebase on Jan 24, 2020
  64. DrahtBot added the label Needs rebase on Feb 1, 2020
  65. MarkLTZ referenced this in commit 136a27c84d on Feb 3, 2020
  66. gui: Add close window shortcut
    CMD+W/CTRL+W is the standard shortcut to close a window without
exiting the program.
    f5a3a5b9ab
  67. IPGlider force-pushed on Feb 5, 2020
  68. DrahtBot removed the label Needs rebase on Feb 5, 2020
  69. hebasto approved
  70. hebasto commented at 5:49 AM on May 4, 2020: member

    ACK f5a3a5b9ab362c58fa424261f313aa9cf46d2a98, tested on Linux Mint 19.3 by manually opening available dialogs and sub-windows, and applying the Ctrl+W shortcut. Also tested with "Minimize on close" option enabled / disabled.

  71. jonasschnelli commented at 9:52 AM on May 4, 2020: contributor

    Tested ACK f5a3a5b9ab362c58fa424261f313aa9cf46d2a98

  72. jonasschnelli merged this on May 4, 2020
  73. jonasschnelli closed this on May 4, 2020
  74. sidhujag referenced this in commit e0e6a4ebd0 on May 4, 2020
  75. luke-jr referenced this in commit 7d2ece8401 on Jun 9, 2020
  76. Fabcien referenced this in commit 1e8c9d8760 on Jan 26, 2021
  77. DrahtBot locked this on Aug 16, 2022
Contributors
IPGliderjonasschnellipromagpracticalswiftlaanwjluke-jrhebastofanquakeDrahtBotGChuf
Labels
FeatureGUImacOS
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-02 18:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me