Introduce Wallet Select Dropdown

[johnny9]

WalletSelect is a Popup that appears after clicking the main WalletBadge in the DesktopNavigation bar. It contains a ListView that allows the user to select one of the wallets listed in the wallet directory.

This PR uses the arrow icon that is shared with the Tooltip so it is based off of that commit.

[johnny9]

Update from 4ef3149 to 5d191d1

• Updated include guard on wallet list model
• Added copyright header to walletlistmodel.cpp
• Adjusted padding on Add Wallet icon
• Changed closePolicy on WalletSelect Popup to CloseOnPressOutside
• Rebased with main

[GBKS]

so what's that "X" for?

The idea is that you can close the wallet that is currently open (result is that there would be no open wallet). And if you press "x" on a close wallet, you would remove it from the application altogether. We don't have those anywhere else in the UI yet on mobile.

Are there other options you may want to directly take on the wallet from here? Like changing the name? If we have more, we could instead use an ellipsis ... to access a sub-menu.

[hebasto]

@pablomartin4btc

tACK 4ef3149

Want to re-ACK?

[hebasto]

@D33r-Gee

tACK on WSL Ubuntu 22.04

To ensure that someone's ACK is related to a certain version of the branch, it is wise to mention the top commit hash. Besides, the merge script needs a top commit hash to count an ACK.

[pablomartin4btc]

so what's that "X" for?

The idea is that you can close the wallet that is currently open (result is that there would be no open wallet). And if you press "x" on a close wallet, you would remove it from the application altogether. We don't have those anywhere else in the UI yet on mobile.

Mmm... then we need to indicate somehow when the wallet is open and the "x" would close it and when it's closed and the "x" will remove it from the UI... and how do you get it back when has been removed? It's not very intuitive at the moment.

Also, I'm not sure if listing all wallets available in the datadir would be practical... perhaps this can be configured as an option and disabled as default or if there are more then 5/ 10 wallets asking the user and persist that into the config?

Are there other options you may want to directly take on the wallet from here? Like changing the name? If we have more, we could instead use an ellipsis ... to access a sub-menu.

Yeah, there's no "renaming" feature in QT currently, it could be useful and we can put the "remove" from the app or "close" the wallet... still, how do you added back once removed?

[pablomartin4btc]

@pablomartin4btc

tACK 4ef3149

Want to re-ACK?

Checking... somehow I missed 5d191d1bedda3f10ab4457518cd5a5f249a23d73.

[GBKS]

Pablo and I spoke about the wallet selector and I did another design iteration based on my current understanding. Here's a video that covers it. Still have some minor questions in there, please take a look and let me know how it looks.

[GBKS]

Thanks for the feedback.

ordering the wallet list on last opened criteria (last 5? then alphabetical?);

Why not always use the last open date?

scroll solution

I will mock this up

icon for remove wallet looks fine but for closing looks a bit simple...

I'll do another revision of this design as we discussed yesterday, where the wallet options are in a small overlay menu

once we remove a wallet, perhaps after the user clicks on the "Add +" we show these options: create a new or "add previously removed walet" or "link a wallet dir"?

The current design is that in the "Add wallet" screen, you can import. From there you select the wallet file you want to add. Does that work?

we still have the issue when there are many wallets

Why is this an issue?

[pablomartin4btc]

ordering the wallet list on last opened criteria (last 5? then alphabetical?);

Why not always use the last open date?

Depends on the amount of wallets we are talking about, that's what I call "issue", perhaps on the mobile version doesn't make sense to have as many, not sure. I'll ask other devs.

once we remove a wallet, perhaps after the user clicks on the "Add +" we show these options: create a new or "add previously removed walet" or "link a wallet dir"?

The current design is that in the "Add wallet" screen, you can import. From there you select the wallet file you want to add. Does that work?

Ok, maybe that's the use case. Is it possible for you to do a prototype as you did in the previous video to see how it would work? From removing the wallet from the list to add it back (if it's doable with the tool you are using).

we still have the issue when there are many wallets

Why is this an issue?

Performance will be impacted loading multiple wallets but what I'm not convinced about is building the list of wallets in the menu with whatever amount of wallets there are in the data directory. I'll ask around, perhaps there's nothing wrong with it and I'm just overly worried.

[D33r-Gee]

re-tACK 5d191d1 on WSL Ubuntu 22.04 works as expected...

Does it need rebase?

[D33r-Gee]

tACK https://github.com/bitcoin-core/gui-qml/commit/411a08a55bfeba795ede692233134e31ca58cbe7 on WSL Ubuntu 22.04

Works as expected...

Ubuntu 22.04 Screenshot


[D33r-Gee]

tACK 833fc38 on WSL Ubuntu 22.04

Works as expected... Only comment is about a missing ending line in WalletBadge.qml

Screenshot from Ubuntu


[johnny9]

from 833fc38 to 366c605

• New line at the end of WalletBadge.qml
• Constrained width of wallet names as suggested by Marnix
• Constrained height of wallet list as suggested by Marnix and they are now scrollable
• Fixed colors to match latest figma

previous (833fc38):

update (366c605)

Will address other significant changes as new PRs as to not make this one too large for now. For instance, load/delete buttons in the wallet list I plan to have as a follow up as well as the logic for ordering the list based on recent use.

[johnny9]

Update from 366c605 to 0939f2b

• Fixed typo in comment in WalletBadge.qml

[D33r-Gee]

tACK 0939f2b on WSL Ubuntu 22.04

Works as expected and looks great

Ubuntu Screenshot


[hebasto]

re-ack 0939f2b

FWIW, "ack" is not counted by the merge script. The "ACK" is expected.