feat!: a new token list approach #18965
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Jenkins BuildsClick to see older builds (582)
|
saledjenic
force-pushed
the
feat/new-token-list
branch
from
5b2deb1 to
a814eb4
Compare
saledjenic
force-pushed
the
feat/new-token-list
branch
7 times, most recently
from
475b5a4 to
cde8469
Compare
saledjenic
force-pushed
the
feat/new-token-list
branch
5 times, most recently
from
436c456 to
4bf30f0
Compare
saledjenic
force-pushed
the
feat/new-token-list
branch
6 times, most recently
from
c5c43c8 to
f8b2bfc
Compare
saledjenic
force-pushed
the
feat/new-token-list
branch
7 times, most recently
from
0f34a00 to
6d9de92
Compare
saledjenic
force-pushed
the
feat/new-token-list
branch
2 times, most recently
from
677d1f5 to
0786ddf
Compare
micieslak
reviewed
Dec 8, 2025
Member
micieslak
left a comment
micieslak
left a comment
There was a problem hiding this comment.
Huge work! LGTM in general, just some things regarding code structure I think are easy and worth to do imo. Plus some other really minor comments.
| e => { accounts.add(e.accountAddress) })) | ||
|
|
||
| accountsSelector.model = [...accounts.values()] | ||
| Qt.callLater(() => { |
Member
There was a problem hiding this comment.
Hmm, why is this callLater needed here?
Contributor
Author
There was a problem hiding this comment.
@micieslak if the last opened page in the Storybook was CollectiblesSelectionAdaptor without callLater the Storybook crashes, cause Component.onCompleted is accessing accountsSelector before it is initialized.
| @@ -1,11 +1,74 @@ | |||
| import QtQuick | |||
|
|
|||
| import StatusQ.Core.Utils | |||
Member
There was a problem hiding this comment.
Please keep all stubs empty, just plain QtObject {}. If it's necessary to share some stub implementation among multiple SB pages or tests, create implementation under storybook/mocks, by inheriting from the store type`, like
import AppLayouts.Wallet.stores
TokenStore {
/* impl there */`
}This approach has some advantages, described in guideline. And will allow for further improvements in the future when all stubs are refactored to that form.
Comment on lines
+51
to
+52
| const token = SQUtils.ModelUtils.getByKey(d.selectedHolding.item.tokens, "chainId", root.selectedNetworkChainId) | ||
| return token.key |
Member
There was a problem hiding this comment.
Suggested change
| const token = SQUtils.ModelUtils.getByKey(d.selectedHolding.item.tokens, "chainId", root.selectedNetworkChainId) | |
| return token.key | |
| return SQUtils.ModelUtils.getByKey(d.selectedHolding.item.tokens, "chainId", root.selectedNetworkChainId, "key") |
| sectionName (optional) [string] - text to be rendered as a section | ||
| **/ | ||
| property alias model: sfpm.sourceModel | ||
| property var model |
Member
There was a problem hiding this comment.
Please update the model description instead of removing it :)
| return | ||
| } | ||
|
|
||
| let entry = SQUtils.ModelUtils.getByKey( |
saledjenic
force-pushed
the
feat/new-token-list
branch
from
f533577 to
9f2f8fe
Compare
Contributor
Author
|
@micieslak will prepare a separate PR tomorrow addressing your comments.
@caybro true, but that's not mandatory, since there are some issues in building them. There are also issues with windows build, but doesn't mean we should not merge this, since it's in the middle of the merging process (statusgo was already merged today).
@anastasiyaig yes, but Alex's PR should help, will see in a bit, I just rebased. |
siddarthkay
force-pushed
the
feat/new-token-list
branch
from
3a2223f to
9f2f8fe
Compare
Contributor
|
@siddarthkay I've seen your commit |
Contributor
|
@alexjba : yeah i removed that commit from this PR, I'm doing that bit in a separate branch. |
Contributor
|
windows CI is fixed here : #19537 |
Instead of using all tokens (atm there are ~15k unique tokens that the app operates with) in all parts of the app, a new token list, containing only tokens of interest, is being used. There is also a new list containing all tokens introduced that will be used in places where we really need all tokens, like drop-down list for Swap.
…ng and faster search - Introduced new model modes to token selector adapter for handling token groups, including lazy loading and search results. - Updated the TokenGroupsModel to support lazy loading of items and improved search functionality. - Refactored the UI components to utilize the new models, improving UX when dealing with large token models. - Loading indicator and debounce mechanisms added for search inputs to optimize performance.
saledjenic
force-pushed
the
feat/new-token-list
branch
from
9f2f8fe to
d3e93ce
Compare
Member
✔️ status-desktop/prs/linux/x86_64/tests-nim/PR-18965#71 🔹 ~9 min 52 sec 🔹 d3e93ce 🔹 📦 tests/nim package |
Member
Member
Member
Member
Member
✔️ status-desktop/e2e/prspr18965 🔹 ~15 min 🔹 d3e93ce 🔹 📦 tests/e2e package |
Contributor
Author
|
@micieslak your comments are addressed in this PR #19561 please have a look. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Depends on: