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 ++++++++++++++++----- 3 files changed, 220 insertions(+), 54 deletions(-) (limited to 'ui/app/components/gas-customization/gas-modal-page-container/advanced-tab-content') 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) + }) }) }) -- cgit