From 4779bd9fe5921d31bfe01fce5df5f2ef8a7f5b9b Mon Sep 17 00:00:00 2001 From: Jyoti Puri Date: Fri, 12 Nov 2021 08:52:54 +0530 Subject: [PATCH] Show warning in confirm transaction screen for low/high gas values. (#12545) --- app/_locales/en/messages.json | 3 + .../app/gas-timing/gas-timing.component.js | 27 ++++++--- ui/components/app/gas-timing/index.scss | 5 ++ ui/hooks/gasFeeInput/useGasFeeInputs.js | 4 +- .../confirm-transaction-base.component.js | 2 + .../gas-details-item/gas-details-item.js | 27 ++++++--- .../gas-details-item/gas-details-item.scss | 4 ++ .../gas-details-item/gas-details-item.test.js | 34 ++++++++++- .../low-priority-message/index.js | 1 + .../low-priority-message.js | 24 ++++++++ .../low-priority-message.scss | 3 + .../low-priority-message.test.js | 59 +++++++++++++++++++ ui/pages/pages.scss | 1 + 13 files changed, 175 insertions(+), 19 deletions(-) create mode 100644 ui/pages/confirm-transaction-base/low-priority-message/index.js create mode 100644 ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.js create mode 100644 ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.scss create mode 100644 ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.test.js diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index 84e09c001..2bc416fb2 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -1349,6 +1349,9 @@ "lockTimeTooGreat": { "message": "Lock time is too great" }, + "lowPriorityMessage": { + "message": "Future transactions will queue after this one. This price was last seen was some time ago." + }, "mainnet": { "message": "Ethereum Mainnet" }, diff --git a/ui/components/app/gas-timing/gas-timing.component.js b/ui/components/app/gas-timing/gas-timing.component.js index 972805ffb..41fc6bed5 100644 --- a/ui/components/app/gas-timing/gas-timing.component.js +++ b/ui/components/app/gas-timing/gas-timing.component.js @@ -24,6 +24,7 @@ import InfoTooltip from '../../ui/info-tooltip/info-tooltip'; import { getGasFeeTimeEstimate } from '../../../store/actions'; import { GAS_FORM_ERRORS } from '../../../helpers/constants/gas'; +import { useGasFeeContext } from '../../../contexts/gasFee'; // Once we reach this second threshold, we switch to minutes as a unit const SECOND_CUTOFF = 90; @@ -49,6 +50,7 @@ export default function GasTiming({ const [customEstimatedTime, setCustomEstimatedTime] = useState(null); const t = useContext(I18nContext); + const { estimateToUse } = useGasFeeContext(); // If the user has chosen a value lower than the low gas fee estimate, // We'll need to use the useEffect hook below to make a call to calculate @@ -94,12 +96,17 @@ export default function GasTiming({ previousIsUnknownLow, ]); - const unknownProcessingTimeText = ( - <> - {t('editGasTooLow')}{' '} - - - ); + let unknownProcessingTimeText; + if (EIP_1559_V2) { + unknownProcessingTimeText = t('editGasTooLow'); + } else { + unknownProcessingTimeText = ( + <> + {t('editGasTooLow')}{' '} + + + ); + } if ( gasWarnings?.maxPriorityFee === GAS_FORM_ERRORS.MAX_PRIORITY_FEE_TOO_LOW || @@ -148,8 +155,9 @@ export default function GasTiming({ ]); } } else { - attitude = 'negative'; - + if (!EIP_1559_V2 || estimateToUse === 'low') { + attitude = 'negative'; + } // If the user has chosen a value less than our low estimate, // calculate a potential wait time if (isUnknownLow) { @@ -191,7 +199,8 @@ export default function GasTiming({ {text} diff --git a/ui/components/app/gas-timing/index.scss b/ui/components/app/gas-timing/index.scss index 52b0f58c1..20d0b451f 100644 --- a/ui/components/app/gas-timing/index.scss +++ b/ui/components/app/gas-timing/index.scss @@ -14,6 +14,11 @@ font-weight: bold; } + &--negative-V2 { + color: $secondary-1; + font-weight: bold; + } + .info-tooltip { display: inline-block; margin-inline-start: 4px; diff --git a/ui/hooks/gasFeeInput/useGasFeeInputs.js b/ui/hooks/gasFeeInput/useGasFeeInputs.js index 96ad40f73..696a9a5a3 100644 --- a/ui/hooks/gasFeeInput/useGasFeeInputs.js +++ b/ui/hooks/gasFeeInput/useGasFeeInputs.js @@ -4,9 +4,9 @@ import { useSelector } from 'react-redux'; import { getAdvancedInlineGasShown } from '../../selectors'; import { hexToDecimal } from '../../helpers/utils/conversions.util'; import { - EDIT_GAS_MODES, - GAS_RECOMMENDATIONS, CUSTOM_GAS_ESTIMATE, + GAS_RECOMMENDATIONS, + EDIT_GAS_MODES, } from '../../../shared/constants/gas'; import { GAS_FORM_ERRORS } from '../../helpers/constants/gas'; diff --git a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js index d7168b233..aa4eac2f2 100644 --- a/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js +++ b/ui/pages/confirm-transaction-base/confirm-transaction-base.component.js @@ -54,6 +54,7 @@ import Typography from '../../components/ui/typography/typography'; import { MIN_GAS_LIMIT_DEC } from '../send/send.constants'; import GasDetailsItem from './gas-details-item'; +import LowPriorityMessage from './low-priority-message'; // eslint-disable-next-line prefer-destructuring const EIP_1559_V2 = process.env.EIP_1559_V2; @@ -414,6 +415,7 @@ export default class ConfirmTransactionBase extends Component { return (
+ {EIP_1559_V2 && } this.handleEditGas()} rows={[ diff --git a/ui/pages/confirm-transaction-base/gas-details-item/gas-details-item.js b/ui/pages/confirm-transaction-base/gas-details-item/gas-details-item.js index b36951cd4..6ac1e0be0 100644 --- a/ui/pages/confirm-transaction-base/gas-details-item/gas-details-item.js +++ b/ui/pages/confirm-transaction-base/gas-details-item/gas-details-item.js @@ -1,5 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; +import classNames from 'classnames'; import { COLORS } from '../../../helpers/constants/design-system'; import { PRIMARY, SECONDARY } from '../../../helpers/constants/common'; @@ -14,11 +15,12 @@ import InfoTooltip from '../../../components/ui/info-tooltip/info-tooltip'; import LoadingHeartBeat from '../../../components/ui/loading-heartbeat'; import TransactionDetailItem from '../../../components/app/transaction-detail-item/transaction-detail-item.component'; import UserPreferencedCurrencyDisplay from '../../../components/app/user-preferenced-currency-display'; +import { useGasFeeContext } from '../../../contexts/gasFee'; const HeartBeat = () => process.env.IN_TEST === 'true' ? null : ; -const GasDetailItem = ({ +const GasDetailsItem = ({ hexMaximumTransactionFee, hexMinimumTransactionFee, isMainnet, @@ -29,6 +31,8 @@ const GasDetailItem = ({ useNativeCurrencyAsPrimaryCurrency, }) => { const t = useI18nContext(); + const { estimateToUse } = useGasFeeContext(); + return ( } - position="top" + position="bottom" /> } @@ -88,9 +92,18 @@ const GasDetailItem = ({
} subText={t('editGasSubTextFee', [ - - - + + + + {estimateToUse === 'high' && '⚠ '} + +
({ + disconnectGasFeeEstimatePoller: jest.fn(), + getGasFeeEstimatesAndStartPolling: jest + .fn() + .mockImplementation(() => Promise.resolve()), + addPollingTokenToAppState: jest.fn(), +})); + const render = (props) => { const store = configureStore({ metamask: { @@ -15,10 +24,23 @@ const render = (props) => { useNativeCurrencyAsPrimaryCurrency: true, }, provider: {}, + cachedBalances: {}, + accounts: { + '0xAddress': { + address: '0xAddress', + balance: '0x176e5b6f173ebe66', + }, + }, + selectedAddress: '0xAddress', }, }); - return renderWithProvider(, store); + return renderWithProvider( + + + , + store, + ); }; describe('GasDetailsItem', () => { @@ -29,4 +51,14 @@ describe('GasDetailsItem', () => { expect(screen.queryByText('Max fee:')).toBeInTheDocument(); expect(screen.queryByText('ETH')).toBeInTheDocument(); }); + + it('should show warning icon if estimates are high', () => { + render({ defaultEstimateToUse: 'high' }); + expect(screen.queryByText('⚠ Max fee:')).toBeInTheDocument(); + }); + + it('should not show warning icon if estimates are not high', () => { + render({ defaultEstimateToUse: 'low' }); + expect(screen.queryByText('Max fee:')).toBeInTheDocument(); + }); }); diff --git a/ui/pages/confirm-transaction-base/low-priority-message/index.js b/ui/pages/confirm-transaction-base/low-priority-message/index.js new file mode 100644 index 000000000..7b2838d48 --- /dev/null +++ b/ui/pages/confirm-transaction-base/low-priority-message/index.js @@ -0,0 +1 @@ +export { default } from './low-priority-message'; diff --git a/ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.js b/ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.js new file mode 100644 index 000000000..41bb5d132 --- /dev/null +++ b/ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.js @@ -0,0 +1,24 @@ +import React from 'react'; + +import ActionableMessage from '../../../components/ui/actionable-message/actionable-message'; +import { useGasFeeContext } from '../../../contexts/gasFee'; +import { useI18nContext } from '../../../hooks/useI18nContext'; + +const LowPriorityMessage = () => { + const { estimateToUse } = useGasFeeContext(); + const t = useI18nContext(); + + if (estimateToUse !== 'low') return null; + return ( +
+ +
+ ); +}; + +export default LowPriorityMessage; diff --git a/ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.scss b/ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.scss new file mode 100644 index 000000000..1a99a03af --- /dev/null +++ b/ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.scss @@ -0,0 +1,3 @@ +.low-priority-message { + margin-top: 20px; +} diff --git a/ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.test.js b/ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.test.js new file mode 100644 index 000000000..9d8e233f3 --- /dev/null +++ b/ui/pages/confirm-transaction-base/low-priority-message/low-priority-message.test.js @@ -0,0 +1,59 @@ +import React from 'react'; + +import { renderWithProvider } from '../../../../test/lib/render-helpers'; +import { ETH } from '../../../helpers/constants/common'; +import { GasFeeContextProvider } from '../../../contexts/gasFee'; +import configureStore from '../../../store/store'; + +import LowPriorityMessage from './low-priority-message'; + +jest.mock('../../../store/actions', () => ({ + disconnectGasFeeEstimatePoller: jest.fn(), + getGasFeeEstimatesAndStartPolling: jest + .fn() + .mockImplementation(() => Promise.resolve()), + addPollingTokenToAppState: jest.fn(), +})); + +const render = (props) => { + const store = configureStore({ + metamask: { + nativeCurrency: ETH, + preferences: { + useNativeCurrencyAsPrimaryCurrency: true, + }, + provider: {}, + cachedBalances: {}, + accounts: { + '0xAddress': { + address: '0xAddress', + balance: '0x176e5b6f173ebe66', + }, + }, + selectedAddress: '0xAddress', + }, + }); + + return renderWithProvider( + + + , + store, + ); +}; + +describe('LowPriorityMessage', () => { + it('should returning warning message for low gas estimate', () => { + render({ transaction: { userFeeLevel: 'low' } }); + expect( + document.getElementsByClassName('actionable-message--warning'), + ).toHaveLength(1); + }); + + it('should return null for gas estimate other than low', () => { + render({ transaction: { userFeeLevel: 'high' } }); + expect( + document.getElementsByClassName('actionable-message--warning'), + ).toHaveLength(0); + }); +}); diff --git a/ui/pages/pages.scss b/ui/pages/pages.scss index df9de3f4e..878344063 100644 --- a/ui/pages/pages.scss +++ b/ui/pages/pages.scss @@ -6,6 +6,7 @@ @import 'confirm-decrypt-message/confirm-decrypt-message'; @import 'confirm-encryption-public-key/confirm-encryption-public-key'; @import 'confirm-transaction-base/gas-details-item/gas-details-item'; +@import 'confirm-transaction-base/low-priority-message/low-priority-message'; @import 'confirmation/confirmation'; @import 'connected-sites/index'; @import 'connected-accounts/index';