-
Notifications
You must be signed in to change notification settings - Fork 979
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
fix(wallets): no routes render on input amount #20112
Conversation
d91db35
to
0074c5e
Compare
Jenkins BuildsClick to see older builds (28)
|
@BalogunofAfrica in the video, I see it is showing "no routes found" after disabling a network and while still loading, is that correct behaviour? |
Not related to this PR: it seems on smaller devices when trying to display routes for the 3 networks it will be cropped from the bottom cc @J-Son89 |
Thanks @BalogunofAfrica for addressing this! Also, when the amount is invalid after disabling a network we shouldn't fetch routes again. Instead, we should reset the values of the sender and receiver cards to zero (similar to what we do there
|
Yes according to the original issue here: #19910 |
We should not load new routes in that case, the input is invalid at that point so we can immediately show the error to the user and avoid a loading state. Please check with design team if any doubts. |
@@ -334,7 +336,7 @@ | |||
:fees fee-formatted | |||
:amount amount-text | |||
:receiver (address/get-shortened-key to-address)}]) | |||
(when (or no-routes-found? limit-insufficient?) | |||
(when no-routes-found? |
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.
this component and it's bindings is becoming larger and larger. Do we think we can componentize some smaller parts of this?
e.g we make a [no-routes ...] component in this view just to make this block slightly smaller? 🤔 🙏
f550d44
to
2005f35
Compare
#(when input-error (debounce/clear-all)) | ||
(fn [] | ||
(when input-error (debounce/clear-all)) | ||
(when routes (rf/dispatch [:wallet/reset-network-amounts-to-zero]))) |
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.
I made a mistake, we should check for (and limit-insufficient? routes)
(assoc-in [:wallet :ui :send :loading-suggested-routes?] true))})) | ||
{:db (assoc-in db [:wallet :ui :send :disabled-from-chain-ids] chain-ids)})) | ||
|
||
(rf/reg-event-fx :wallet/reset-network-amounts-to-zero |
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.
I think for #16974 we would just need to set a locked-network-values map and maybe modify network-amounts utils function to handle a locked type to render in the card. This utils is very simple, just map and modify current values to zero.
1e46f27
to
62b04c0
Compare
85% of end-end tests have passed
Failed tests (6)Click to expandClass TestWalletOneDevice:
Class TestWalletMultipleDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Expected to fail tests (2)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Passed tests (44)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestDeepLinksOneDevice:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestWalletOneDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
|
62b04c0
to
4769f3b
Compare
4769f3b
to
07723b3
Compare
Hi @BalogunofAfrica ! Thanks for your PR. |
07723b3
to
27090df
Compare
27090df
to
1a5ec02
Compare
fixes #19910
fixes #20111
fixes #20136
Summary
Render "No routes Found" only based on the flows in 1 and 2
Platforms
Functional
Steps to test