Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wallet enabling rework #1232

Merged
merged 22 commits into from
Sep 2, 2021
Merged

Wallet enabling rework #1232

merged 22 commits into from
Sep 2, 2021

Conversation

ghost
Copy link

@ghost ghost commented Aug 31, 2021

Will close #1212 when ready.

It's in a draft stage for the moment.

@ghost ghost requested a review from Milerius August 31, 2021 09:05
@Milerius Milerius requested a review from smk762 August 31, 2021 11:24
@Milerius
Copy link
Contributor

Milerius commented Aug 31, 2021

You can start checking @smk762

i suggest:

  • try with 5-10 coins enabled, logout/login multiple times
  • same with 50~coins
  • same with 100~coins

If you have any suggestion on the behaviour meanwhile coins are getting "enabled" please feel free.

One of my suggestion would be to split into more batches if more than 5 coins are enabled at the same time or on starting.

What do you think @tonymorony ?

This way if one coin cause the whole batch to not be answered we can have more batch

we can also do a batch less solution, and enable one by one

@smk762
Copy link
Collaborator

smk762 commented Aug 31, 2021

  • batch all parent coins first
  • batch the rest in groups of 5 or 10

@ghost
Copy link
Author

ghost commented Sep 1, 2021

Detected various frontend issues:

  • Notifications panel UX not good
  • Dashboard "Show only coins with balance" switch also affects the wallet page which should not be the case
  • Improve dashboard's coins list loading time

@Milerius
Copy link
Contributor

Milerius commented Sep 1, 2021

I found the reason of the slowing bug when enabling lot of coins, it's due too:

src/core/atomicdex/models/qt.portfolio.model.cpp:42

  • m_model_proxy->setDynamicSortFilter(true);

This property make Qt dynamically recalculate the whole sorting/filtering each time a bunch of coins are added, and this actions can be slow if repeated too often.

Few ideas how to solve that:

  • At construction, set this property to false
  • When the number of active coins reach the number of enabled coins, re-enable it to true
  • If the user require to add too much coins at once (let's say more than 20) then we need to turn off this property to off again, and post enabling, re-enable it + resort.

can you take care of that @SylEze ?

With property to true:

Enregistrement.de.l.ecran.2021-09-01.a.11.36.33.mov

With property to false (no filtering - no sort):

Enregistrement.de.l.ecran.2021-09-01.a.11.38.13.mov

@smk762
Copy link
Collaborator

smk762 commented Sep 1, 2021

Balance from enable works great, seen as soon as added to list.

@tonymorony tonymorony marked this pull request as ready for review September 2, 2021 08:34
@tonymorony tonymorony self-requested a review September 2, 2021 08:34
Copy link
Contributor

@tonymorony tonymorony left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's resolving the problem of indefinitely hang of app startup with a lot of assets

however, there are two more problems which we need to resolve @SylEze @Milerius :

  1. if you start the app with 100-200 assets app became unresponsive/hangs for 30 seconds or so
  2. when there are a lot of assets there is a loading spinner on tabs switching to the dashboard

@tonymorony tonymorony merged commit 4295810 into dev Sep 2, 2021
@smk762 smk762 deleted the wallet_enabling_rework branch November 7, 2021 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE REQUEST]: Add possibility to remove not working asset from assets list
3 participants