From ce11fad81c951d41f169496b48843c64a76d4849 Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Mon, 18 May 2020 14:51:29 -0300
Subject: [PATCH] Improve account options menu (#8607)

The account options menu is now much faster, and it correctly closes
when 'Switch account' is selected.

A static width had to be set on the menu so that it could be positioned
reliably. Without this width set, it was rendered as a different size
before positioning than after, which resulted in it being positioned
incorrectly. A `z-index` had to be added (equal to the `z-index` used
by the popover component) to ensure it wasn't rendered beneath the
popover.

The menu is automatically positioned relative to the account options
button, appearing below the button by default but above it instead if
there isn't room below. It is positioned to be inside the bounds of the
popover as well.

The account options button is now a `<button>` rather than a `<i>`.
This required a few additional style rules to overrule the default
button styles. Additionally the size was increased so that it matches
the designs more closely.

The callbacks for connecting, disconnecting, and switching accounts
have been updated to use state and props to determine the correct
address to use, rather than being bound to the correct address
parameter in the render function. This means we aren't creating a new
function upon each render anymore.

The `showAccountOptions` method still needs to be bound once per
account, but this was switched to use more readable syntax (`.bind`,
instead of the double arrow function).

`react-popper` and `@popperjs/core` were both added as dependencies.
These should be used for any UI requiring relative positioning (e.g.
tooltips, menus, etc.). Older versions of these libraries are already
in our codebase as transitive dependencies of the tooltip library we're
using.
---
 package.json                                  |  2 +
 ...nnected-accounts-list-options.component.js | 72 +++++++++++--------
 .../connected-accounts-list.component.js      | 39 +++++++---
 .../app/connected-accounts-list/index.scss    | 21 +++++-
 yarn.lock                                     | 18 +++++
 5 files changed, 110 insertions(+), 42 deletions(-)

diff --git a/package.json b/package.json
index a932ea7fc..fe530d16b 100644
--- a/package.json
+++ b/package.json
@@ -78,6 +78,7 @@
     "@metamask/eth-token-tracker": "^2.0.0",
     "@metamask/etherscan-link": "^1.1.0",
     "@metamask/inpage-provider": "^5.0.0",
+    "@popperjs/core": "^2.4.0",
     "@reduxjs/toolkit": "^1.3.2",
     "@sentry/browser": "^5.11.1",
     "@sentry/integrations": "^5.11.1",
@@ -154,6 +155,7 @@
     "react-idle-timer": "^4.2.5",
     "react-inspector": "^2.3.0",
     "react-media": "^1.8.0",
+    "react-popper": "^2.2.3",
     "react-redux": "^7.2.0",
     "react-router-dom": "^5.1.2",
     "react-select": "^1.0.0",
diff --git a/ui/app/components/app/connected-accounts-list/connected-accounts-list-options/connected-accounts-list-options.component.js b/ui/app/components/app/connected-accounts-list/connected-accounts-list-options/connected-accounts-list-options.component.js
index e1eb29def..de7b5411c 100644
--- a/ui/app/components/app/connected-accounts-list/connected-accounts-list-options/connected-accounts-list-options.component.js
+++ b/ui/app/components/app/connected-accounts-list/connected-accounts-list-options/connected-accounts-list-options.component.js
@@ -1,34 +1,46 @@
 import PropTypes from 'prop-types'
-import React, { PureComponent } from 'react'
-import { Tooltip } from 'react-tippy'
+import React, { useRef, useState } from 'react'
+import { createPortal } from 'react-dom'
+import { usePopper } from 'react-popper'
 
-export default class ConnectedAccountsListOptions extends PureComponent {
-  static propTypes = {
-    children: PropTypes.node.isRequired,
-  }
+const ConnectedAccountsListOptions = ({ children, onShowOptions, onHideOptions, show }) => {
+  const [optionsButtonElement, setOptionsButtonElement] = useState(null)
+  const [popperElement, setPopperElement] = useState(null)
+  const popoverContainerElement = useRef(document.getElementById('popover-content'))
 
-  render () {
-    return (
-      <Tooltip
-        arrow={false}
-        animation="none"
-        animateFill={false}
-        transitionFlip={false}
-        hideDuration={0}
-        duration={0}
-        trigger="click"
-        interactive
-        theme="none"
-        position="bottom-end"
-        unmountHTMLWhenHide
-        html={(
-          <div className="connected-accounts-options">
-            {this.props.children}
-          </div>
-        )}
-      >
-        <i className="fas fa-ellipsis-v" />
-      </Tooltip>
-    )
-  }
+  const { attributes, styles } = usePopper(optionsButtonElement, popperElement, {
+    modifiers: [{ name: 'preventOverflow', options: { altBoundary: true } }],
+  })
+  return (
+    <>
+      <button className="fas fa-ellipsis-v connected-accounts-options__button" onClick={onShowOptions} ref={setOptionsButtonElement} />
+      {
+        show
+          ? createPortal(
+            <>
+              <div className="connected-accounts-options__background" onClick={onHideOptions} />
+              <div
+                className="connected-accounts-options"
+                ref={setPopperElement}
+                style={styles.popper}
+                {...attributes.popper}
+              >
+                { children }
+              </div>
+            </>,
+            popoverContainerElement.current
+          )
+          : null
+      }
+    </>
+  )
 }
+
+ConnectedAccountsListOptions.propTypes = {
+  children: PropTypes.node.isRequired,
+  onHideOptions: PropTypes.func.isRequired,
+  onShowOptions: PropTypes.func.isRequired,
+  show: PropTypes.bool.isRequired,
+}
+
+export default ConnectedAccountsListOptions
diff --git a/ui/app/components/app/connected-accounts-list/connected-accounts-list.component.js b/ui/app/components/app/connected-accounts-list/connected-accounts-list.component.js
index 50bb32698..58fe3d74a 100644
--- a/ui/app/components/app/connected-accounts-list/connected-accounts-list.component.js
+++ b/ui/app/components/app/connected-accounts-list/connected-accounts-list.component.js
@@ -36,16 +36,30 @@ export default class ConnectedAccountsList extends PureComponent {
     setSelectedAddress: PropTypes.func.isRequired,
   }
 
-  connectAccount = (address) => () => {
-    this.props.addPermittedAccount(address)
+  state = {
+    accountWithOptionsShown: null,
   }
 
-  disconnectAccount = (address) => () => {
-    this.props.removePermittedAccount(address)
+  connectAccount = () => {
+    this.props.addPermittedAccount(this.props.accountToConnect?.address)
   }
 
-  switchAccount = (address) => () => {
-    this.props.setSelectedAddress(address)
+  disconnectAccount = () => {
+    this.hideAccountOptions()
+    this.props.removePermittedAccount(this.state.accountWithOptionsShown)
+  }
+
+  switchAccount = () => {
+    this.hideAccountOptions()
+    this.props.setSelectedAddress(this.state.accountWithOptionsShown)
+  }
+
+  hideAccountOptions = () => {
+    this.setState({ accountWithOptionsShown: null })
+  }
+
+  showAccountOptions = (address) => {
+    this.setState({ accountWithOptionsShown: address })
   }
 
   renderUnconnectedAccount () {
@@ -66,7 +80,7 @@ export default class ConnectedAccountsList extends PureComponent {
           <>
             {t('statusNotConnected')}
             &nbsp;&middot;&nbsp;
-            <a className="connected-accounts-list__account-status-link" onClick={this.connectAccount(address)}>
+            <a className="connected-accounts-list__account-status-link" onClick={this.connectAccount}>
               {t('connect')}
             </a>
           </>
@@ -77,6 +91,7 @@ export default class ConnectedAccountsList extends PureComponent {
 
   render () {
     const { connectedAccounts, permissions, selectedAddress } = this.props
+    const { accountWithOptionsShown } = this.state
     const { t } = this.context
 
     return (
@@ -90,12 +105,16 @@ export default class ConnectedAccountsList extends PureComponent {
               name={`${name} (…${address.substr(-4, 4)})`}
               status={index === 0 ? t('primary') : `${t('lastActive')}: ${DateTime.fromMillis(lastActive).toISODate()}`}
               options={(
-                <ConnectedAccountsListOptions>
+                <ConnectedAccountsListOptions
+                  onHideOptions={this.hideAccountOptions}
+                  onShowOptions={this.showAccountOptions.bind(null, address)}
+                  show={accountWithOptionsShown === address}
+                >
                   {
                     address === selectedAddress ? null : (
                       <ConnectedAccountsListOptionsItem
                         iconClassNames="fas fa-random"
-                        onClick={this.switchAccount(address)}
+                        onClick={this.switchAccount}
                       >
                         {t('switchToThisAccount')}
                       </ConnectedAccountsListOptionsItem>
@@ -103,7 +122,7 @@ export default class ConnectedAccountsList extends PureComponent {
                   }
                   <ConnectedAccountsListOptionsItem
                     iconClassNames="fas fa-ban"
-                    onClick={this.disconnectAccount(address)}
+                    onClick={this.disconnectAccount}
                   >
                     {t('disconnectThisAccount')}
                   </ConnectedAccountsListOptionsItem>
diff --git a/ui/app/components/app/connected-accounts-list/index.scss b/ui/app/components/app/connected-accounts-list/index.scss
index 3c9f78313..249802a3e 100644
--- a/ui/app/components/app/connected-accounts-list/index.scss
+++ b/ui/app/components/app/connected-accounts-list/index.scss
@@ -66,7 +66,7 @@
   background: $white;
   box-shadow: 0 2px 10px rgba(0, 0, 0, 0.214);
   border-radius: 8px;
-  min-width: 200px;
+  width: 225px;
   color: $Black-100;
   display: flex;
   flex-direction: column;
@@ -79,6 +79,8 @@
   font-weight: normal;
   line-height: 20px;
 
+  z-index: 1050;
+
   &__row {
     background: none;
     font-family: inherit;
@@ -95,6 +97,21 @@
       margin-right: 8px;
     }
   }
+
+  &__button {
+    background: inherit;
+    color: $Grey-500;
+    font-size: 22px;
+  }
+
+  &__background {
+    position: absolute;
+    top: 0;
+    left: 0;
+    width: 100vw;
+    height: 100vh;
+    z-index: 1050;
+  }
 }
 
 .connected-accounts-permissions {
@@ -104,7 +121,7 @@
 
   font-size: 12px;
   line-height: 17px;
-  color: #6A737D;
+  color: $Grey-500;
 
   strong {
     font-weight: bold;
diff --git a/yarn.lock b/yarn.lock
index e54850f2e..24217b8ea 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -2007,6 +2007,11 @@
     "@nodelib/fs.scandir" "2.1.3"
     fastq "^1.6.0"
 
+"@popperjs/core@^2.4.0":
+  version "2.4.0"
+  resolved "https://registry.yarnpkg.com/@popperjs/core/-/core-2.4.0.tgz#0e1bdf8d021e7ea58affade33d9d607e11365915"
+  integrity sha512-NMrDy6EWh9TPdSRiHmHH2ye1v5U0gBD7pRYwSwJvomx7Bm4GG04vu63dYiVzebLOx2obPpJugew06xVP0Nk7hA==
+
 "@protobufjs/utf8@^1.1.0":
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/@protobufjs/utf8/-/utf8-1.1.0.tgz#a777360b5b39a1a2e5106f8e858f2fd2d060c570"
@@ -23327,6 +23332,11 @@ react-fast-compare@2.0.4:
   resolved "https://registry.yarnpkg.com/react-fast-compare/-/react-fast-compare-2.0.4.tgz#e84b4d455b0fec113e0402c329352715196f81f9"
   integrity sha512-suNP+J1VU1MWFKcyt7RtjiSWUjvidmQSlqu+eHslq+342xCbGTYmC0mEhPCOHxlW0CywylOC1u2DFAT+bv4dBw==
 
+react-fast-compare@^3.0.1:
+  version "3.1.1"
+  resolved "https://registry.yarnpkg.com/react-fast-compare/-/react-fast-compare-3.1.1.tgz#0becf31e3812fa70dc231e259f40d892d4767900"
+  integrity sha512-SCsAORWK59BvauR2L1BTdjQbJcSGJJz03U0awektk2hshLKrITDDFTlgGCqIZpTDlPC/NFlZee6xTMzXPVLiHw==
+
 react-focus-lock@^2.1.0:
   version "2.2.1"
   resolved "https://registry.yarnpkg.com/react-focus-lock/-/react-focus-lock-2.2.1.tgz#1d12887416925dc53481914b7cedd39494a3b24a"
@@ -23466,6 +23476,14 @@ react-popper@^1.3.3:
     typed-styles "^0.0.7"
     warning "^4.0.2"
 
+react-popper@^2.2.3:
+  version "2.2.3"
+  resolved "https://registry.yarnpkg.com/react-popper/-/react-popper-2.2.3.tgz#33d425fa6975d4bd54d9acd64897a89d904b9d97"
+  integrity sha512-mOEiMNT1249js0jJvkrOjyHsGvqcJd3aGW/agkiMoZk3bZ1fXN1wQszIQSjHIai48fE67+zwF8Cs+C4fWqlfjw==
+  dependencies:
+    react-fast-compare "^3.0.1"
+    warning "^4.0.2"
+
 react-redux@^7.2.0:
   version "7.2.0"
   resolved "https://registry.yarnpkg.com/react-redux/-/react-redux-7.2.0.tgz#f970f62192b3981642fec46fd0db18a074fe879d"