-
Notifications
You must be signed in to change notification settings - Fork 985
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
Added support for different currencies in token price calculation #18078
Conversation
Jenkins BuildsClick to see older builds (43)
|
ab49638
to
0f61265
Compare
0f61265
to
041a7d4
Compare
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.
Good work, just some small comments 👍
041a7d4
to
e7bad19
Compare
75cbfd1
to
0825a2e
Compare
0825a2e
to
f8bd043
Compare
81% of end-end tests have passed
Failed tests (3)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (6)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Passed tests (39)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
|
:<- [:profile/currency] | ||
:<- [:profile/currency-symbol] | ||
(fn [[{:keys [tokens color]} currency currency-symbol]] | ||
(mapv #(calc-token-value % color currency currency-symbol) tokens))) |
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.
@smohamedjavid is this taking into account the different network configuration of a given account? 🤔
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.
@J-Son89 - Not at this moment. Will work on that separately. 👍
50% of end-end tests have passed
Failed tests (1)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Passed tests (1)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
|
hi @smohamedjavid thank you for PR. No issues from my side. PR is ready to be merged |
…rice calculation Signed-off-by: Mohamed Javid <19339952+smohamedjavid@users.noreply.github.com>
Thank you for testing the PR @VolodLytvynenko. Were you able to see the |
f8bd043
to
87afbf4
Compare
@smohamedjavid yes. The usdc.e is shown. Thank you! |
@smohamedjavid, I missed one question and included a follow-up issue: link. |
@VolodLytvynenko - No worries, I will check that issue 👍
No, this was not in the scope of this PR, and this requires an update in |
fixes #17984 & #18160
Summary
This PR
wallet.-.currency.mp4
Testing notes
The new wallet home screen displays
€0.00
in the cumulative balance of all wallet accounts and tokens as its temporary data.Platforms
Steps to test
Advanced > Set currency
status: ready