qt: Add Window menu #14573

pull promag wants to merge 3 commits into bitcoin:master from promag:2018-10-overhaul-menus changing 3 files +70 −9
  1. promag commented at 3:05 PM on October 25, 2018: member

    Overall this PR does the following:

    • add top level menu Window
    • add Minimize and Zoom actions to Window menu
    • move Sending/Receiving address to Window
    • remove Help->Debug window
    • add one menu entry for each debug window tab

    This removes the access to address book from the File menu.

    With wallet support: <img width="522" alt="screenshot 2018-12-11 at 00 33 05" src="https://user-images.githubusercontent.com/3534524/49770451-5bec0800-fcdc-11e8-91d6-f8f850ead92d.png">

    Without wallet support: <img width="593" alt="screenshot 2018-12-11 at 12 55 21" src="https://user-images.githubusercontent.com/3534524/49802183-19f6ac80-fd44-11e8-9973-36fcfb4f129e.png">

  2. fanquake added the label GUI on Oct 25, 2018
  3. in src/qt/bitcoingui.cpp:387 in 87ea35c369 outdated 
     386 | -        file->addSeparator();
 387 |      }
 388 |      file->addAction(quitAction);
 389 |  
 390 | -    QMenu *settings = appMenuBar->addMenu(tr("&Settings"));
 391 | +    QMenu *settings = appMenuBar->addMenu(tr("&Wallet"));
    ken2812221 commented at 2:20 PM on October 27, 2018:

    I would suggest to put this into if(walletFrame)

  4. ken2812221 commented at 2:20 PM on October 27, 2018: contributor

    Concept ACK

  5. hebasto commented at 2:37 PM on October 28, 2018: member

    Concept ACK. Could move "Backup Wallet..." from File menu to Wallet menu? Agree with @Sjors's comment

    From Apple's Human Interface Guidelines:

    Provide a Window menu even if your app has only one window. Include the Minimize and Zoom menu items so people using Full Keyboard Access can invoke these functions using their keyboard.

  6. promag renamed this:
    qt: Add Wallet and Window menus
    qt: Add Window menus
    on Oct 30, 2018
  7. promag renamed this:
    qt: Add Window menus
    qt: Add Window menu
    on Oct 30, 2018
  8. DrahtBot commented at 11:47 PM on October 30, 2018: 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:

    • #14879 (qt: Add warning messages to the debug window by hebasto)
    • #9849 (Qt: Network Watch tool by luke-jr)

    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.

  9. promag force-pushed on Oct 31, 2018
  10. promag force-pushed on Oct 31, 2018
  11. promag commented at 4:19 PM on October 31, 2018: member

    Removed the changes to the "Settings" menu.

  12. laanwj commented at 2:47 PM on November 12, 2018: member

    I don't know what the conventions around this are, nowadays, but Concept ACK.

    Edit: I don't understand why "address book" would belong in the Window menu, it doesn' seem window related functionality but wallet related functionality! oh, because it's an auxiliary window

  13. meshcollider commented at 2:49 PM on November 12, 2018: contributor

    Concept ACK, I would slightly prefer the name "View" but this is fine with me too

  14. jonasschnelli commented at 8:28 AM on November 13, 2018: contributor

    Tested a bit. I think the "minimize" entry should toggle to "maximise" once minimized or at least maximises when selecting "minimize" in already minimized state. Rest looks fine.

  15. promag commented at 8:34 AM on November 13, 2018: member

    Thanks @jonasschnelli, I'll see if others also toggle.

    I have to test in windows.

  16. promag commented at 3:34 PM on November 14, 2018: member

    @jonasschnelli on macos, if the active window is minimized then "Minimize" action is disabled. And "Zoom" action toggles between maximized and normal.

    Also, some software add the main window to this menu, and I think we could too.

  17. Sjors commented at 2:19 PM on November 19, 2018: member

    @hebasto wrote:

    Could move "Backup Wallet..." from File menu to Wallet menu?

    Why? Loading, saving and backing up the wallet seem like document / File operations.

    Moving message signing and verification from File to Window would make more sense to me.

    Even more clear IMO, but haven't thought very deeply about it yet, is to have a Wallet top level menu. In that case I'd be more in favour of adding Backup into that, as well as sign/verify, Sending/Receiving address. The Window menu would be used for Minimize, Debug Window.

    I would also be fine with dropping the "Address Book" level and just having direct menu items for Sending and Receiving addresses.

    Definitely out of scope, but I would still like to see each wallet represented as a window (rather than the little dropdown we have now). However I wouldn't want closing a window to cause unloading a wallet, so it's not obvious how this would work.

  18. hebasto commented at 9:32 PM on November 22, 2018: member

    IMO, it would be better to avoid using native widgets (Native Widgets vs Alien Widgets) and related methods (QWidget::windowHandle()) and objects (QWindow). @Sjors

    Why? Loading, saving and backing up the wallet seem like document / File operations.

    Agree. @jonasschnelli

    I think the "minimize" entry should toggle to "maximise" once minimized or at least maximises when selecting "minimize" in already minimized state. @promag on macos, if the active window is minimized then "Minimize" action is disabled. And "Zoom" action toggles between maximized and normal.

    On macOS, if the active window is minimized the both "Minimize" and "Zoom" actions should be disabled. That is not the case for 3f779b3b99b674b7beecc9a30fa37bda1d722387.

  19. in src/qt/bitcoingui.cpp:400 in 3f779b3b99 outdated 
     393 | @@ -396,11 +394,32 @@ void BitcoinGUI::createMenuBar()
 394 |      }
 395 |      settings->addAction(optionsAction);
 396 |  
 397 | -    QMenu *help = appMenuBar->addMenu(tr("&Help"));
 398 | +    QMenu* window_menu = appMenuBar->addMenu(tr("&Window"));
 399 | +    QAction* minimize_action = window_menu->addAction(tr("Minimize"), this, &QMainWindow::showMinimized, QKeySequence(Qt::CTRL + Qt::Key_M));
 400 | +    QObject::connect(windowHandle(), &QWindow::windowStateChanged, [this, minimize_action] {
 401 | +        minimize_action->setEnabled(windowHandle()->windowState() != Qt::WindowMinimized);
    hebasto commented at 9:59 PM on November 22, 2018:

    From Qt docs:

    The window state is a OR'ed combination of Qt::WindowState: Qt::WindowMinimized, Qt::WindowMaximized, Qt::WindowFullScreen, and Qt::WindowActive.

    IMO, comparing windowState() without bitmasking is error prone.

    Could it be just: minimize_action->setEnabled(isMinimized());

    promag commented at 12:09 AM on December 11, 2018:

    Does not apply now since it's using QWindow now.

  20. in src/qt/bitcoingui.cpp:404 in 3f779b3b99 outdated 
     400 | +    QObject::connect(windowHandle(), &QWindow::windowStateChanged, [this, minimize_action] {
 401 | +        minimize_action->setEnabled(windowHandle()->windowState() != Qt::WindowMinimized);
 402 | +    });
 403 | +#ifdef Q_OS_MAC
 404 | +    window_menu->addAction(tr("Zoom"), [this] {
 405 | +        if (windowHandle()->windowState() == Qt::WindowMaximized) {
    hebasto commented at 10:00 PM on November 22, 2018:

    The same: if (isMaximized()) {

    promag commented at 12:09 AM on December 11, 2018:

    Same as above.

  21. hebasto changes_requested
  22. promag commented at 12:11 AM on December 11, 2018: member

    Updated after feedback, thank you! Also updated screenshot. Windows/linux feedback would be awesome.

  23. promag force-pushed on Dec 11, 2018
  24. promag commented at 12:23 AM on December 11, 2018: member

    @Sjors

    Definitely out of scope, but I would still like to see each wallet represented as a window (rather than the little dropdown we have now)

    I think in the future we could have multiple windows, each could have multiple wallets.

  25. promag commented at 12:32 AM on December 11, 2018: member

    Pushed a commit to remove ellipsis from the sending/receiving addresses menu actions. Please see rationale in https://stackoverflow.com/a/637708 (also updated screenshot)

  26. Sjors commented at 11:31 AM on December 11, 2018: member

    tACK 6c72f91

    A few nits, but don't let that stop anyone from merging:

    • you could move Sign & Verify Message to Window (they do need ellipses)
    • I don't find the Zoom item very useful, but don't mind it either (I'm just not a Bitcoin Core Window Maximalist)
    • when ./configure --disable-wallet or disablewallet=1 you could still show Information, Console, Network Traffic and Peers in the Window menu. There may be some accessibility advantage to that.
  27. promag commented at 11:39 AM on December 11, 2018: member
    • you could move Sign & Verify Message to Window (they do need ellipses)

    I'll leave that for later after we decide about top level menu "Wallet".

    • I don't find the Zoom item very useful, but don't mind it either (I'm just not a Bitcoin Core Window Maximalist)

    Minimize and Zoom operate on the focus window, which can be the main window, address book dialogs, debug window..

    • when ./configure --disable-wallet or disablewallet=1 you could still show Information, Console, Network Traffic and Peers in the Window menu.

    I'll update with this suggestion, thanks @Sjors.

  28. promag force-pushed on Dec 11, 2018
  29. qt: Allow to inspect RPCConsole tabs 9ea38d0222
  30. qt: Add Window menu a96c0df35e
  31. qt: Remove ellipsis from sending/receiving addresses
    Considering https://stackoverflow.com/a/637708 the ellipsis in these
menu actions should be removed.
    95a5a9fccb
  32. promag force-pushed on Dec 11, 2018
  33. Sjors commented at 5:01 PM on December 11, 2018: member

    tACK 9ea38d0

    git range-diff `git merge-base --all HEAD 6c72f91`...6c72f91 HEAD~3...HEAD

    <img width="629" alt="schermafbeelding 2018-12-11 om 17 39 04" src="https://user-images.githubusercontent.com/10217/49816643-9dc58e80-fd6e-11e8-919d-fa451cf1ebe0.png">

  34. jonasschnelli commented at 7:24 PM on December 11, 2018: contributor

    Tested ACK 9ea38d022281713e7f79a219b37651ac5648d695

  35. meshcollider added this to the "Blockers" column in a project
  36. promag commented at 6:04 PM on December 13, 2018: member

    @ken2812221 do you think you have the time to test this?

  37. meshcollider commented at 1:20 AM on December 14, 2018: contributor

    Tested ACK https://github.com/bitcoin/bitcoin/pull/14573/commits/9ea38d022281713e7f79a219b37651ac5648d695 on windows 10

    I'm a little confused about the "Main Window" option, isn't the menu part of the main window implying that it is already focused? Also perhaps "Restore" should toggle between "Maximise" & "Restore"?

    image

  38. promag commented at 1:43 AM on December 14, 2018: member

    @MeshCollider thanks!

    This was inspired on macos since it has a menu bar per application, not per window. Forgot that in windows it has a menu bar in each window..

    So next changes will be:

    • first section are actions on the current window (focused window on macos)
    • then comes the main windows — we only allow 1 main window for now — and this is used to bring it to front (I'll remove in windows, maybe linux too?)
    • then comes some dialogs — address book for now
    • then comes the debug dialogs
  39. Sjors commented at 10:11 AM on December 15, 2018: member

    Rather than explicitly handling this per OS, does QT have any way to abstract that?

  40. promag commented at 1:15 PM on December 15, 2018: member

    Not that I'm aware of.

  41. jonasschnelli merged this on Dec 16, 2018
  42. jonasschnelli closed this on Dec 16, 2018
  43. jonasschnelli referenced this in commit dba0f4c5c7 on Dec 16, 2018
  44. fanquake removed this from the "Blockers" column in a project
  45. promag deleted the branch on Dec 16, 2018
  46. MarcoFalke commented at 12:40 AM on December 17, 2018: member

    This does not compile because the addAction function was only introduced in Qt 5.6.

  47. Empact commented at 4:10 AM on December 17, 2018: member

    An example of the failure: https://travis-ci.org/bitcoin/bitcoin/jobs/468706526 Min Qt version is 5.2, haven't found those docs readily available but the 5.5 QMenu api is here: http://doc.qt.io/archives/qt-5.5/qmenu.html

  48. laanwj referenced this in commit bfd7e54097 on Dec 17, 2018
  49. fanquake added the label Needs release notes on Dec 17, 2018
  50. luke-jr referenced this in commit a48491882a on Dec 24, 2018
  51. luke-jr referenced this in commit cad5bfd061 on Dec 24, 2018
  52. luke-jr referenced this in commit ca546a5779 on Dec 24, 2018
  53. luke-jr referenced this in commit b992e517ad on Dec 26, 2018
  54. luke-jr referenced this in commit 697622ff1d on Dec 26, 2018
  55. fanquake removed the label Needs release note on Mar 20, 2019
  56. molxyz commented at 8:05 PM on April 23, 2019: none

    Hello, I also have a question on that button Main Window on windows bitcoin. If it has no purpose please remove that button before the official release to avoid confusions for windows users.

    Thank you.

    Please remove this button:

    image

  57. promag commented at 8:39 PM on April 23, 2019: member

    @molxyz from the above #14573 (comment)

    • then comes the main windows — we only allow 1 main window for now — and this is used to bring it to front (I'll remove in windows, maybe linux too?)

    Sorry for forgetting about this, but at this stage I'm afraid it won't be removed for 0.18 (unless we have to do RC5).

  58. molxyz commented at 8:44 PM on April 23, 2019: none

    @promag Hi, I'll be happy to run tests for RC5. Thanks.

  59. laanwj referenced this in commit 75656988ac on Aug 1, 2019
  60. jonasschnelli referenced this in commit b265ffe323 on Oct 10, 2019
  61. sidhujag referenced this in commit 70b4e7317f on Oct 11, 2019
  62. jasonbcox referenced this in commit 066a318ae6 on Oct 12, 2020
  63. jasonbcox referenced this in commit b01ba2ed25 on Oct 13, 2020
  64. deadalnix referenced this in commit 326b3c7453 on Oct 28, 2020
  65. ftrader referenced this in commit 459b3bb5d0 on Apr 14, 2021
  66. ftrader referenced this in commit b515ce60ff on Apr 14, 2021
  67. ftrader referenced this in commit 401d66d279 on Apr 14, 2021
  68. linuxsh2 referenced this in commit 0ccfb45431 on Jul 30, 2021
  69. Munkybooty referenced this in commit 1da9de31f6 on Aug 3, 2021
  70. christiancfifi referenced this in commit 6733a11bdc on Aug 24, 2021
  71. christiancfifi referenced this in commit ea528f8998 on Aug 24, 2021
  72. christiancfifi referenced this in commit 1b04658278 on Aug 24, 2021
  73. christiancfifi referenced this in commit 59933c0265 on Aug 24, 2021
  74. christiancfifi referenced this in commit e7b85714c6 on Aug 25, 2021
  75. christiancfifi referenced this in commit 269df516a8 on Aug 25, 2021
  76. christiancfifi referenced this in commit b90cc3fff9 on Aug 25, 2021
  77. christiancfifi referenced this in commit 49cfdcf0f9 on Aug 25, 2021
  78. christiancfifi referenced this in commit c77cc5d5b2 on Aug 25, 2021
  79. christiancfifi referenced this in commit 39d1ef864f on Aug 25, 2021
  80. christiancfifi referenced this in commit 5365c2f0a4 on Aug 25, 2021
  81. christiancfifi referenced this in commit ca2508fb30 on Aug 25, 2021
  82. christiancfifi referenced this in commit f6fe83b7fe on Aug 26, 2021
  83. christiancfifi referenced this in commit 4dfc1832e7 on Aug 26, 2021
  84. christiancfifi referenced this in commit e9956d267e on Aug 26, 2021
  85. christiancfifi referenced this in commit ef4fe3e783 on Aug 28, 2021
  86. christiancfifi referenced this in commit 706241de39 on Aug 28, 2021
  87. christiancfifi referenced this in commit f97a0e8fb1 on Aug 28, 2021
  88. christiancfifi referenced this in commit d71ced389b on Aug 29, 2021
  89. christiancfifi referenced this in commit 6b44e6e0dc on Aug 29, 2021
  90. christiancfifi referenced this in commit 4e02ba7ff7 on Aug 29, 2021
  91. christiancfifi referenced this in commit fb23bb0b89 on Aug 29, 2021
  92. christiancfifi referenced this in commit f78179d790 on Aug 29, 2021
  93. christiancfifi referenced this in commit 67f109eaee on Aug 29, 2021
  94. christiancfifi referenced this in commit 1df9a7ff22 on Aug 29, 2021
  95. christiancfifi referenced this in commit e8cc282214 on Aug 29, 2021
  96. christiancfifi referenced this in commit 146871ad57 on Aug 29, 2021
  97. christiancfifi referenced this in commit 9e51f2c971 on Sep 10, 2021
  98. christiancfifi referenced this in commit 44d8bfc018 on Sep 10, 2021
  99. christiancfifi referenced this in commit 97f7a88acd on Sep 10, 2021
  100. christiancfifi referenced this in commit 5948eff6b5 on Sep 10, 2021
  101. christiancfifi referenced this in commit 7277d60191 on Sep 10, 2021
  102. christiancfifi referenced this in commit 29baf3335c on Sep 10, 2021
  103. christiancfifi referenced this in commit cb54330856 on Sep 10, 2021
  104. christiancfifi referenced this in commit 50cc786850 on Sep 10, 2021
  105. christiancfifi referenced this in commit 9cbbc3176c on Sep 10, 2021
  106. christiancfifi referenced this in commit 22aa2f8748 on Sep 11, 2021
  107. christiancfifi referenced this in commit 550afc39b5 on Sep 11, 2021
  108. christiancfifi referenced this in commit 78fbde01bb on Sep 11, 2021
  109. christiancfifi referenced this in commit 2662672a9b on Sep 11, 2021
  110. christiancfifi referenced this in commit a05ada38a9 on Sep 11, 2021
  111. christiancfifi referenced this in commit 3056af473c on Sep 11, 2021
  112. christiancfifi referenced this in commit 60cfe10074 on Sep 11, 2021
  113. Munkybooty referenced this in commit 72d7426df6 on Nov 25, 2021
  114. Munkybooty referenced this in commit b08bcee9fd on Nov 30, 2021
  115. DrahtBot locked this on Dec 16, 2021
  116. gades referenced this in commit 713b14781e on May 9, 2022
  117. gades referenced this in commit 62aecff410 on May 9, 2022
Contributors
promagken2812221hebastoDrahtBotlaanwjmeshcolliderjonasschnelliSjorsMarcoFalkeEmpactmolxyz
Labels
GUI
Linked (view graph)
#14979 [Qt] Restore < Qt5.6 compatibility for addAction#14983 One Travis instance should run minimum supported QT version#15023 GUI: Restore RPC Console to non-wallet tray icon menu#16514 gui: Remove unused RPCConsole::tabFocus
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 15:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me