-
Notifications
You must be signed in to change notification settings - Fork 394
[Wallets] Sergei / Suggestion to avoid loading when the user switches between wallets #11139
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
[Wallets] Sergei / Suggestion to avoid loading when the user switches between wallets #11139
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Kudos, SonarCloud Quality Gate passed!
|
|
A production App ID was automatically generated for this PR. (log)
Click here to copy & paste above information. |
|
🚀 Smoke test run (2) passed successfully! |
|
🚨 Lighthouse report for the changes in this PR:
Lighthouse ran with https://deriv-app-git-fork-sergei-deriv-sergei-suggestion-to-avo-12b8e9.binary.sx/ |
| const { data, ...rest } = useQuery('authorize', { | ||
| payload: { authorize: current_token || '' }, | ||
| options: { enabled: Boolean(current_token) }, | ||
| options: { enabled: Boolean(current_token), keepPreviousData: true }, |
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.
does this have effects only on wallets, or is it used elsewhere too?
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.
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.
could it cause a component to use stale data from previous authorisation e.g. while submitting a transfer, or reseting a balance, in case of new authorisation data being delayed/erorrer (e.g. poor network connection, which is common among our customers). E.g. lets say that navigation worked correctly and moved on to new route/component, but authorisation is still providing stale data from previous account.
I think best way to go would be to somehow explicitly support loading and error state - at least it ensures no weird edge cases.
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.
Had a thought to add this before, and farzin was saying better not to hide the actual problem. I think multiple authorization could solve this issue https://app.clickup.com/t/20696747/PLA-623
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.
@wojciech-deriv Understood your point, didn't think about this before. I'm not sure it could be a problem in our project, but we don't need to risk. Anyway, we can add this property to the hook which works with account_list endpoint when it will be created by BE team, but for now maybe would be better leave it as it now.
@farhan-nurzi-deriv yeah, hope this issue will be solved by this solution, thank you!









Changes:
Add
keepPreviousDatatouseAuthorizehook. When the endpointaccounts_listwill be created and separated fromauthorizerequest then we can movekeepPreviousDataproperty to the new hook.You can read about keepPreviousData property here
Video:
[Desktop]
How it was:
Desktop.Loading.when.switching.between.wallets.mov
How it now:
Desktop.Without.loading.mov
[Responsive]
How it was:
Responsive.Loading.when.swithes.between.wallets.mov
How it now:
Responsive.Without.loading.mov