Transaction components #1512

EtDu · 2020-04-27T03:47:53Z

Description

Create components for transactions created from dApps redesign. #1501

Checklist

Create TransactionReviewFeeCard component
TransactionReviewFeeCard create snapshot test
Confirm: Use TransactionReviewFeeCard
CustomGas: implement custom gas modal redesign
Confirm & Approve: use new CustomGas
Add in fix for iOS Crash after editing gas price that dapp suggested #1473
Fix gas error not updating properly

Issue

Resolves #1497
Resolves #1500
Resolves #1473

EtDu · 2020-04-27T04:01:35Z

primaryCurrency = FIAT

primaryCurrency = ETH

@cjeria is this what you had in mind?

EtDu · 2020-04-29T05:59:03Z

Basic network fee selection

EtDu · 2020-04-30T14:10:53Z

Advanced gas controls

Gas limit error

Gas price error

Notes:

If gas limit and gas price errors are both present, it will only show one at a time (gas price) until it is resolved, then it will show the other.
Showing/hiding the error does not displace any elements on the screen

estebanmino · 2020-05-11T18:47:17Z

app/components/Views/SendFlow/CustomGas/index.js

+	buttonNext: {
+		flex: 1
+	},
+	radio: {}


I think you can remove this

missed that, I wonder if the linter could pick up on empty styles

estebanmino · 2020-05-11T18:49:55Z

app/components/Views/SendFlow/CustomGas/index.js

+		const { warningGasLimit, warningGasPrice } = this.state;
+		const { gasError } = this.props;
+		let gasErrorMessage;
+		if ((warningGasLimit && warningGasPrice) || warningGasPrice) {


isn't this equivalent to if (warningGasPrice) ?

Yes, double logic at its finest

estebanmino · 2020-05-11T18:51:09Z

app/components/Views/SendFlow/CustomGas/index.js

+		}
+		return (
+			<View style={styles.warningWrapper}>
+				<View style={[styles.warningTextWrapper, !gasError ? styles.invisible : null]}>


what about not returning any component if there's no error? I don't know if this is using space on the view even though is rendering nothing

It is using space on the view on purpose, that's why I hide and show it with opacity: 0. If it doesn't initially take space, then when the error appears it will push everything else upward- which doesn't look good.

Let me know however if this is the desired effect

estebanmino

added small comments, looks good!

estebanmino · 2020-05-26T20:29:08Z

app/components/Views/SendFlow/Confirm/index.js

-				confirmDisabled={!!gasError}
-				cancelButtonMode={'neutral'}
-				confirmButtonMode={'confirm'}
+			<Modal


I wonder why you didn't use ActionModal here, you can disable the cancel button if you want and add a style prop there to put it on the bottom of the screen. In terms of reusability of components, now if we change the color of these models, we'll have to change this one separately from the others.
Same comment for https://github.com/MetaMask/metamask-mobile/pull/1512/files#diff-1d09ed350c73498ec258583166d70e87R461

I mean is working and already approved, but I think we can reuse things here

Cool, resolved

…ETH transaction fee, move renderIfGasEstimationReady to TransactionReviewFeeCard, remove adjust transaction fee button, pass props to TransactionReviewFeeCard

… other snapshots

…ge conflict recovery)

…en updating gas limit

…rice is empty

…nge it when CustomGas is ready so the button can be disabled

ibrahimtaveras00

Overall this looks and feels pretty solid across both OS's and multiple screen sizes!

Only noticed two issues:

Issue 1:

When I edit network fee and go to advanced, I'm still able to tap save when I do not have anything entered in the gas limit input field

seen here = https://recordit.co/3sK3vd9Jt5

When I tap send, I then get the error

Issue 2:

Continuing from issue one, when I get into this state and click Edit again and attempt to change to either slow/average/fast and tap save, the advanced section doesn't update thus I still have a network fee of $0.00. Is the value supposed to update after saving gas speed?

Seen here = https://recordit.co/c2F2d2tvA4

I'm sure the fix for one will ultimately fix issue 2 as they're related.

ibrahimtaveras00

Fix looks good on both OS's, QA Passed 👍

estebanmino

🚀

* Locales: change Transaction fee to Network fee per design * Confirm: Use TransactionReviewFeeCard, add primaryCurrency prop, add ETH transaction fee, move renderIfGasEstimationReady to TransactionReviewFeeCard, remove adjust transaction fee button, pass props to TransactionReviewFeeCard * Create new TransactionReviewFeeCard component * TransactionReviewFeeCard: fix proptypes, create snapshot test, update other snapshots * Confirm: use slide up modal for custom gas * Confirm: Remove CustomGas style wrappers into CustomGas component, make CustomGas the only child of Modal * Locales: edit network fee, basic, save, cost explanation reword * CustomGas: Implement redesign. Still need to modify styling of individual gas selectors * CustomGas: fix gas selector styling * snapshot update * Approve: Use Modal in place of Action Modal, use updated CustomGas * Confirm: add gasError prop to CustomGas * CustomGas: disable save button if gas error * CustomGas: Implement advanced options design * update snapshot * CustomGas: fix padding on non iphoneX devices * Confirm: remove unused TransactionSummary * CustomGas: remove double logic and empty styles * Confirm: fix, change props.transactionState to state.transaction (merge conflict recovery) * snapshot update * SendFlow/CustomGas: fix button not disabling and error not showing when updating gas limit * SendFlow/CustomGas: Fix #1473 crash when modifying gas limit if gas price is empty * Confirm: add styles for ActionModal, use ActionModal * SendFlow/CustomGas: modify styles, remove footer container * ActionModal: add props * ActionContent: add props * Confirm + CutomGas: add ready state, pass down prop to change it, change it when CustomGas is ready so the button can be disabled * Approve: add in ActionModal styles, use ActionModal * snapshot update * SendFlow/CustomGas: QA fix save not disabled on certain case Co-authored-by: Ibrahim Taveras <ibrahimtaveras00@gmail.com>

