diff options
author | Dan J Miller <danjm.com@gmail.com> | 2018-12-11 05:51:00 +0800 |
---|---|---|
committer | Dan Finlay <542863+danfinlay@users.noreply.github.com> | 2018-12-11 05:51:00 +0800 |
commit | 1fbdce8916151df2b31eebc5de29a1365e5dadff (patch) | |
tree | 2333aa889dd6a6ec83a1665591006daca8e68859 /ui/app/components | |
parent | 49971e9ec250888746546f62fa176ed129bf9c74 (diff) | |
download | tangerine-wallet-browser-1fbdce8916151df2b31eebc5de29a1365e5dadff.tar tangerine-wallet-browser-1fbdce8916151df2b31eebc5de29a1365e5dadff.tar.gz tangerine-wallet-browser-1fbdce8916151df2b31eebc5de29a1365e5dadff.tar.bz2 tangerine-wallet-browser-1fbdce8916151df2b31eebc5de29a1365e5dadff.tar.lz tangerine-wallet-browser-1fbdce8916151df2b31eebc5de29a1365e5dadff.tar.xz tangerine-wallet-browser-1fbdce8916151df2b31eebc5de29a1365e5dadff.tar.zst tangerine-wallet-browser-1fbdce8916151df2b31eebc5de29a1365e5dadff.zip |
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.
Diffstat (limited to 'ui/app/components')
8 files changed, 239 insertions, 59 deletions
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 ( <div className="advanced-tab__gas-edit-row__input-wrapper"> <input className={classnames('advanced-tab__gas-edit-row__input', { - 'advanced-tab__gas-edit-row__input--error': insufficientBalance, + 'advanced-tab__gas-edit-row__input--error': isInError && errorType === 'error', + 'advanced-tab__gas-edit-row__input--warning': isInError && errorType === 'warning', })} type="number" value={value} - min={min} onChange={event => onChange(Number(event.target.value))} /> <div className={classnames('advanced-tab__gas-edit-row__input-arrows', { - 'advanced-tab__gas-edit-row__input-arrows--error': insufficientBalance, + 'advanced-tab__gas-edit-row__input--error': isInError && errorType === 'error', + 'advanced-tab__gas-edit-row__input--warning': isInError && errorType === 'warning', })}> <div className="advanced-tab__gas-edit-row__input-arrows__i-wrap" onClick={() => onChange(value + 1)}><i className="fa fa-sm fa-angle-up" /></div> <div className="advanced-tab__gas-edit-row__input-arrows__i-wrap" onClick={() => onChange(value - 1)}><i className="fa fa-sm fa-angle-down" /></div> </div> - {insufficientBalance && <div className="advanced-tab__gas-edit-row__insufficient-balance"> - Insufficient Balance - </div>} + { isInError + ? <div className={`advanced-tab__gas-edit-row__${errorType}-text`}> + { errorText } + </div> + : null } </div> ) } @@ -83,23 +120,45 @@ export default class AdvancedTabContent extends Component { ) } - renderGasEditRow (labelKey, ...gasInputArgs) { + renderGasEditRow (gasInputArgs) { return ( <div className="advanced-tab__gas-edit-row"> <div className="advanced-tab__gas-edit-row__label"> - { this.context.t(labelKey) } + { this.context.t(gasInputArgs.labelKey) } { this.infoButton(() => {}) } </div> - { this.gasInput(...gasInputArgs) } + { this.gasInput(gasInputArgs) } </div> ) } - renderGasEditRows (customGasPrice, updateCustomGasPrice, customGasLimit, updateCustomGasLimit, insufficientBalance) { + renderGasEditRows ({ + customGasPrice, + updateCustomGasPrice, + customGasLimit, + updateCustomGasLimit, + insufficientBalance, + customPriceIsSafe, + isSpeedUp, + }) { return ( <div className="advanced-tab__gas-edit-rows"> - { 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, + }) } </div> ) } @@ -115,19 +174,23 @@ export default class AdvancedTabContent extends Component { totalFee, gasChartProps, gasEstimatesLoading, + customPriceIsSafe, + isSpeedUp, } = this.props return ( <div className="advanced-tab"> { this.renderDataSummary(totalFee, timeRemaining) } <div className="advanced-tab__fee-chart"> - { this.renderGasEditRows( - customGasPrice, - updateCustomGasPrice, - customGasLimit, - updateCustomGasLimit, - insufficientBalance - ) } + { this.renderGasEditRows({ + customGasPrice, + updateCustomGasPrice, + customGasLimit, + updateCustomGasLimit, + insufficientBalance, + customPriceIsSafe, + isSpeedUp, + }) } <div className="advanced-tab__fee-chart__title">Live Gas Price Predictions</div> {!gasEstimatesLoading ? <GasPriceChart {...gasChartProps} updateCustomGasPrice={updateCustomGasPrice} /> 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, diff --git a/ui/app/components/transaction-list-item/transaction-list-item.container.js b/ui/app/components/transaction-list-item/transaction-list-item.container.js index 73d9d8250..e08d3232f 100644 --- a/ui/app/components/transaction-list-item/transaction-list-item.container.js +++ b/ui/app/components/transaction-list-item/transaction-list-item.container.js @@ -11,7 +11,7 @@ import { formatDate } from '../../util' import { fetchBasicGasAndTimeEstimates, fetchGasEstimates, - setCustomGasPrice, + setCustomGasPriceForRetry, setCustomGasLimit, } from '../../ducks/gas.duck' @@ -21,7 +21,7 @@ const mapDispatchToProps = dispatch => { fetchGasEstimates: (blockTime) => dispatch(fetchGasEstimates(blockTime)), setSelectedToken: tokenAddress => dispatch(setSelectedToken(tokenAddress)), retryTransaction: (transaction, gasPrice) => { - dispatch(setCustomGasPrice(gasPrice || transaction.txParams.gasPrice)) + dispatch(setCustomGasPriceForRetry(gasPrice || transaction.txParams.gasPrice)) dispatch(setCustomGasLimit(transaction.txParams.gas)) dispatch(showSidebar({ transitionName: 'sidebar-left', |