-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
chore(@desktop/wallet): Simplify the wallet networks service #13995
Conversation
5963be3
to
b97e016
Compare
Jenkins BuildsClick to see older builds (57)
|
ff17f9a
to
4e33673
Compare
3f3bb33
to
bde077c
Compare
✔️ status-desktop/prs/linux/x86_64/tests-nim/PR-13995#12 🔹 ~6 min 45 sec 🔹 bde077c 🔹 📦 tests/nim package |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great simplification of networks handling. The common source of truth will hopefully help avoid network related issues in the future. I still don't like how test mode is everywhere instead of just having available networks, maybe in a future we can improve on this also.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
There's some cleanup we can do on the collectibles side by left-joining network info instead of inserting it into each entry (https://github.com/status-im/status-desktop/blob/master/src/app/modules/shared_models/collectibles_entry.nim#L16)
Yes we could create a follow up task for that so that its not lost. |
bde077c
to
fd78a4d
Compare
fixes #12717
What does the PR do
Simplifying the networks service and creating only 2 base types -
Moving all the layer1, layer2 , enabled submodels to qml by using SortFilterProxyModels
I will be moving the networks submodule in profile in the next PR as this PR is already too bug.
Affected areas
Networks in UI (all app)
StatusQ checklist
Screenshot of functionality (including design for comparison)