gui: Create wallet menu option follow-ups #16822

pull jonatack wants to merge 3 commits into bitcoin:master from jonatack:createwallet-followup changing 8 files +23 −19

jonatack commented at 12:19 PM on September 7, 2019: member
Closes #16820. The wallet name escaping issue in that issue predates #15450 and is fixed by #16826.
- rename encrypt() to encryptWallet(), and blank() to makeBlankWallet() // EDIT: updated to isEncryptWalletChecked() isDisablePrivateKeysChecked() isMakeBlankWalletChecked()
- fix naming of askPasshprase() to askPassphrase()
- fix passphrase labels and tooltip in createwalletdialog.ui and askpassphrasedialog.ui
- fix grammar of labels in askpassphrase dialog and WalletController::closeWallet
- fix autofocus in CreateWalletActivity::askPassphrase()
Squashed down to three commits.

Reviewers, to test manually: build, launch the gui wallet, and look at labels/tooltips/focus with the create wallet, encrypt wallet, change password, and close wallet commands.
fanquake added the label GUI on Sep 7, 2019
fanquake added the label Wallet on Sep 7, 2019
fanquake removed the label Wallet on Sep 7, 2019
jonatack force-pushed on Sep 8, 2019
jonatack renamed this:
~~[WIP] gui: Createwallet followups~~
gui: Create wallet menu option follow-ups
on Sep 8, 2019

in src/qt/guiutil.cpp:426 in 66af07e762 outdated

 422 | @@ -423,7 +423,7 @@ bool ToolTipToRichTextFilter::eventFilter(QObject *obj, QEvent *evt)
 423 |          QString tooltip = widget->toolTip();
 424 |          if(tooltip.size() > size_threshold && !tooltip.startsWith("<qt") && !Qt::mightBeRichText(tooltip))
 425 |          {
 426 | -            // Envelop with <qt></qt> to make sure Qt detects this as rich text
 427 | +            // Envelope with <qt></qt> to make sure Qt detects this as rich text

michaelfolkson commented at 10:40 AM on September 8, 2019:

Nit. I think this should be envelop rather than envelope. Envelope is a noun, envelop is a verb.

jonatack commented at 10:53 AM on September 8, 2019:

Done, thanks! That seemed misspelled to me but you are correct.

jonatack force-pushed on Sep 8, 2019
jonatack commented at 11:25 AM on September 8, 2019: member

Lone CI failure is a time-out.
promag commented at 11:28 AM on September 8, 2019: member

Concept ACK, restarted travis job.
in src/qt/createwalletdialog.cpp:49 in d4072b32d4 outdated
```
  45 | @@ -45,7 +46,7 @@ QString CreateWalletDialog::walletName() const
  46 |      return ui->wallet_name_line_edit->text();
  47 |  }
  48 |  
  49 | -bool CreateWalletDialog::encrypt() const
  50 | +bool CreateWalletDialog::encryptWallet() const
```
promag commented at 11:32 AM on September 8, 2019:

nit, both previous and current suggest an action. How about isEncryptChecked and isMakeBlankChecked?

jonatack commented at 12:45 PM on September 8, 2019:

Done, updated disablePrivateKeys as well for consistency. Updated commit message and PR description.
jonatack force-pushed on Sep 8, 2019
michaelfolkson commented at 1:16 PM on September 8, 2019: contributor

ACK 52d3743cc0bb57dba2a9bedb74bbfdde3a8d28ae. Labels and tooltips correctly update.
promag commented at 5:30 PM on September 8, 2019: member

Code review ACK 52d3743cc0bb57dba2a9bedb74bbfdde3a8d28ae.
DrahtBot commented at 9:13 PM on September 8, 2019: member



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

No conflicts as of last run.

in src/qt/walletcontroller.cpp:78 in 52d3743cc0 outdated

  74 | @@ -75,7 +75,7 @@ void WalletController::closeWallet(WalletModel* wallet_model, QWidget* parent)
  75 |  {
  76 |      QMessageBox box(parent);
  77 |      box.setWindowTitle(tr("Close wallet"));
  78 | -    box.setText(tr("Are you sure you wish to close wallet <i>%1</i>?").arg(wallet_model->getDisplayName()));
  79 | +    box.setText(tr("Are you sure you wish to close the wallet <i>%1</i>?").arg(wallet_model->getDisplayName()));

jonatack commented at 6:49 AM on September 9, 2019:

This PR and #16826 both touch this one line. Trivial rebase for the whichever is not merged first.

fanquake added the label Needs rebase on Sep 9, 2019
gui: rename encrypt(), blank(), and askPasshprase()
as well as disablePrivateKeys() to be consistent in naming.
43aa9b0d79

gui: fix passphrase labels/tooltip in createwalletdialog/askpassphrasedialog

UI improvements:
- update remaining GUI wallet labels and tooltips from passwords to passphrases
- improve grammar of labels in askpassphrase dialog and WalletController::closeWallet

539d9403af

jonatack force-pushed on Sep 9, 2019
gui: fix autofocus in CreateWalletActivity::askPassphrase() cad3ab5db8
jonatack force-pushed on Sep 9, 2019
jonatack referenced this in commit 91f70fd925 on Sep 9, 2019
DrahtBot removed the label Needs rebase on Sep 9, 2019
laanwj added this to the milestone 0.19.0 on Sep 10, 2019
kanzure commented at 12:49 PM on September 10, 2019: contributor

I am generally in favor of migrating from "password" to "passphrase". I made this comment in -wizards that nobody should talk about 'the password' in bip39 because 'word' already means a single word from the dictionary which is definitely low entropy. And then I was directed here.
jonatack commented at 1:18 PM on September 10, 2019: member

Yes, removing the last user-facing references to "password" has been on my to-do list since June. Since PRs to change UI can invite bikeshedding (there's been a bit of that lately), I waited until related code or a PR (in this case, the PR this one follows up on) touched it to push the changes in 539d9403af956c76ae0149a58c07c71a6b58ac69.
jb55 commented at 7:08 PM on September 13, 2019: member

Approach ACK cad3ab5db835e1d0c44c6a5fc0e4b2d1661dcd5c
instagibbs commented at 8:03 PM on September 13, 2019: member

code review and tACK cad3ab5db835e1d0c44c6a5fc0e4b2d1661dcd5c
fanquake approved
fanquake commented at 8:38 AM on September 16, 2019: member

ACK cad3ab5db835e1d0c44c6a5fc0e4b2d1661dcd5c

Tested the auto focusing behaviour on macOS. Other changes seem ok.

<img width="618" alt="to to for" src="https://user-images.githubusercontent.com/863730/64944134-32ca5a80-d8a0-11e9-9659-9f2111f73012.png">
fanquake referenced this in commit a953429a0e on Sep 16, 2019
fanquake merged this on Sep 16, 2019
fanquake closed this on Sep 16, 2019
jonatack deleted the branch on Sep 16, 2019
sidhujag referenced this in commit 2b7ff1f581 on Sep 16, 2019
jasonbcox referenced this in commit 075c236162 on Oct 20, 2020
deadalnix referenced this in commit 334559d6fb on Oct 20, 2020
jasonbcox referenced this in commit 5c69e06912 on Oct 21, 2020
DrahtBot locked this on Dec 16, 2021