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

Make send flows consistent #1465

Merged
merged 27 commits into from
May 12, 2020
Merged

Make send flows consistent #1465

merged 27 commits into from
May 12, 2020

Conversation

rickycodes
Copy link
Member

@rickycodes rickycodes commented Apr 1, 2020

Description

Reuse components from the new send flow in certain areas to make things consistent

Before:

After:

Checklist

  • There is a related GitHub issue
  • Tests are included if applicable
  • Any added code is fully documented

Issue

Resolves #1440

@cjeria
Copy link

cjeria commented Apr 1, 2020

Looking good Ricky! So @omnat and I took a look together and wanted to point out a few things for this screen. See the screenshot for notes. Let us know if you have any questions @rickycodes

image

@danjm
Copy link
Contributor

danjm commented Apr 15, 2020

@cjeria Here are updated designs. Let me know what you think of the disabling of the accounts that have insufficient funds: https://streamable.com/35d8vo

@danjm
Copy link
Contributor

danjm commented Apr 15, 2020

New designs now working for payment request as well: https://streamable.com/dskued

@danjm
Copy link
Contributor

danjm commented Apr 15, 2020

However, those designs do not allow the user to edit as many fields as the existing payment request screen. Will discuss live with Christian.

@danjm danjm marked this pull request as ready for review April 22, 2020 16:16
@danjm
Copy link
Contributor

danjm commented Apr 22, 2020

Demo video of new designs applied to instapay txes: https://streamable.com/6qzdpa

@danjm danjm changed the title WIP: Make send flows consistent Make send flows consistent Apr 22, 2020
Copy link
Contributor

@estebanmino estebanmino left a comment

Choose a reason for hiding this comment

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

LGTM in terms of code, left some important concerns tho

Comment on lines 897 to 949
const { transaction: existingTransaction } = state;
const { transaction: ownPropsTransaction } = ownProps;

const identities = state.engine.backgroundState.PreferencesController.identities;
const network = state.engine.backgroundState.NetworkController.network;
let transactionState;

const { to, ensRecipient } = ownPropsTransaction || existingTransaction || {};

