gui: Remove unused menu items for Windows and Linux #16735

GChuf commented at 5:53 PM on August 27, 2019: contributor

Removed "Main Window" and "Restore" menu option for Windows and linux Keep the options for macOS

GChuf commented at 5:55 PM on August 27, 2019: contributor

This was already mentioned by @molxyz (at least for "Main Window") in #14573.

I'm not entirely sure what's up with "Restore", but for me on Windows and Linux, when I click it, I only get this shadow of the menu around. I also tried adding a shortcut to see if it would do something different (whether minimized or not), but it behaved the same. Not sure what it does on macOS.

Screenshot_74

Before and after for linux (and windows):

Screenshot_71 Screenshot_75

p.s. this does not touch entries on macOS and keeps everything else behaving just as it did before.

if macOS:
 add "Zoom" entry
 if wallet enabled, add  "Main Window", "SendingAddresses", "ReceivingAddress" entries

else:
 if wallet enabled, add "SendingAddresses", "ReceivingAddress" entries

molxyz commented at 5:58 PM on August 27, 2019: none

@GChuf Thank you! I made this request several times but for some reason it seemed to go into deaf ears. Hopefully this time someone will listen to you. Thank you very much! :)

GChuf commented at 6:01 PM on August 27, 2019: contributor

@molxyz surprised to see you here actually, given that it was an "old" issue and I haven't seen you around yet, but I'm glad you responded! Once I saw your comment I confirmed to myself this is an issue that needs to be addressed.

MarcoFalke commented at 6:09 PM on August 27, 2019: member

Concept ACK

hebasto commented at 7:26 PM on August 27, 2019: member

Concept ACK

DrahtBot added the label GUI on Aug 27, 2019

emilengler commented at 8:21 PM on August 27, 2019: contributor

Concept ACK

promag commented at 1:32 AM on August 28, 2019: member

I made this request several times but for some reason it seemed to go into deaf ears.

Sorry but does #14573 (comment) means several?

Concept ACK.

Maybe also drop Minimize in windows? IIRC the context menu of the window icon has that option?

promag commented at 1:37 AM on August 28, 2019: member

I'm not entirely sure what's up with "Restore", but for me on Windows and Linux, when I click it, I only get this shadow of the menu around

That sounds like a Qt bug? Which one are you using?

Not sure what it does on macOS.

In macos there's the "Zoom" menu item, it toggle between maximized and normal.

fanquake renamed this:
~~GUI: Remove unused menu items for Windows and Linux~~
gui: Remove unused menu items for Windows and Linux
on Aug 28, 2019

in src/qt/bitcoingui.cpp:496 in 2483dcc496 outdated

 491 | @@ -502,6 +492,13 @@ void BitcoinGUI::createMenuBar()
 492 |          window_menu->addAction(usedSendingAddressesAction);
 493 |          window_menu->addAction(usedReceivingAddressesAction);
 494 |      }
 495 | +#else
 496 | +    if (walletFrame) {

hebasto commented at 8:57 AM on August 28, 2019:

Could code duplication be avoided?

GChuf commented at 1:12 PM on August 28, 2019:

I dont see any nice, clean way if we want to keep current behavior. Not duplicating sending/receiving addresses would require duplicating check for macos.

GChuf commented at 1:29 PM on August 28, 2019:

That said, it would save us a few symbols, though not any lines...

GChuf commented at 2:22 PM on August 28, 2019:

... I take it back, I was thinking about this when I was away from my computer. Looks like I managed to decrease the line count by 5 by doing what I had in mind. Thanks for the idea.

GChuf commented at 2:02 PM on August 28, 2019: contributor

@promag I wouldnt like to drop minimize on windows (and/or linux), it can still act as a nice shortcut (ctrl + m). Even if the shortcut is there, menu entry needs to be as well for people to see the shortcut exists.

Supposedly, windows equivalent of Zoom is Magnify, I don't think people use it much, if at all.

Using qt 5.5.1, and i'm pretty sure older versions behaved the same.

MarcoFalke added the label Needs gitian build on Aug 28, 2019

DrahtBot removed the label Needs gitian build on Aug 29, 2019

molxyz commented at 10:47 AM on August 30, 2019: none

Sorry but does #14573 (comment) means several? @promag Excuse me? You think I would lie on this GH repo? Here are the requests I made for this same issue: #15555 (comment) #14573 (comment) #16386 (comment)

That means "several" in English AFAIK. I probably should have made a separate new issue on its own now I think of it, but I gave up.

And I wasn't the only one, Meshcollider also made the same request to you before I saw this useless button on my wallet. #14573 (comment)

Your remark to me is inappropriate, i'm not upset but please don't create things on GUI that don't have any use or purpose, that's all I'm asking for. Thank you.

promag commented at 11:10 AM on August 30, 2019: member

@molxyz I had no intention in being inappropriate, just genuinely curious which were those several requests, because I was only aware of that one.

it seemed to go into deaf ears. Hopefully this time someone will listen to you.

I did reply to you and @meshcollider and explained that it was too late to change for 0.18.

but please don't create things on GUI that don't have any use or purpose

Yeah I don't want either :trollface: and you are also free to submit changes like I am.

Anyway, I already agreed to this change: #16735 (comment) so lets get this done for 0.19.

in src/qt/bitcoingui.cpp:487 in b448fb152f outdated

 497 | -        connect(main_window_action, &QAction::triggered, [this] {
 498 | -            GUIUtil::bringToFront(this);
 499 | -        });
 500 | -
 501 | +#ifdef Q_OS_MAC		
 502 | +    window_menu->addSeparator();