From 1fbdce8916151df2b31eebc5de29a1365e5dadff Mon Sep 17 00:00:00 2001 From: Dan J Miller Date: Mon, 10 Dec 2018 18:21:00 -0330 Subject: Improve ux for low gas price set (#5862) * Show user warning if they set gas price below safelow minimum, error if 0. * Properly cache basic price estimate data. * Default retry price to recommended price if original price was 0x0 * Use mock fetch in send-new-ui integration tests. --- .../advanced-tab-content.component.js | 103 +++++++++++--- .../advanced-tab-content/index.scss | 14 +- .../tests/advanced-tab-content-component.test.js | 157 ++++++++++++++++----- .../gas-modal-page-container.component.js | 10 +- .../gas-modal-page-container.container.js | 5 +- .../gas-modal-page-container-component.test.js | 1 + .../gas-modal-page-container-container.test.js | 4 +- 7 files changed, 237 insertions(+), 57 deletions(-) (limited to 'ui/app/components/gas-customization/gas-modal-page-container') diff --git a/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/advanced-tab-content.component.js b/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/advanced-tab-content.component.js index ba738ff75..7c3142d0d 100644 --- a/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/advanced-tab-content.component.js +++ b/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/advanced-tab-content.component.js @@ -21,6 +21,8 @@ export default class AdvancedTabContent extends Component { timeRemaining: PropTypes.string, gasChartProps: PropTypes.object, insufficientBalance: PropTypes.bool, + customPriceIsSafe: PropTypes.bool, + isSpeedUp: PropTypes.bool, } constructor (props) { @@ -37,27 +39,62 @@ export default class AdvancedTabContent extends Component { } } - gasInput (value, onChange, min, insufficientBalance, showGWEI) { + gasInputError ({ labelKey, insufficientBalance, customPriceIsSafe, isSpeedUp, value }) { + let errorText + let errorType + let isInError = true + + + if (insufficientBalance) { + errorText = 'Insufficient Balance' + errorType = 'error' + } else if (labelKey === 'gasPrice' && isSpeedUp && value === 0) { + errorText = 'Zero gas price on speed up' + errorType = 'error' + } else if (labelKey === 'gasPrice' && !customPriceIsSafe) { + errorText = 'Gas Price Extremely Low' + errorType = 'warning' + } else { + isInError = false + } + + return { + isInError, + errorText, + errorType, + } + } + + gasInput ({ labelKey, value, onChange, insufficientBalance, showGWEI, customPriceIsSafe, isSpeedUp }) { + const { + isInError, + errorText, + errorType, + } = this.gasInputError({ labelKey, insufficientBalance, customPriceIsSafe, isSpeedUp, value }) + return (
onChange(Number(event.target.value))} />
onChange(value + 1)}>
onChange(value - 1)}>
- {insufficientBalance &&
- Insufficient Balance -
} + { isInError + ?
+ { errorText } +
+ : null }
) } @@ -83,23 +120,45 @@ export default class AdvancedTabContent extends Component { ) } - renderGasEditRow (labelKey, ...gasInputArgs) { + renderGasEditRow (gasInputArgs) { return (
- { this.context.t(labelKey) } + { this.context.t(gasInputArgs.labelKey) } { this.infoButton(() => {}) }
- { this.gasInput(...gasInputArgs) } + { this.gasInput(gasInputArgs) }
) } - renderGasEditRows (customGasPrice, updateCustomGasPrice, customGasLimit, updateCustomGasLimit, insufficientBalance) { + renderGasEditRows ({ + customGasPrice, + updateCustomGasPrice, + customGasLimit, + updateCustomGasLimit, + insufficientBalance, + customPriceIsSafe, + isSpeedUp, + }) { return (
- { this.renderGasEditRow('gasPrice', customGasPrice, updateCustomGasPrice, customGasPrice, insufficientBalance, true) } - { this.renderGasEditRow('gasLimit', customGasLimit, this.onChangeGasLimit, customGasLimit, insufficientBalance) } + { this.renderGasEditRow({ + labelKey: 'gasPrice', + value: customGasPrice, + onChange: updateCustomGasPrice, + insufficientBalance, + customPriceIsSafe, + showGWEI: true, + isSpeedUp, + }) } + { this.renderGasEditRow({ + labelKey: 'gasLimit', + value: customGasLimit, + onChange: this.onChangeGasLimit, + insufficientBalance, + customPriceIsSafe, + }) }
) } @@ -115,19 +174,23 @@ export default class AdvancedTabContent extends Component { totalFee, gasChartProps, gasEstimatesLoading, + customPriceIsSafe, + isSpeedUp, } = this.props return (
{ this.renderDataSummary(totalFee, timeRemaining) }
- { this.renderGasEditRows( - customGasPrice, - updateCustomGasPrice, - customGasLimit, - updateCustomGasLimit, - insufficientBalance - ) } + { this.renderGasEditRows({ + customGasPrice, + updateCustomGasPrice, + customGasLimit, + updateCustomGasLimit, + insufficientBalance, + customPriceIsSafe, + isSpeedUp, + }) }
Live Gas Price Predictions
{!gasEstimatesLoading ? diff --git a/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/index.scss b/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/index.scss index 88c69faf4..53cb84791 100644 --- a/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/index.scss +++ b/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/index.scss @@ -102,11 +102,15 @@ } } - &__insufficient-balance { + &__error-text { font-size: 12px; color: red; } + &__warning-text { + font-size: 12px; + color: orange; + } &__input-wrapper { position: relative; @@ -128,6 +132,10 @@ border: 1px solid $red; } + &__input--warning { + border: 1px solid $orange; + } + &__input-arrows { position: absolute; top: 7px; @@ -169,6 +177,10 @@ border: 1px solid $red; } + &__input-arrows--warning { + border: 1px solid $orange; + } + input[type="number"]::-webkit-inner-spin-button { -webkit-appearance: none; -moz-appearance: none; diff --git a/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/tests/advanced-tab-content-component.test.js b/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/tests/advanced-tab-content-component.test.js index d6920454d..00242e430 100644 --- a/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/tests/advanced-tab-content-component.test.js +++ b/ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content/tests/advanced-tab-content-component.test.js @@ -16,6 +16,7 @@ sinon.spy(AdvancedTabContent.prototype, 'renderGasEditRow') sinon.spy(AdvancedTabContent.prototype, 'gasInput') sinon.spy(AdvancedTabContent.prototype, 'renderGasEditRows') sinon.spy(AdvancedTabContent.prototype, 'renderDataSummary') +sinon.spy(AdvancedTabContent.prototype, 'gasInputError') describe('AdvancedTabContent Component', function () { let wrapper @@ -29,6 +30,8 @@ describe('AdvancedTabContent Component', function () { timeRemaining={21500} totalFee={'$0.25'} insufficientBalance={false} + customPriceIsSafe={true} + isSpeedUp={false} />, { context: { t: (str1, str2) => str2 ? str1 + str2 : str1 } }) }) @@ -86,9 +89,15 @@ describe('AdvancedTabContent Component', function () { it('should call renderGasEditRows with the expected params', () => { assert.equal(AdvancedTabContent.prototype.renderGasEditRows.callCount, 1) const renderGasEditRowArgs = AdvancedTabContent.prototype.renderGasEditRows.getCall(0).args - assert.deepEqual(renderGasEditRowArgs, [ - 11, propsMethodSpies.updateCustomGasPrice, 23456, propsMethodSpies.updateCustomGasLimit, false, - ]) + assert.deepEqual(renderGasEditRowArgs, [{ + customGasPrice: 11, + customGasLimit: 23456, + insufficientBalance: false, + customPriceIsSafe: true, + updateCustomGasPrice: propsMethodSpies.updateCustomGasPrice, + updateCustomGasLimit: propsMethodSpies.updateCustomGasLimit, + isSpeedUp: false, + }]) }) }) @@ -124,9 +133,10 @@ describe('AdvancedTabContent Component', function () { beforeEach(() => { AdvancedTabContent.prototype.gasInput.resetHistory() - gasEditRow = shallow(wrapper.instance().renderGasEditRow( - 'mockLabelKey', 'argA', 'argB' - )) + gasEditRow = shallow(wrapper.instance().renderGasEditRow({ + labelKey: 'mockLabelKey', + someArg: 'argA', + })) }) it('should render the gas-edit-row root node', () => { @@ -149,7 +159,7 @@ describe('AdvancedTabContent Component', function () { it('should call this.gasInput with the correct args', () => { const gasInputSpyArgs = AdvancedTabContent.prototype.gasInput.args - assert.deepEqual(gasInputSpyArgs[0], [ 'argA', 'argB' ]) + assert.deepEqual(gasInputSpyArgs[0], [ { labelKey: 'mockLabelKey', someArg: 'argA' } ]) }) }) @@ -188,12 +198,22 @@ describe('AdvancedTabContent Component', function () { it('should call this.renderGasEditRow twice, with the expected args', () => { const renderGasEditRowSpyArgs = AdvancedTabContent.prototype.renderGasEditRow.args assert.equal(renderGasEditRowSpyArgs.length, 2) - assert.deepEqual(renderGasEditRowSpyArgs[0].map(String), [ - 'gasPrice', 'mockGasPrice', () => 'mockUpdateCustomGasPriceReturn', 'mockGasPrice', false, true, - ].map(String)) - assert.deepEqual(renderGasEditRowSpyArgs[1].map(String), [ - 'gasLimit', 'mockGasLimit', () => 'mockOnChangeGasLimit', 'mockGasLimit', false, - ].map(String)) + assert.deepEqual(renderGasEditRowSpyArgs[0].map(String), [{ + labelKey: 'gasPrice', + value: 'mockGasLimit', + onChange: () => 'mockOnChangeGasLimit', + insufficientBalance: false, + customPriceIsSafe: true, + showGWEI: true, + }].map(String)) + assert.deepEqual(renderGasEditRowSpyArgs[1].map(String), [{ + labelKey: 'gasPrice', + value: 'mockGasPrice', + onChange: () => 'mockUpdateCustomGasPriceReturn', + insufficientBalance: false, + customPriceIsSafe: true, + showGWEI: true, + }].map(String)) }) }) @@ -219,13 +239,16 @@ describe('AdvancedTabContent Component', function () { beforeEach(() => { AdvancedTabContent.prototype.renderGasEditRow.resetHistory() - gasInput = shallow(wrapper.instance().gasInput( - 321, - value => value + 7, - 0, - false, - 8 - )) + AdvancedTabContent.prototype.gasInputError.resetHistory() + gasInput = shallow(wrapper.instance().gasInput({ + labelKey: 'gasPrice', + value: 321, + onChange: value => value + 7, + insufficientBalance: false, + showGWEI: true, + customPriceIsSafe: true, + isSpeedUp: false, + })) }) it('should render the input-wrapper root node', () => { @@ -237,12 +260,6 @@ describe('AdvancedTabContent Component', function () { assert(gasInput.children().at(0).hasClass('advanced-tab__gas-edit-row__input')) }) - it('should pass the correct value min and precision props to the input', () => { - const inputProps = gasInput.find('input').props() - assert.equal(inputProps.min, 0) - assert.equal(inputProps.value, 321) - }) - it('should call the passed onChange method with the value of the input onChange event', () => { const inputOnChange = gasInput.find('input').props().onChange assert.equal(inputOnChange({ target: { value: 8} }), 15) @@ -256,18 +273,92 @@ describe('AdvancedTabContent Component', function () { }) it('should call onChange with the value incremented decremented when its onchange method is called', () => { - gasInput = shallow(wrapper.instance().gasInput( - 321, - value => value + 7, - 0, - 8, - false - )) const upArrow = gasInput.find('.advanced-tab__gas-edit-row__input-arrows__i-wrap').at(0) assert.equal(upArrow.props().onClick(), 329) const downArrow = gasInput.find('.advanced-tab__gas-edit-row__input-arrows__i-wrap').at(1) assert.equal(downArrow.props().onClick(), 327) }) + + it('should call gasInputError with the expected params', () => { + assert.equal(AdvancedTabContent.prototype.gasInputError.callCount, 1) + const gasInputErrorArgs = AdvancedTabContent.prototype.gasInputError.getCall(0).args + assert.deepEqual(gasInputErrorArgs, [{ + labelKey: 'gasPrice', + insufficientBalance: false, + customPriceIsSafe: true, + value: 321, + isSpeedUp: false, + }]) + }) + }) + + describe('gasInputError()', () => { + let gasInputError + + beforeEach(() => { + AdvancedTabContent.prototype.renderGasEditRow.resetHistory() + gasInputError = wrapper.instance().gasInputError({ + labelKey: '', + insufficientBalance: false, + customPriceIsSafe: true, + isSpeedUp: false, + }) + }) + + it('should return an insufficientBalance error', () => { + const gasInputError = wrapper.instance().gasInputError({ + labelKey: 'gasPrice', + insufficientBalance: true, + customPriceIsSafe: true, + isSpeedUp: false, + value: 1, + }) + assert.deepEqual(gasInputError, { + isInError: true, + errorText: 'Insufficient Balance', + errorType: 'error', + }) + }) + + it('should return a zero gas on retry error', () => { + const gasInputError = wrapper.instance().gasInputError({ + labelKey: 'gasPrice', + insufficientBalance: false, + customPriceIsSafe: false, + isSpeedUp: true, + value: 0, + }) + assert.deepEqual(gasInputError, { + isInError: true, + errorText: 'Zero gas price on speed up', + errorType: 'error', + }) + }) + + it('should return a low gas warning', () => { + const gasInputError = wrapper.instance().gasInputError({ + labelKey: 'gasPrice', + insufficientBalance: false, + customPriceIsSafe: false, + isSpeedUp: false, + value: 1, + }) + assert.deepEqual(gasInputError, { + isInError: true, + errorText: 'Gas Price Extremely Low', + errorType: 'warning', + }) + }) + + it('should return isInError false if there is no error', () => { + gasInputError = wrapper.instance().gasInputError({ + labelKey: 'gasPrice', + insufficientBalance: false, + customPriceIsSafe: true, + value: 1, + }) + assert.equal(gasInputError.isInError, false) + }) }) }) diff --git a/ui/app/components/gas-customization/gas-modal-page-container/gas-modal-page-container.component.js b/ui/app/components/gas-customization/gas-modal-page-container/gas-modal-page-container.component.js index be91bef0f..64c2be353 100644 --- a/ui/app/components/gas-customization/gas-modal-page-container/gas-modal-page-container.component.js +++ b/ui/app/components/gas-customization/gas-modal-page-container/gas-modal-page-container.component.js @@ -35,6 +35,9 @@ export default class GasModalPageContainer extends Component { PropTypes.string, PropTypes.number, ]), + customPriceIsSafe: PropTypes.bool, + isSpeedUp: PropTypes.bool, + disableSave: PropTypes.bool, } state = {} @@ -69,6 +72,8 @@ export default class GasModalPageContainer extends Component { currentTimeEstimate, insufficientBalance, gasEstimatesLoading, + customPriceIsSafe, + isSpeedUp, }) { const { transactionFee } = this.props return ( @@ -83,6 +88,8 @@ export default class GasModalPageContainer extends Component { gasChartProps={gasChartProps} insufficientBalance={insufficientBalance} gasEstimatesLoading={gasEstimatesLoading} + customPriceIsSafe={customPriceIsSafe} + isSpeedUp={isSpeedUp} /> ) } @@ -153,6 +160,7 @@ export default class GasModalPageContainer extends Component { onSubmit, customModalGasPriceInHex, customModalGasLimitInHex, + disableSave, ...tabProps } = this.props @@ -162,7 +170,7 @@ export default class GasModalPageContainer extends Component { title={this.context.t('customGas')} subtitle={this.context.t('customGasSubTitle')} tabsComponent={this.renderTabs(infoRowProps, tabProps)} - disabled={tabProps.insufficientBalance} + disabled={disableSave} onCancel={() => cancelAndClose()} onClose={() => cancelAndClose()} onSubmit={() => { diff --git a/ui/app/components/gas-customization/gas-modal-page-container/gas-modal-page-container.container.js b/ui/app/components/gas-customization/gas-modal-page-container/gas-modal-page-container.container.js index c619a0988..dde0f2b94 100644 --- a/ui/app/components/gas-customization/gas-modal-page-container/gas-modal-page-container.container.js +++ b/ui/app/components/gas-customization/gas-modal-page-container/gas-modal-page-container.container.js @@ -40,6 +40,7 @@ import { getEstimatedGasTimes, getRenderableBasicEstimateData, getBasicGasEstimateBlockTime, + isCustomPriceSafe, } from '../../../selectors/custom-gas' import { submittedPendingTransactionsSelector, @@ -107,6 +108,7 @@ const mapStateToProps = (state, ownProps) => { newTotalFiat, currentTimeEstimate: getRenderableTimeEstimate(customGasPrice, gasPrices, estimatedTimes), blockTime: getBasicGasEstimateBlockTime(state), + customPriceIsSafe: isCustomPriceSafe(state), gasPriceButtonGroupProps: { buttonDataLoading, defaultActiveButtonIndex: getDefaultActiveButtonIndex(gasButtonInfo, customModalGasPriceInHex), @@ -167,7 +169,7 @@ const mapDispatchToProps = dispatch => { } const mergeProps = (stateProps, dispatchProps, ownProps) => { - const { gasPriceButtonGroupProps, isConfirm, isSpeedUp, txId } = stateProps + const { gasPriceButtonGroupProps, isConfirm, txId, isSpeedUp, insufficientBalance, customGasPrice } = stateProps const { updateCustomGasPrice: dispatchUpdateCustomGasPrice, hideGasButtonGroup: dispatchHideGasButtonGroup, @@ -208,6 +210,7 @@ const mergeProps = (stateProps, dispatchProps, ownProps) => { dispatchHideSidebar() } }, + disableSave: insufficientBalance || (isSpeedUp && customGasPrice === 0), } } diff --git a/ui/app/components/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-component.test.js b/ui/app/components/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-component.test.js index 2ba2fa9e7..f068c40d0 100644 --- a/ui/app/components/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-component.test.js +++ b/ui/app/components/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-component.test.js @@ -78,6 +78,7 @@ describe('GasModalPageContainer Component', function () { customGasPriceInHex={'mockCustomGasPriceInHex'} customGasLimitInHex={'mockCustomGasLimitInHex'} insufficientBalance={false} + disableSave={false} />, { context: { t: (str1, str2) => str2 ? str1 + str2 : str1 } }) }) diff --git a/ui/app/components/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-container.test.js b/ui/app/components/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-container.test.js index 512832866..077ec471d 100644 --- a/ui/app/components/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-container.test.js +++ b/ui/app/components/gas-customization/gas-modal-page-container/tests/gas-modal-page-container-container.test.js @@ -75,6 +75,7 @@ describe('gas-modal-page-container container', () => { gas: { basicEstimates: { blockTime: 12, + safeLow: 2, }, customData: { limit: 'aaaaaaaa', @@ -107,9 +108,10 @@ describe('gas-modal-page-container container', () => { blockTime: 12, customModalGasLimitInHex: 'aaaaaaaa', customModalGasPriceInHex: 'ffffffff', + customPriceIsSafe: true, gasChartProps: { 'currentPrice': 4.294967295, - estimatedTimes: ['31', '62', '93', '124'], + estimatedTimes: [31, 62, 93, 124], estimatedTimesMax: '31', gasPrices: [3, 4, 5, 6], gasPricesMax: 6, -- cgit v1.2.3