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

Screens for editing transaction settings #21838

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

vkjr
Copy link
Contributor

@vkjr vkjr commented Dec 17, 2024

fixes #21023 #21024

Summary

Implemented screens for editing Max base fee, Priority fee, Nonce and Max gas amount.
This is only UI implementation and it is hidden under feature flag.
status-go pr is not merged yet, so integration with status-go will be made separately

Screen.Recording.2024-12-17.at.15.50.35.mov

Review notes

Figma designs
In figma designs "max base fee" and "priority fee" use token-input component with swap button that allows switching between fiat and crypto. In my implementation there is no switching because it is not practical. 1 gwei is just a fraction of cent and user will never operate in fiat to set fees. Also we never operate with values less than 1 cent in other parts of our app. (cc @xAlisher)

Testing notes

Platforms

  • Android
  • iOS

Areas that maybe impacted

Functional
  • wallet / transactions

Steps to test

  • Open Status
  • Start send flow, reach transaction confirmation screen
  • Press transaction settings button, select "custom" menu and try different options

status: ready

@vkjr vkjr self-assigned this Dec 17, 2024
@status-im-auto
Copy link
Member

status-im-auto commented Dec 17, 2024

Jenkins Builds

Click to see older builds (16)
Commit #️⃣ Finished (UTC) Duration Platform Result
46862df #1 2024-12-17 15:39:32 ~2 min ios 📄log
46862df #1 2024-12-17 15:41:31 ~4 min tests 📄log
✔️ 46862df #1 2024-12-17 15:44:35 ~8 min android-e2e 🤖apk 📲
✔️ 46862df #1 2024-12-17 15:45:13 ~8 min android 🤖apk 📲
✔️ dd4e08a #2 2024-12-23 12:51:00 ~4 min tests 📄log
✔️ dd4e08a #2 2024-12-23 12:53:00 ~6 min android-e2e 🤖apk 📲
✔️ dd4e08a #2 2024-12-23 12:53:17 ~6 min ios 📱ipa 📲
✔️ dd4e08a #2 2024-12-23 12:54:44 ~8 min android 🤖apk 📲
✔️ 31c80d2 #3 2025-01-06 12:54:56 ~4 min tests 📄log
✔️ 31c80d2 #3 2025-01-06 12:57:35 ~6 min ios 📱ipa 📲
✔️ 31c80d2 #3 2025-01-06 12:58:07 ~7 min android-e2e 🤖apk 📲
✔️ 31c80d2 #3 2025-01-06 12:58:46 ~8 min android 🤖apk 📲
✔️ c5a2efd #4 2025-01-07 15:06:32 ~5 min tests 📄log
✔️ c5a2efd #4 2025-01-07 15:08:29 ~7 min ios 📱ipa 📲
✔️ c5a2efd #4 2025-01-07 15:10:44 ~9 min android-e2e 🤖apk 📲
✔️ c5a2efd #4 2025-01-07 15:11:08 ~9 min android 🤖apk 📲
Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 34177a6 #5 2025-01-07 17:57:13 ~4 min tests 📄log
✔️ 34177a6 #5 2025-01-07 17:59:34 ~7 min android-e2e 🤖apk 📲
✔️ 34177a6 #5 2025-01-07 18:00:52 ~8 min android 🤖apk 📲
✔️ 34177a6 #5 2025-01-07 18:02:31 ~10 min ios 📱ipa 📲
✔️ a79acef #6 2025-01-09 14:26:49 ~5 min tests 📄log
✔️ a79acef #6 2025-01-09 14:29:09 ~7 min ios 📱ipa 📲
✔️ a79acef #6 2025-01-09 14:29:20 ~7 min android-e2e 🤖apk 📲
✔️ a79acef #6 2025-01-09 14:29:51 ~8 min android 🤖apk 📲

@@ -7,8 +7,8 @@
[react-native.core :as rn]
[react-native.platform :as platform]))

