Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve ux for low gas price set #5862

Merged
merged 4 commits into from
Dec 10, 2018
Merged

Conversation

danjm
Copy link
Contributor

@danjm danjm commented Nov 30, 2018

Fixes #5782 according to the plan described here #5786 (comment)

if the original tx price was greater than 0, continue to force retries to be priced at 110% of the original tx price
if a user is ever setting the gas price to below our recommended minimum, we should show a warning
if a user is retrying a transaction that had a gas price of 0, then we:

  • autofill gas price with our recommended gas price
  • still allow the user to set it as low as they wish
  • prevent them from sending a retry transaction with 0 gas price, and show an error message in that case

Below demonstrates the new behaviours for a user trying to speed up a transaction with a 0 gas price:

peek 2018-11-30 12-35

@danjm danjm force-pushed the improve-ux-for-low-gas-price-set branch from 1552b56 to ba02b4a Compare December 3, 2018 19:09
@danjm danjm force-pushed the new-gas-customize-feature-branch-d branch 2 times, most recently from 92c2e92 to 8194309 Compare December 4, 2018 03:36
@danjm danjm changed the base branch from new-gas-customize-feature-branch-d to develop December 5, 2018 05:34
@danjm danjm force-pushed the improve-ux-for-low-gas-price-set branch 3 times, most recently from 98ca699 to b8643a9 Compare December 10, 2018 21:12
@@ -167,7 +169,7 @@ const mapDispatchToProps = dispatch => {
}

const mergeProps = (stateProps, dispatchProps, ownProps) => {
const { gasPriceButtonGroupProps, isConfirm, isSpeedUp, txId } = stateProps
const { gasPriceButtonGroupProps, isConfirm, txId } = stateProps
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was isSpeedUp pulled out of here and used as stateProps.isSpeedUp?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know...

alextsg
alextsg previously approved these changes Dec 10, 2018
@danjm danjm force-pushed the improve-ux-for-low-gas-price-set branch from b8643a9 to f6640c5 Compare December 10, 2018 21:30
@metamaskbot
Copy link
Collaborator

Builds ready [b8643a9]: mascara, chrome, firefox, edge, opera

@metamaskbot
Copy link
Collaborator

Builds ready [f6640c5]: mascara, chrome, firefox, edge, opera

@danfinlay danfinlay merged commit 1fbdce8 into develop Dec 10, 2018
@whymarrh whymarrh deleted the improve-ux-for-low-gas-price-set branch December 10, 2018 21:55
danfinlay pushed a commit that referenced this pull request Dec 11, 2018
* Adds new gas customization modal container (without content)

* Adds the content of the advanced tab - w/o chart or dynamic content - to gas customize modal.

* Use correct message key in gas-modal-page-container.component.js

* Use BEM for css in gas-modal-page-container

* Split advanced-tab-content.component.js  render() method into smaller pieces; add translations to the same file.

* Remove gas slider from advance-tab-content.component

* Add tests for advanced-tab-component.js and subcomponents.

* Improve styling of advanced-tab-content gasInput row

* Adds basic tab content to gas customizer, with styled button group (static, for now).

* Connect the gas-button-group component to redux and a live api.

* Improvements to propdefaults in button-group.component and basic-tab-content.component

* Styling fixes for gas customization advanced tab content.

* Adds gas-duck.test.js tests.

* Connects remained of the gas customization component to redux.

* Integrate gas buttons with the send screen.

* Test updates and additions for button integration with send screen.

* Adds redesign for the customize gas advanced tab.

* Adds not yet functional gas price chart.

* Gas price chart improvements, redesign, bug fixes, and set up to receive external data

* Read only connection of gas price chart to redux

* Clean up for advanced gas tab customization changes.

* Complete integration of gas chart with redux.

* Add control arrows to advanced gas tab inputs.

* Lint and unit test fixes.

* Clean up gas chart code.

* Update tests, plus some lint fixes, for gas-price-chart

* Improve data management and tests for gas-modal-page-container price estimates.

* Clean up for mmui-i11-custom-gas-price-chart branch

* Redesign of gas customization basic tab.

* Adds createSpeedUpTransaction to txController

* Connect gas price chart to gas station api.

* Adds speed up slide-in gas customization sidebar

* Update e2e tests for new gas customization modal.

* Fixes for components that break e2e gas customization tests, plus unit test updates.

* Remove gas customization integration tests (in favour of e2e tests)

* Add gas data to integration test json data set.

* Add c3 and d3 to the separate dependencies bundle.

* Make gas customization modal responsive.

* Fix "fastest" translation message; change to sentence case

* Uses more reliable api on main send screen; caches basic api results in modal

* Add loading spinners when waiting for APIs in the gas customization modal

* Modify results of API data to better fit gas chart: remove outliers, pad data

* Clear custom gas data on hiding of gas customization modal.

* Improve responsiveness of customize speed up slider.

* Final gas customization fixes

* Fix styling of send screen in extension view when hex data on.

* Replace height: 100% rule with workaround for flexbox quirks

* Fill in more Polish message translations

* Update lockfile to fix errors

npm has informed me that the lockfile has "errors":

    npm ERR! code ELOCKVERIFY
    npm ERR! Errors were found in your package-lock.json, run  npm install  to fix them.
    npm ERR!     Missing: c3@^0.6.7
    npm ERR!     Invalid: lock file's [email protected] does not satisfy d3@^5.7.0

* circleci: Disable npm audit when installing packages

Auditing packages when installing here doesn't help anyone as the summary
isn't visible and vulnerabilities don't produce a non-zero exit code. We
will have `npm audit` as an extra CI job.

* npm audit fix

* circleci: Replace nsp with npm audit

Refs #4751

* Remove beefy dependency and its usages

Refs #4768
Refs #5389

This changeset removes the beefy package that:

1. Was last published 2 yrs ago
2. Brought with it 1 moderate and 1 critical vulnerability
3. Was only used in scripts that no longer work

* npm uninstall open

* Update ganache-core to mitigate vuln

                       === npm audit security report ===

> # Run  npm install --save-dev [email protected]  to resolve 1 vulnerability
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Moderate      │ Memory Exposure                                              │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ bl                                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ ganache-core [dev]                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ ganache-core > level-sublevel > levelup > bl                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/596                       │
└───────────────┴──────────────────────────────────────────────────────────────┘

* Deduplicate package.json file

From `npm install`:

> npm WARN The package css-loader is included as both a dev and production dependency.
> npm WARN The package eslint-plugin-react is included as both a dev and production dependency.
> npm WARN The package file-loader is included as both a dev and production dependency.
> npm WARN The package gulp is included as both a dev and production dependency.

It's also worth noting that the Gulp version we were using was inconsistent and there is
a published v4 release on GitHub.

* Fix race condition in network controller lookup() method.

* Group transactions by nonce (#5886)

* fix formatting of 32-byte strings in personal_sign (#5878)

* Bump json-rpc-engine to v4.0.0

* Bump package lock, mostly to https links

* 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.

* Show Failed transaction in the browser notification for on-chain failures (#5904)

* Changelog and version bump for 5.2.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants