qt: Clean system tray icon menu for '-disablewallet' mode #14383

hebasto · 2018-10-03T16:18:30Z

There is a Debug window leftover in the system tray icon menu after #3392 merging.
This PR makes both the app menu and the systray icon menu consistent.

maflcko · 2018-10-03T21:52:57Z

src/qt/bitcoingui.cpp

Don't forget to install clang-format and run the https://github.com/bitcoin/bitcoin/tree/master/contrib/devtools#clang-format-diffpy script to preempt whitespace nitpicking.

Right. Done.

DrahtBot · 2018-10-04T05:30:14Z

Coverage	Change (pull 14383)	Reference (master)
Lines	-0.0132 %	87.0471 %
Functions	+0.0000 %	84.1130 %
Branches	-0.0104 %	51.5403 %

promag

Tested ACK 7a002b4.

promag · 2018-10-04T23:28:29Z

src/qt/bitcoingui.cpp

Why not if (enableWallet) {?

I would prefer if (enableWallet) as well. Using walletFrame seems to be an older convention: b7f4b6d

Sjors

Before and after screenshots are very useful for this kind of change, especially for concept ACK's (let's see if BitcoinACKS falls for this.

Before:

After:

I'm confused: why would you remove the debug window menu item? It would make more sense to me to remove the Send, Receive and Sign Message items (and verify, even though in theory that shouldn't need a wallet).

Sjors · 2018-10-05T04:05:07Z

src/qt/bitcoingui.cpp

I would prefer if (enableWallet) as well. Using walletFrame seems to be an older convention: b7f4b6d

hebasto · 2018-10-05T22:40:30Z

@Sjors
Thank you for your review.

I'm confused: why would you remove the debug window menu item?

In the node mode:

bitcoin/src/qt/bitcoingui.cpp

Lines 114 to 117 in fa7e969

    
                   /* When compiled without wallet or -disablewallet is provided, 
        
                    * the central widget is the rpc console. 
        
                    */ 
        
                   setCentralWidget(rpcConsole);

and

bitcoin/src/qt/bitcoingui.cpp

Line 343 in fa7e969

    
           connect(openRPCConsoleAction, &QAction::triggered, this, &BitcoinGUI::showDebugWindow);

effectively disable this slot:

bitcoin/src/qt/bitcoingui.cpp

Lines 662 to 668 in fa7e969

    
           void BitcoinGUI::showDebugWindow() 
        
           { 
        
               rpcConsole->showNormal(); 
        
               rpcConsole->show(); 
        
               rpcConsole->raise(); 
        
               rpcConsole->activateWindow(); 
        
           }

The Debug window menu item is already removed from the main window Help menu for the node mode:

bitcoin/src/qt/bitcoingui.cpp

Lines 400 to 403 in fa7e969

    
           if(walletFrame) 
        
           { 
        
               help->addAction(openRPCConsoleAction); 
        
           }

and this PR aims to align the system tray icon menu with the main window Help menu.

Ref bitcoin#3392

hebasto · 2018-10-06T00:12:43Z

All thoughts above are implemented.

Master [wallet mode]

This PR [wallet mode]

Master [node mode]

This PR [node mode]

@promag @Sjors please re-review and test on macos.

fanquake · 2018-10-09T07:04:08Z

tACK 36323e2
Tested on macOS 10.14

master (1d14174):

master (1d14174) -disablewallet:

PR (36323e2):

PR (36323e2) -disablwallet:

promag · 2018-10-09T07:10:44Z

utACK 36323e2.

Sjors · 2018-10-10T02:54:09Z

tACK 36323e2

Ref bitcoin#3392 Github-Pull: bitcoin#14383 Rebased-From: 36323e2

27beb83 qt: All tray menu actions call showNormalIfMinimized (João Barbosa) c470bbd qt: Use GUIUtil::bringToFront where possible (João Barbosa) ac73c7d qt: Add GUIUtil::bringToFront (João Barbosa) 0c2fb87 Remove obj_c for macOS Dock icon menu (Hennadii Stepanov) 9034714 Use Qt signal for macOS Dock icon click event (Hennadii Stepanov) 4d4bc37 Remove obj_c for macOS Dock icon setting (Hennadii Stepanov) d2ed162 Clean systray icon menu for -disablewallet mode (Hennadii Stepanov) 298dc15 gui: Favor macOS show / hide action in dock menu (João Barbosa) Pull request description: Backport #14123 #14133 #14383 and #14597 to 0.17 branch to fix #13606 (comment). Tree-SHA512: 543c80e7e2130870e801e0c9a69b06b9eea27c288478fc5dddeb662f7f3ec5b56b30916e5a9a629fced3fffcb8be77e2cd155e75cfd0a4392299add9730840f4

f33efa8 GUI: Restore RPC Console to non-wallet tray icon menu (Luke Dashjr) Pull request description: #14383 moved the debug window's menu position, to make it conditional on wallet mode. The rationale given was to match the behaviour of the 'Help' menu. #14573 replaced the 'Help' menu's conditional debug window with an unconditional list of items in the new 'Window' menu. This PR reverts the no-longer-applicable part of #14383, putting the debug window back on the tray menu unconditionally, and in the position it previously had. ACKs for top commit: jonasschnelli: Tested ACK f33efa8 - the debug window is also accessible from the menu (though directly the subpages which counts IMO). Tree-SHA512: c04a588fed37a8c31cb413baaa346e3c1c18724f9b40d64b8528c517f65290930d577bccf0a794180e968e84d3c52e9fa3fdc8a40bbc5fe3418eaddd73481271

… menu f33efa8 GUI: Restore RPC Console to non-wallet tray icon menu (Luke Dashjr) Pull request description: bitcoin#14383 moved the debug window's menu position, to make it conditional on wallet mode. The rationale given was to match the behaviour of the 'Help' menu. bitcoin#14573 replaced the 'Help' menu's conditional debug window with an unconditional list of items in the new 'Window' menu. This PR reverts the no-longer-applicable part of bitcoin#14383, putting the debug window back on the tray menu unconditionally, and in the position it previously had. ACKs for top commit: jonasschnelli: Tested ACK f33efa8 - the debug window is also accessible from the menu (though directly the subpages which counts IMO). Tree-SHA512: c04a588fed37a8c31cb413baaa346e3c1c18724f9b40d64b8528c517f65290930d577bccf0a794180e968e84d3c52e9fa3fdc8a40bbc5fe3418eaddd73481271

Summary: This is a backport of Core [[bitcoin/bitcoin#14383 | PR14383]] and [[bitcoin/bitcoin#15023 | PR15023]] PR 14383 description: > There is a Debug window leftover in the system tray icon menu after [[bitcoin/bitcoin#3392 | PR3392]] merging. > This PR makes both the app menu and the systray icon menu consistent. PR15023 description: > [[bitcoin/bitcoin#14383 | PR14383]] moved the debug window's menu position, to make it conditional on wallet mode. The rationale given was to match the behaviour of the 'Help' menu. > > [[bitcoin/bitcoin#14573 | PR14573]] replaced the 'Help' menu's conditional debug window with an unconditional list of items in the new 'Window' menu. > > This PR reverts the no-longer-applicable part of #14383, putting the debug window back on the tray menu unconditionally, and in the position it previously had. Note: PR14573 was backported in D7892 Test Plan: `ninja && src/qt/bitcoin-qt -disablewallet` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, Fabien Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, Fabien Subscribers: deadalnix, Fabien Differential Revision: https://reviews.bitcoinabc.org/D8146

…let' mode 36323e2 Clean systray icon menu for -disablewallet mode (Hennadii Stepanov) Pull request description: There is a `Debug window` leftover in the system tray icon menu after dashpay#3392 merging. This PR makes both the app menu and the systray icon menu consistent. Tree-SHA512: c9ef58785fe2a54bc6f778140a16001748ed8c46da948656822b86fdc2e224203cd467857f71d00ce56fc73ff2590c46d8c234a54c261c1141d83039de6fee1e

… menu f33efa8 GUI: Restore RPC Console to non-wallet tray icon menu (Luke Dashjr) Pull request description: bitcoin#14383 moved the debug window's menu position, to make it conditional on wallet mode. The rationale given was to match the behaviour of the 'Help' menu. bitcoin#14573 replaced the 'Help' menu's conditional debug window with an unconditional list of items in the new 'Window' menu. This PR reverts the no-longer-applicable part of bitcoin#14383, putting the debug window back on the tray menu unconditionally, and in the position it previously had. ACKs for top commit: jonasschnelli: Tested ACK f33efa8 - the debug window is also accessible from the menu (though directly the subpages which counts IMO). Tree-SHA512: c04a588fed37a8c31cb413baaa346e3c1c18724f9b40d64b8528c517f65290930d577bccf0a794180e968e84d3c52e9fa3fdc8a40bbc5fe3418eaddd73481271

maflcko reviewed Oct 3, 2018

View reviewed changes

hebasto force-pushed the 20181003-disablewallet-systray branch from 92135c9 to 7a002b4 Compare October 3, 2018 22:13

fanquake added the GUI label Oct 3, 2018

promag reviewed Oct 4, 2018

View reviewed changes

Sjors reviewed Oct 5, 2018

View reviewed changes

src/qt/bitcoingui.cpp Outdated

Copy link

Member

Sjors Oct 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if (enableWallet) as well. Using walletFrame seems to be an older convention: b7f4b6d

Clean systray icon menu for -disablewallet mode

36323e2

Ref bitcoin#3392

hebasto force-pushed the 20181003-disablewallet-systray branch from 7a002b4 to 36323e2 Compare October 5, 2018 23:55

hebasto changed the title ~~qt: Extend -disablewallet mode to system tray icon~~ qt: Clean system tray icon menu for '-disablewallet' mode Oct 6, 2018

maflcko assigned jonasschnelli Oct 9, 2018

laanwj merged commit 36323e2 into bitcoin:master Oct 16, 2018

hebasto deleted the 20181003-disablewallet-systray branch October 16, 2018 05:01

luke-jr mentioned this pull request Dec 21, 2018

GUI: Restore RPC Console to non-wallet tray icon menu #15023

Merged

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Dec 24, 2018

Clean systray icon menu for -disablewallet mode

6ffb8b9

Ref bitcoin#3392 Github-Pull: bitcoin#14383 Rebased-From: 36323e2

promag pushed a commit to promag/bitcoin that referenced this pull request Dec 30, 2018

Clean systray icon menu for -disablewallet mode

d2ed162

Ref bitcoin#3392 Github-Pull: bitcoin#14383 Rebased-From: 36323e2

promag mentioned this pull request Dec 30, 2018

0.17: GUI Backports #14123 #14133 #14383 #14597 #15065

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

qt: Clean system tray icon menu for '-disablewallet' mode #14383

qt: Clean system tray icon menu for '-disablewallet' mode #14383

Uh oh!

Conversation

hebasto commented Oct 3, 2018

Uh oh!

maflcko Oct 3, 2018

Choose a reason for hiding this comment

Uh oh!

hebasto Oct 3, 2018

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Oct 4, 2018

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

promag Oct 4, 2018

Choose a reason for hiding this comment

Uh oh!

Sjors Oct 5, 2018

Choose a reason for hiding this comment

Uh oh!

Sjors left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sjors Oct 5, 2018

Choose a reason for hiding this comment

Uh oh!

hebasto commented Oct 5, 2018

Uh oh!

hebasto commented Oct 6, 2018

Uh oh!

fanquake commented Oct 9, 2018

Uh oh!

promag commented Oct 9, 2018

Uh oh!

Sjors commented Oct 10, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Sjors left a comment •

edited

Loading