Skip to content
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

Show cancel button in wallet password prompt #1765

Merged

Conversation

devinbileck
Copy link
Member

Fixes #1181

When launching the application, there was no way to cancel the wallet password prompt and quit the application. Attempting to close the application window did not quit the application (at least not in Windows 10).

So a cancel button has been added:
image

Copy link
Contributor

@ManfredKarrer ManfredKarrer left a comment

Choose a reason for hiding this comment

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

NACK
I agree that it is good to add a cancel button here but it must be done differently.

The WalletPasswordWindow is used in several places and the close button must not trigger a shitdonw in the other use cases.
The hideCloseButton call can be removed in MainViewModel at line294.
To add the shut down functionality need to be added there as well so it does not affect the other use cases.

Following code should do it:

 bisqSetup.setRequestWalletPasswordHandler(aesKeyHandler -> walletPasswordWindow
                .onAesKey(aesKeyHandler::accept)
                .onClose(()-> BisqApp.getShutDownHandler().run())
                .show());

@devinbileck
Copy link
Member Author

Thanks @ManfredKarrer. I completely missed that. I have made the suggested changes.

Copy link
Contributor

@ManfredKarrer ManfredKarrer left a comment

Choose a reason for hiding this comment

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

ACK.
Looks good now. Thanks!

@ManfredKarrer ManfredKarrer merged commit 56965c1 into bisq-network:master Oct 10, 2018
@devinbileck devinbileck deleted the cancel-wallet-password-prompt branch October 10, 2018 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Login password dialog doesn't provide 'exit' button
2 participants