feat(@desktop/wallet): Adapt Token Selector #16733

Khushboo-dev-cpp · 2024-11-08T11:23:12Z

What does the PR do

Adapts the TokenSelectors and its components to adapt to new design from Ben here -
https://www.figma.com/design/FkFClTCYKf83RJWoifWgoX/Wallet-v2?node-id=25229-108897&t=Ech2sOd4zCGhGFzC-4
https://www.figma.com/design/FkFClTCYKf83RJWoifWgoX/Wallet-v2?node-id=25232-114042&t=x7Ukt7ixxp7Ji84n-4

Affected areas

TokenSelector
SendModal

Architecture compliance

I am familiar with the application architecture and agreed good practices.
My PR is consistent with this document: Status Desktop Architecture Guide

Screenshot of functionality (including design for comparison)

I've checked the design and this PR matches it

Token Selector Button size is not 24 not 21.
https://github.com/user-attachments/assets/6947b3d8-4256-4f4e-83ac-454781c6db41

Searchable Assets and Collectibles now have padding of 4px and spacing of 4x.
On Hover Collectible shows contract network
Only 5.5 items are shown
On search the top item is highlighted and user can press enter to quickly select it.
https://github.com/user-attachments/assets/d06cc015-3c1b-448c-8377-042616db8a62

Empty Collectibles list state

status-im-auto · 2024-11-08T11:30:23Z

Jenkins Builds

Click to see older builds (14)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`9e7dad7`	#1	2024-11-08 11:30:22	~6 min	`macos/aarch64`	🍎`dmg`
✔️	`9e7dad7`	#1	2024-11-08 11:31:42	~8 min	`tests/nim`	📄`log`
❌	`9e7dad7`	#1	2024-11-08 11:35:03	~11 min	`tests/ui`	📄`log`
✔️	`9e7dad7`	#1	2024-11-08 11:40:37	~17 min	`linux/x86_64`	📦`tgz`
✔️	`9e7dad7`	#1	2024-11-08 11:41:58	~18 min	`macos/x86_64`	🍎`dmg`
✔️	`9e7dad7`	#1	2024-11-08 11:45:47	~22 min	`windows/x86_64`	💿`exe`
✔️	`9e7dad7`	#1	2024-11-08 11:47:20	~23 min	`linux-nix/x86_64`	📦`tgz`

✔️	`a7ca3e6`	#2	2024-11-22 17:07:53	~5 min	`macos/aarch64`	🍎`dmg`
✔️	`a7ca3e6`	#2	2024-11-22 17:10:55	~8 min	`tests/nim`	📄`log`
✔️	`a7ca3e6`	#2	2024-11-22 17:14:44	~12 min	`tests/ui`	📄`log`
✔️	`a7ca3e6`	#2	2024-11-22 17:19:04	~16 min	`macos/x86_64`	🍎`dmg`
✔️	`a7ca3e6`	#2	2024-11-22 17:19:34	~17 min	`linux-nix/x86_64`	📦`tgz`
✔️	`a7ca3e6`	#2	2024-11-22 17:25:25	~22 min	`linux/x86_64`	📦`tgz`
✔️	`a7ca3e6`	#2	2024-11-22 17:27:56	~25 min	`windows/x86_64`	💿`exe`

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
✔️	`0b153d8`	#3	2024-11-25 09:39:45	~4 min	`macos/aarch64`	🍎`dmg`
✔️	`0b153d8`	#3	2024-11-25 09:43:05	~8 min	`tests/nim`	📄`log`
✔️	`0b153d8`	#3	2024-11-25 09:47:37	~12 min	`tests/ui`	📄`log`
✔️	`0b153d8`	#3	2024-11-25 09:50:38	~15 min	`macos/x86_64`	🍎`dmg`
✔️	`0b153d8`	#3	2024-11-25 09:52:44	~17 min	`linux-nix/x86_64`	📦`tgz`
✔️	`0b153d8`	#3	2024-11-25 09:56:10	~21 min	`linux/x86_64`	📦`tgz`
✔️	`0b153d8`	#3	2024-11-25 09:56:26	~21 min	`windows/x86_64`	💿`exe`

✔️	`ae4b190`	#5	2024-11-26 14:29:18	~7 min	`macos/aarch64`	🍎`dmg`
✔️	`ae4b190`	#5	2024-11-26 14:29:23	~7 min	`tests/nim`	📄`log`
❌	`ae4b190`	#5	2024-11-26 14:34:22	~12 min	`tests/ui`	📄`log`
✔️	`ae4b190`	#5	2024-11-26 14:36:10	~14 min	`linux-nix/x86_64`	📦`tgz`
✔️	`ae4b190`	#5	2024-11-26 14:38:31	~16 min	`linux/x86_64`	📦`tgz`
✔️	`ae4b190`	#5	2024-11-26 14:38:59	~16 min	`macos/x86_64`	🍎`dmg`
✔️	`ae4b190`	#5	2024-11-26 14:43:54	~21 min	`windows/x86_64`	💿`exe`

caybro

Otherwise LGTM

ui/app/AppLayouts/Wallet/stores/CollectiblesStore.qml

ui/app/AppLayouts/Wallet/panels/SearchableCollectiblesPanel.qml

micieslak

LGTM in general, but there are several details imo worth amending to simplify/sort out things.

ui/app/AppLayouts/Wallet/panels/SearchableAssetsPanel.qml

micieslak · 2024-11-18T12:22:06Z

ui/app/AppLayouts/Wallet/views/TokenSelectorAssetDelegate.qml

