Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Closed
fanquake wants to merge 1 commit into from
Closed

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

fanquake wants to merge 1 commit into from

Conversation

@fanquake
Copy link
Member

@fanquake fanquake commented Aug 2, 2018

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

screenshot 2018-08-02 at 4 21 57 pm

Fixes #13828.

@fanquake fanquake added the GUI label Aug 2, 2018
@fanquake fanquake requested review from Sjors and jonasschnelli August 2, 2018 08:24
@laanwj
Copy link
Member

laanwj commented Aug 2, 2018

I don't think this message makes much sense when the wallet is disabled? It talks about a wallet and spending coins.

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 2, 2018 •
edited
Loading

No more conflicts as of last run.

@Sjors
Copy link
Member

Sjors commented Aug 2, 2018

Concept ACK, though indeed removing the wallet specific warning makes sense.

@DrahtBot DrahtBot mentioned this pull request Aug 2, 2018
Merged
@promag
Copy link
Contributor

promag commented Aug 3, 2018

FYI in #11826 there is discussion about changing the overlay and I was planning to work on that after 0.17 branch.

@jonasschnelli
Copy link
Contributor

jonasschnelli commented Aug 5, 2018

I think the modal overlay with the sync-info should show but the warning part (icon / text) should only be visible when a wallet is loaded.

Sjors
Sjors approved these changes Aug 11, 2018
View reviewed changes
Copy link
Member

@Sjors Sjors left a comment •
edited
Loading

Choose a reason for hiding this comment

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

tACK 68c6fff, it's a strict improvement over master, despite the unnecessary wallet related text, so would be good to get into 0.17.

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 21, 2018

Needs rebase

@luke-jr
Copy link
Member

luke-jr commented Aug 26, 2018

Seems like it would be better to just add the missing data to the main Information tab.

@laanwj
Copy link
Member

laanwj commented Aug 29, 2018

Seems like it would be better to just add the missing data to the main Information tab.

In principle that's per-wallet and supposed to have wallet information, not node/global information.
That's why there is no other such information there.
Though there would certainly be place for it...

@fanquake
Copy link
Member Author

fanquake commented Oct 9, 2018

@promag I'm closing and labelling up for grabs. Feel free to work on this.

@fanquake fanquake closed this Oct 9, 2018
@benthecarman benthecarman mentioned this pull request Jan 3, 2019
Merged
@fanquake fanquake deleted the allow-modal-disablewallet branch January 3, 2019 06:46
@fanquake
Copy link
Member Author

fanquake commented Jan 3, 2019

Removing up for grabs, as this has been picked up in #15084.

jonasschnelli added a commit that referenced this pull request Oct 18, 2019
@jonasschnelli
Merge #15084: gui: don't disable the sync overlay when wallet is disa…
b9b58f8 
…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
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Oct 18, 2019
@jonasschnelli
Merge bitcoin#15084: gui: don't disable the sync overlay when wallet …
9cc1aa0 
…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
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jonasschnelli jonasschnelli Awaiting requested review from jonasschnelli

1 more reviewer

@Sjors Sjors Sjors approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

GUI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Missing and unaccesible sync modal when using QT without wallet

7 participants

@fanquake @laanwj @DrahtBot @Sjors @promag @jonasschnelli @luke-jr