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

Update account options menu design #8654

Merged
merged 1 commit into from
May 27, 2020

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented May 26, 2020

The AccountDetailsDropdown component has been rewritten to use the new Menu component, and to follow the latest designs.

This should be functionally equivalent. A couple of the icons have changed, but that's about it.

Support for a subtitle was added to MenuItem to support the origin subtitle used for the explorer link for custom RPC endpoints.

A few adjustments were required to test/helper.js to accommodate the use of Menu from a JSDOM context (this is the first time it's been used in a unit test). A popover-content element was added to the fake DOM, and another global was added that react-popper used internally.

An additional driver method (clickPoint) was added to the e2e driver to allow clicking the background behind the menu to dismiss it. This wasn't possible using the clickElement method, because that method would refuse to click an obscured element. The only non-obscured
element to click was the menu backdrop, and that didn't work either because the center was obscured by the menu (Selenium clicks the center of whichever element is targeted).

@Gudahtt
Copy link
Member Author

Gudahtt commented May 26, 2020

This was done now because a separate change I was working on locally relating to the asset page resulted in the position of the account options menu being totally broken. Instead of fixing that issue, I figured we'd might as well replace the whole thing.

@Gudahtt Gudahtt force-pushed the update-account-options-menu-design branch from 61e5601 to c89ffaa Compare May 26, 2020 22:52
@Gudahtt
Copy link
Member Author

Gudahtt commented May 26, 2020

Here are the new designs I was following: https://www.figma.com/file/b8U583mpXW1FsPWfGFNT8C/Wallet-home-(popup-%2B-fullscreen)?node-id=217%3A104

Here are screenshots of the new account options menu:

Typical case:

new-options

With a custom RPC endpoint and while an imported account is selected:

new-options-custom-rpc

@Gudahtt Gudahtt force-pushed the update-account-options-menu-design branch from c89ffaa to f0a3f52 Compare May 26, 2020 23:57
@metamaskbot
Copy link
Collaborator

Builds ready [f0a3f52]
Page Load Metrics (599 ± 50 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint3110541189
domContentLoaded34569659810450
load34669759910450
domInteractive34469559710450

@Gudahtt Gudahtt force-pushed the update-account-options-menu-design branch from f0a3f52 to 6557f5e Compare May 27, 2020 13:39
The `AccountDetailsDropdown` component has been rewritten to use the
new `Menu` component, and to follow the latest designs.

This should be functionally equivalent. A couple of the icons have
changed, but that's about it.

Support for a subtitle was added to `MenuItem` to support the `origin`
subtitle used for the explorer link for custom RPC endpoints.

A few adjustments were required to `test/helper.js` to accommodate
the use of `Menu` from a JSDOM context (this is the first time it's
been used in a unit test). A `popover-content` element was added to the
fake DOM, and another global was added that `react-popper` used
internally.

An additional driver method (`clickPoint`) was added to the e2e driver
to allow clicking the background behind the menu to dismiss it. This
wasn't possible using the `clickElement` method, because that method
would refuse to click an obscured element. The only non-obscured
element to click was the menu backdrop, and that didn't work either
because the center was obscured by the menu (Selenium clicks the center
of whichever element is targeted).
@Gudahtt Gudahtt force-pushed the update-account-options-menu-design branch from 6557f5e to 77d6f9e Compare May 27, 2020 13:40
@metamaskbot
Copy link
Collaborator

Builds ready [77d6f9e]
Page Load Metrics (657 ± 51 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint308043157
domContentLoaded35880565510651
load35980765710651
domInteractive35880565510651

@Gudahtt Gudahtt marked this pull request as ready for review May 27, 2020 14:06
@Gudahtt Gudahtt requested a review from a team as a code owner May 27, 2020 14:06
Copy link
Contributor

@brad-decker brad-decker left a comment

Choose a reason for hiding this comment

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

image
:shipit:

Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

LGTM!

@Gudahtt Gudahtt merged commit a6f2156 into develop May 27, 2020
@Gudahtt Gudahtt deleted the update-account-options-menu-design branch May 27, 2020 15:31
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