Add wallet name to address book page title #757
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ReviewsSee the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update. ConflictsReviewers, 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 9fc21cf. 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. |
src/qt/addresstablemodel.cpp
Outdated
| @@ -451,3 +451,5 @@ void AddressTableModel::emitDataChanged(int idx) | |||
| { | |||
| Q_EMIT dataChanged(index(idx, 0, QModelIndex()), index(idx, columns.length()-1, QModelIndex())); | |||
| } | |||
|
|
|||
| QString AddressTableModel::GetWalletDisplayName() const { return walletModel->getDisplayName(); }; | |||
There was a problem hiding this comment.
nit: This shouldn't be flattened to a single line.
There was a problem hiding this comment.
I was following the same convention as in 7 lines above to keep it consistent:
gui/src/qt/addresstablemodel.cpp
Line 448 in 9fc21cf
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.
pablomartin4btc
force-pushed
the
gui-wallet-on-address-page-title
branch
from
9fc21cf to
e604229
Compare
Extend addresstablemodel to return the display name from the wallet and set it to the addressbookpage window title when its model is set.
pablomartin4btc
force-pushed
the
gui-wallet-on-address-page-title
branch
from
e604229 to
58c9b50
Compare
58c9b50 gui: Add wallet name to address book page (pablomartin4btc) Pull request description: 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.  ACKs for top commit: hebasto: ACK 58c9b50, tested on Ubuntu 22.04. Tree-SHA512: 82febc020653560281da144cd35c672c49ca9f48b23d173eb19395e9ab4d045500295a9b5f24c82efdbf6e7ea70c87e733207cb6a31d3f84828b27e3b2df558b
It fixes #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.