From 7b8e87e32a7978ce864ce162949f9bb0295bc53c Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Fri, 10 Jan 2020 10:53:54 -0330 Subject: [PATCH] Fix gas estimate for tokens (#7753) * Make gas estimate update on debounced token amount change, not just on blur after change * Updated tests * Ensure `updateGas` is bound early Co-authored-by: Mark Stacey --- test/e2e/metamask-ui.spec.js | 1 + .../ui/token-input/token-input.component.js | 2 +- .../send-amount-row.component.js | 7 +++-- .../tests/send-amount-row-component.test.js | 12 ++++---- .../send/send-footer/send-footer.component.js | 5 ++-- .../send/send-footer/send-footer.container.js | 2 ++ .../tests/send-footer-component.test.js | 14 ++++++++++ .../tests/send-footer-container.test.js | 28 +------------------ 8 files changed, 33 insertions(+), 38 deletions(-) diff --git a/test/e2e/metamask-ui.spec.js b/test/e2e/metamask-ui.spec.js index a0efe56f9..38c7ac528 100644 --- a/test/e2e/metamask-ui.spec.js +++ b/test/e2e/metamask-ui.spec.js @@ -965,6 +965,7 @@ describe('MetaMask', function () { // Continue to next screen const nextScreen = await findElement(driver, By.xpath(`//button[contains(text(), 'Next')]`)) + await driver.wait(until.elementIsEnabled(nextScreen)) await nextScreen.click() await delay(regularDelayMs) }) diff --git a/ui/app/components/ui/token-input/token-input.component.js b/ui/app/components/ui/token-input/token-input.component.js index c28af5fde..316463f5b 100644 --- a/ui/app/components/ui/token-input/token-input.component.js +++ b/ui/app/components/ui/token-input/token-input.component.js @@ -78,7 +78,7 @@ export default class TokenInput extends PureComponent { } handleBlur = () => { - this.props.onBlur(this.state.hexValue) + this.props.onBlur && this.props.onBlur(this.state.hexValue) } renderConversionComponent () { diff --git a/ui/app/pages/send/send-content/send-amount-row/send-amount-row.component.js b/ui/app/pages/send/send-content/send-amount-row/send-amount-row.component.js index 495de58b5..0b46d48c5 100644 --- a/ui/app/pages/send/send-content/send-amount-row/send-amount-row.component.js +++ b/ui/app/pages/send/send-content/send-amount-row/send-amount-row.component.js @@ -1,5 +1,6 @@ import React, { Component } from 'react' import PropTypes from 'prop-types' +import debounce from 'lodash.debounce' import SendRowWrapper from '../send-row-wrapper' import AmountMaxButton from './amount-max-button' import UserPreferencedCurrencyInput from '../../../../components/app/user-preferenced-currency-input' @@ -32,6 +33,8 @@ export default class SendAmountRow extends Component { t: PropTypes.func, } + updateGas = debounce(this.updateGas.bind(this), 500) + validateAmount (amount) { const { amountConversionRate, @@ -90,8 +93,8 @@ export default class SendAmountRow extends Component { return ( this.validateAmount(newAmount)} - onBlur={newAmount => { + onChange={newAmount => { + this.validateAmount(newAmount) this.updateGas(newAmount) this.updateAmount(newAmount) }} diff --git a/ui/app/pages/send/send-content/send-amount-row/tests/send-amount-row-component.test.js b/ui/app/pages/send/send-content/send-amount-row/tests/send-amount-row-component.test.js index bfc0f7e64..41a36e992 100644 --- a/ui/app/pages/send/send-content/send-amount-row/tests/send-amount-row-component.test.js +++ b/ui/app/pages/send/send-content/send-amount-row/tests/send-amount-row-component.test.js @@ -8,6 +8,8 @@ import SendRowWrapper from '../../send-row-wrapper/send-row-wrapper.component' import AmountMaxButton from '../amount-max-button/amount-max-button.container' import UserPreferencedTokenInput from '../../../../../components/app/user-preferenced-token-input' +import timeout from '../../../../../../lib/test-timeout' + const propsMethodSpies = { setMaxModeTo: sinon.spy(), updateSendAmount: sinon.spy(), @@ -153,9 +155,8 @@ describe('SendAmountRow Component', function () { assert(wrapper.find(SendRowWrapper).childAt(1).is(UserPreferencedTokenInput)) }) - it('should render the UserPreferencedTokenInput with the correct props', () => { + it('should render the UserPreferencedTokenInput with the correct props', async () => { const { - onBlur, onChange, error, value, @@ -164,8 +165,9 @@ describe('SendAmountRow Component', function () { assert.equal(value, 'mockAmount') assert.equal(SendAmountRow.prototype.updateGas.callCount, 0) assert.equal(SendAmountRow.prototype.updateAmount.callCount, 0) - onBlur('mockNewAmount') - assert.equal(SendAmountRow.prototype.updateGas.callCount, 1) + assert.equal(SendAmountRow.prototype.validateAmount.callCount, 0) + onChange('mockNewAmount') + await timeout(501) assert.deepEqual( SendAmountRow.prototype.updateGas.getCall(0).args, ['mockNewAmount'] @@ -175,8 +177,6 @@ describe('SendAmountRow Component', function () { SendAmountRow.prototype.updateAmount.getCall(0).args, ['mockNewAmount'] ) - assert.equal(SendAmountRow.prototype.validateAmount.callCount, 0) - onChange('mockNewAmount') assert.equal(SendAmountRow.prototype.validateAmount.callCount, 1) assert.deepEqual( SendAmountRow.prototype.validateAmount.getCall(0).args, diff --git a/ui/app/pages/send/send-footer/send-footer.component.js b/ui/app/pages/send/send-footer/send-footer.component.js index f637fb8c7..20ab50c3c 100644 --- a/ui/app/pages/send/send-footer/send-footer.component.js +++ b/ui/app/pages/send/send-footer/send-footer.component.js @@ -28,6 +28,7 @@ export default class SendFooter extends Component { update: PropTypes.func, sendErrors: PropTypes.object, gasEstimateType: PropTypes.string, + gasIsLoading: PropTypes.bool, } static contextTypes = { @@ -102,10 +103,10 @@ export default class SendFooter extends Component { } formShouldBeDisabled () { - const { data, inError, selectedToken, tokenBalance, gasTotal, to, gasLimit } = this.props + const { data, inError, selectedToken, tokenBalance, gasTotal, to, gasLimit, gasIsLoading } = this.props const missingTokenBalance = selectedToken && !tokenBalance const gasLimitTooLow = gasLimit < 5208 // 5208 is hex value of 21000, minimum gas limit - const shouldBeDisabled = inError || !gasTotal || missingTokenBalance || !(data || to) || gasLimitTooLow + const shouldBeDisabled = inError || !gasTotal || missingTokenBalance || !(data || to) || gasLimitTooLow || gasIsLoading return shouldBeDisabled } diff --git a/ui/app/pages/send/send-footer/send-footer.container.js b/ui/app/pages/send/send-footer/send-footer.container.js index 42c12f2d5..52348ca89 100644 --- a/ui/app/pages/send/send-footer/send-footer.container.js +++ b/ui/app/pages/send/send-footer/send-footer.container.js @@ -23,6 +23,7 @@ import { getUnapprovedTxs, getSendErrors, } from '../send.selectors' +import { getGasIsLoading } from '../../../selectors/selectors' import { isSendFormInError, } from './send-footer.selectors' @@ -62,6 +63,7 @@ function mapStateToProps (state) { unapprovedTxs: getUnapprovedTxs(state), sendErrors: getSendErrors(state), gasEstimateType, + gasIsLoading: getGasIsLoading(state), } } diff --git a/ui/app/pages/send/send-footer/tests/send-footer-component.test.js b/ui/app/pages/send/send-footer/tests/send-footer-component.test.js index e1dd0d631..4f35e1b25 100644 --- a/ui/app/pages/send/send-footer/tests/send-footer-component.test.js +++ b/ui/app/pages/send/send-footer/tests/send-footer-component.test.js @@ -81,22 +81,34 @@ describe('SendFooter Component', function () { 'should return true if inError is truthy': { inError: true, expectedResult: true, + gasIsLoading: false, }, 'should return true if gasTotal is falsy': { inError: false, gasTotal: false, expectedResult: true, + gasIsLoading: false, }, 'should return true if to is truthy': { to: '0xsomevalidAddress', inError: false, gasTotal: false, expectedResult: true, + gasIsLoading: false, }, 'should return true if selectedToken is truthy and tokenBalance is falsy': { selectedToken: true, tokenBalance: null, expectedResult: true, + gasIsLoading: false, + }, + 'should return true if gasIsLoading is truthy but all other params are falsy': { + inError: false, + gasTotal: null, + selectedToken: null, + tokenBalance: 0, + expectedResult: true, + gasIsLoading: true, }, 'should return false if inError is false and all other params are truthy': { inError: false, @@ -104,7 +116,9 @@ describe('SendFooter Component', function () { selectedToken: true, tokenBalance: 123, expectedResult: false, + gasIsLoading: false, }, + } Object.entries(config).map(([description, obj]) => { it(description, () => { diff --git a/ui/app/pages/send/send-footer/tests/send-footer-container.test.js b/ui/app/pages/send/send-footer/tests/send-footer-container.test.js index b8af19017..70bc078c8 100644 --- a/ui/app/pages/send/send-footer/tests/send-footer-container.test.js +++ b/ui/app/pages/send/send-footer/tests/send-footer-container.test.js @@ -2,7 +2,6 @@ import assert from 'assert' import proxyquire from 'proxyquire' import sinon from 'sinon' -let mapStateToProps let mapDispatchToProps const actionSpies = { @@ -22,8 +21,7 @@ const utilsStubs = { proxyquire('../send-footer.container.js', { 'react-redux': { - connect: (ms, md) => { - mapStateToProps = ms + connect: (_, md) => { mapDispatchToProps = md return () => ({}) }, @@ -55,30 +53,6 @@ proxyquire('../send-footer.container.js', { describe('send-footer container', () => { - describe('mapStateToProps()', () => { - - it('should map the correct properties to props', () => { - assert.deepEqual(mapStateToProps('mockState'), { - amount: 'mockAmount:mockState', - data: 'mockHexData:mockState', - selectedToken: 'mockSelectedToken:mockState', - editingTransactionId: 'mockEditingTransactionId:mockState', - from: 'mockFromObject:mockState', - gasLimit: 'mockGasLimit:mockState', - gasPrice: 'mockGasPrice:mockState', - gasTotal: 'mockGasTotal:mockState', - inError: 'mockInError:mockState', - to: 'mockTo:mockState', - toAccounts: 'mockToAccounts:mockState', - tokenBalance: 'mockTokenBalance:mockState', - unapprovedTxs: 'mockUnapprovedTxs:mockState', - sendErrors: 'mockSendErrors:mockState', - gasEstimateType: 'mockGasEstimateType:mockState', - }) - }) - - }) - describe('mapDispatchToProps()', () => { let dispatchSpy let mapDispatchToPropsObject