-
Notifications
You must be signed in to change notification settings - Fork 38.7k
gui: don't disable the sync overlay when wallet is disabled #15084
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
gui: don't disable the sync overlay when wallet is disabled #15084
Conversation
|
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? |
|
Concept ACK |
ebb011e to
8822215
Compare
|
I added a flag in the constructor as @promag suggested, and changed the text to be more clear. |
9d41519 to
fa76f31
Compare
Sjors
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK fa76f31
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.
fa76f31 to
815c729
Compare
|
@Sjors do you agree with "the information tab is good enough to not have the overlay"? |
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 |
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. |
|
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. |
|
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? |
815c729 to
b3b6b6f
Compare
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. |
|
Tested ACK b3b6b6f |
…bled b3b6b6f gui: don't disable the sync overlay when wallet is disabled (Ben Carman) Pull request description: 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](https://imgur.com/6LsoT2l) Fixes #13828. ACKs for top commit: jonasschnelli: Tested ACK b3b6b6f Tree-SHA512: 325bc22a0b692bfb8fcc9d84e02dfc506146028b97b3609e23c2c45288c79b8aead1ad2e9b8d692f5f6771b4d2aee63fbe71bfaeaf17d260865da32ab3631e07
…is disabled b3b6b6f gui: don't disable the sync overlay when wallet is disabled (Ben Carman) Pull request description: Continuation of bitcoin#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](https://imgur.com/6LsoT2l) Fixes bitcoin#13828. ACKs for top commit: jonasschnelli: Tested ACK b3b6b6f Tree-SHA512: 325bc22a0b692bfb8fcc9d84e02dfc506146028b97b3609e23c2c45288c79b8aead1ad2e9b8d692f5f6771b4d2aee63fbe71bfaeaf17d260865da32ab3631e07
Summary: When running with -disablewallet the sync modal is now available by clicking on the progress bar or syncing icon. This is a backport of Core [[bitcoin/bitcoin#15084 | PR15084]] Test Plan: `ninja && src/qt/bitcoin-qt -disablewallet` Click on the progress bar or syncing icon to show the sync overlay dialog. Reviewers: O1 Bitcoin ABC, #bitcoin_abc, majcosta Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, majcosta Subscribers: majcosta Differential Revision: https://reviews.bitcoinabc.org/D8161
Continuation of #13848.
When running with
-disablewalletthe sync modal is now available by clicking on the progress bar orsyncingicon.Current Image of what the window looks like
Fixes #13828.