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 |
|---|---|
 |
 |
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:
0diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp
1index 3cceb5ca5..875dd0c14 100644
2--- a/src/qt/walletcontroller.cpp
3+++ b/src/qt/walletcontroller.cpp
4@@ -206,6 +206,7 @@ void WalletControllerActivity::showProgressDialog(const QString& label_text)
5 m_progress_dialog->setLabelText(label_text);
6 m_progress_dialog->setRange(0, 0);
7 m_progress_dialog->setCancelButton(nullptr);
8+ m_progress_dialog->setWindowTitle(PACKAGE_NAME);
9 m_progress_dialog->setWindowModality(Qt::ApplicationModal);
10 GUIUtil::PolishProgressDialog(m_progress_dialog);
11 // 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 |
|---|---|
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:
0$ git grep -n setWindowTitle
1src/qt/addressbookpage.cpp:84: case SendingTab: setWindowTitle(tr("Choose the address to send coins to")); break;
2src/qt/addressbookpage.cpp:85: case ReceivingTab: setWindowTitle(tr("Choose the address to receive coins with")); break;
3src/qt/addressbookpage.cpp:96: case SendingTab: setWindowTitle(tr("Sending addresses")); break;
4src/qt/addressbookpage.cpp:97: case ReceivingTab: setWindowTitle(tr("Receiving addresses")); break;
5src/qt/askpassphrasedialog.cpp:51: setWindowTitle(tr("Encrypt wallet"));
6src/qt/askpassphrasedialog.cpp:59: setWindowTitle(tr("Unlock wallet"));
7src/qt/askpassphrasedialog.cpp:62: setWindowTitle(tr("Change passphrase"));
8src/qt/bitcoingui.cpp:1387: setWindowTitle(window_title);
9src/qt/editaddressdialog.cpp:29: setWindowTitle(tr("New sending address"));
10src/qt/editaddressdialog.cpp:32: setWindowTitle(tr("Edit receiving address"));
11src/qt/editaddressdialog.cpp:36: setWindowTitle(tr("Edit sending address"));
12src/qt/psbtoperationsdialog.cpp:28: setWindowTitle("PSBT Operations");
13src/qt/receiverequestdialog.cpp:49: setWindowTitle(tr("Request payment to %1").arg(info.label.isEmpty() ? info.address : info.label));
14src/qt/sendcoinsdialog.cpp:1037: setWindowTitle(title); // On macOS, the window title is ignored (as required by the macOS Guidelines).
15src/qt/splashscreen.cpp:122: setWindowTitle(titleText + " " + titleAddText);
16src/qt/transactiondescdialog.cpp:18: setWindowTitle(tr("Details for %1").arg(idx.data(TransactionTableModel::TxHashRole).toString()));
17src/qt/utilitydialog.cpp:41: setWindowTitle(tr("About %1").arg(PACKAGE_NAME));
18src/qt/utilitydialog.cpp:60: setWindowTitle(tr("Command-line options"));
19src/qt/utilitydialog.cpp:158: shutdownWindow->setWindowTitle(window->windowTitle());
20src/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:
0showProgressDialog(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.
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?
0 showProgressDialog(tr("Create Wallet"), tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped()));
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”/ ?
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:
0@@ -350,10 +350,10 @@ LoadWalletsActivity::LoadWalletsActivity(WalletController* wallet_controller, QW
1 void LoadWalletsActivity::load()
2 {
3 showProgressDialog(
4- //: Title of progress window when which is displayed when wallets are being loaded.
5- tr("Load Wallet"),
6- //: Describing what is being done in background.
7- tr("Loading wallets…"));
8+ //: Title of progress window when which is displayed when wallets are being loaded.
9+ tr("Load Wallet"),
10+ //: Describing what is being done in background.
11+ tr("Loading wallets…"));
12
13 QTimer::singleShot(0, worker(), [this] {
14 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.
0 //: Title of window indicating the progress of the creation of a new wallet.
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
0@@ -236,7 +236,7 @@ void CreateWalletActivity::createWallet()
1 //: Title of progress window when Create Wallet action is selected.
2 tr("Create Wallet"),
3 //: Describing what is being done in background.
4- tr("Creating Wallet <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped()));
5+ tr("Creating Wallet") + QString(" <b>%1</b>…").arg(m_create_wallet_dialog->walletName().toHtmlEscaped()));
6
7 std::string name = m_create_wallet_dialog->walletName().toStdString();
8 uint64_t flags = 0;
9@@ -331,7 +331,7 @@ void OpenWalletActivity::open(const std::string& path)
10 //: Title of progress window when Open Wallet action is selected.
11 tr("Open Wallet"),
12 //: Describing what is being done in the background.
13- tr("Opening Wallet <b>%1</b>…").arg(name.toHtmlEscaped()));
14+ tr("Opening Wallet") + QString(" <b>%1</b>…").arg(name.toHtmlEscaped()));
15
16 QTimer::singleShot(0, worker(), [this, path] {
17 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.
0 //: Descriptive text of the create wallet progress window which indicates to the user what wallet is currently being created.
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.
0 //: Title of window indicating the progress of opening a wallet.
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"),
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.
0 //: Descriptive text of the open wallet progress window which indicates to the user what wallet is currently being opened.
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.
0 //: Descriptive text of the load wallets progress window which indicates to the user that wallets are currently being loaded.
Updated from a086a37 to ccecfa86a010704ac5247fc637de28e71b3e71b2 (pr409.07 -> pr409.08)
Addressed @hebasto and @jarolrod comments
Changes:
Load Wallet -> Load Wallets
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?
0 //: Title of progress window which is displayed when wallets are being loaded.
?
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.