Throughout the GUI, the title of the window, tells about the purpose of the window. This was not true for the title of wallet loading wallet. This PR fixes this issue by renaming the wallet loading window title to 'Open Wallet'
Changes introduced in this PR (Runned Bitcoin-GUI on signet network)
| Master | PR |
|---|---|
 |
 |
This will also break the settings in the registry, IIRC
Considering the following
user could end with two installed versions of Bitcoin Core, no?
Approach NACK.
Why not modify WalletControllerActivity::showProgressDialog?
Or use QGuiApplication::applicationDisplayName? But not sure about the latter...
This is quite an invasive change, why not just do:
diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp
index 3cceb5ca5..875dd0c14 100644
--- a/src/qt/walletcontroller.cpp
+++ b/src/qt/walletcontroller.cpp
@@ -206,6 +206,7 @@ void WalletControllerActivity::showProgressDialog(const QString& label_text)
m_progress_dialog->setLabelText(label_text);
m_progress_dialog->setRange(0, 0);
m_progress_dialog->setCancelButton(nullptr);
+ m_progress_dialog->setWindowTitle(PACKAGE_NAME);
m_progress_dialog->setWindowModality(Qt::ApplicationModal);
GUIUtil::PolishProgressDialog(m_progress_dialog);
// The setValue call forces QProgressDialog to start the internal duration estimation.
~ACK abcc83a71876b6fff4d853392e10a06f0c7a1c38~ (see #409 (comment)), tested on Linux Mint 20.2 (Qt 5.12.8):
| master | this PR |
|---|---|
 |
 |
@ShaMan239 Before merging, could you update the PR title and description to describe use cases more precise:
tACK abcc83a71876b6fff4d853392e10a06f0c7a1c38
tested on macOS 12 and Ubuntu 20.04, thanks for picking up the suggestion!
It should be noted that the behavior on master for macOS was a slightly different problem. For macOS on master, there was no window title at all. Now there is one. This is shown in the screenshots below.
| Master | PR |
|---|---|
| <img width="330" alt="Screen Shot 2021-08-26 at 4 39 15 PM" src="https://user-images.githubusercontent.com/23396902/131032845-f526cee6-2490-4ed1-b108-0fdb6ea57015.png"> | <img width="330" alt="Screen Shot 2021-08-26 at 4 09 36 PM" src="https://user-images.githubusercontent.com/23396902/131029405-5eedc690-a7b3-46de-86e2-6e41f36953ed.png"> |
Hmm, is this approach consistent with our code base? Consider git grep -n setWindowTitle.
Hmm, is this approach consistent with our code base? Consider
git grep -n setWindowTitle.
Why isn't it? https://github.com/bitcoin-core/gui/blob/b8d45a3c2015568ecc699736bd750f36321d948f/src/qt/utilitydialog.cpp#L41
Here is is essentially the same, just appends more information to PACKAGE_NAME
https://github.com/bitcoin-core/gui/blob/b8d45a3c2015568ecc699736bd750f36321d948f/src/qt/bitcoingui.cpp#L1375-L1387
Hmm, is this approach consistent with our code base? Consider
git grep -n setWindowTitle.Why isn't it?
Consider other short-living auxiliary windows:
$ git grep -n setWindowTitle
src/qt/addressbookpage.cpp:84: case SendingTab: setWindowTitle(tr("Choose the address to send coins to")); break;
src/qt/addressbookpage.cpp:85: case ReceivingTab: setWindowTitle(tr("Choose the address to receive coins with")); break;
src/qt/addressbookpage.cpp:96: case SendingTab: setWindowTitle(tr("Sending addresses")); break;
src/qt/addressbookpage.cpp:97: case ReceivingTab: setWindowTitle(tr("Receiving addresses")); break;
src/qt/askpassphrasedialog.cpp:51: setWindowTitle(tr("Encrypt wallet"));
src/qt/askpassphrasedialog.cpp:59: setWindowTitle(tr("Unlock wallet"));
src/qt/askpassphrasedialog.cpp:62: setWindowTitle(tr("Change passphrase"));
src/qt/bitcoingui.cpp:1387: setWindowTitle(window_title);
src/qt/editaddressdialog.cpp:29: setWindowTitle(tr("New sending address"));
src/qt/editaddressdialog.cpp:32: setWindowTitle(tr("Edit receiving address"));
src/qt/editaddressdialog.cpp:36: setWindowTitle(tr("Edit sending address"));
src/qt/psbtoperationsdialog.cpp:28: setWindowTitle("PSBT Operations");
src/qt/receiverequestdialog.cpp:49: setWindowTitle(tr("Request payment to %1").arg(info.label.isEmpty() ? info.address : info.label));
src/qt/sendcoinsdialog.cpp:1037: setWindowTitle(title); // On macOS, the window title is ignored (as required by the macOS Guidelines).
src/qt/splashscreen.cpp:122: setWindowTitle(titleText + " " + titleAddText);
src/qt/transactiondescdialog.cpp:18: setWindowTitle(tr("Details for %1").arg(idx.data(TransactionTableModel::TxHashRole).toString()));
src/qt/utilitydialog.cpp:41: setWindowTitle(tr("About %1").arg(PACKAGE_NAME));
src/qt/utilitydialog.cpp:60: setWindowTitle(tr("Command-line options"));
src/qt/utilitydialog.cpp:158: shutdownWindow->setWindowTitle(window->windowTitle());
src/qt/walletcontroller.cpp:87: box.setWindowTitle(tr("Close wallet"));
202 | @@ -203,6 +203,7 @@ void WalletControllerActivity::showProgressDialog(const QString& label_text) 203 | assert(!m_progress_dialog); 204 | m_progress_dialog = new QProgressDialog(m_parent_widget); 205 | 206 | + m_progress_dialog->setWindowTitle(PACKAGE_NAME);
Search for setWindowTitle, typically windows/dialogs use titles. I'd add a title argument to showProgressDialog and update all call sites, like:
showProgressDialog(tr("Open Wallet"), tr("Opening Wallet <b>%1</b>…").arg(name.toHtmlEscaped()));
which uses the same text as the menu item.
Edit: looks like @hebasto already pointed this.
Concept ACK.
Updated from abcc83a to 58dca474673f190e947db4bd350926f1a3bd592c (pr409.02 -> pr409.03) Addressed Comments by @hebasto and @promag. Thanks, @hebasto, for pointing out the problem, and thanks, @promag, for the suggestion.
Changes made:
showProgressDialog function to accept two arguments.showProgressDialog with the new title as “Open Wallet.”251 | @@ -251,7 +252,7 @@ void CreateWalletActivity::askPassphrase() 252 | 253 | void CreateWalletActivity::createWallet() 254 | { 255 | - showProgressDialog(tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped())); 256 | + showProgressDialog(tr("Open Wallet"), tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped()));
Create Wallet is more appropriate than Open Wallet here, no?
showProgressDialog(tr("Create Wallet"), tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped()));
Addressed in d9e421c. Thanks for pointing that out.
251 | @@ -251,7 +252,7 @@ void CreateWalletActivity::askPassphrase() 252 | 253 | void CreateWalletActivity::createWallet() 254 | { 255 | - showProgressDialog(tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped())); 256 | + showProgressDialog(tr("Create Wallet"), tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped()));
nit
... which uses the same text as the menu item.
Maybe mention this in the translator comment?
ACK d9e421c3ba937fd37529a74dff07dcc6ec1cdc26
In commit message maybe s/"qt: Fix window title of wallet loading window"/"qt: Fix WalletControllerActivity progress dialog title"/ ?
ACK 8e11d89ea90be5433905ed207ff9a5a5dbe3e8cb
Updated from e4bf5804bdeee2b16d5caea64b97f002d8d625e5 to a086a3757b1edb06b601457142d36628ae15441d (pr409.06 -> pr409.07)
Changes:
348 | @@ -340,7 +349,11 @@ LoadWalletsActivity::LoadWalletsActivity(WalletController* wallet_controller, QW 349 | 350 | void LoadWalletsActivity::load() 351 | { 352 | - showProgressDialog(tr("Loading wallets…")); 353 | + showProgressDialog( 354 | + //: Title of progress window when which is displayed when wallets are being loaded. 355 | + tr("Load Wallet"), 356 | + //: Describing what is being done in background. 357 | + tr("Loading wallets…"));
Please fix indentation.
nit: s/"Load Wallet"/"Load Wallets"/ (singular -> plural).
@shaavan apply clang format:
@@ -350,10 +350,10 @@ LoadWalletsActivity::LoadWalletsActivity(WalletController* wallet_controller, QW
void LoadWalletsActivity::load()
{
showProgressDialog(
- //: Title of progress window when which is displayed when wallets are being loaded.
- tr("Load Wallet"),
- //: Describing what is being done in background.
- tr("Loading wallets…"));
+ //: Title of progress window when which is displayed when wallets are being loaded.
+ tr("Load Wallet"),
+ //: Describing what is being done in background.
+ tr("Loading wallets…"));
QTimer::singleShot(0, worker(), [this] {
for (auto& wallet : node().walletClient().getWallets()) {
nit: s/"Load Wallet"/"Load Wallets"/ (singular -> plural).
Fixed
apply clang-format:
Applied
Addressed in ccecfa8
231 | @@ -231,7 +232,11 @@ void CreateWalletActivity::askPassphrase() 232 | 233 | void CreateWalletActivity::createWallet() 234 | { 235 | - showProgressDialog(tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped())); 236 | + showProgressDialog( 237 | + //: Title of progress window when Create Wallet action is selected.
//: Title of window indicating the progress of the creation of a new wallet.
Addressed in ccecfa8
231 | @@ -231,7 +232,11 @@ void CreateWalletActivity::askPassphrase() 232 | 233 | void CreateWalletActivity::createWallet() 234 | { 235 | - showProgressDialog(tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped())); 236 | + showProgressDialog( 237 | + //: Title of progress window when Create Wallet action is selected. 238 | + tr("Create Wallet"), 239 | + //: Describing what is being done in background. 240 | + tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped()));
while here, can we break up the HTML tags from the string to be translated? Keeping these tags as part of the string is error-prone
@@ -236,7 +236,7 @@ void CreateWalletActivity::createWallet()
//: Title of progress window when Create Wallet action is selected.
tr("Create Wallet"),
//: Describing what is being done in background.
- tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped()));
+ tr("Creating Wallet") + QString(" <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped()));
std::string name = m_create_wallet_dialog->walletName().toStdString();
uint64_t flags = 0;
@@ -331,7 +331,7 @@ void OpenWalletActivity::open(const std::string& path)
//: Title of progress window when Open Wallet action is selected.
tr("Open Wallet"),
//: Describing what is being done in the background.
- tr("Opening Wallet <b>%1</b>…").arg(name.toHtmlEscaped()));
+ tr("Opening Wallet") + QString(" <b>%1</b>…").arg(name.toHtmlEscaped()));
QTimer::singleShot(0, worker(), [this, path] {
std::unique_ptr<interfaces::Wallet> wallet = node().walletClient().loadWallet(path, m_error_message, m_warning_message);
Keeping these tags as part of the string is error-prone
Oh, I was not aware of it. Thanks for enlightening me! Addressed in ccecfa8
while here, can we break up the HTML tags from the string to be translated? Keeping these tags as part of the string is error-prone
Why?
231 | @@ -231,7 +232,11 @@ void CreateWalletActivity::askPassphrase() 232 | 233 | void CreateWalletActivity::createWallet() 234 | { 235 | - showProgressDialog(tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped())); 236 | + showProgressDialog( 237 | + //: Title of progress window when Create Wallet action is selected. 238 | + tr("Create Wallet"), 239 | + //: Describing what is being done in background.
//: Descriptive text of the create wallet progress window which indicates to the user what wallet is currently being created.
This translator comment description looks a lot better! Addressed in ccecfa8
326 | @@ -322,7 +327,11 @@ void OpenWalletActivity::open(const std::string& path) 327 | { 328 | QString name = path.empty() ? QString("["+tr("default wallet")+"]") : QString::fromStdString(path); 329 | 330 | - showProgressDialog(tr("Opening Wallet <b>%1</b>…").arg(name.toHtmlEscaped())); 331 | + showProgressDialog( 332 | + //: Title of progress window when Open Wallet action is selected.
//: Title of window indicating the progress of opening a wallet.
Addressed in ccecfa8
348 | @@ -340,7 +349,11 @@ LoadWalletsActivity::LoadWalletsActivity(WalletController* wallet_controller, QW 349 | 350 | void LoadWalletsActivity::load() 351 | { 352 | - showProgressDialog(tr("Loading wallets…")); 353 | + showProgressDialog( 354 | + //: Title of progress window when which is displayed when wallets are being loaded. 355 | + tr("Load Wallet"),
Addressed in ccecfa8
326 | @@ -322,7 +327,11 @@ void OpenWalletActivity::open(const std::string& path) 327 | { 328 | QString name = path.empty() ? QString("["+tr("default wallet")+"]") : QString::fromStdString(path); 329 | 330 | - showProgressDialog(tr("Opening Wallet <b>%1</b>…").arg(name.toHtmlEscaped())); 331 | + showProgressDialog( 332 | + //: Title of progress window when Open Wallet action is selected. 333 | + tr("Open Wallet"), 334 | + //: Describing what is being done in the background.
//: Descriptive text of the open wallet progress window which indicates to the user what wallet is currently being opened.
Addressed in ccecfa8
348 | @@ -340,7 +349,11 @@ LoadWalletsActivity::LoadWalletsActivity(WalletController* wallet_controller, QW 349 | 350 | void LoadWalletsActivity::load() 351 | { 352 | - showProgressDialog(tr("Loading wallets…")); 353 | + showProgressDialog( 354 | + //: Title of progress window when which is displayed when wallets are being loaded. 355 | + tr("Load Wallet"), 356 | + //: Describing what is being done in background.
//: Descriptive text of the load wallets progress window which indicates to the user that wallets are currently being loaded.
Addressed in ccecfa8
You forgot this in your recent push
Oh! Thanks for catching it! Addressed in 01bff8f
Updated from a086a37 to ccecfa86a010704ac5247fc637de28e71b3e71b2 (pr409.07 -> pr409.08)
Addressed @hebasto and @jarolrod comments
Changes:
Load Wallet -> Load Wallets
I agree with hebasto. The strings shouldn't be broken up. Some languages might need to reorder the sentence.
350 | @@ -340,7 +351,11 @@ LoadWalletsActivity::LoadWalletsActivity(WalletController* wallet_controller, QW 351 | 352 | void LoadWalletsActivity::load() 353 | { 354 | - showProgressDialog(tr("Loading wallets…")); 355 | + showProgressDialog( 356 | + //: Title of progress window when which is displayed when wallets are being loaded.
Just to be sure, is the first "when" correct?
//: Title of progress window which is displayed when wallets are being loaded.
?
Oops! It shouldn't have been there. Let me quickly correct it!
Done Corrected! f86fe19
ACK 9de84f66bede3aeb7974e0b8840370ba9e5fd0e3
utACK f86fe193329e057b7c6b624c3b3f11d5ae04826f
Almost there, just need to update one of the translator comments: #409 (review)
Tested on macOS 12, here are all the new window titles in action:
| open wallet | create wallet | load wallet |
|---|---|---|
 |
 |
 |
The WalletControllerActivity progress dialog had title of "Bitcoin-Qt".
The window title for opening wallet window should be "Open Wallet",
for creating wallet window should be "Create Wallet", and for the window
that is displayed when wallets are being loaded at startup should be
"Load Wallets". This PR fixes that.
ACK 01bff8f0494ba1c02c087b7186c892785f326fd9, tested on Linux Mint 20.2 (Qt 5.12.8).