qt: Add Window menu #14573

promag · 2018-10-25T15:05:40Z

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:

Without wallet support:

ken2812221

Concept ACK

src/qt/bitcoingui.cpp

hebasto · 2018-10-28T14:37:24Z

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.

DrahtBot · 2018-10-30T23:47:58Z

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

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.

promag · 2018-10-31T16:19:19Z

Removed the changes to the "Settings" menu.

laanwj · 2018-11-12T14:47:23Z

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

meshcollider · 2018-11-12T14:49:20Z

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

jonasschnelli · 2018-11-13T08:28:19Z

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.

promag · 2018-11-13T08:34:50Z

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

I have to test in windows.

promag · 2018-11-14T15:34:24Z

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

Sjors · 2018-11-19T14:19:47Z

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

hebasto · 2018-11-22T21:32:56Z

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

src/qt/bitcoingui.cpp

promag · 2018-12-11T00:11:10Z

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

promag · 2018-12-11T00:23:50Z

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

promag · 2018-12-11T00:32:20Z

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)

Sjors · 2018-12-11T11:31:16Z

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.

promag · 2018-12-11T11:39:28Z

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.

Considering https://stackoverflow.com/a/637708 the ellipsis in these menu actions should be removed.

Sjors · 2018-12-11T17:01:17Z

tACK 9ea38d0

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

95a5a9f qt: Remove ellipsis from sending/receiving addresses (João Barbosa) a96c0df qt: Add Window menu (João Barbosa) 9ea38d0 qt: Allow to inspect RPCConsole tabs (João Barbosa) Pull request description: 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"> Tree-SHA512: 4fb03702efe18df7bae33950e462940162abe634c55d0214b8920812127b763234cc9b73f27b3702502a37b6d49bdd6c50b7c8d9a3daea75cecb0136556dd1ea

The Dash UI changes were introduced in 83420a1 ("qt: Replace usage of QTabBar with custom replacement (dashpay#3560)", 2020-07-14)

3e21b69 [Qt] Restore < Qt5.6 compatibility for addAction (Jonas Schnelli) Pull request description: bitcoin#14573 broke < Qt5.6 compatibility due to calling the lambda version of `addAction` that was added in Qt5.6. This PR re-enables < Qt5.6 compatibility. Tree-SHA512: b3cf055d88a76713d100be05b2298d4091967e1a43de176af2647f59e76b98b216493dd12a6d68a942ae7946f2026e33dd8e8d20fc44a9a9614a3690ad9a2417

95a5a9f qt: Remove ellipsis from sending/receiving addresses (João Barbosa) a96c0df qt: Add Window menu (João Barbosa) 9ea38d0 qt: Allow to inspect RPCConsole tabs (João Barbosa) Pull request description: 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"> Tree-SHA512: 4fb03702efe18df7bae33950e462940162abe634c55d0214b8920812127b763234cc9b73f27b3702502a37b6d49bdd6c50b7c8d9a3daea75cecb0136556dd1ea

The Dash UI changes were introduced in 83420a1 ("qt: Replace usage of QTabBar with custom replacement (dashpay#3560)", 2020-07-14)

3e21b69 [Qt] Restore < Qt5.6 compatibility for addAction (Jonas Schnelli) Pull request description: bitcoin#14573 broke < Qt5.6 compatibility due to calling the lambda version of `addAction` that was added in Qt5.6. This PR re-enables < Qt5.6 compatibility. Tree-SHA512: b3cf055d88a76713d100be05b2298d4091967e1a43de176af2647f59e76b98b216493dd12a6d68a942ae7946f2026e33dd8e8d20fc44a9a9614a3690ad9a2417

b078067 gui: Remove unused RPCConsole::tabFocus (João Barbosa) Pull request description: Added in bitcoin#14573 but not used, so begone. ACKs for top commit: practicalswift: utACK b078067 hebasto: ACK b078067 laanwj: ACK b078067, there's nothing really to test here Tree-SHA512: 237276dea4d174b5fca34855447146f79c3faaae7179f4245c70e2070b49282d95f886b1be6d2a33713c81a254f4483a4e4bf850053a8dcb18a3a897bd3da08e

