Never disable HD status icon

stratospher commented at 9:51 pm on October 5, 2021: contributor

#8517 introduced the HD status icon in the bottom menu bar. At the time, it may have made sense to disable the HD status icon (which would display the icon at 50% opacity) when the wallet is not HD or watch-only.

Since then, we have had many changes to our UI. Namely, the ability to switch between dark and light mode on macOS. Additionally, since version 0.16, the Bitcoin Core client only generates HD wallets. Non-HD Wallets cannot be created anymore, only imported.

This PR proposes never to disable the HD status icon in the bottom menu bar. Instead, we always leave the HD status icon enabled for a better user interface. There’s no good reason for it to remain disabled, and it’s hard to see the watch-only icon in macOS dark mode. If a user has imported a non-HD wallet, we also want the icon to be as clear as possible in the bottom menu bar.

on macOS

	Master	PR
Dark Mode
Light Mode

on ubuntu 20.04

Master	PR

qt: never disable HD status icon

Make the watch-only icon in the bottom bar enabled by default for a better user interface.
Currently, it's disabled by default with a 50% opacity which makes it hard to see the icon in dark mode.

35e814c1cf

jarolrod added the label UI on Oct 5, 2021

jarolrod commented at 2:52 am on October 6, 2021: member

ACK 35e814c1cfedf17e7751b730ae1e36099801be4c

Tested on macOS 12. I think this is nice & small UI improvement. It’s a bit of an eyesore when using a watch-only wallet. One nice thing about this change is that the icon (in the cases of a watch-only and non-hd wallet) will be properly colorized when switching between light and dark mode on macOS. As shown in your screenshots with Ubuntu, it’s a small UI improvement regardless of platform.

Watch-Only Icon

master	pr

Since I don’t have a non-HD wallet available to import, I looked at the visual changes this PR would have on the HD disabled icon with the following patch:

0@@ -1301,7 +1301,7 @@ bool BitcoinGUI::handlePaymentRequest(const SendCoinsRecipient& recipient)
1 
2 void BitcoinGUI::setHDStatus(bool privkeyDisabled, int hdEnabled)
3 {
4-    labelWalletHDStatusIcon->setThemedPixmap(privkeyDisabled ? QStringLiteral(":/icons/eye") : hdEnabled ? QStringLiteral(":/icons/hd_enabled") : QStringLiteral(":/icons/hd_disabled"), STATUSBAR_ICONSIZE, STATUSBAR_ICONSIZE);
5+    labelWalletHDStatusIcon->setThemedPixmap(QStringLiteral(":/icons/hd_disabled"), STATUSBAR_ICONSIZE, STATUSBAR_ICONSIZE);
6     labelWalletHDStatusIcon->setToolTip(privkeyDisabled ? tr("Private key <b>disabled</b>") : hdEnabled ? tr("HD key generation is <b>enabled</b>") : tr("HD key generation is <b>disabled</b>"));

HD disabled icon

master + patch	pr + patch

hebasto commented at 12:28 pm on October 7, 2021: member

~Screenshots in the OP seem misleading as they do not show thelabelWalletHDStatusIcon.~

Additionally, since version 0.16, the Bitcoin Core client only generates HD wallets. Non-HD Wallets cannot be created anymore, only imported.

It does not mean people stopped using non-HD wallets.

Concept ACK. The labelWalletHDStatusIcon is not clickable, therefore, disabling it makes no sense.

Is a “non-HD” label is more clear to read and understand than a crossed “HD” one?

cc @GBKS @bosch-0

shaavan commented at 1:15 pm on October 7, 2021: contributor

Tested successfully on Ubuntu 20.04

This PR allows displaying of the non-HD enabled icons without reduced opacity.

I have noticed an issue with this PR that I want to address by showing the difference between Master and PR screenshots.

Scenario	Master	PR
HD + Private keys
non HD + Private keys
HD + no Private keys
non HD + no Private keys

As you might notice, the icons displayed in the last two scenarios in the PR have become precisely the same, which will probably introduce unnecessary confusion. And I don’t think inducing any kind of UX confusion, however small, is favorable.

Is a “non-HD” label is more clear to read and understand than a crossed “HD” one?

I would love to listen to the Designers’ take on this matter before forming an opinion for this PR.

jarolrod commented at 2:53 pm on October 15, 2021: member

Screenshots in the OP seem misleading as they do not show thelabelWalletHDStatusIcon.

They certainly are showing the labelWalletHDStatusIcon, but in the included screenshots it is currently set to the watch-only icon. Seem’s that the reason for including the screenshot would be to highlight the visual change in the case of watch-only. In the case of HD enabled, it is already going to be set enabled. OP could have included screenshots showing the visual difference in the case that HD wallet status is disabled. Before this PR, this icon would be disabled, with this PR it is always enabled. In all of these mentioned cases, the icon displayed is the labelWalletHDStatusIcon.

As you might notice, the icons displayed in the last two scenarios in the PR have become precisely the same, which will probably introduce unnecessary confusion. And I don’t think inducing any kind of UX confusion, however small, is favorable.

The point of the PR is to never disable the labelWalletHDStatusIcon, this behavior is correct. There’s no reason to disable the icon.

Is a “non-HD” label is more clear to read and understand than a crossed “HD” one? g an opinion for this PR.

This is out of the scope of this PR.

hebasto commented at 9:41 pm on October 19, 2021: member

Screenshots in the OP seem misleading as they do not show thelabelWalletHDStatusIcon.

They certainly are showing the labelWalletHDStatusIcon, but in the included screenshots it is currently set to the watch-only icon.

Right.

hebasto approved

hebasto commented at 9:44 pm on October 19, 2021: member

ACK 35e814c1cfedf17e7751b730ae1e36099801be4c

hebasto merged this on Oct 19, 2021

hebasto closed this on Oct 19, 2021

sidhujag referenced this in commit 48d2d79dd8 on Oct 20, 2021

laanwj commented at 4:49 pm on October 21, 2021: member

Please change your git author name to something else than = for future commits (unless you really want to be credited as that).

luke-jr commented at 9:12 pm on July 16, 2022: member

As you might notice, the icons displayed in the last two scenarios in the PR have become precisely the same, which will probably introduce unnecessary confusion. And I don’t think inducing any kind of UX confusion, however small, is favorable.

Is there in fact a meaningful distinction? HD deals with how addresses are generated from private keys, but watch-only doesn’t have private keys… If there’s a reason to distinguish, please open an issue before this gets lost. ;)

luke-jr commented at 7:27 pm on September 4, 2022: member

Watch-only wallets were introduced in 0.17, while the ability to create non-HD wallets was removed in 0.16, so even if it’s possible to make a watch-only non-HD wallet (I don’t think it is), it should be weird enough to not care about how it gets displayed in the GUI.

delta1 referenced this in commit e59e22f32f on May 13, 2023

bitcoin-core locked this on Sep 4, 2023

Never disable HD status icon #447