gui: don't disable the sync overlay when wallet is disabled

benthecarman commented at 6:43 am on January 3, 2019: contributor

Continuation of #13848.

When running with -disablewallet the sync modal is now available by clicking on the progress bar or syncing icon.

Current Image of what the window looks like

Fixes #13828.

fanquake added the label GUI on Jan 3, 2019

promag commented at 2:35 pm on January 3, 2019: member

Note: not very fond of the overlay.

With the wallet disabled I think the information tab is good enough to not have the overlay. Maybe the information tab could have the same progress bar?

Also, it’s a pity to add wallet dependency to the overlay code, maybe add a flag to the constructor?

in src/qt/modaloverlay.cpp:45 in ebb011e36e outdated

40+    enableWallet = WalletModel::isWalletEnabled();
41+#endif
42+    if (!enableWallet)
43+    {
44+        ui->infoText->setVisible(false);
45+        ui->infoTextStrong->setText("Bitcoin Core is not fully synced yet.");

jonasschnelli commented at 8:22 pm on January 3, 2019:

This eventually should be written in positive form (“is syncing” instead of “not filly synced”)… could also a bit more explanatory

jonasschnelli commented at 8:22 pm on January 3, 2019: contributor

Concept ACK

benthecarman force-pushed on Jan 3, 2019

benthecarman commented at 11:52 pm on January 3, 2019: contributor

I added a flag in the constructor as @promag suggested, and changed the text to be more clear.

in src/qt/modaloverlay.cpp:15 in 8822215604 outdated

11@@ -12,7 +12,7 @@
12 #include <QResizeEvent>
13 #include <QPropertyAnimation>
14 
15-ModalOverlay::ModalOverlay(QWidget *parent) :
16+ModalOverlay::ModalOverlay(QWidget *parent, bool enableWallet) :

promag commented at 0:01 am on January 4, 2019:

In Qt *parent is usually the last argument. Also use enable_wallet instead.

in src/qt/modaloverlay.cpp:32 in 8822215604 outdated

28@@ -29,6 +29,11 @@ userClosed(false)
29 
30     blockProcessTime.clear();
31     setVisible(false);
32+    if (!enableWallet)

promag commented at 0:01 am on January 4, 2019:

nit, { here.

benthecarman force-pushed on Jan 4, 2019

in src/qt/modaloverlay.cpp:34 in fa76f3182d outdated

28@@ -29,6 +29,10 @@ userClosed(false)
29 
30     blockProcessTime.clear();
31     setVisible(false);
32+    if (!enable_wallet) {
33+        ui->infoText->setVisible(false);
34+        ui->infoTextStrong->setText("Bitcoin Core is currently syncing.  It will download headers and blocks from peers and validate them until reaching the tip of the best block chain.");

Sjors commented at 1:47 pm on January 9, 2019:

Nit: leave out “best”

Sjors approved

Sjors commented at 1:56 pm on January 9, 2019: member

tACK fa76f31 @promag:

With the wallet disabled I think the information tab is good enough to not have the overlay. Maybe the information tab could have the same progress bar?

Agreed, but not necessary for this PR.

benthecarman force-pushed on Jan 9, 2019

promag commented at 5:59 pm on January 10, 2019: member

@Sjors do you agree with “the information tab is good enough to not have the overlay”?

Sjors commented at 12:44 pm on January 15, 2019: member

With the wallet disabled I think the information tab is good enough to not have the overlay. Maybe the information tab could have the same progress bar?

Yes, but I think it should wait for another PR. I doubt many people use QT compiled without wallet. This PR fixes a bug with that, so let’s just merge that.

re-tACK 815c729

laanwj commented at 1:26 pm on February 12, 2019: member

With the wallet disabled I think the information tab is good enough to not have the overlay. Maybe the information tab could have the same progress bar?

Tend to agree here. The overlay is good for wallet users, but I don’t think it’s really necessary without wallet with the information tab as first display. It has more or less the same information.

jonasschnelli commented at 6:41 am on March 15, 2019: contributor

it is probably nice when one directly see what’s going on during the sync rather then requiring to open the “debug window”.

I see the point that the overlay (as it is) was designed for wallet users (it overlays and warns). But showing relevant information on first sight for non-wallet users seems to be a nice addition.

in src/qt/modaloverlay.cpp:34 in 815c7293b9 outdated

28@@ -29,6 +29,10 @@ userClosed(false)
29 
30     blockProcessTime.clear();
31     setVisible(false);
32+    if (!enable_wallet) {
33+        ui->infoText->setVisible(false);
34+        ui->infoTextStrong->setText("Bitcoin Core is currently syncing.  It will download headers and blocks from peers and validate them until reaching the tip of the block chain.");

jonasschnelli commented at 9:38 am on October 9, 2019:

needs a tr()

jonasschnelli commented at 9:41 am on October 9, 2019: contributor

Tested a bit and I think it makes sense to add this. It won’t show automatically at the start if wallets are disabled.

Does the exclamation mark warning icon need to be changed in node mode though?

jonasschnelli added this to the milestone 0.19.0 on Oct 9, 2019

jonasschnelli removed this from the milestone 0.19.0 on Oct 9, 2019

jonasschnelli added this to the milestone 0.20.0 on Oct 9, 2019

gui: don't disable the sync overlay when wallet is disabled b3b6b6f62f

benthecarman force-pushed on Oct 13, 2019

benthecarman commented at 3:49 am on October 18, 2019: contributor

Does the exclamation mark warning icon need to be changed in node mode though?

I think it makes sense to keep it, it’ll bring attention to the fact that the screen means the node is not fully synced.

jonasschnelli commented at 7:13 am on October 18, 2019: contributor

Tested ACK b3b6b6f62fcabea9818e8049dba714d0d0ef8ab6

jonasschnelli referenced this in commit b9b58f8f68 on Oct 18, 2019

jonasschnelli merged this on Oct 18, 2019

jonasschnelli closed this on Oct 18, 2019

sidhujag referenced this in commit 9cc1aa0811 on Oct 18, 2019

MarkLTZ referenced this in commit eee566b731 on Nov 17, 2019

deadalnix referenced this in commit 5a179f986c on Oct 29, 2020

DrahtBot locked this on Dec 16, 2021

gui: don’t disable the sync overlay when wallet is disabled #15084