Skip to content

Conversation

@benthecarman
Copy link
Contributor

@benthecarman benthecarman commented Jan 3, 2019

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.

@promag
Copy link
Contributor

promag commented Jan 3, 2019

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?

@jonasschnelli
Copy link
Contributor

Concept ACK

@benthecarman benthecarman force-pushed the sync_overlay_without_wallet branch from ebb011e to 8822215 Compare January 3, 2019 23:51
@benthecarman
Copy link
Contributor Author

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

@benthecarman benthecarman force-pushed the sync_overlay_without_wallet branch 2 times, most recently from 9d41519 to fa76f31 Compare January 4, 2019 00:13
Copy link
Member

@Sjors Sjors left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 benthecarman force-pushed the sync_overlay_without_wallet branch from fa76f31 to 815c729 Compare January 9, 2019 19:35
@promag
Copy link
Contributor

promag commented Jan 10, 2019

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

@Sjors
Copy link
Member

Sjors commented Jan 15, 2019

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
Copy link
Member

laanwj commented Feb 12, 2019

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
Copy link
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.

@jonasschnelli
Copy link
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 jonasschnelli modified the milestones: 0.19.0, 0.20.0 Oct 9, 2019
@benthecarman benthecarman force-pushed the sync_overlay_without_wallet branch from 815c729 to b3b6b6f Compare October 13, 2019 15:31
@benthecarman
Copy link
Contributor Author

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
Copy link
Contributor

Tested ACK b3b6b6f

jonasschnelli added a commit that referenced this pull request Oct 18, 2019
…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
@jonasschnelli jonasschnelli merged commit b3b6b6f into bitcoin:master Oct 18, 2019
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 18, 2019
…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
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Nov 17, 2019
deadalnix pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 29, 2020
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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
@benthecarman benthecarman deleted the sync_overlay_without_wallet branch October 10, 2025 02:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing and unaccesible sync modal when using QT without wallet

6 participants