From e2fbc7ce8ea42bd8befdea894ddc8ed0fbdf27be Mon Sep 17 00:00:00 2001 From: David Walsh Date: Mon, 2 Aug 2021 18:53:13 -0500 Subject: [PATCH] Remove button group for non-EIP-1559 networks (#11712) * Remove button group for non-EIP-1559 networks * Fix tests...maybe * Remove unnecessary props, as well as gas display * Remove unused string * test progress * fix test * fix test * add customizes gas block to improve e2e pass rate Co-authored-by: Alex --- app/_locales/en/messages.json | 3 - test/e2e/metamask-ui.spec.js | 33 +++--- test/e2e/tests/from-import-ui.spec.js | 9 -- test/e2e/tests/metamask-responsive-ui.spec.js | 8 -- test/e2e/tests/send-edit.spec.js | 19 ++-- test/e2e/tests/send-eth.spec.js | 66 ------------ .../send-gas-row/send-gas-row.component.js | 102 ++---------------- .../send-gas-row.component.test.js | 96 +---------------- 8 files changed, 39 insertions(+), 297 deletions(-) diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 6d5f24176..74e74f5ff 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -904,9 +904,6 @@ "gasEstimatesUnavailableWarning": { "message": "Our low, medium and high estimates are not available." }, - "gasFee": { - "message": "Gas Fee" - }, "gasFeeEstimate": { "message": "Estimate" }, diff --git a/test/e2e/metamask-ui.spec.js b/test/e2e/metamask-ui.spec.js index 3fb432e94..1b19b45ec 100644 --- a/test/e2e/metamask-ui.spec.js +++ b/test/e2e/metamask-ui.spec.js @@ -561,19 +561,9 @@ describe('MetaMask', function () { driver.fill('.unit-input__input', '1'); }); - it('opens customize gas modal and saves options to continue', async function () { - await driver.clickElement('.advanced-gas-options-btn'); - - // wait for gas modal to be visible - const gasModal = await driver.findVisibleElement('span .modal'); - await driver.findElement('.page-container__title'); - await driver.clickElement({ text: 'Save', tag: 'button' }); - // wait for gas modal to be removed from DOM. - await gasModal.waitForElementState('hidden'); - }); - it('transitions to the confirm screen', async function () { // Continue to next screen + await driver.delay(largeDelayMs); await driver.clickElement({ text: 'Next', tag: 'button' }); await driver.delay(regularDelayMs); }); @@ -610,6 +600,18 @@ describe('MetaMask', function () { await driver.delay(regularDelayMs); }); + it('customizes gas', async function () { + await driver.clickElement({ text: 'Edit', tag: 'button' }); + await driver.delay(largeDelayMs); + const inputs = await driver.findElements('input[type="number"]'); + const gasLimitInput = inputs[0]; + const gasPriceInput = inputs[1]; + await gasLimitInput.fill('100000'); + await gasPriceInput.fill('100'); + await driver.delay(1000); + await driver.clickElement({ text: 'Save', tag: 'button' }, 10000); + }); + it('submits the transaction', async function () { await driver.clickElement({ text: 'Confirm', tag: 'button' }); await driver.delay(regularDelayMs); @@ -670,7 +672,7 @@ describe('MetaMask', function () { it('customizes gas', async function () { await driver.clickElement({ text: 'Edit', tag: 'button' }); - await driver.delay(regularDelayMs); + await driver.delay(largeDelayMs); await driver.clickElement( { text: 'Edit suggested gas fee', tag: 'button' }, 10000, @@ -791,13 +793,14 @@ describe('MetaMask', function () { it('customizes gas', async function () { await driver.clickElement('.confirm-approve-content__small-blue-text'); await driver.delay(regularDelayMs); - await driver.clickElement( - '.edit-gas-display__dapp-acknowledgement-button', + { text: 'Edit suggested gas fee', tag: 'button' }, + 10000, ); + await driver.delay(regularDelayMs); const [gasLimitInput, gasPriceInput] = await driver.findElements( - '.advanced-gas-controls input[type="number"]', + 'input[type="number"]', ); await gasPriceInput.fill('10'); diff --git a/test/e2e/tests/from-import-ui.spec.js b/test/e2e/tests/from-import-ui.spec.js index b8c0abc52..176b51e48 100644 --- a/test/e2e/tests/from-import-ui.spec.js +++ b/test/e2e/tests/from-import-ui.spec.js @@ -126,15 +126,6 @@ describe('Metamask Import UI', function () { ); await driver.fill('.unit-input__input', '1'); - // Set the gas limit - await driver.clickElement('.advanced-gas-options-btn'); - - // wait for gas modal to be visible - const gasModal = await driver.findVisibleElement('span .modal'); - await driver.clickElement({ text: 'Save', tag: 'button' }); - // wait for gas modal to be removed from DOM - await gasModal.waitForElementState('hidden'); - // Continue to next screen await driver.clickElement({ text: 'Next', tag: 'button' }); diff --git a/test/e2e/tests/metamask-responsive-ui.spec.js b/test/e2e/tests/metamask-responsive-ui.spec.js index a50bdfff9..e512ce9f4 100644 --- a/test/e2e/tests/metamask-responsive-ui.spec.js +++ b/test/e2e/tests/metamask-responsive-ui.spec.js @@ -173,14 +173,6 @@ describe('Metamask Responsive UI', function () { const inputValue = await inputAmount.getAttribute('value'); assert.equal(inputValue, '1'); - // opens and closes the gas modal - await driver.clickElement('.advanced-gas-options-btn'); - // wait for gas modal to be visible - const gasModal = await driver.findVisibleElement('span .modal'); - await driver.clickElement('.page-container__header-close-text'); - // wait for gas modal to be removed from dom - await gasModal.waitForElementState('hidden'); - // confirming transcation await driver.clickElement({ text: 'Next', tag: 'button' }); await driver.clickElement({ text: 'Confirm', tag: 'button' }); diff --git a/test/e2e/tests/send-edit.spec.js b/test/e2e/tests/send-edit.spec.js index c4b8a5ad5..81772df7b 100644 --- a/test/e2e/tests/send-edit.spec.js +++ b/test/e2e/tests/send-edit.spec.js @@ -42,14 +42,14 @@ describe('Editing Confirm Transaction', function () { ); await driver.fill('.unit-input__input', '2.2'); - await driver.clickElement('.advanced-gas-options-btn'); + await driver.clickElement({ text: 'Next', tag: 'button' }); + await driver.delay(regularDelayMs); - // wait for gas modal to be visible - const gasModal = await driver.findVisibleElement('span .modal'); + await driver.clickElement({ text: 'Edit', tag: 'button' }); - const [gasPriceInput, gasLimitInput] = await driver.findElements( - '.advanced-gas-inputs__gas-edit-row__input', + const [gasLimitInput, gasPriceInput] = await driver.findElements( + 'input[type="number"]', ); await gasPriceInput.fill('8'); @@ -59,19 +59,16 @@ describe('Editing Confirm Transaction', function () { await driver.delay(largeDelayMs); await driver.clickElement({ text: 'Save', tag: 'button' }); - // Wait for gas modal to be removed from DOM - await gasModal.waitForElementState('hidden'); - await driver.clickElement({ text: 'Next', tag: 'button' }); // has correct updated value on the confirm screen the transaction const editedTransactionAmounts = await driver.findElements( - '.currency-display-component__text', + '.transaction-detail-item__row .transaction-detail-item__detail-text .currency-display-component__text', ); const editedTransactionAmount = editedTransactionAmounts[0]; - assert.equal(await editedTransactionAmount.getText(), '2.2'); + assert.equal(await editedTransactionAmount.getText(), '0.0008'); const editedTransactionFee = editedTransactionAmounts[1]; - assert.equal(await editedTransactionFee.getText(), '0.0008'); + assert.equal(await editedTransactionFee.getText(), '2.2008'); // confirms the transaction await driver.clickElement({ text: 'Confirm', tag: 'button' }); diff --git a/test/e2e/tests/send-eth.spec.js b/test/e2e/tests/send-eth.spec.js index 8ac10f6ea..1fbebfaa7 100644 --- a/test/e2e/tests/send-eth.spec.js +++ b/test/e2e/tests/send-eth.spec.js @@ -87,61 +87,6 @@ describe('Send ETH from inside MetaMask using default gas', function () { }); }); -describe('Send ETH from inside MetaMask using fast gas option', function () { - const ganacheOptions = { - accounts: [ - { - secretKey: - '0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC', - balance: 25000000000000000000, - }, - ], - }; - it('finds the transaction in the transactions list', async function () { - await withFixtures( - { - fixtures: 'imported-account', - ganacheOptions, - title: this.test.title, - }, - async ({ driver }) => { - await driver.navigate(); - await driver.fill('#password', 'correct horse battery staple'); - await driver.press('#password', driver.Key.ENTER); - - await driver.clickElement('[data-testid="eth-overview-send"]'); - - await driver.fill( - 'input[placeholder="Search, public address (0x), or ENS"]', - '0x2f318C334780961FB129D2a6c30D0763d9a5C970', - ); - - const inputAmount = await driver.findElement('.unit-input__input'); - await inputAmount.fill('1'); - - const inputValue = await inputAmount.getAttribute('value'); - assert.equal(inputValue, '1'); - - // Set the gas price - await driver.clickElement({ text: 'Fast', tag: 'button/div/div' }); - - // Continue to next screen - await driver.clickElement({ text: 'Next', tag: 'button' }); - - await driver.clickElement({ text: 'Confirm', tag: 'button' }); - - await driver.waitForSelector( - '.transaction-list__completed-transactions .transaction-list-item', - ); - await driver.waitForSelector({ - css: '.transaction-list-item__primary-currency', - text: '-1 ETH', - }); - }, - ); - }); -}); - describe('Send ETH from inside MetaMask using advanced gas modal', function () { const ganacheOptions = { accounts: [ @@ -177,17 +122,6 @@ describe('Send ETH from inside MetaMask using advanced gas modal', function () { const inputValue = await inputAmount.getAttribute('value'); assert.equal(inputValue, '1'); - // Set the gas limit - await driver.clickElement('.advanced-gas-options-btn'); - - // wait for gas modal to be visible - const gasModal = await driver.findVisibleElement('span .modal'); - - await driver.clickElement({ text: 'Save', tag: 'button' }); - - // Wait for gas modal to be removed from DOM - await gasModal.waitForElementState('hidden'); - // Continue to next screen await driver.clickElement({ text: 'Next', tag: 'button' }); diff --git a/ui/pages/send/send-content/send-gas-row/send-gas-row.component.js b/ui/pages/send/send-content/send-gas-row/send-gas-row.component.js index e14b3d9cd..3ddb37224 100644 --- a/ui/pages/send/send-content/send-gas-row/send-gas-row.component.js +++ b/ui/pages/send/send-content/send-gas-row/send-gas-row.component.js @@ -1,23 +1,14 @@ import React, { Component } from 'react'; import PropTypes from 'prop-types'; import SendRowWrapper from '../send-row-wrapper'; -import GasPriceButtonGroup from '../../../../components/app/gas-customization/gas-price-button-group'; import AdvancedGasInputs from '../../../../components/app/gas-customization/advanced-gas-inputs'; import { GAS_INPUT_MODES } from '../../../../ducks/send'; -import GasFeeDisplay from './gas-fee-display/gas-fee-display.component'; export default class SendGasRow extends Component { static propTypes = { - gasFeeError: PropTypes.bool, - gasLoadingError: PropTypes.bool, - gasTotal: PropTypes.string, - showLegacyCustomizeGasModal: PropTypes.func, updateGasPrice: PropTypes.func, updateGasLimit: PropTypes.func, gasInputMode: PropTypes.oneOf(Object.values(GAS_INPUT_MODES)), - gasPriceButtonGroupProps: PropTypes.object, - advancedInlineGasShown: PropTypes.bool, - resetGasButtons: PropTypes.func, gasPrice: PropTypes.string, gasLimit: PropTypes.string, insufficientBalance: PropTypes.bool, @@ -29,69 +20,23 @@ export default class SendGasRow extends Component { trackEvent: PropTypes.func, }; - renderAdvancedOptionsButton() { - const { trackEvent } = this.context; - const { showLegacyCustomizeGasModal } = this.props; - return ( -
{ - trackEvent({ - category: 'Transactions', - event: 'Clicked "Advanced Options"', - }); - showLegacyCustomizeGasModal(); - }} - > - {this.context.t('advancedOptions')} -
- ); - } - - renderContent() { + render() { const { - gasLoadingError, - gasTotal, - gasPriceButtonGroupProps, - gasInputMode, - resetGasButtons, updateGasPrice, updateGasLimit, gasPrice, gasLimit, insufficientBalance, minimumGasLimit, + gasInputMode, } = this.props; - const { trackEvent } = this.context; - const gasPriceButtonGroup = ( -
- { - trackEvent({ - category: 'Transactions', - event: 'User Clicked Gas Estimate Button', - properties: { - gasEstimateType: opts.gasEstimateType.toLowerCase(), - }, - }); - await gasPriceButtonGroupProps.handleGasPriceSelection(opts); - }} - /> -
- ); - const gasFeeDisplay = ( - - ); - const advancedGasInputs = ( -
+ if (gasInputMode !== GAS_INPUT_MODES.INLINE) { + return null; + } + + return ( + -
- ); - // Tests should behave in same way as mainnet, but are using Localhost - switch (gasInputMode) { - case GAS_INPUT_MODES.BASIC: - return gasPriceButtonGroup; - case GAS_INPUT_MODES.INLINE: - return advancedGasInputs; - case GAS_INPUT_MODES.CUSTOM: - default: - return gasFeeDisplay; - } - } - - render() { - const { gasFeeError, gasInputMode, advancedInlineGasShown } = this.props; - - return ( - <> - - {this.renderContent()} - - {gasInputMode === GAS_INPUT_MODES.BASIC || advancedInlineGasShown ? ( - {this.renderAdvancedOptionsButton()} - ) : null} - + ); } } diff --git a/ui/pages/send/send-content/send-gas-row/send-gas-row.component.test.js b/ui/pages/send/send-content/send-gas-row/send-gas-row.component.test.js index b8f8ec41d..e9ed78243 100644 --- a/ui/pages/send/send-content/send-gas-row/send-gas-row.component.test.js +++ b/ui/pages/send/send-content/send-gas-row/send-gas-row.component.test.js @@ -1,19 +1,9 @@ import React from 'react'; import { shallow } from 'enzyme'; -import sinon from 'sinon'; import SendRowWrapper from '../send-row-wrapper/send-row-wrapper.component'; -import GasPriceButtonGroup from '../../../../components/app/gas-customization/gas-price-button-group'; import { GAS_INPUT_MODES } from '../../../../ducks/send'; import SendGasRow from './send-gas-row.component'; -import GasFeeDisplay from './gas-fee-display/gas-fee-display.component'; - -const propsMethodSpies = { - showCustomizeGasModal: sinon.spy(), - showLegacyCustomizeGasModal: sinon.spy(), - resetGasButtons: sinon.spy(), -}; - describe('SendGasRow Component', () => { let wrapper; @@ -24,97 +14,19 @@ describe('SendGasRow Component', () => { conversionRate={20} convertedCurrency="mockConvertedCurrency" gasFeeError - gasLoadingError={false} - gasTotal="mockGasTotal" - gasInputMode={GAS_INPUT_MODES.CUSTOM} - showCustomizeGasModal={propsMethodSpies.showCustomizeGasModal} - showLegacyCustomizeGasModal={ - propsMethodSpies.showLegacyCustomizeGasModal - } - resetGasButtons={propsMethodSpies.resetGasButtons} - gasPriceButtonGroupProps={{ - someGasPriceButtonGroupProp: 'foo', - anotherGasPriceButtonGroupProp: 'bar', - }} + gasInputMode={GAS_INPUT_MODES.INLINE} />, { context: { t: (str) => `${str}_t`, trackEvent: () => ({}) } }, ); wrapper.setProps({ isMainnet: true }); }); - afterEach(() => { - propsMethodSpies.resetGasButtons.resetHistory(); - }); - it('should render a SendRowWrapper component', () => { - expect(wrapper.name()).toStrictEqual('Fragment'); - expect(wrapper.at(0).find(SendRowWrapper)).toHaveLength(1); + expect(wrapper.is(SendRowWrapper)).toStrictEqual(true); }); - it('should pass the correct props to SendRowWrapper', () => { - const { label, showError, errorType } = wrapper - .find(SendRowWrapper) - .first() - .props(); - - expect(label).toStrictEqual('transactionFee_t:'); - expect(showError).toStrictEqual(true); - expect(errorType).toStrictEqual('gasFee'); - }); - - it('should render a GasFeeDisplay as a child of the SendRowWrapper', () => { - expect( - wrapper.find(SendRowWrapper).first().childAt(0).is(GasFeeDisplay), - ).toStrictEqual(true); - }); - - it('should render the GasFeeDisplay', () => { - const { gasLoadingError, gasTotal, onReset } = wrapper - .find(SendRowWrapper) - .first() - .childAt(0) - .props(); - expect(gasLoadingError).toStrictEqual(false); - expect(gasTotal).toStrictEqual('mockGasTotal'); - expect(propsMethodSpies.resetGasButtons.callCount).toStrictEqual(0); - onReset(); - expect(propsMethodSpies.resetGasButtons.callCount).toStrictEqual(1); - }); - - it('should render the GasPriceButtonGroup if gasInputMode is BASIC', () => { - wrapper.setProps({ gasInputMode: GAS_INPUT_MODES.BASIC }); - const rendered = wrapper.find(SendRowWrapper).first().childAt(0); - expect(wrapper.children()).toHaveLength(2); - - const gasPriceButtonGroup = rendered.childAt(0); - expect(gasPriceButtonGroup.is(GasPriceButtonGroup)).toStrictEqual(true); - expect( - gasPriceButtonGroup.hasClass('gas-price-button-group--small'), - ).toStrictEqual(true); - expect(gasPriceButtonGroup.props().showCheck).toStrictEqual(false); - expect( - gasPriceButtonGroup.props().someGasPriceButtonGroupProp, - ).toStrictEqual('foo'); - expect( - gasPriceButtonGroup.props().anotherGasPriceButtonGroupProp, - ).toStrictEqual('bar'); - }); - - it('should render an advanced options button if gasInputMode is BASIC', () => { - wrapper.setProps({ gasInputMode: GAS_INPUT_MODES.BASIC }); - const rendered = wrapper.find(SendRowWrapper).last(); - expect(wrapper.children()).toHaveLength(2); - - const advancedOptionsButton = rendered.childAt(0); - expect(advancedOptionsButton.text()).toStrictEqual('advancedOptions_t'); - - expect( - propsMethodSpies.showLegacyCustomizeGasModal.callCount, - ).toStrictEqual(0); - advancedOptionsButton.props().onClick(); - expect( - propsMethodSpies.showLegacyCustomizeGasModal.callCount, - ).toStrictEqual(1); + it('should render an AdvancedGasInputs as a child of the SendRowWrapper', () => { + expect(wrapper.first().childAt(0)).toBeDefined(); }); }); });