EtDu marked this pull request as draft May 7, 2020 13:22

EtDu marked this pull request as ready for review May 9, 2020 07:39

EtDu force-pushed the transaction-components branch from 900e7a0 to f18248d Compare May 9, 2020 08:18

estebanmino reviewed May 11, 2020

View reviewed changes

rickycodes self-requested a review May 12, 2020 03:01

rickycodes mentioned this pull request May 13, 2020

Complete redesigns for approval flow #1558

Merged

4 tasks

rickycodes removed their request for review May 13, 2020 17:51

EtDu force-pushed the transaction-components branch from 8a38295 to dd4ed7a Compare May 14, 2020 06:41

EtDu marked this pull request as draft May 14, 2020 13:38

EtDu force-pushed the transaction-components branch from 90a451b to d48dc94 Compare May 18, 2020 09:53

EtDu marked this pull request as ready for review May 18, 2020 13:11

EtDu mentioned this pull request May 21, 2020

iOS Crash after editing gas price that dapp suggested #1473

Closed

EtDu force-pushed the transaction-components branch from d48dc94 to e7c03ea Compare May 22, 2020 11:26

omnat added next release and removed next release labels May 26, 2020

ibrahimtaveras00 requested a review from a team as a code owner May 26, 2020 17:55

estebanmino reviewed May 26, 2020

View reviewed changes

EtDu force-pushed the transaction-components branch from 8d783bb to 18a47ff Compare May 28, 2020 11:00

estebanmino added the needs-qa Any New Features that needs a full manual QA prior to being added to a release. label May 28, 2020

estebanmino requested a review from ibrahimtaveras00 May 28, 2020 14:22

EtDu added 4 commits June 2, 2020 16:51

Locales: change Transaction fee to Network fee per design

8fb2e31

Confirm: Use TransactionReviewFeeCard, add primaryCurrency prop, add …

96d30ea

…ETH transaction fee, move renderIfGasEstimationReady to TransactionReviewFeeCard, remove adjust transaction fee button, pass props to TransactionReviewFeeCard

Create new TransactionReviewFeeCard component

e2a5d16

TransactionReviewFeeCard: fix proptypes, create snapshot test, update…

9edc7be

… other snapshots

EtDu added 16 commits June 2, 2020 16:51

CustomGas: Implement advanced options design

237446c

update snapshot

498b6b8

CustomGas: fix padding on non iphoneX devices

0d92c7f

Confirm: remove unused TransactionSummary

1bd58db

CustomGas: remove double logic and empty styles

7b9630e

Confirm: fix, change props.transactionState to state.transaction (mer…

85f27c8

…ge conflict recovery)

snapshot update

2194e1d

SendFlow/CustomGas: fix button not disabling and error not showing wh…

9c64219

…en updating gas limit

SendFlow/CustomGas: Fix #1473 crash when modifying gas limit if gas p…

4099b84

…rice is empty

Confirm: add styles for ActionModal, use ActionModal

9791e30

SendFlow/CustomGas: modify styles, remove footer container

6789f8d

ActionModal: add props

299c24b

ActionContent: add props

cd5486b

Confirm + CutomGas: add ready state, pass down prop to change it, cha…

d3c821c

…nge it when CustomGas is ready so the button can be disabled

Approve: add in ActionModal styles, use ActionModal

2ce0c2a

snapshot update

7b77976

EtDu force-pushed the transaction-components branch from 1900922 to 7b77976 Compare June 2, 2020 08:52

ibrahimtaveras00 suggested changes Jun 5, 2020

View reviewed changes

ibrahimtaveras00 added QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed and removed needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Jun 5, 2020

SendFlow/CustomGas: QA fix save not disabled on certain case

bc18c92

EtDu force-pushed the transaction-components branch from e5e48fe to bc18c92 Compare June 5, 2020 09:34

Merge branch 'develop' into transaction-components

1d4e9db

ibrahimtaveras00 approved these changes Jun 5, 2020

View reviewed changes

ibrahimtaveras00 added QA Passed A successful QA run through has been done and removed QA'd - Issues Found QA has been complete, however issues have been discovered that need to be addressed labels Jun 5, 2020

estebanmino approved these changes Jun 9, 2020

View reviewed changes

EtDu merged commit b41d78e into develop Jun 9, 2020

EtDu deleted the transaction-components branch June 9, 2020 05:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Transaction components #1512

Transaction components #1512

EtDu commented Apr 27, 2020 •

edited

Loading

EtDu commented Apr 27, 2020

EtDu commented Apr 29, 2020

EtDu commented Apr 30, 2020 •

edited

Loading

estebanmino May 11, 2020

EtDu May 13, 2020

estebanmino May 11, 2020

EtDu May 13, 2020

estebanmino May 11, 2020

EtDu May 13, 2020

estebanmino left a comment

estebanmino May 26, 2020

estebanmino May 26, 2020

EtDu May 28, 2020

ibrahimtaveras00 left a comment

ibrahimtaveras00 left a comment

estebanmino left a comment

Transaction components #1512

Transaction components #1512

Conversation

EtDu commented Apr 27, 2020 • edited Loading

EtDu commented Apr 27, 2020

EtDu commented Apr 29, 2020

EtDu commented Apr 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

estebanmino left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

estebanmino left a comment

Choose a reason for hiding this comment

EtDu commented Apr 27, 2020 •

edited

Loading

EtDu commented Apr 30, 2020 •

edited

Loading