Implement asset page #8696

Gudahtt · 2020-05-29T02:44:21Z

A new page has been created for viewing assets. This replaces the old selectedToken state, which previously would augment the home page to show token-specific information.

The new asset page shows the standard token overview as seen previously on the home page, plus a history filtered to show just transactions relevant to that token.

The actions that were available in the old token list menu have been moved to a "Token Options" menu that mirrors the "Account Options" menu.

The selectedTokenAddress state has been removed, as it is no longer being used for anything.

A new Redux store has been added to track state related to browser history. The most recent "overview" page (i.e. the home page or the asset page) is currently being tracked, so that actions taken from the asset page can return the user back to the asset page when the action has finished.

Gudahtt · 2020-05-29T03:38:45Z

~~This depends upon #8695~~ #8695 has been merged.

metamaskbot · 2020-05-30T00:07:27Z

Builds ready [cc2720c]

builds: chrome, firefox, opera
bundle viz: background, ui, inpage, contentscript, ui-libs, bg-libs, phishing-detect
dep viz: background
all artifacts

Page Load Metrics (602 ± 29 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	29	38	33	3	1
		domContentLoaded	340	633	601	61	29
		load	342	635	602	61	29
		domInteractive	340	633	600	61	29

Gudahtt · 2020-05-30T00:13:37Z

Screenshots:

Home (popup):

Asset page - Eth (popup):

Asset page - TST token (popup):

Asset page - TST token with menu open (popup):

Home (fullscreen):

Asset page - Eth (fullscreen):

Asset page - TST token with menu open (fullscreen):

rekmarks · 2020-05-30T00:52:12Z

Fantastic!

This will take some time to review of course, but one initial piece of feedback:

Were there any designs for the asset rows being highlighted on hover, with a link/pointer cursor? I imagine we want at least the pointer cursor, but a subtle highlight would be best, IMO.

Gudahtt · 2020-05-30T01:06:01Z

Yes, good point! The hover state was in the design and is missing. Cursor pointer seems appropriate as well. I'll add those

Gudahtt · 2020-05-30T03:15:43Z

I've updated the asset list items to have a grey background on hover, cursor: pointer, and a border similar to the transaction list items.

These things are still TODO:

Add the inline "Send [token]" button on the token list items, but only on large viewports
Use the ListItem component for both lists (some minor refactoring is required first here), and update the styles to get more consistent heights, spacing, etc. across both lists
Update tab styles to match design more closely
Expand the "warning" state to show the entire warning inline on large viewports

None of these seem trivial, nor do they seem like blockers, so I'd prefer to deal with them in a future PR if possible to avoid blocking the RC, and to keep this monstrously sized PR from getting any larger

metamaskbot · 2020-05-30T03:25:32Z

Builds ready [7861a2a]

builds: chrome, firefox, opera
bundle viz: background, ui, inpage, contentscript, ui-libs, bg-libs, phishing-detect
dep viz: background
all artifacts

Page Load Metrics (521 ± 58 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	29	79	35	10	5
		domContentLoaded	328	635	520	121	58
		load	329	636	521	121	58
		domInteractive	327	635	519	121	58

ui/app/ducks/metamask/metamask.js

ui/app/pages/asset/asset.js

brad-decker · 2020-06-01T15:44:46Z

Q: Should clicking on ETH show only ETH transactions? I know in the previous version of the UI clicking ETH was just like 'unselecting' a token, but with the new UI and the header "Account 1 / ETH" I'm not sure I'd expect token transactions to show up. -- Not at all a blocker for merge just curious.

Gudahtt · 2020-06-01T15:52:02Z

Q: Should clicking on ETH show only ETH transactions?

Yes, that was the intention for now, since all transactions use Eth for gas. It seemed strange to allow the eth balance to change without showing why it changed in the transaction history.

Maybe we should filter out signature requests in that view though? 🤔 🤷‍♂️ I hadn't thought of that, since they're only shown when pending.

brad-decker · 2020-06-01T15:55:00Z

Perhaps. Maybe, in a later iteration, we can highlight token transfers on the eth screen in some way that makes it evident that they are token transfers, but that indicates the gas prices paid in eth.

Gudahtt · 2020-06-01T15:56:57Z

Yeah, perhaps by showing the gas cost instead of the token balance that was transferred. I was just thinking that too. Not sure how to make clear that it's the gas price we're showing though, and not the transfer amount converted to Eth.

brad-decker

LGTM

ui/app/pages/asset/asset.scss

A new page has been created for viewing assets. This replaces the old `selectedToken` state, which previously would augment the home page to show token-specific information. The new asset page shows the standard token overview as seen previously on the home page, plus a history filtered to show just transactions relevant to that token. The actions that were available in the old token list menu have been moved to a "Token Options" menu that mirrors the "Account Options" menu. The `selectedTokenAddress` state has been removed, as it is no longer being used for anything. `getMetaMetricState` has been renamed to `getBackgroundMetaMetricState` because its sole purpose is extracting data from the background state to send metrics from the background. It's not really a selector, but it was convenient for it to use the same selectors the UI uses to extract background data, so I left it there for now.

The `history` store tracks state related to browser history. The most recent "overview" page (i.e. the home page or the asset page) is currently being tracked, so that actions taken from the asset page can return the user back to the asset page when the action has finished.

This action also handles clearing other `send` state relating to tokens when switching back to the native currency.

The asset list items now have borders, a grey background on hover, and a pointer cursor.

The `.asset__container` element doesn't need to set `overflow-y` or any scrollbar-related rules because that element doesn't have a defined `height` or `max-height`; it can grow as long as it wants. There can be no overflow, and no scrollbar.

brad-decker

LGTM

rekmarks · 2020-06-01T16:29:08Z

Why do we use the history state/ducks (i.e. mostRecentOverviewPage) instead of history.goBack()?

Gudahtt · 2020-06-01T16:43:52Z

history.goBack doesn't work because we use the HashRouter, which doesn't keep any history.

rekmarks

Great work!

metamaskbot · 2020-06-01T16:59:30Z

Builds ready [d07d54b]

builds: chrome, firefox, opera
bundle viz: background, ui, inpage, contentscript, ui-libs, bg-libs, phishing-detect
dep viz: background
all artifacts

Page Load Metrics (834 ± 81 ms)

Platform	Page	Metric	Min (ms)	Max (ms)	Average (ms)	StandardDeviation (ms)	MarginOfError (ms)
Chrome	Home	firstPaint	NaN	NaN	NaN	NaN	NaN
		domContentLoaded	484	1300	832	168	81
		load	486	1311	834	169	81
		domInteractive	484	1300	831	168	81

brad-decker · 2020-06-01T17:12:49Z

* origin/develop: (689 commits) Implement asset page (#8696) fix crash on signature request (#8709) Fix accounts menu styling (#8707) Delete docs/porting_to_new_environment.md (#8704) Remove unused `getToErrorObject` parameters (#8705) hide connected-status on metamask ext (#8703) Stop adding permissions middleware to trusted connections (#8701) Use `send` state for send flow token (#8695) do not display extension id in connection modal (#8699) Fix tab content disappearing during scrolling on macOS Firefox (#8702) close details when button is pressed (#8694) Refactor token selectors (#8671) Update eth_accounts permission description (#8693) Extract selected token from token input (#8692) Fix propType for Home defaultHomeActiveTabName (#8683) Fix create account form styling (#8689) Remove unused `getSelectedTokenAssetImage` selector (#8691) Remove `getTxParams` (#8676) do not show account mismatch alert on details (#8678) Fix connect hardware styling (#8680) ...

Gudahtt force-pushed the asset-page branch from 294238f to d728081 Compare May 29, 2020 02:57

Gudahtt force-pushed the use-send-token-in-send-flow branch from 2d3f2ea to 091e782 Compare May 29, 2020 03:07

Gudahtt force-pushed the asset-page branch 3 times, most recently from c814567 to aede31f Compare May 29, 2020 03:37

Gudahtt force-pushed the asset-page branch 3 times, most recently from 64cb6fc to aa4b0c5 Compare May 29, 2020 15:01

Base automatically changed from use-send-token-in-send-flow to develop May 29, 2020 17:46

Gudahtt force-pushed the asset-page branch 7 times, most recently from 05bfe32 to cc2720c Compare May 29, 2020 23:36

Gudahtt marked this pull request as ready for review May 29, 2020 23:51

Gudahtt requested review from kumavis and a team as code owners May 29, 2020 23:51

rekmarks reviewed Jun 1, 2020

View reviewed changes

ui/app/ducks/metamask/metamask.js Show resolved Hide resolved

rekmarks reviewed Jun 1, 2020

View reviewed changes

ui/app/pages/asset/asset.js Show resolved Hide resolved

brad-decker previously approved these changes Jun 1, 2020

View reviewed changes

rekmarks reviewed Jun 1, 2020

View reviewed changes

ui/app/pages/asset/asset.scss Outdated Show resolved Hide resolved

Gudahtt added 5 commits June 1, 2020 13:24

Create separate action for changing send token

7e5f54e

This action also handles clearing other `send` state relating to tokens when switching back to the native currency.

Update asset list item styles

f1aabd3

The asset list items now have borders, a grey background on hover, and a pointer cursor.

Remove unnecessary CSS rules

d07d54b

The `.asset__container` element doesn't need to set `overflow-y` or any scrollbar-related rules because that element doesn't have a defined `height` or `max-height`; it can grow as long as it wants. There can be no overflow, and no scrollbar.

Gudahtt dismissed brad-decker’s stale review via d07d54b June 1, 2020 16:24

Gudahtt force-pushed the asset-page branch from 7861a2a to d07d54b Compare June 1, 2020 16:24

brad-decker approved these changes Jun 1, 2020

View reviewed changes

rekmarks approved these changes Jun 1, 2020

View reviewed changes

Gudahtt merged commit df85ab6 into develop Jun 1, 2020

Gudahtt deleted the asset-page branch June 1, 2020 17:54

brad-decker mentioned this pull request Jul 27, 2020

Refactor the wallet view eth/token list #4472

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement asset page #8696

Implement asset page #8696

Gudahtt commented May 29, 2020 •

edited

Loading

Gudahtt commented May 29, 2020 •

edited

Loading

metamaskbot commented May 30, 2020

Gudahtt commented May 30, 2020

rekmarks commented May 30, 2020 •

edited

Loading

Gudahtt commented May 30, 2020

Gudahtt commented May 30, 2020 •

edited

Loading

metamaskbot commented May 30, 2020

brad-decker commented Jun 1, 2020

Gudahtt commented Jun 1, 2020

brad-decker commented Jun 1, 2020

Gudahtt commented Jun 1, 2020

brad-decker left a comment

brad-decker left a comment

rekmarks commented Jun 1, 2020 •

edited

Loading

Gudahtt commented Jun 1, 2020

rekmarks left a comment

metamaskbot commented Jun 1, 2020

brad-decker commented Jun 1, 2020

Implement asset page #8696

Implement asset page #8696

Conversation

Gudahtt commented May 29, 2020 • edited Loading

Gudahtt commented May 29, 2020 • edited Loading

metamaskbot commented May 30, 2020

Gudahtt commented May 30, 2020

rekmarks commented May 30, 2020 • edited Loading

Gudahtt commented May 30, 2020

Gudahtt commented May 30, 2020 • edited Loading

metamaskbot commented May 30, 2020

brad-decker commented Jun 1, 2020

Gudahtt commented Jun 1, 2020

brad-decker commented Jun 1, 2020

Gudahtt commented Jun 1, 2020

brad-decker left a comment

Choose a reason for hiding this comment

brad-decker left a comment

Choose a reason for hiding this comment

rekmarks commented Jun 1, 2020 • edited Loading

Gudahtt commented Jun 1, 2020

rekmarks left a comment

Choose a reason for hiding this comment

metamaskbot commented Jun 1, 2020

brad-decker commented Jun 1, 2020

Gudahtt commented May 29, 2020 •

edited

Loading

Gudahtt commented May 29, 2020 •

edited

Loading

rekmarks commented May 30, 2020 •

edited

Loading

Gudahtt commented May 30, 2020 •

edited

Loading

rekmarks commented Jun 1, 2020 •

edited

Loading