Group transactions by nonce #5886

alextsg · 2018-12-05T23:59:59Z

bdresser · 2018-12-06T00:23:28Z

yessss @alextsg this looks awesome!!!!

@cjeria re-looking at these - kinda seems like there are 4 transactions confirmed or 3 transactions failed when really we mean four attempts, and one finally confirmed. Maybe it'd make sense to take the count/number outside the status pill so they seem less linked? (sry in advance Alex, we can also do this as a follow-up if that's a pain.)

alextsg · 2018-12-06T01:10:33Z

@bdresser np, that won't be a huge change if we decide to do that.

bdresser · 2018-12-06T20:00:53Z

Notes from kyo sync:

Current Etherscan button pops open the most recent tx – how do we link to Etherscan for the previous transactions? We can use the tiny little up arrow, could even link the whole line.
Remove the transaction counter

cjeria · 2018-12-06T22:08:43Z

@alextsg @bdresser Here's an option for repositioning the attempts counter outside of the pill and adding a bit more copy for context. Thoughts?

alextsg · 2018-12-06T23:10:00Z

@cjeria I'm not too sure about the "No attempts made" option because we'd have a clickable link within a clickable container, and if that can be a confusing interaction for users. Also, I kind of see Cancel being grouped together with Speed Up in terms of functionality. Would it make sense to keep them together as they were before?

alextsg · 2018-12-07T05:36:14Z

Updated to remove the transaction counter and link each activity in the activity log to Etherscan.

cjeria · 2018-12-07T15:00:48Z

@alextsg

I'm not too sure about the "No attempts made" option because we'd have a clickable link within a clickable container

I agree. This was an attempt at surfacing the speed up button to address some complaints we've recently received from support tickets that some users don't see or know that they can speed up the transaction. Let's keep it as you have it for now and we'll iterate on this some more. Maybe @bahnju can take a pass at some other options for surfacing up the speed up label per tx?

Re: the clickable activity link to etherscan - is the blue highlighted activity in your screenshot a hover state? If so, I'd suggest we make the hover state solid black on hover and use a lighter grey (same color as activity icons #9B9B9B) for the default state of all activity lines and cursor:pointer to indicate it's also a link.

Also, can we lighten up the details container bg color to #FAFBFC like the designs?

Sorry for the nitpicks!

alextsg · 2018-12-07T18:12:50Z

@cjeria Yeah the blue was a hover state, and sure I'll make those color changes. I welcome the nitpicks!

alextsg · 2018-12-07T18:24:02Z

Updated:

whymarrh

A few small nitpicks. Some general feedback:

It might be good to document the new functions added to ui/app/selectors/transactions.js from now while it's still fresh
I skipped reading the CSS

...action-activity-log/transaction-activity-log-icon/transaction-activity-log-icon.component.js

ui/app/components/transaction-list-item/transaction-list-item.component.js

ui/app/selectors/transactions.js

alextsg · 2018-12-08T08:15:35Z

Updated PR

whymarrh · 2018-12-08T18:49:43Z

LGTM

* 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

This state hasn't been used since #5886. The nonce we display in our UI is now from the background, rather than queried directly from the front-end. This also means we save making this network call each time a pending transaction is added, and each time the transaction list is mounted.

alextsg assigned whymarrh and danjm Dec 5, 2018

alextsg requested review from danjm, frankiebee and whymarrh as code owners December 5, 2018 23:59

alextsg force-pushed the group-tx-by-nonce branch from 1861c81 to 557a82f Compare December 6, 2018 01:18

alextsg force-pushed the group-tx-by-nonce branch 2 times, most recently from 9a27ebf to d0ac065 Compare December 7, 2018 05:19

alextsg force-pushed the group-tx-by-nonce branch from d0ac065 to 87fb7fc Compare December 7, 2018 18:16

whymarrh suggested changes Dec 8, 2018

View reviewed changes

Group transactions by nonce

06baa05

alextsg force-pushed the group-tx-by-nonce branch from 87fb7fc to 06baa05 Compare December 8, 2018 07:59

whymarrh approved these changes Dec 8, 2018

View reviewed changes

alextsg merged commit d8ab9cc into MetaMask:develop Dec 9, 2018

alextsg deleted the group-tx-by-nonce branch December 9, 2018 20:48

Gudahtt mentioned this pull request Apr 23, 2020

Remove unused network nonce state #8395

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Group transactions by nonce #5886

Group transactions by nonce #5886

alextsg commented Dec 5, 2018

bdresser commented Dec 6, 2018

alextsg commented Dec 6, 2018

bdresser commented Dec 6, 2018

cjeria commented Dec 6, 2018

alextsg commented Dec 6, 2018

alextsg commented Dec 7, 2018

cjeria commented Dec 7, 2018 •

edited

Loading

alextsg commented Dec 7, 2018

alextsg commented Dec 7, 2018

whymarrh left a comment

alextsg commented Dec 8, 2018

whymarrh commented Dec 8, 2018

Group transactions by nonce #5886

Group transactions by nonce #5886

Conversation

alextsg commented Dec 5, 2018

bdresser commented Dec 6, 2018

alextsg commented Dec 6, 2018

bdresser commented Dec 6, 2018

cjeria commented Dec 6, 2018

alextsg commented Dec 6, 2018

alextsg commented Dec 7, 2018

cjeria commented Dec 7, 2018 • edited Loading

alextsg commented Dec 7, 2018

alextsg commented Dec 7, 2018

whymarrh left a comment

Choose a reason for hiding this comment

alextsg commented Dec 8, 2018

whymarrh commented Dec 8, 2018

cjeria commented Dec 7, 2018 •

edited

Loading