Show estimated fees on wallet input selection screen #18309
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (68)
|
briansztamfater
force-pushed
the
feat/show-fees-with-route
branch
4 times, most recently
from
1663978
to
61d6133
Compare
briansztamfater
changed the title
briansztamfater
requested review from
OmarBasem,
ulisesmac,
J-Son89,
shivekkhurana,
vkjr,
smohamedjavid and
mmilad75
and removed request for
OmarBasem and
ulisesmac
J-Son89
reviewed
Dec 29, 2023
J-Son89
reviewed
Dec 29, 2023
| {:icon-only? true | ||
| :type :outline | ||
| :size 32 | ||
| :inner-style {:opacity 1} |
There was a problem hiding this comment.
why do you need this :inner-style {:opacity 1}?
There was a problem hiding this comment.
This is because when in loading state the button is disabled which leads the button to have an opacity of 0.3, so I need to force opacity to be 1 on inner-style to avoid this behavior wether if the button is disabled or enabled.
There was a problem hiding this comment.
So we want that the button looks enabled even when it's disabled?
Is it too bad to show it as disabled? or just trigger nothing when the routes are being loaded?
Just thinking if this needs to be addressed in a more general way if we face it again.
briansztamfater
force-pushed
the
feat/show-fees-with-route
branch
4 times, most recently
from
f6937f9
to
3aa69b2
Compare
Cool, I'll check the issues and let you know when they are ready to re-test. Thanks @VolodLytvynenko ! |
briansztamfater
force-pushed
the
feat/show-fees-with-route
branch
from
8a59445
to
e8ad383
Compare
briansztamfater
force-pushed
the
feat/show-fees-with-route
branch
4 times, most recently
from
b1239b2
to
c94c07a
Compare
|
Hi @VolodLytvynenko, I resumed the work on this PR and should be ready to be tested again. Let me know how testing goes! |
31% of end-end tests have passedFailed tests (32)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (15)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
44% of end-end tests have passedFailed tests (18)Click to expandClass TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Passed tests (14)Click to expandClass TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
|
briansztamfater
force-pushed
the
feat/show-fees-with-route
branch
2 times, most recently
from
7399934
to
b36da79
Compare
38% of end-end tests have passedFailed tests (29)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (18)Click to expandClass TestGroupChatMultipleDeviceMergedNewUI:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
|
40% of end-end tests have passedFailed tests (28)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestDeepLinksOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (19)Click to expandClass TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
|
hi @briansztamfater thank you for the PR. Apologies for the testing delay. The PR looks good, but it seems there's an issue with the network fee calculation. However, it appears this issue is related to the backend rather than the UI. If that's the case, the PR can be merged. ISSUE 5: Incorrect Optimism Gas Fee displaySteps:
Actual result:The displayed gas fee is consistently less than $0.01 USD for transactions on the Optimism network. However, upon checking the actual transaction fees on Etherscan, they are substantially higher: Transaction 1 (0.0001 ETH): Actual fee on Etherscan is $0.25 USD. Transaction 2 (0.0005 ETH): Actual fee on Etherscan is $0.26 USD. Expected result:The fee corresponds to the actual fee shown in the etherscan |
Signed-off-by: Brian Sztamfater <brian@status.im>
briansztamfater
force-pushed
the
feat/show-fees-with-route
branch
from
b36da79
to
b007d38
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #18186
Summary
This PR implements UI for showing estimated fees on the input selection screen for the wallet send flow
Platforms
Functional
Steps to test
status: ready