The Dash UI changes were introduced in 83420a1 ("qt: Replace usage of QTabBar with custom replacement (dashpay#3560)", 2020-07-14)

fanquake added the GUI label Oct 25, 2018

ken2812221 reviewed Oct 27, 2018

View reviewed changes

src/qt/bitcoingui.cpp Outdated Show resolved Hide resolved

promag changed the title ~~qt: Add Wallet and Window menus~~ qt: Add Window menus Oct 30, 2018

promag changed the title ~~qt: Add Window menus~~ qt: Add Window menu Oct 30, 2018

promag mentioned this pull request Oct 30, 2018

Qt: Network Watch tool #9849

Closed

promag force-pushed the 2018-10-overhaul-menus branch 2 times, most recently from 039c146 to 3f779b3 Compare October 31, 2018 16:18

hebasto requested changes Nov 22, 2018

View reviewed changes

src/qt/bitcoingui.cpp Outdated Show resolved Hide resolved

src/qt/bitcoingui.cpp Outdated Show resolved Hide resolved

promag force-pushed the 2018-10-overhaul-menus branch from 3f779b3 to 46cbcb0 Compare December 11, 2018 00:11

promag force-pushed the 2018-10-overhaul-menus branch from 6c72f91 to 4768926 Compare December 11, 2018 13:15

promag added 3 commits December 11, 2018 13:15

qt: Allow to inspect RPCConsole tabs

9ea38d0

qt: Add Window menu

a96c0df

qt: Remove ellipsis from sending/receiving addresses

95a5a9f

Considering https://stackoverflow.com/a/637708 the ellipsis in these menu actions should be removed.

promag force-pushed the 2018-10-overhaul-menus branch from 4768926 to 95a5a9f Compare December 11, 2018 13:15

christiancfifi added a commit to christiancfifi/dash that referenced this pull request Sep 11, 2021

Fix bitcoin#14573: Alter to work with Dash UI

a05ada3

The Dash UI changes were introduced in 83420a1 ("qt: Replace usage of QTabBar with custom replacement (dashpay#3560)", 2020-07-14)

christiancfifi added a commit to christiancfifi/dash that referenced this pull request Sep 11, 2021

Fix bitcoin#14573: Drop Tools menu, move actions to File menu

3056af4

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

gades pushed a commit to cosanta/cosanta-core that referenced this pull request May 9, 2022

Fix bitcoin#14573: Alter to work with Dash UI

713b147

The Dash UI changes were introduced in 83420a1 ("qt: Replace usage of QTabBar with custom replacement (dashpay#3560)", 2020-07-14)

gades pushed a commit to cosanta/cosanta-core that referenced this pull request May 9, 2022

Fix bitcoin#14573: Drop Tools menu, move actions to File menu

62aecff

qt: Add Window menu #14573

qt: Add Window menu #14573

Uh oh!

Conversation

promag commented Oct 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ken2812221 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hebasto commented Oct 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Oct 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

promag commented Oct 31, 2018

Uh oh!

laanwj commented Nov 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

meshcollider commented Nov 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonasschnelli commented Nov 13, 2018

Uh oh!

promag commented Nov 13, 2018

Uh oh!

promag commented Nov 14, 2018

Uh oh!

Sjors commented Nov 19, 2018

Uh oh!

hebasto commented Nov 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

promag commented Dec 11, 2018

Uh oh!

promag commented Dec 11, 2018

Uh oh!

promag commented Dec 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sjors commented Dec 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

promag commented Dec 11, 2018

Uh oh!

Sjors commented Dec 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

promag commented Oct 25, 2018 •

edited

Loading

hebasto commented Oct 28, 2018 •

edited

Loading

DrahtBot commented Oct 30, 2018 •

edited

Loading

laanwj commented Nov 12, 2018 •

edited

Loading

meshcollider commented Nov 12, 2018 •

edited

Loading

hebasto commented Nov 22, 2018 •

edited

Loading

promag commented Dec 11, 2018 •

edited

Loading

Sjors commented Dec 11, 2018 •

edited

Loading