@@ -17,6 +17,7 @@ ItemDelegate {
    required property string symbol
    required property string currencyBalanceAsString
    required property string iconSource
+    required property bool isFirstSearchEntry


This property seems not necessary as we already have highlighted property there which could be used to highlight first element if necessary.

Currently we can highlight even 3 items at once, what doesn't seem to be correct:

(first, selected and hovered)

Setting highlighted in SearchableAssetsPanel` seems to solve it nicely:

highlighted: !!searchBox.text ? (index === 0) : (model.tokensKey === root.highlightedKey)

@micieslak the thing is that I dont want it to be highlighted but it should be the same as hovered state which is a readonly property. check line 40.

but perhaps the name could be more generic ?

But isn't the situation on the screenshot a bit strange?

Maybe if we want to have the first item "hovered" by default, that hover should disappear when we explicitly hover over another item? And go back to first when we are completely outside with the mouse cursor?

aaah I dont know but I overlooked that issue. Yes you are right, should be hovered by default but then should change if the user manually hovers over anther item.

I can check how to correct this logic and rename this property to something like - automaticHoverOn ?

@micieslak updated property name and behaviour now

Screen.Recording.2024-11-22.at.5.39.42.PM.mov

ui/app/AppLayouts/Wallet/stores/CollectiblesStore.qml

caybro

LGTM, just some smaller details

ui/app/AppLayouts/Wallet/panels/SearchableAssetsPanel.qml

ui/app/AppLayouts/Wallet/panels/SearchableCollectiblesPanel.qml

ui/app/AppLayouts/Wallet/views/TokenSelectorCollectibleDelegate.qml

fixes #16702

caybro

LGTM!

micieslak

Looks great! Thank you for introducing the proposed amendments!

micieslak · 2024-11-26T22:54:55Z

ui/app/AppLayouts/Wallet/controls/TokenSelector.qml

@@ -34,6 +34,11 @@ Control {
        tokenSelectorPanel.highlightedKey = key ?? ""
    }

+    QObject {
+        id: d


we keep empty line after id

micieslak · 2024-11-26T22:55:10Z

ui/app/AppLayouts/Wallet/controls/TokenSelector.qml

+        onClosed: tokenSelectorPanel.clear()
+
+        contentItem: Item {
+


not necessary empty line here

micieslak · 2024-11-26T22:58:30Z

ui/app/AppLayouts/Wallet/panels/SearchableAssetsPanel.qml

@@ -99,6 +110,7 @@ Control {
                highlighted: model.tokensKey === root.highlightedKey
                enabled: model.tokensKey !== root.nonInteractiveKey
                balancesListInteractive: !ListView.view.moving
+                isAutoHovered: d.validSearchResultExists && index === 0 && !listViewHoverHandler.hovered


Nice and clean, here and in TokenSelectorAssetDelegate :)

micieslak · 2024-11-26T23:04:18Z

ui/imports/shared/popups/send/SendModal.qml

@@ -409,6 +409,10 @@ StatusDialog {
                            sourceComponent: CollectiblesSelectionAdaptor {
                                accountKey: popup.selectedAccount.address

+                                networksModel: SortFilterProxyModel {


Ideally we should take just networsModel as a top-level input param and use it as it is, with no extra shenanigans inside. So not in store, but also not here as it shouldn't be this component responsibility to do that. But ofc we can try to improve it later, as it probably triggers more work.

Khushboo-dev-cpp marked this pull request as ready for review November 8, 2024 11:34

Khushboo-dev-cpp requested review from micieslak, a team, caybro and alexjba as code owners November 8, 2024 11:34

Khushboo-dev-cpp requested review from Cuteivist and removed request for a team November 8, 2024 11:34

caybro reviewed Nov 18, 2024

View reviewed changes

ui/app/AppLayouts/Wallet/stores/CollectiblesStore.qml Outdated Show resolved Hide resolved

ui/app/AppLayouts/Wallet/panels/SearchableCollectiblesPanel.qml Outdated Show resolved Hide resolved

micieslak requested changes Nov 18, 2024

View reviewed changes

Khushboo-dev-cpp force-pushed the feat/16702 branch 2 times, most recently from a7ca3e6 to 0b153d8 Compare November 25, 2024 09:34

caybro reviewed Nov 26, 2024

View reviewed changes

Khushboo-dev-cpp force-pushed the feat/16702 branch from 0b153d8 to f96d34b Compare November 26, 2024 14:20

feat(@desktop/wallet): Adapt Token Selector

ae4b190

fixes #16702

Khushboo-dev-cpp force-pushed the feat/16702 branch from f96d34b to ae4b190 Compare November 26, 2024 14:21

caybro approved these changes Nov 26, 2024

View reviewed changes

micieslak approved these changes Nov 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(@desktop/wallet): Adapt Token Selector #16733

feat(@desktop/wallet): Adapt Token Selector #16733

Khushboo-dev-cpp commented Nov 8, 2024 •

edited

Loading

status-im-auto commented Nov 8, 2024 •

edited

Loading

caybro left a comment

micieslak left a comment

micieslak Nov 18, 2024

Khushboo-dev-cpp Nov 22, 2024

Khushboo-dev-cpp Nov 22, 2024

micieslak Nov 22, 2024

Khushboo-dev-cpp Nov 22, 2024 •

edited

Loading

Khushboo-dev-cpp Nov 22, 2024

caybro left a comment

caybro left a comment

micieslak left a comment

micieslak Nov 26, 2024

micieslak Nov 26, 2024

micieslak Nov 26, 2024

micieslak Nov 26, 2024

feat(@desktop/wallet): Adapt Token Selector #16733

Are you sure you want to change the base?

feat(@desktop/wallet): Adapt Token Selector #16733

Conversation

Khushboo-dev-cpp commented Nov 8, 2024 • edited Loading

What does the PR do

Affected areas

Architecture compliance

Screenshot of functionality (including design for comparison)

status-im-auto commented Nov 8, 2024 • edited Loading

Jenkins Builds

caybro left a comment

Choose a reason for hiding this comment

micieslak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Khushboo-dev-cpp Nov 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caybro left a comment

Choose a reason for hiding this comment

caybro left a comment

Choose a reason for hiding this comment

micieslak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Khushboo-dev-cpp commented Nov 8, 2024 •

edited

Loading

status-im-auto commented Nov 8, 2024 •

edited

Loading

Khushboo-dev-cpp Nov 22, 2024 •

edited

Loading