It fixes bitcoin-core/gui#756.
Each address book page window it’s now labeled with the wallet name they were opened with, so the user can easily identify which addresses belong to which wallet even when there are many windows opened. It’s a helpful enhancement for users managing multiple wallets.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | hebasto |
| Stale ACK | MarnixCroes |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
Tested 9fc21cfd3bdf2ab7d446a5f78318ce1b95b0dbbe. It works as expected.
However, I’m curious, doesn’t the ability to open “Sending/Receiving addresses” windows for multiple wallets rather confuse the user than improve their experience?
Maybe, it makes sense to make those windows modal?
Making those windows modal could be an alternative but it would change the present behaviour unexpectedly for those users that are used to it. Perhaps a current use case could be that a user may want to make transfers between wallets and needs to see their addresses.
Some other alternative could be that when a user selects a different wallet a dialog shows up asking to confirm the preference to close the addresses book page/ s (sending/ receiving/ both) hide them temporarily (until the user selects the wallet back) or just keep them open? Then we can also save it as a setting so that dialog doesn’t show up anymore or ask for confirmation every time.
326+
327+ if (mode == ForEditing) {
328+ switch(tab)
329+ {
330+ case SendingTab: setWindowTitle(tr("Sending addresses - ").append(walletName)); break;
331+ case ReceivingTab: setWindowTitle(tr("Receiving addresses - ").append(walletName)); break;
0 case SendingTab: setWindowTitle(tr("Sending addresses - %s").arg(walletName)); break;
1 case ReceivingTab: setWindowTitle(tr("Receiving addresses - %s").arg(walletName)); break;
%s doesn’t actually work.
329+ {
330+ case SendingTab: setWindowTitle(tr("Sending addresses - ").append(walletName)); break;
331+ case ReceivingTab: setWindowTitle(tr("Receiving addresses - ").append(walletName)); break;
332+ }
333+ }
334+}
450@@ -451,3 +451,5 @@ void AddressTableModel::emitDataChanged(int idx)
451 {
452 Q_EMIT dataChanged(index(idx, 0, QModelIndex()), index(idx, columns.length()-1, QModelIndex()));
453 }
454+
455+QString AddressTableModel::GetWalletDisplayName() const { return walletModel->getDisplayName(); };
I was following the same convention as in 7 lines above to keep it consistent:
I’m happy to change it, or move it to the header file maybe? I saw other places in the code where we have single line in headers with the implementation, but there were mostly getters and setters I think.
Extend addresstablemodel to return the display name from the wallet and
set it to the addressbookpage window title when its model is set.
326+
327+ if (mode == ForEditing) {
328+ switch(tab)
329+ {
330+ case SendingTab: setWindowTitle(tr("Sending addresses - %s").arg(walletName)); break;
331+ case ReceivingTab: setWindowTitle(tr("Receiving addresses - %s").arg(walletName)); break;
0 case SendingTab: setWindowTitle(tr("Sending addresses - %1").arg(walletName)); break;
1 case ReceivingTab: setWindowTitle(tr("Receiving addresses - %1").arg(walletName)); break;
Please refer to https://doc.qt.io/qt-5/qstring.html#arg