if (to || ensRecipient) {
const { data, from, gas, gasPrice, to, value, ensRecipient, selectedAsset } =
ownPropsTransaction || existingTransaction;

const selectedAddress = state.engine.backgroundState.PreferencesController.selectedAddress;
const fromAddress = from || selectedAddress;

const addressBook = state.engine.backgroundState.AddressBookController.addressBook;
const transactionToName =
addressBook &&
to &&
getTransactionToName({
addressBook,
network,
toAddress: to,
identities,
ensRecipient
});

transactionState = {
...ownPropsTransaction,
transaction: { data, from: fromAddress, gas, gasPrice, to, value },
transactionTo: to,
transactionToName,
transactionFromName: identities[fromAddress].name,
transactionValue: value,
selectedAsset: selectedAsset || { isETH: true, symbol: 'ETH' }
};
Copy link
Contributor

Choose a reason for hiding this comment

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

can we move all of this to componentDidMount (or didUpdatechecking for changes on state if needed)? My concern with this way is performance, all of this is going to be executed when state changes, so many times for data that is not going to change

Copy link
Contributor

@danjm danjm Apr 30, 2020

Choose a reason for hiding this comment

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

Copy link
Contributor

@danjm danjm May 1, 2020

Choose a reason for hiding this comment

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

With the latest commit 08b4a49, this is now all handled before the transaction is dispatched to redux.

Comment on lines 31 to 32
import { prepareTransaction } from '../../../../actions/newTransaction';
import { newTransaction } from '../../../../actions/transaction';
Copy link
Contributor

Choose a reason for hiding this comment

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

I've seen this in other files too, I don't feel safe using to sets of reducers and actions for the same transaction. The idea before this PR of having a new actions/newTransaction was to keep using the old flow for app / deeplink txs in its own state, now that we're unifying these we should use the same redux state to handle all txs, wdyt?

Copy link
Contributor

@danjm danjm Apr 30, 2020

Choose a reason for hiding this comment

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

Is this what you had in mind: https://github.com/MetaMask/metamask-mobile/compare/consistent-send-flow...consistent-send-flow-tx-state-refactor?expand=1

A couple bugs to work out, but wanted to share what I've done so far to make sure it matches what you were thinking

Copy link
Contributor

@danjm danjm May 1, 2020

Choose a reason for hiding this comment

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

The two reducers are combined in 08b4a49,

Comment on lines 963 to 1007
const { transaction: existingTransaction, newTransaction } = state;

const { isDeepLinkTransaction, paymentChannelTransaction, symbol } = existingTransaction;
let selectedAsset;

if (paymentChannelTransaction) {
selectedAsset = existingTransaction.selectedAsset;
} else if (isDeepLinkTransaction) {
selectedAsset = { symbol, isETH: symbol === 'ETH' };
} else {
selectedAsset = newTransaction.selectedAsset;
}

return {
Copy link
Contributor

Choose a reason for hiding this comment

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

added a comment below about this, we have too many state updates coming from redux this could impact performance

Copy link
Contributor

@danjm danjm Apr 30, 2020

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

With the latest commit 08b4a49, this is now all handled before the transaction is dispatched to redux.

Comment on lines 15 to 17
import { setSelectedAsset, prepareTransaction } from '../../../../actions/newTransaction';
import { setTransactionObject } from '../../../../actions/transaction';
Copy link
Contributor

Choose a reason for hiding this comment

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

Added a comment in /confirm/index.js about this

Copy link
Contributor

@danjm danjm Apr 30, 2020

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

The two reducers are combined in 08b4a49,

@estebanmino estebanmino added next release needs-qa Any New Features that needs a full manual QA prior to being added to a release. labels Apr 27, 2020
@omnat omnat added the blocker label Apr 27, 2020
@danjm danjm force-pushed the consistent-send-flow branch 4 times, most recently from 1216a23 to 08b4a49 Compare May 1, 2020 11:35
@@ -32,6 +33,22 @@ const migrations = {
state.engine.backgroundState.AssetsController.tokens = migratedTokens;

return state;
},
// Combine the transactions reducer and newTransaction reducer
2: state => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is needed, this state is only used when sending txs if that flow finishes the state should be erased

Copy link
Contributor

Choose a reason for hiding this comment

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

In that case we only need a migration to delete the newTransaction property from state... an error is thrown otherwise

selectedAsset: state.newTransaction.selectedAsset,
selectedAsset: state.transaction.paymentChannelTransaction
? state.transaction.selectedAsset
: state.transaction.selectedAsset,
Copy link
Contributor

Choose a reason for hiding this comment

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

was this intended?

Copy link
Contributor

Choose a reason for hiding this comment

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

lol, no the ternary statement is no longer needed and it can just be selectedAsset: state.transaction.selectedAsset

@ibrahimtaveras00
Copy link
Contributor

ibrahimtaveras00 commented May 6, 2020

Issue 1:

It's a little confusing at first how to edit the txn fee if you don't pay attention right away to the wording, but making it blue to stand out should be easier and more identifiable
cc: @cjeria

image

Issue 2:

In this scenario, Account 2 does not have MANA as an asset but Account 1 does. I initiate the send flow starting with Account 1. Once I get to the Confirm view, I then switch accounts to Account 2 (which does not have any MANA) and then submit the txn, which causes issues.

seen here = https://recordit.co/9MnnthlYIL

Screen Shot 2020-05-06 at 5 56 54 PM

steps to reproduce:

  • start an account that has token that another account doesn't have
  • go through the flow till you get to confirm view
  • on confirm view, switch to a different account that does not have that current token

This also happens if Account 1 has a collectbile that Account 2 doesn't have. I start the flow with account 1 sending a crypto kitty and then on confirm view, I switch to account 2 which does not own that crypto kitty; seen here = http://recordit.co/DkQ9PvItSh

Screen Shot 2020-05-06 at 6 03 08 PM

@omnat
Copy link
Contributor

omnat commented May 6, 2020

Issue 1:
Yes, per the designs this was meant to be blue. Let's make it blue.

Issue 2:
Account shouldn't be switchable when user is initiating the Send flow. This should be possible only when a payment request link/QR is opened (that lands on confirm screen)

cc @cjeria

@ibrahimtaveras00
Copy link
Contributor

ibrahimtaveras00 commented May 6, 2020

Issue 3:

I am unable to transact on dapps

Here I attempted to donate ETH on Ropsten

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

I can't really perform any txn on this site

Seen here = https://recordit.co/0gPWKlFzLx

Similar to above issue, I can't complete my txn

seen here = https://recordit.co/PF81D9n3IP

Screen Shot 2020-05-06 at 7 15 26 PM

Screen Shot 2020-05-06 at 7 15 46 PM

Issue 4:

For InstaPay, looks like we're not detecting the balance on the account. When I made it to the amount view, I entered a balance way more than what I had (I have 17.73 SAI and attempted to send 200 SAI) and didn't get the insufficient funds warning.

Seen here = http://recordit.co/IW6J81GRQw

@ibrahimtaveras00 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 May 6, 2020
Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

Found issues 1-4

@omnat
Copy link
Contributor

omnat commented May 7, 2020

Issue 3 is the most critical and a definite blocker.

@ibrahimtaveras00
Copy link
Contributor

ibrahimtaveras00 commented May 8, 2020

Issue 2 fixed 👍

One thing to keep in mind @omnat @cjeria, that since we do still allow the switching of accounts ONLY on payment requests, there is still a chance of a user switching to an account that doesn't hold an ERC20 Token that the requester is requesting and they can proceed with the txn. See the flow here = http://recordit.co/cqoppARTwW

  • Basically the requester is requesting 1 MANA
  • I am on Account 1 which does have MANA
  • Once on Confirm view, I then switch to Account 4 which doesn't have MANA
  • I then proceed with the txn

image

Issue 5A: Fixed 👍

Issue 5B: Fixed although the position of the Cancel relative to the normal send flow is flopped

Seen here:
image

Issue 5C: Fixed 👍

Issue 6: Fixed 👍

Issue 7: Fixed 👍

Issue 8:
While testing the fix, I did find an issue with InstaPay where I can only send whole number values and not anything with a decimal after 1; seen here = https://recordit.co/9PGPeI4umS

Issue 9:

Approval of token isn't working
Seen here:
Rinkeby = http://recordit.co/UvCCUJSNZH
Mainnet = http://recordit.co/CoiORWKd7B

Issue 10:
This one is weird, it seems like all the transaction history dates are wrong (showing a future date)

Mainnet example:
image

Rinkeby example:
image

@omnat
Copy link
Contributor

omnat commented May 8, 2020

Issue 2: We should do another check against the balance of the chosen account (same as we'd do for the default account that opens the payment request too). And if there's insufficient amount, we should throw the error 'insufficient funds' (and show the requested asset balance in From field), and disable Send button.

@danjm
Copy link
Contributor

danjm commented May 11, 2020

Issue 8 fixed in ce3b3b6
Issue 9 fixed in 6c9d7de
Issue 10 fixed in a separate PR as it is also on develop: #1557

@danjm
Copy link
Contributor

danjm commented May 11, 2020

I think the best improvement for the outstanding concern of issue 2 would be prevent switching to accounts with 0 tokens. However, that will require a non-trivial update to a gaba controller, so I would recommend we create a separate task for that improvement.

nevermind... just read Omna's comment on this above

@ibrahimtaveras00
Copy link
Contributor

ibrahimtaveras00 commented May 11, 2020

Issues 8 and 9 have been fixed 👍

Also I see issue 2 is still outstanding as well as the button placement of issue 5B

@danjm
Copy link
Contributor

danjm commented May 11, 2020

90890cf implements @omnat's suggestion for Issue 2 (#1465 (comment))

@ibrahimtaveras00
Copy link
Contributor

ibrahimtaveras00 commented May 11, 2020

In regards to Issue 2, I'm still able to switch to an account that doesn't hold that ERC20 token and tap send and continue

seen here = http://recordit.co/7U8Gt5Wy6l

In txn history I end up seeing Not Available Undefined

Screen Shot 2020-05-11 at 6 56 20 PM

Steps:

  • Tap on your link from the deeplinks site for an ERC20 token transfer
  • once on confirm view, switch to an account that doesn't have any value for that specified ERC20 token that's requested from above
  • tap send

In addition there's another issue on iOS when coming directly from an account that doesn't have enough of the ERC20 token value; seen here = http://recordit.co/BarvA8Bae6

steps:

  • start on an account that doesn't have the ERC20 token that is being requested or doesn't have the value requested
  • tap on the deeplink
  • when arriving on confirm view, Android correctly shows the insufficient funds warning, while iOS doesn't
    Screen Shot 2020-05-11 at 7 42 40 PM
  • when user taps send on iOS, there's a txn error
    Screen Shot 2020-05-11 at 7 40 21 PM

@danjm
Copy link
Contributor

danjm commented May 11, 2020

Addressed navbar button position issue 5b in 218a2ca

https://streamable.com/8wmscl

@danjm
Copy link
Contributor

danjm commented May 11, 2020

Okay, issue 2 should be fixed for real with c47dccc

https://streamable.com/m833h1

@ibrahimtaveras00
Copy link
Contributor

@danjm all fixes look good now 👍

I did another regression on txn's just to make sure nothing else broke. There was only 1 issue I found through my regression.

When going to instaPay via drawer, and then attempting to send a value below 1, results in the following error:

Screen Shot 2020-05-11 at 10 40 29 PM

flow seen here = https://recordit.co/nR7RdRNiQG

All other flows looked good.

selectedAsset: {
address: '0x89d24A6b4CcB1B6fAA2625fE562bDD9a23260359',
decimals: 18,
logo: 'dai.svg',
Copy link
Contributor

Choose a reason for hiding this comment

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

initializePaymentChannels = () => {
PaymentChannelsClient.init(this.props.selectedAddress);
PaymentChannelsClient.hub.on('state::change', this.onPaymentChannelStateChange);
Copy link
Contributor

Choose a reason for hiding this comment

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

what is this for?

Copy link
Contributor

Choose a reason for hiding this comment

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

When we get a 'payment::request' event (below), we end up calling this.initiatePaymentChannelRequest.

this.initiatePaymentChannelRequest calls setTransactionObject to set the transaction state of the payment channel request, and then navigates to the confirm screen. To ensure the balance of the payment channel is available when we get to the confirm screen, we pass it as the assetBalance property.

This listener gets that balance when it becomes available on the first state event from the PaymentChannelsClient.

Copy link
Contributor

Choose a reason for hiding this comment

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

cool I think it makes sense, PaymentChannelsClient is polling every 5 seconds I believe, emitting state::change, so I'm worried about performance

Copy link
Contributor

Choose a reason for hiding this comment

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

The setState call in this.onPaymentChannelStateChange is only called once because of the paymentChannelReady state property. So I don't think there will a performance impact.

@@ -475,9 +475,11 @@ class PaymentChannel extends PureComponent {
address: '0x89d24A6b4CcB1B6fAA2625fE562bDD9a23260359',
decimals: 18,
logo: 'dai.svg',
Copy link
Contributor

Choose a reason for hiding this comment

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

hu I see the old logo here, not sure which one to use now

Copy link
Contributor

Choose a reason for hiding this comment

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

@danjm
Copy link
Contributor

danjm commented May 12, 2020

Fix the issue with sending DAI and SAI < 1 via instapay in 393e251

Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

@estebanmino I've made some comments that explain the reason for the various code changes. I hope it all makes sense, and please ask any questions. Please do let me know if you think any of it was unnecessary or could have been done in a simpler way.

@@ -1,9 +1,103 @@
import TransactionTypes from '../../core/TransactionTypes';

Copy link
Contributor

@danjm danjm May 12, 2020

Choose a reason for hiding this comment

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

Code changes here, except for the function added at very bottom, are just copy and pastes of the code that was in app/actions/newTransaction/index.js

This was done to combine the transaction and newTransaction states, as suggested in this comment

Copy link
Contributor

@danjm danjm May 12, 2020

Choose a reason for hiding this comment

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

Along with a similar combination of the reducer files, these changes cause file and property name changes throughout the PR.

* @param {string} config.transactionFromName - Resolved address book name for from address
* @param {object} config.selectedAsset - Asset to start the transaction with
* @param {string} config.assetType - The selectedAsset's type
*/
Copy link
Contributor

@danjm danjm May 12, 2020

Choose a reason for hiding this comment

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

prepareFullTransaction is not used and can be removed.

@@ -425,7 +425,19 @@ class Main extends PureComponent {
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

The majority of changes in this file are needed to properly handling payment channel requests and direct them to the existing confirm screen.

const { forceReload } = this.state;
return (
<React.Fragment>
<View style={styles.flex}>
{!forceReload ? <MainNavigator navigation={this.props.navigation} /> : this.renderLoader()}
{!forceReload ? (
Copy link
Contributor

Choose a reason for hiding this comment

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

These changes allow us to customize the navbar if we are handling payment channel transactions or editing payment requests.

@@ -17,6 +17,9 @@ const styles = StyleSheet.create({
paddingVertical: 20,
Copy link
Contributor

Choose a reason for hiding this comment

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

The changes in this file give the user additional information when attempting to switch accounts on the confirm screen, which is an addition that is new with this PR, possible when editing deep link / QR code transactions. Same for the AccountList component.

* @returns - Whether there is an error with the amount
*/
validateAmount = transaction => {
const {
Copy link
Contributor

Choose a reason for hiding this comment

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

Validation of amount is now needed on this screen because editing of tx from can now be done from this screen (now that it supports payment requests).

};

onPaymentChannelSend = async () => {
this.setState({ transactionConfirmed: true });
Copy link
Contributor

Choose a reason for hiding this comment

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

This send method is needed for payment channel transactions.

@@ -574,34 +763,103 @@ class Confirm extends PureComponent {
);
};

render = () => {
getBalanceError = balance => {
const {
Copy link
Contributor

Choose a reason for hiding this comment

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

This and other methods added below but before render support the account switching functionality.

@@ -14,7 +14,7 @@ import Engine from '../../../../core/Engine';
import { isValidAddress, toChecksumAddress } from 'ethereumjs-util';
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes in this file allow the first screen of the standard send flow to be used for payment channel transactions created within metamask-mobile

@@ -1,6 +1,6 @@
import React, { PureComponent } from 'react';
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes here accomodate the design changes shown in this comment #1465 (comment)

Copy link
Contributor

@ibrahimtaveras00 ibrahimtaveras00 left a comment

Choose a reason for hiding this comment

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

All fixes look good on both OS's, QA Passed 👍

@ibrahimtaveras00 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 May 12, 2020
@danjm danjm merged commit 97a5ee3 into develop May 12, 2020
@danjm danjm deleted the consistent-send-flow branch May 12, 2020 20:57
estebanmino added a commit that referenced this pull request May 15, 2020
* Loosen nvmrc (#1524)

Co-authored-by: Esteban Miño <efmino@uc.cl>

* bugfix/check for sai method (#1545)

* update docs link in readme (#1521)

Co-authored-by: Esteban Miño <efmino@uc.cl>

* Add settings to nav bar (#1544)

* Add settings to nav bar

* Remove settings Icon

* Add icons for all drawer links

* Improvement/tx status notification (#1475)

* simple not update

* text update

* wip

* delete old details

* delete old confirm

* almost done withtx details

* modal working

* modal title

* rm transfer element

* clean

* fix transfer

* transfer and payment channel

* decodeTransferFromTx

* decodeDeploymentTx

* decodeConfirmTx

* onpress

* status

* close on view web

* more cleanup

* payment

* showing not

* closer?

* comment

* tx details and not

* animated

* tx not

* enable access view on not

* animated

* rename

* using txnnot manager

* working

* receive

* rm unused

* rm logs

* handle browser not

* parse date

* handle asset details

* tx summary rename props

* Refactor names in details

* handle primary currency

* missing props

* almost there

* working but browser

* finally

* one more thing

* done

* snaps

* missing locales

* update ethereum address

* snaps

* handle instapay txs

* snaps

* feeless tx

* data check

* No fee

* instance._hideTransactionNotification

* fix instapay notifications

* elevation

* fix remaining issues

* apeed up cancel

* transaction modal

* speed cancel

* speedup cancel ui

* working

* added engine methods

* done

* snaps

* fix qaing

* fix ios build

* one snap

* remove test

* status text fix

* cancelled

* margin

* snaps

* fix insufficient funds

* doc

* Transaction Header Component (#1487)

* Remove redundant imports, remove redundant styles, comment typo correction, remove renderPageInformation(), split props line by line in render(), swap rendering renderPageInformation() with TransactionHeader component, pass props

* added lock and warning icons

* removed domain prop

* new TransactionHeader component, imports, styling, prop types, lock/warning icon change based on URL protocol, network status indicator (color) changes if network is online/not online

* re-generated snapshot for SignatureRequest, created new test for TransactionHeader

* network changes based on selected network

* update snapshot

* update snapshot

* remove function, use css for network capitalization

* move network status logic to renderNetworkStatusIndicator()

* render icon logic moved to renderSecureIcon()

* add comments

* update snapshot

* remove redundant getTrackingParams, use props directly

* remove png icons from image dir, use react native svg icons (FontAwesome), update snapshot

* TransactionHeader: use 'Ethereum' instead of 'Mainnet'

* Add shortnames to networks util, TransactionHeader: use networks util to display network name, update snapshot

* Remove redundant imports, remove redundant styles, comment typo correction, remove renderPageInformation(), split props line by line in render(), swap rendering renderPageInformation() with TransactionHeader component, pass props

* added lock and warning icons

* removed domain prop

* new TransactionHeader component, imports, styling, prop types, lock/warning icon change based on URL protocol, network status indicator (color) changes if network is online/not online

* re-generated snapshot for SignatureRequest, created new test for TransactionHeader

* network changes based on selected network

* update snapshot

* update snapshot

* remove function, use css for network capitalization

* move network status logic to renderNetworkStatusIndicator()

* render icon logic moved to renderSecureIcon()

* add comments

* update snapshot

* remove redundant getTrackingParams, use props directly

* remove png icons from image dir, use react native svg icons (FontAwesome), update snapshot

* TransactionHeader: use 'Ethereum' instead of 'Mainnet'

* Add shortnames to networks util, TransactionHeader: use networks util to display network name, update snapshot

* fixed import error

Co-authored-by: Esteban Miño <efmino@uc.cl>

* Feature/block screenshots (#1495)

* native

* test on wallet

* block in some screens

* ios check

* rm asyncs

* helper

* missing places

* Detect if site has been added to Favorites (#1538)

* Detect if site has been added to Favorites

Previously we were `Alert`ing after attempting to `addBookmark`.

Instead, we remove the option from the menu entirely.

closes: #1511

* Rename isFavorite -> isBookmark

* Use "web-search" keyboardType on iOS (#1539)

* Use web-search keyboard

Use web-search for the omnibar keyboard

* Add new mobile provider (#1517)

Co-authored-by: Esteban Miño <efmino@uc.cl>

* Sig request design fixed (#1493)

* new folder for AccountInfoCard component, remove signature_request.title from message, personal & typed sign components, remove redundant style

* SignatureRequest: remove account information from top, proptypes, props, styles, imports

* SignatureRequest: change signing message to 'Sign this message?', make bold and larger

* remove keyboard aware scroll view

* Add AccountInfoCard component to SignatureRequest

* AccountInfoCard: implement proper styling

* AccountInfoCard: use renderShortAddress, fix styles, use conversionRate to display $ amount

* ActionView: add isSigning prop, disables scroll when true, SignatureRequest: pass isSigning=true to ActionView

* AccountInfoCard: remove top level view

* SignatureRequest: apply styles & layout, add website + arrow icons

* Signing components: update styling, rename informationRow --> informationCol

* remove message style

* TypedSign: put back message style, add messageWrapper style, ensure data fits in box and hides overflow

* AccountInfoCard: add snapshot test

* update snapshots

* styling of 'sign this message'

* update snapshot

* update snapshot

* SignatureRequest: Always render arrow if children coming from TypedSign

* SignatureRequest: change to regular component with state to show expanded message content, wrap touchableWithoutFeedback around the message children and move rendering to renderActionViewChildren, tapping the message currently does nothing

* fix dapp icon style, fix render inf loop

* remove textwrap for typed sign. Now renders properly for V1, V3 & V4

* AccountInfoCard: fix paddings, identicon style, use widths instead of flex, float address to left, fix font weights

* TypedSign: use width instead of flex

* SignatureRequest: remove website icon wrapper, fix arrow positioning, remove assetLogo style

* temp disable warning to match style, ensure message fits within box

* PersonalSign: create message wrapper, ensure message fits within box

* Message & Personal Sign: use ellipses mode for text wrapper, drop messageWrapper styles

* SignatureRequest: remove shouldRenderArrow, add shouldRenderArrow prop

* MessageSign: change to stateful component, add renderArrow state, decides if should render arrow upon text component layout, then adjusts the text accordingly

* PersonalSign: change to stateful component, add renderArrow state, decides if should render arrow upon text component layout, then adjusts the text accordingly

* TypedSign: shouldRenderArrow always passes down as true. Will never be a situation where an entire typed message fits in the box

* SignatureRequest: change back to pure component, change handleMessageTap into prop

* change handleMessageTap to toggleExpandedMessage

* TypedSign: implement message expansion and retraction

* modify message, add message_from

* new ExpandedMessage component, rendered by typed, personal & regular message components

* TypedSign: use ExpandedMessage component

* ExpandedMessage: test

* SignatureRequest: add renderArrow prop, make box not expandable if false

* MessageSign: implement message expanding and retracting

* PersonalSign: implement message expand & retract

* ExpandedMessage: add mock fns, update all snapshots

* ActionView: get rid of top border

* new button styles

* signing components: ensure a top left and right rounded border

* change Cancel & Sign to lowercase

* snapshot update

* adjust style for android

* use unique button style for signing components, fix percentage in stylesheet

* change isSigning prop to noScroll

* snapshot update

* update more snapshots

* Signing components: revert to pure component

* AccountInfoCard: use weiToFiat & hexToBn helpers to display fiat value, add currentCurrency prop

* Signing components: shift renderRootView() contents into render()

* update snapshot

* AccountInfoCard, SignatureRequest: fix paddings per design

* AccountInfoCard: remove bottom margin

* TransactionHeader: remove margins, use padding

* MessageSign: larger min height, showWarning prop

* WarningMessage: use flexstart alignment for bell icon

* locales: change eth_sign_warning

* WarningMessage: add object as secondary prop type for warning message

* SignatureRequest: use WarningMessage component, fix paddings, use renderWarning as prop for WarningMessage

* snapshot update yo

* SignatureRequest: move AccountInfoCard ontop of message children

* snapshot update

* AccountInfoCard: remove width

* ActionView: remove no scroll - small devices

* Signing Components: remove root style, move to SignatureRequest

* SignatureRequest: remove style redundancies, add in root style

* SignatureRequest: fix height of modal based on screen height

* ExpandedMessage: fix styling, move scrollview to signing components renderMessage

* ExpandedMessage: Put the scroll back

* PersonalSign: remove expandedMessage text wrapper

* SignatureRequest: fix up styling, add more overhead (reduced from signing components)

* Signing components: reduce view hierarchy, move to SignatureRequest

* Locales: add Read more to signature_request

* AccountInfoCard: add operation prop, if operation is signing, only display the account name and address

* TypedSign: add shouldTruncateMessage & truncateMessage in state

* PersonalSign: remove console log

* Signing components: change renderArrow to truncateMessage

* Nav/Main: add showExpandedMessage to state, add toggleExpandedMessage, configure expanded signing modal to go back on android back button press, pass down props to signing components

* Signing Components: move showExpandedMessage out of state, move out toggleExpandedMessage

* TypedSign: create different messageWrapper height for iOS & Android, fix text clipping mid-line

* ExpandedMessage: fix scrollview

* snapshot update

* AccountInfoCard: use getTicker

* Signing components: change margin bottom from 5 to 4

* Device: new getMediumDevice, SignatureRequest: use getMediumDevice

* ActionView, styledButtonStyles: add cancel button style for signing components

* snapshot update

* SignatureRequest: fix the domain logo not being a circle

* update snapshot

* Use gaba@1.11.0 (#1552)

* Fix settings everywhere (#1556)

* Fix day and month numbers in toDateFormat (#1557)

* Make send flows consistent (#1465)

* Move components and styles from Confirm into TransactionReview

* Add ActionView back in

* Add missing styles

* Revert TransactionReview changes

* Update send screen: from accounts editable and redesign gas edit link

* Use sendflow confirm for payment requests and when editing

* Update sendflow/confirm tests

* Use new send flow designs for instapayment / payment channel transactions

* Use existing send flow screens for deep link transactions

* Fix editing of payment request transactions

* Fix unit tests on consistent-send-flow branch

* Fix navigation for deep link tx edits on the amount screen.

* Refactor: combine transaction and newTransaction reducers

* Fix bugs on consistent-send-flow

* Fix confirm and edit of transactions created from dapps

* Update transaction edit text color

* Only allow changing from field on confirm screen of payment requests

* Fix amount validation for payment channel transactions

* Fix qr payment requests, payment channel payment requests, and token payment requests; plus other small fixes

* Fix token approvals

* Fix sending of decimals on payment channles

* Show correct error messages when accounts are changed and/or token balances are insufficient

* Update navbar options in edit mode

* Ensure tokens cannot be sent in cases where user has not added the token

* Correctly validate payment channel transaction on mount/update

* Use sai.svg instead of dai.svg

Co-authored-by: Dan Miller <danjm.com@gmail.com>
Co-authored-by: Esteban Miño <efmino@uc.cl>

* Use setTimeout hack (again) to get paste context in token search (#1548)

* Use setTimeout hack (again) to get paste context in token search

* Update test

Co-authored-by: Esteban Miño <efmino@uc.cl>

* V0.2.16 (#1561)

* bump

* changelog

* rm entry

* Fix amount validation for large token payment requests (#1572)

* Fix validating of amount when sending a collectible (#1565)

* Fix validating of amount when sending a collectible

* Validate collectible ownership on amount screen.

* Ensure correct updating of collectible transaction after edit on the amount screen

* Ensure collectibles that use 'transfer' method show a fee in tx history list (#1574)

Co-authored-by: Esteban Miño <efmino@uc.cl>

* Disable confirm screen edit button when no tokens of a payment request (#1570)

* Disable confirm screen edit button when account has no tokens of a payment request

* Ensure account switching from undefined token balance accounts enables edit on pay reqs

* Improve logic in componentDidUpdate of send/index.js

* v0.2.16 changelog (#1575)

* Instapay deposit navbar cancel (#1582)

* fix

* works

* rm log

* add this to changelog and update date

* amount title

Co-authored-by: Erik Marks <25517051+rekmarks@users.noreply.github.com>
Co-authored-by: Jenny Pollack <jennypollack3@gmail.com>
Co-authored-by: ricky <ricky.miller@gmail.com>
Co-authored-by: Etienne Dusseault <etienne.dusseault@gmail.com>
Co-authored-by: Whymarrh Whitby <whymarrh.whitby@gmail.com>
Co-authored-by: Ibrahim Taveras <ibrahimtaveras00@gmail.com>
Co-authored-by: Dan J Miller <danjm.com@gmail.com>
rickycodes added a commit that referenced this pull request Jan 31, 2022
* Move components and styles from Confirm into TransactionReview

* Add ActionView back in

* Add missing styles

* Revert TransactionReview changes

* Update send screen: from accounts editable and redesign gas edit link

* Use sendflow confirm for payment requests and when editing

* Update sendflow/confirm tests

* Use new send flow designs for instapayment / payment channel transactions

* Use existing send flow screens for deep link transactions

* Fix editing of payment request transactions

* Fix unit tests on consistent-send-flow branch

* Fix navigation for deep link tx edits on the amount screen.

* Refactor: combine transaction and newTransaction reducers

* Fix bugs on consistent-send-flow

* Fix confirm and edit of transactions created from dapps

* Update transaction edit text color

* Only allow changing from field on confirm screen of payment requests

* Fix amount validation for payment channel transactions

* Fix qr payment requests, payment channel payment requests, and token payment requests; plus other small fixes

* Fix token approvals

* Fix sending of decimals on payment channles

* Show correct error messages when accounts are changed and/or token balances are insufficient

* Update navbar options in edit mode

* Ensure tokens cannot be sent in cases where user has not added the token

* Correctly validate payment channel transaction on mount/update

* Use sai.svg instead of dai.svg

Co-authored-by: Dan Miller <danjm.com@gmail.com>
Co-authored-by: Esteban Miño <efmino@uc.cl>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next release QA Passed A successful QA run through has been done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make send flows consistent
6 participants