Add currency conversions for signing sheet - Fixes #8027 #9063

acolytec3 · 2019-09-27T20:06:08Z

fixes #8027

Summary

Displays the amount and fee associated with a transaction in the User's wallet's currency

Functional

wallet / transactions

Steps to test

Open Status
Select Wallet from bottom taskbar
Select account tile
Select "Send"
Fill out required transaction details
Select "sign transaction"
Validate that amount/fee values display a converted wallet currency amount next to them

status: ready

status-github-bot · 2019-09-27T20:06:15Z

Pull Request Checklist

Docs: Updated the documentation, if affected
Docs: Added or updated inline comments explaining intention of the code
Tests: Ensured that all new UI elements have been assigned accessibility IDs
Tests: Signaled need for E2E tests with label, if applicable
Tests: Briefly described what was tested and what platforms were used
UI: In case of UI changes, ensured that UI matches Figma
UI: In case of UI changes, requested review from a Core UI designer
UI: In case of UI changes, included screenshots of implementation

status-im-auto · 2019-09-27T20:07:44Z

Jenkins Builds

Click to see older builds (36)

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
❌	`9f87e03`	#1	2019-09-27 20:07:44	~1 min	`linux`	📄`log`
❌	`9f87e03`	#1	2019-09-27 20:16:09	~9 min	`ios`	📄`log`
❌	`9f87e03`	#1	2019-09-27 20:21:21	~14 min	`windows`	📄`log`
❌	`9f87e03`	#1	2019-09-27 20:22:03	~15 min	`macos`	📄`log`
❌	`9f87e03`	#1	2019-09-27 20:24:42	~18 min	`android-e2e`	📄`log`
❌	`9f87e03`	#1	2019-09-27 20:24:54	~18 min	`android`	📄`log`

❌	`32df077`	#2	2019-09-28 01:02:06	~27 sec	`linux`	📄`log`
❌	`32df077`	#2	2019-09-28 01:05:32	~3 min	`ios`	📄`log`
❌	`32df077`	#2	2019-09-28 01:13:51	~12 min	`android-e2e`	📄`log`
❌	`32df077`	#2	2019-09-28 01:14:06	~12 min	`android`	📄`log`
✔️	`32df077`	#2	2019-09-28 01:14:22	~12 min	`macos`	📦`dmg`
✔️	`32df077`	#2	2019-09-28 01:15:30	~13 min	`windows`	📦`exe`

✔️	`819b68b`	#3	2019-09-28 10:15:55	~11 min	`ios`	📦`ipa` 📲
❌	`819b68b`	#3	2019-09-28 10:16:23	~12 min	`android-e2e`	📄`log`
✔️	`819b68b`	#3	2019-09-28 10:17:25	~12 min	`macos`	📦`dmg`
✔️	`819b68b`	#3	2019-09-28 10:18:31	~14 min	`linux`	📦`App`
✔️	`819b68b`	#3	2019-09-28 10:18:32	~14 min	`android`	📦`apk` 📲
✔️	`819b68b`	#3	2019-09-28 10:19:58	~15 min	`windows`	📦`exe`

✔️	`e104a6b`	#4	2019-09-29 01:35:11	~9 min	`ios`	📦`ipa` 📲
❌	`e104a6b`	#4	2019-09-29 01:38:05	~12 min	`android-e2e`	📄`log`
❌	`e104a6b`	#4	2019-09-29 01:38:11	~12 min	`android`	📄`log`
✔️	`e104a6b`	#4	2019-09-29 01:38:40	~13 min	`linux`	📦`App`
✔️	`e104a6b`	#4	2019-09-29 01:38:55	~13 min	`macos`	📦`dmg`
✔️	`e104a6b`	#4	2019-09-29 01:39:04	~13 min	`windows`	📦`exe`

❌	`1153594`	#5	2019-09-30 17:09:44	~38 min	`ios`	📄`log`
❌	`1153594`	#5	2019-09-30 17:09:57	~39 min	`macos`	📄`log`
❌	`1153594`	#5	2019-09-30 17:10:11	~39 min	`android-e2e`	📄`log`
❌	`1153594`	#5	2019-09-30 17:26:22	~55 min	`android`	📄`log`
❌	`1153594`	#5	2019-09-30 17:26:29	~55 min	`windows`	📄`log`
❌	`1153594`	#5	2019-09-30 17:26:47	~55 min	`linux`	📄`log`

❌	`765b0bb`	#6	2019-09-30 23:11:44	~31 sec	`android`	📄`log`
❌	`765b0bb`	#6	2019-09-30 23:11:44	~28 sec	`ios`	📄`log`
❌	`765b0bb`	#6	2019-09-30 23:11:48	~29 sec	`linux`	📄`log`
❌	`765b0bb`	#6	2019-09-30 23:11:49	~26 sec	`macos`	📄`log`
❌	`765b0bb`	#6	2019-09-30 23:11:50	~26 sec	`windows`	📄`log`
✔️	`765b0bb`	#6	2019-09-30 23:22:34	~11 min	`android-e2e`	📦`apk` 📲

❔	Commit	#️⃣	Finished (UTC)	Duration	Platform	Result
❌	`23f3d1b`	#7	2019-10-01 02:20:47	~4 min	`ios`	📄`log`
✔️	`23f3d1b`	#7	2019-10-01 02:29:29	~12 min	`android`	📦`apk` 📲
✔️	`23f3d1b`	#7	2019-10-01 02:29:34	~12 min	`macos`	📦`dmg`
✔️	`23f3d1b`	#7	2019-10-01 02:29:52	~13 min	`windows`	📦`exe`
✔️	`23f3d1b`	#7	2019-10-01 02:29:58	~13 min	`linux`	📦`App`
✔️	`23f3d1b`	#7	2019-10-01 02:30:33	~14 min	`android-e2e`	📦`apk` 📲

✔️	`d930c1f`	#8	2019-10-04 11:03:50	~12 min	`ios`	📦`ipa` 📲
✔️	`d930c1f`	#8	2019-10-04 11:08:20	~16 min	`android-e2e`	📦`apk` 📲
✔️	`d930c1f`	#8	2019-10-04 11:09:24	~17 min	`android`	📦`apk` 📲
✔️	`d930c1f`	#8	2019-10-04 11:09:56	~18 min	`linux`	📦`App`
✔️	`d930c1f`	#8	2019-10-04 11:12:26	~20 min	`macos`	📦`dmg`
✔️	`d930c1f`	#8	2019-10-04 11:16:23	~24 min	`windows`	📦`exe`

acolytec3 · 2019-09-28T01:24:32Z

@yenda How do I get access to the Jenkins logs to see what it doesn't like? I've run the tests and linter and both pass at this point.

cammellos · 2019-09-28T04:45:56Z

@acolytec3 does not seems related to the pr, tests fails because we run out of space on jenkins (maybe just make sure they pass locally in the meantime), @jakubgs could you have a look please?

jakubgs · 2019-09-28T08:21:33Z

I've cleaned up the hosts, should be good now:

 $ ansible ci-slave-linux -m shell -a 'df -h /'

linux-05.do-ams3.ci.misc | CHANGED | rc=0 >>
Filesystem      Size  Used Avail Use% Mounted on
/dev/vda1       310G   90G  221G  29% /

linux-04.do-ams3.ci.misc | CHANGED | rc=0 >>
Filesystem      Size  Used Avail Use% Mounted on
/dev/vda1       310G   53G  258G  17% /

linux-01.do-ams3.ci.misc | CHANGED | rc=0 >>
Filesystem      Size  Used Avail Use% Mounted on
/dev/vda1       310G   98G  213G  32% /

linux-03.do-ams3.ci.misc | CHANGED | rc=0 >>
Filesystem      Size  Used Avail Use% Mounted on
/dev/vda1       310G   71G  240G  23% /

linux-02.do-ams3.ci.misc | CHANGED | rc=0 >>
Filesystem      Size  Used Avail Use% Mounted on
/dev/vda1       310G   94G  217G  31% /

Please push again to rebuild everything.

acolytec3 · 2019-09-28T10:11:27Z

Done, squashed and updated.

Below is a screengrab of how the currency conversion looks.

2 questions:

The original render in the issue shows a spacing dot of some sort between the crypto and wallet currency amounts and I wasn't sure if there was a specific character I should use as that spacer or if there is some prebuilt component to place there. I just have a blank space currently.
Do you want me to insert the "~" for either or both currency fields? Here again, the render has it displayed for the fee but not the value section.

yenda · 2019-09-28T10:23:46Z

@errorists could you answer those questions?

@acolytec3 before a designer comes by to answer your question I think you should use an interpunct for now https://en.wikipedia.org/wiki/Interpunct and also be mindful of the font-color, the value is black. You can check the nested-text component and how it is used to achieve that within one text element. I'd use the tilde (~) for both

acolytec3 · 2019-09-29T01:31:51Z

@yenda I've incorporated a nested-text element into the presentation so now the text of the wallet-currency is now black. I think the only thing left is now I have an odd spacing issue where there's a gap between the value and the currency symbol as shown below. Does nested-text take a padding or margin style element that would shrink this gap? I looked at the prop functions for nested text and it looks like all the styling options are font related and not spacing.

Also, since I already had the code basically written, I went ahead and added the loading spinner as well. The bounty can still be paid out to @alexjg since I know he was planning to work on it. Below is a screen-grab.

errorists · 2019-09-30T08:23:35Z

yes, Eric is right, both should use ~. What I couldn't help but notice is the huuge space between value and currency, it's not right, should be a single space character. Also the space around the • bullet character (is that a bullet there? looks super small) should be a single space.

yenda · 2019-09-30T08:39:02Z

src/status_im/ui/screens/signing/views.cljs

             [separator]])
          [list-item/list-item
           {:type        :small
            :title       :t/network-fee
            :error       gas-error
-            :accessories [[acc-text fee fee-display-symbol] :chevron]
+            :accessories [[acc-text fee fee-display-symbol] "·" (if converted-fee-value [react/nested-text {:styles {:color :colors/black}} (str "~" (i18n/format-currency converted-fee-value (:code wallet-currency)))]


don't use acc-text here, if you look at the definition above in this namespace it is just a nested-text with 2 parts and here you have more. You also misused nested-text here, you should use nested-text instead of acc-text. here is an example of how to use it:

https://github.com/status-im/status-react/blob/496f3f1cc78703c3d629f9f458a0e966b04e5c61/src/status_im/ui/screens/privacy_policy/views.cljs

as you can see you give a global style and then you can have as many arguments as you want, strings or vector, the strings get the global style and the vectors are tuples of style + string, the style is merged with the global style so you only have to put what changes there

andmironov · 2019-09-30T08:50:36Z

Hey! Spotted there are error tooltips on the screens, just wanted to make sure the error states stay consistent, here's an issue about those #9037 (comment)

cc @rachelhamlin

hesterbruikman · 2019-09-30T11:01:31Z

Hey! Spotted there are error tooltips on the screens, just wanted to make sure the error states stay consistent, here's an issue about those #9037 (comment)

On the tooltip. There is a component here: https://github.com/status-im/status-react/blob/bf6b328996581c77fff437efcbf9d411e00d2ef9/src/status_im/ui/components/tooltip/styles.cljs

@yenda do you know if it's up to date, following these designs?: https://www.figma.com/file/cb4p8AxLtTF3q1L6JYDnKN15/Index?node-id=4290%3A1769

src/status_im/ui/screens/signing/views.cljs

acolytec3 · 2019-10-01T02:19:27Z

@yenda @errorists I've updated the PR with the fixes for the nested-text element and seems to have resolved the spacing elements as well. I also changed out the bullet I was using to the larger one from @errorists's comment so let me know if there are any more visual things I need to work on.

errorists · 2019-10-01T06:53:32Z

Looks good now, thanks @acolytec3

src/status_im/ui/screens/signing/views.cljs

andmironov

looks good to me as well!

statustestbot · 2019-10-04T14:24:23Z

100% of end-end tests have passed

Total executed tests: 47
Failed tests: 0
Passed tests: 47

Passed tests (47)

Click to expand

1. test_block_user_from_public_chat
Device sessions Device 2: Steps, video, logs Device 1: Steps, video, logs
2. test_filters_from_daap
Device sessions Device 1: Steps, video, logs
3. test_copy_and_paste_messages
Device sessions Device 1: Steps, video, logs
4. test_send_transaction_from_daap
Device sessions Device 1: Steps, video, logs
5. test_deploy_contract_from_daap
Device sessions Device 1: Steps, video, logs
6. test_open_transaction_on_etherscan
Device sessions Device 1: Steps, video, logs
7. test_public_chat_messaging
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
8. test_long_press_to_delete_1_1_chat
Device sessions Device 1: Steps, video, logs
9. test_password_in_logcat_sign_in
Device sessions Device 1: Steps, video, logs
10. test_text_message_1_1_chat
Device sessions Device 2: Steps, video, logs Device 1: Steps, video, logs
11. test_add_to_contacts
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
12. test_sign_typed_message
Device sessions Device 1: Steps, video, logs
13. test_unread_messages_counter_1_1_chat
Device sessions Device 2: Steps, video, logs Device 1: Steps, video, logs
14. test_ens_in_public_chat
Device sessions Device 2: Steps, video, logs Device 1: Steps, video, logs
15. test_logcat_send_transaction_from_daap
Device sessions Device 1: Steps, video, logs
16. test_send_message_in_group_chat
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
17. test_logcat_send_transaction_from_wallet
Device sessions Device 1: Steps, video, logs
18. test_send_token_with_7_decimals
Device sessions Device 1: Steps, video, logs
19. test_offline_messaging_1_1_chat
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
20. test_modify_transaction_fee_values
Device sessions Device 1: Steps, video, logs
21. test_send_eth_from_wallet_to_address
Device sessions Device 1: Steps, video, logs
22. test_add_account_to_multiaccount_instance
Device sessions Device 1: Steps, video, logs
23. test_manage_assets
Device sessions Device 1: Steps, video, logs
24. test_long_press_to_delete_public_chat
Device sessions Device 1: Steps, video, logs
25. test_send_emoji
Device sessions Device 1: Steps, video, logs
26. test_search_chat_on_home
Device sessions Device 1: Steps, video, logs
27. test_logcat_recovering_account
Device sessions Device 1: Steps, video, logs
28. test_can_add_existing_ens
Device sessions Device 1: Steps, video, logs
29. test_messaging_in_different_networks
Device sessions Device 2: Steps, video, logs Device 1: Steps, video, logs
30. test_logcat_backup_recovery_phrase
Device sessions Device 1: Steps, video, logs
31. test_logcat_sign_message_from_daap
Device sessions Device 1: Steps, video, logs
32. test_switch_users_and_add_new_account
Device sessions Device 1: Steps, video, logs
33. test_send_stt_from_wallet
Device sessions Device 1: Steps, video, logs
34. test_login_with_new_account
Device sessions Device 1: Steps, video, logs
35. test_start_chat_with_ens
Device sessions Device 1: Steps, video, logs
36. test_add_contact_from_public_chat
Device sessions Device 2: Steps, video, logs Device 1: Steps, video, logs
37. test_send_two_transactions_one_after_another_in_dapp
Device sessions Device 1: Steps, video, logs
38. test_password_in_logcat_creating_account
Device sessions Device 1: Steps, video, logs
39. test_backup_recovery_phrase
Device sessions Device 1: Steps, video, logs
40. test_offline_status
Device sessions Device 1: Steps, video, logs
41. test_open_google_com_via_open_dapp
Device sessions Device 1: Steps, video, logs
42. test_unread_messages_counter_public_chat
Device sessions Device 1: Steps, video, logs Device 2: Steps, video, logs
43. test_sign_message_from_daap
Device sessions Device 1: Steps, video, logs
44. test_user_can_remove_profile_picture (TestRail link is not found)
Device sessions Device 1: Steps, video, logs
45. test_share_contact_code_and_wallet_address
Device sessions Device 1: Steps, video, logs
46. test_refresh_button_browsing_app_webview
Device sessions Device 1: Steps, video, logs
47. test_backup_recovery_phrase_warning_from_wallet
Device sessions Device 1: Steps, video, logs

churik · 2019-10-04T15:37:23Z

two questions:

there is no value on transaction fee:

Should it be added separately @flexsurfer ?

for some currecncies (e.g MDL) - we don't have separate symbol, so screen looks like:

Is it OK @andmironov ?

From my perspective PR is ready to merge.

Signed-off-by: Andrey Shovkoplyas <motor4ik@gmail.com>

rachelhamlin · 2019-10-07T08:34:52Z

Thanks for calling these out @churik.

Lack of symbol is okay from my POV.
Network fee conversion to local currency would be good for when we update that screen - backlogged issue. Okay for now in my opinion.

Well done @acolytec3! Thanks for your contribution.

acolytec3 requested a review from a team as a code owner September 27, 2019 20:06

auto-assign bot removed the request for review from a team September 27, 2019 20:06

jakubgs force-pushed the develop branch from b3150f6 to acd7e56 Compare September 28, 2019 08:27

acolytec3 force-pushed the currency-conversion branch from 32df077 to 819b68b Compare September 28, 2019 10:04

acolytec3 requested review from jakubgs and pedropombeiro as code owners September 28, 2019 10:04

weekly-digest bot mentioned this pull request Sep 29, 2019

Weekly Digest (22 September, 2019 - 29 September, 2019) #9066

Closed

yenda reviewed Sep 30, 2019

View reviewed changes

flexsurfer added bounty community labels Sep 30, 2019

yenda reviewed Sep 30, 2019

View reviewed changes

src/status_im/ui/screens/signing/views.cljs Outdated Show resolved Hide resolved

yenda reviewed Sep 30, 2019

View reviewed changes

src/status_im/ui/screens/signing/views.cljs Outdated Show resolved Hide resolved

yenda suggested changes Sep 30, 2019

View reviewed changes

src/status_im/ui/screens/signing/views.cljs Outdated Show resolved Hide resolved

acolytec3 force-pushed the currency-conversion branch from 765b0bb to 23f3d1b Compare October 1, 2019 02:16

yenda approved these changes Oct 1, 2019

View reviewed changes

hesterbruikman mentioned this pull request Oct 2, 2019

[#8026] Add loading state for currency conversion #8443

Closed

gitcoinbot mentioned this pull request Oct 2, 2019

[Wallet] Show values in the user's currency #8027

Closed

flexsurfer added design-review request-manual-qa labels Oct 4, 2019

flexsurfer requested a review from andmironov October 4, 2019 09:36

flexsurfer reviewed Oct 4, 2019

View reviewed changes

src/status_im/ui/screens/signing/views.cljs Outdated Show resolved Hide resolved

acolytec3 force-pushed the currency-conversion branch from 23f3d1b to d930c1f Compare October 4, 2019 10:51

andmironov reviewed Oct 4, 2019

View reviewed changes

churik self-assigned this Oct 4, 2019

churik added the Tested - OK label Oct 4, 2019

Add currency conversions for signing sheet

7286747

Signed-off-by: Andrey Shovkoplyas <motor4ik@gmail.com>

flexsurfer force-pushed the currency-conversion branch from d930c1f to 7286747 Compare October 4, 2019 16:14

flexsurfer merged commit 7286747 into status-im:develop Oct 4, 2019

acolytec3 deleted the currency-conversion branch October 4, 2019 18:21

weekly-digest bot mentioned this pull request Oct 6, 2019

Weekly Digest (29 September, 2019 - 6 October, 2019) #9110

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add currency conversions for signing sheet - Fixes #8027 #9063

Add currency conversions for signing sheet - Fixes #8027 #9063

acolytec3 commented Sep 27, 2019

status-github-bot bot commented Sep 27, 2019

status-im-auto commented Sep 27, 2019 •

edited

Loading

acolytec3 commented Sep 28, 2019

cammellos commented Sep 28, 2019

jakubgs commented Sep 28, 2019

acolytec3 commented Sep 28, 2019

yenda commented Sep 28, 2019 •

edited

Loading

acolytec3 commented Sep 29, 2019 •

edited

Loading

errorists commented Sep 30, 2019

yenda Sep 30, 2019

andmironov commented Sep 30, 2019

hesterbruikman commented Sep 30, 2019

acolytec3 commented Oct 1, 2019

errorists commented Oct 1, 2019

andmironov left a comment

statustestbot commented Oct 4, 2019

churik commented Oct 4, 2019 •

edited

Loading

rachelhamlin commented Oct 7, 2019

Add currency conversions for signing sheet - Fixes #8027 #9063

Add currency conversions for signing sheet - Fixes #8027 #9063

Conversation

acolytec3 commented Sep 27, 2019

Summary

Functional

Steps to test

status-github-bot bot commented Sep 27, 2019

Pull Request Checklist

status-im-auto commented Sep 27, 2019 • edited Loading

Jenkins Builds

acolytec3 commented Sep 28, 2019

cammellos commented Sep 28, 2019

jakubgs commented Sep 28, 2019

acolytec3 commented Sep 28, 2019

yenda commented Sep 28, 2019 • edited Loading

acolytec3 commented Sep 29, 2019 • edited Loading

errorists commented Sep 30, 2019

yenda Sep 30, 2019

Choose a reason for hiding this comment

andmironov commented Sep 30, 2019

hesterbruikman commented Sep 30, 2019

acolytec3 commented Oct 1, 2019

errorists commented Oct 1, 2019

andmironov left a comment

Choose a reason for hiding this comment

statustestbot commented Oct 4, 2019

100% of end-end tests have passed

Passed tests (47)

churik commented Oct 4, 2019 • edited Loading

rachelhamlin commented Oct 7, 2019

status-im-auto commented Sep 27, 2019 •

edited

Loading

yenda commented Sep 28, 2019 •

edited

Loading

acolytec3 commented Sep 29, 2019 •

edited

Loading

churik commented Oct 4, 2019 •

edited

Loading