(defn- label-&-counter
[{:keys [label current-chars char-limit variant-colors theme]}]
(defn- label-line
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In designs input field has labels at top-left and top-right sides. In our component we had only characters counter in the right side, so i added a right-side label.

:number (count networks)
:size :size-16}
networks]
(when networks
Copy link
Contributor Author

@vkjr vkjr Dec 17, 2024

Choose a reason for hiding this comment

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

If networks is empty we don't need this component because it eats some space and leads to incorrect alignment of text after it.

(let [theme (quo.theme/use-theme)
window-width (:width (rn/get-window))]
[{:keys [token-symbol on-token-press value error? on-swap currency-symbol show-token-icon?
swappable?]}]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Two new features of token-input:
If not swappable? we won't display the button to swap between crypto and fiat values.
If not show-token-icon? we do not show icon. Because GWEI or UNITS that we enter in transaction settings screens do not have icons.

@@ -78,16 +78,19 @@

(defn set-upper-limit
[state limit]
(when limit
(if limit
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this change set-upper-limit breaks the usage of -> because no value returned when limit not applied

@@ -768,3 +768,41 @@
:always (update-in [:wallet :ui :send] dissoc :amount :route)
(not keep-tx-data?) (update-in [:wallet :ui :send] dissoc :tx-type))
:fx [[:dispatch [:navigate-back]]]})))

(rf/reg-event-fx
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not know yet in what form we will get the recommended and current values for transaction settings from status-go, so for now I've stored test values like that. Not a final form I believe.

{:db (all-screens-params db (first comp-id) screen-params)
:fx [[:navigate-to-within-stack (conj comp-id (:theme db))]]})
{:db (all-screens-params db (first comp-id) screen-params)
:dispatch-n [[:hide-bottom-sheet]]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Bottom sheets should be hidden when navigate to another screen. Copied from :navigate-to event above in the code.

@vkjr vkjr force-pushed the max-and-priority-fee-ui branch from 46862df to dd4e08a Compare December 23, 2024 12:46
@vkjr vkjr requested a review from clauxx January 6, 2025 12:41
@vkjr vkjr force-pushed the max-and-priority-fee-ui branch from dd4e08a to 31c80d2 Compare January 6, 2025 12:50
@vkjr vkjr force-pushed the max-and-priority-fee-ui branch from 31c80d2 to c5a2efd Compare January 7, 2025 15:01
@shivekkhurana shivekkhurana added the wallet-core Issues for mobile wallet team label Jan 7, 2025
@vkjr vkjr force-pushed the max-and-priority-fee-ui branch from 34177a6 to a79acef Compare January 9, 2025 14:21
@status-im-auto
Copy link
Member

100% of end-end tests have passed

Total executed tests: 8
Failed tests: 0
Expected to fail tests: 0
Passed tests: 8

Passed tests (8)

Click to expand

Class TestCommunityOneDeviceMerged:

1. test_restore_multiaccount_with_waku_backup_remove_profile_switch, id: 703133
Device sessions

2. test_community_copy_and_paste_message_in_chat_input, id: 702742
Device sessions

Class TestWalletOneDevice:

1. test_wallet_add_remove_regular_account, id: 727231
2. test_wallet_balance_mainnet, id: 740490

Class TestWalletMultipleDevice:

1. test_wallet_send_asset_from_drawer, id: 727230
2. test_wallet_send_eth, id: 727229

Class TestOneToOneChatMultipleSharedDevicesNewUi:

1. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
Device sessions

Class TestCommunityMultipleDeviceMerged:

1. test_community_message_edit, id: 702843
Device sessions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request-manual-qa wallet-core Issues for mobile wallet team
Projects
Status: E2E Tests
Development

Successfully merging this pull request may close these issues.

🖊️⚙️ Implement Input and Validation for Max Base Fee and Priority Fee in Custom TX Parameters
6 participants