-
Notifications
You must be signed in to change notification settings - Fork 333
fix issue when disabling the auto-enabled blank wallet checkbox #243
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
Conversation
src/qt/createwalletdialog.cpp
Outdated
| }); | ||
|
|
||
| connect(ui->blank_wallet_checkbox, &QCheckBox::toggled, [this](bool checked) { | ||
| // Disable the disable_privkeys_checkbox when blank_wallet_checkbox is false |
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.
Not sure if these kind of comments are helpful. You can see that stated fact by just looking at rather self-explanatory code, with descriptive variable names.
Maybe this message would be more informative:
//Having "Disable Private Keys" checked after unchecking "Make Blank Wallet" makes no sense. Disabling keys means wallet has to be blank.
Or something similar.
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.
Actually, the comment is wrong as the disable_privkeys_checkbox is to be unchecked, not disabled :)
I think just removing it is fine.
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.
Removed the comment in 915e341
leonardojobim
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.
Tested ACK ca6bb0d on Ubuntu 20.04
|
Concept ACK. The logic of checkboxes interaction is correct. |
hebasto
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.
Approach ACK ca6bb0d, please remove comment.
This commit makes it so that when the `Blank Wallet` checkbox is auto-selected after the user selects 'Disable Private' keys, unselecting it will also unselect the 'Disable Private Keys' checkbox, which in turn re-enables the 'Encrypt Wallet' checkbox.
hebasto
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.
ACK 915e341
Talkless
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.
ACK 915e341
… blank wallet checkbox 915e341 qt: fix issue when disabling the auto-enabled blank wallet checkbox (Jarol Rodriguez) Pull request description: As detailed by #151, On `master` a user can create the confusing scenario where you have a disabled `Encrypt Wallet` checkbox and a selected `Disable Private Keys` checkbox after unselecting the auto-enabled `Blank Wallet` checkbox. This commit makes it so that when the `Blank Wallet` checkbox is auto-selected after the user selects `Disable Private keys`, unselecting it will also unselect the `Disable Private Keys` checkbox, which in turn re-enables the `Encrypt Wallet` checkbox. Below are screenshots comparing the behavior of selecting `Disable Private Keys` then unselecting the `Blank Wallet` between `master` and this `PR`: **Master:** | Select `Disable Private Keys` | Unselect `Blank Wallet` | | ----------------------------- | ------------------------ | |  |  | **PR:** | Select `Disable Private Keys` | Unselect `Blank Wallet` | | ----------------------------- | ------------------------ | |  |  | ACKs for top commit: hebasto: ACK 915e341 Talkless: ACK 915e341 Tree-SHA512: ce6ecbc35b94a08cabf0b8a24dbdfc874d82cc8918cc8623dce8172c7fc9c75d63a13b036bae5f7ab2c090f8d020574a542285d1651600813faf5d91e2506a8d

As detailed by #151, On
mastera user can create the confusing scenario where you have a disabledEncrypt Walletcheckbox and a selectedDisable Private Keyscheckbox after unselecting the auto-enabledBlank Walletcheckbox.This commit makes it so that when the
Blank Walletcheckbox is auto-selected after the user selectsDisable Private keys, unselecting it will also unselect theDisable Private Keyscheckbox, which in turn re-enables theEncrypt Walletcheckbox.Below are screenshots comparing the behavior of selecting
Disable Private Keysthen unselecting theBlank Walletbetweenmasterand thisPR:Master:
Disable Private KeysBlank WalletPR:
Disable Private KeysBlank Wallet