Feat/19274/ds popover update account list menu (#19534)

* update account list menu to use ds popover and fix accessibility issue

* close popover if user continues to tab to next items

* remove disable logic not doing anything

* add stylesheet

* add refs to track last menuitem

* cleaned up ref version for MenuItems

* fix test

* add click away option and fix test

* fix e2e test

* undo e2e test

* add account e2e

* fix click outside component

* remove additional line break

* remove commented out code

* add isOpen to story

* set width to 225px

test/e2e/tests/add-account.spec.js

@@ -216,7 +216,7 @@ describe('Add account', function () {
         );
 
         // Create 3rd account with private key
-        await driver.clickElement('.menu__background');
+        await driver.clickElement('.mm-text-field');
         await driver.clickElement({ text: 'Import account', tag: 'button' });
         await driver.fill('#private-key-box', testPrivateKey);
 

ui/components/multichain/account-list-item-menu/account-list-item-menu.js

@@ -1,4 +1,4 @@
-import React, { useContext } from 'react';
+import React, { useContext, useRef, useEffect } from 'react';
 import { useDispatch, useSelector } from 'react-redux';
 import { useHistory } from 'react-router-dom';
 import PropTypes from 'prop-types';
@@ -23,8 +23,15 @@ import { toChecksumHexAddress } from '../../../../shared/modules/hexstring-utils
 ///: END:ONLY_INCLUDE_IN
 import { findKeyringForAddress } from '../../../ducks/metamask/metamask';
 import { NETWORKS_ROUTE } from '../../../helpers/constants/routes';
-import { Menu, MenuItem } from '../../ui/menu';
+import { MenuItem } from '../../ui/menu';
-import { Text, IconName } from '../../component-library';
+import {
+  Text,
+  IconName,
+  Popover,
+  PopoverPosition,
+  ModalFocus,
+  PopoverRole,
+} from '../../component-library';
 import {
   MetaMetricsEventCategory,
   MetaMetricsEventLinkType,
@@ -42,6 +49,7 @@ export const AccountListItemMenu = ({
   closeMenu,
   isRemovable,
   identity,
+  isOpen,
 }) => {
   const t = useI18nContext();
   const trackEvent = useContext(MetaMetricsContext);
@@ -82,116 +90,178 @@ export const AccountListItemMenu = ({
   };
 
   ///: BEGIN:ONLY_INCLUDE_IN(build-mmi)
+  const isCustodial = keyring?.type ? /Custody/u.test(keyring.type) : false;
   const accounts = useSelector(getMetaMaskAccountsOrdered);
-  const isCustodial = /Custody/u.test(keyring.type);
+
   const mmiActions = mmiActionsFactory();
   ///: END:ONLY_INCLUDE_IN
 
-  return (
+  // Handle Tab key press for accessibility inside the popover and will close the popover on the last MenuItem
-    <Menu
+  const lastItemRef = useRef(null);
-      anchorElement={anchorElement}
+  const accountDetailsItemRef = useRef(null);
-      className="account-list-item-menu"
+  const removeAccountItemRef = useRef(null);
-      onHide={onClose}
+  const removeJWTItemRef = useRef(null);
-    >
-      <MenuItem
-        onClick={() => {
-          blockExplorerLinkText.firstPart === 'addBlockExplorer'
-            ? routeToAddBlockExplorerUrl()
-            : openBlockExplorer();
 
-          trackEvent({
+  // Checks the MenuItems from the bottom to top to set lastItemRef on the last MenuItem that is not disabled
-            event: MetaMetricsEventName.BlockExplorerLinkClicked,
+  useEffect(() => {
-            category: MetaMetricsEventCategory.Accounts,
+    if (removeJWTItemRef.current) {
-            properties: {
+      lastItemRef.current = removeJWTItemRef.current;
-              location: 'Account Options',
+    } else if (removeAccountItemRef.current) {
-              chain_id: chainId,
+      lastItemRef.current = removeAccountItemRef.current;
-            },
+    } else {
-          });
+      lastItemRef.current = accountDetailsItemRef.current;
-        }}
+    }
-        subtitle={blockExplorerUrlSubTitle || null}
+  }, [
-        iconName={IconName.Export}
+    removeJWTItemRef.current,
-        data-testid="account-list-menu-open-explorer"
+    removeAccountItemRef.current,
-      >
+    accountDetailsItemRef.current,
-        <Text variant={TextVariant.bodySm}>{t('viewOnExplorer')}</Text>
+  ]);
-      </MenuItem>
+
-      <MenuItem
+  const handleKeyDown = (event) => {
-        onClick={() => {
+    if (event.key === 'Tab' && event.target === lastItemRef.current) {
-          dispatch(setAccountDetailsAddress(identity.address));
+      // If Tab is pressed at the last item to close popover and focus to next element in DOM
-          trackEvent({
+      onClose();
-            event: MetaMetricsEventName.NavAccountDetailsOpened,
+    }
-            category: MetaMetricsEventCategory.Navigation,
+  };
-            properties: {
+
-              location: 'Account Options',
+  // Handle click outside of the popover to close it
-            },
+  const popoverDialogRef = useRef(null);
-          });
+
-          onClose();
+  const handleClickOutside = (event) => {
-          closeMenu?.();
+    if (
-        }}
+      popoverDialogRef?.current &&
-        iconName={IconName.ScanBarcode}
+      !popoverDialogRef.current.contains(event.target)
-        data-testid="account-list-menu-details"
+    ) {
-      >
+      onClose();
-        <Text variant={TextVariant.bodySm}>{t('accountDetails')}</Text>
+    }
-      </MenuItem>
+  };
-      {isRemovable ? (
+
-        <MenuItem
+  useEffect(() => {
-          data-testid="account-list-menu-remove"
+    document.addEventListener('mousedown', handleClickOutside);
-          onClick={() => {
+
-            dispatch(
+    return () => {
-              showModal({
+      document.removeEventListener('mousedown', handleClickOutside);
-                name: 'CONFIRM_REMOVE_ACCOUNT',
+    };
-                identity,
+  }, []);
-              }),
+
-            );
+  return (
-            trackEvent({
+    <Popover
-              event: MetaMetricsEventName.AccountRemoved,
+      className="multichain-account-list-item-menu__popover"
-              category: MetaMetricsEventCategory.Accounts,
+      referenceElement={anchorElement}
-              properties: {
+      role={PopoverRole.Dialog}
-                account_hardware_type: deviceName,
+      position={PopoverPosition.Bottom}
-                chain_id: chainId,
+      offset={[0, 0]}
-                account_type: accountType,
+      padding={0}
-              },
+      isOpen={isOpen}
-            });
+      isPortal
-            onClose();
+      preventOverflow
-            closeMenu?.();
+    >
-          }}
+      <ModalFocus restoreFocus initialFocusRef={anchorElement}>
-          iconName={IconName.Trash}
+        <div onKeyDown={handleKeyDown} ref={popoverDialogRef}>
-        >
-          <Text variant={TextVariant.bodySm}>{t('removeAccount')}</Text>
-        </MenuItem>
-      ) : null}
-      {
-        ///: BEGIN:ONLY_INCLUDE_IN(build-mmi)
-        isCustodial ? (
           <MenuItem
-            data-testid="account-options-menu__remove-jwt"
+            onClick={() => {
-            onClick={async () => {
+              blockExplorerLinkText.firstPart === 'addBlockExplorer'
-              const token = await dispatch(mmiActions.getCustodianToken());
+                ? routeToAddBlockExplorerUrl()
-              const custodyAccountDetails = await dispatch(
+                : openBlockExplorer();
-                mmiActions.getAllCustodianAccountsWithToken(
+
-                  keyring.type.split(' - ')[1],
+              trackEvent({
-                  token,
+                event: MetaMetricsEventName.BlockExplorerLinkClicked,
-                ),
+                category: MetaMetricsEventCategory.Accounts,
-              );
+                properties: {
-              dispatch(
+                  location: 'Account Options',
-                showModal({
+                  chain_id: chainId,
-                  name: 'CONFIRM_REMOVE_JWT',
+                },
-                  token,
+              });
-                  custodyAccountDetails,
+            }}
-                  accounts,
+            subtitle={blockExplorerUrlSubTitle || null}
-                  selectedAddress: toChecksumHexAddress(identity.address),
+            iconName={IconName.Export}
-                }),
+            data-testid="account-list-menu-open-explorer"
-              );
+          >
+            <Text variant={TextVariant.bodySm}>{t('viewOnExplorer')}</Text>
+          </MenuItem>
+          <MenuItem
+            ref={accountDetailsItemRef}
+            onClick={() => {
+              dispatch(setAccountDetailsAddress(identity.address));
+              trackEvent({
+                event: MetaMetricsEventName.NavAccountDetailsOpened,
+                category: MetaMetricsEventCategory.Navigation,
+                properties: {
+                  location: 'Account Options',
+                },
+              });
               onClose();
               closeMenu?.();
             }}
-            iconName={IconName.Trash}
+            iconName={IconName.ScanBarcode}
+            data-testid="account-list-menu-details"
           >
-            <Text variant={TextVariant.bodySm}>{t('removeJWT')}</Text>
+            <Text variant={TextVariant.bodySm}>{t('accountDetails')}</Text>
           </MenuItem>
-        ) : null
+          {isRemovable ? (
-        ///: END:ONLY_INCLUDE_IN
+            <MenuItem
-      }
+              ref={removeAccountItemRef}
-    </Menu>
+              data-testid="account-list-menu-remove"
+              onClick={() => {
+                dispatch(
+                  showModal({
+                    name: 'CONFIRM_REMOVE_ACCOUNT',
+                    identity,
+                  }),
+                );
+                trackEvent({
+                  event: MetaMetricsEventName.AccountRemoved,
+                  category: MetaMetricsEventCategory.Accounts,
+                  properties: {
+                    account_hardware_type: deviceName,
+                    chain_id: chainId,
+                    account_type: accountType,
+                  },
+                });
+                onClose();
+                closeMenu?.();
+              }}
+              iconName={IconName.Trash}
+            >
+              <Text variant={TextVariant.bodySm}>{t('removeAccount')}</Text>
+            </MenuItem>
+          ) : null}
+          {
+            ///: BEGIN:ONLY_INCLUDE_IN(build-mmi)
+            isCustodial ? (
+              <MenuItem
+                ref={removeJWTItemRef}
+                data-testid="account-options-menu__remove-jwt"
+                onClick={async () => {
+                  const token = await dispatch(mmiActions.getCustodianToken());
+                  const custodyAccountDetails = await dispatch(
+                    mmiActions.getAllCustodianAccountsWithToken(
+                      keyring.type.split(' - ')[1],
+                      token,
+                    ),
+                  );
+                  dispatch(
+                    showModal({
+                      name: 'CONFIRM_REMOVE_JWT',
+                      token,
+                      custodyAccountDetails,
+                      accounts,
+                      selectedAddress: toChecksumHexAddress(identity.address),
+                    }),
+                  );
+                  onClose();
+                  closeMenu?.();
+                }}
+                iconName={IconName.Trash}
+              >
+                <Text variant={TextVariant.bodySm}>{t('removeJWT')}</Text>
+              </MenuItem>
+            ) : null
+            ///: END:ONLY_INCLUDE_IN
+          }
+        </div>
+      </ModalFocus>
+    </Popover>
   );
 };
 
@@ -204,6 +274,12 @@ AccountListItemMenu.propTypes = {
    * Function that executes when the menu is closed
    */
   onClose: PropTypes.func.isRequired,
+  /**
+   * Represents if the menu is open or not
+   *
+   * @type {boolean}
+   */
+  isOpen: PropTypes.bool.isRequired,
   /**
    * Function that closes the menu
    */

ui/components/multichain/account-list-item-menu/account-list-item-menu.stories.js

@@ -23,6 +23,9 @@ export default {
     identity: {
       control: 'object',
     },
+    isOpen: {
+      control: 'boolean',
+    },
   },
   args: {
     anchorElement: null,
@@ -34,6 +37,7 @@ export default {
     },
     isRemovable: true,
     blockExplorerUrlSubTitle: 'etherscan.io',
+    isOpen: true,
   },
 };
 

ui/components/multichain/account-list-item-menu/account-list-item-menu.test.js

@@ -17,6 +17,7 @@ const DEFAULT_PROPS = {
   onClose: jest.fn(),
   onHide: jest.fn(),
   isRemovable: false,
+  isOpen: true,
 };
 
 const render = (props = {}) => {

ui/components/multichain/account-list-item-menu/index.scss

@@ -0,0 +1,6 @@
+.multichain-account-list-item-menu__popover {
+  z-index: $popover-in-modal-z-index;
+  overflow: hidden;
+  min-width: 225px;
+  max-width: 225px;
+}

ui/components/multichain/account-list-item/__snapshots__/account-list-item.test.js.snap

@@ -3,7 +3,7 @@
 exports[`AccountListItem renders AccountListItem component and shows account name, address, and balance 1`] = `
 <div>
   <div
-    class="box multichain-account-list-item box--padding-4 box--display-flex box--flex-direction-row box--background-color-transparent"
+    class="mm-box multichain-account-list-item mm-box--padding-4 mm-box--display-flex mm-box--background-color-transparent"
   >
     <div
       class="box mm-text mm-avatar-base mm-avatar-base--size-sm mm-avatar-account mm-text--body-sm mm-text--text-transform-uppercase box--margin-inline-end-2 box--display-flex box--flex-direction-row box--justify-content-center box--align-items-center box--color-text-default box--background-color-background-alternative box--rounded-full box--border-color-transparent box--border-style-solid box--border-width-1"
@@ -49,13 +49,13 @@ exports[`AccountListItem renders AccountListItem component and shows account nam
       </div>
     </div>
     <div
-      class="box multichain-account-list-item__content box--display-flex box--flex-direction-column"
+      class="mm-box multichain-account-list-item__content mm-box--display-flex mm-box--flex-direction-column"
     >
       <div
-        class="box box--display-flex box--flex-direction-column"
+        class="mm-box mm-box--display-flex mm-box--flex-direction-column"
       >
         <div
-          class="box box--display-flex box--flex-direction-row box--justify-content-space-between"
+          class="mm-box mm-box--display-flex mm-box--justify-content-space-between"
         >
           <div
             class="box mm-text multichain-account-list-item__account-name mm-text--body-md mm-text--ellipsis box--margin-inline-end-2 box--flex-direction-row box--color-text-default"
@@ -95,10 +95,10 @@ exports[`AccountListItem renders AccountListItem component and shows account nam
         </div>
       </div>
       <div
-        class="box box--display-flex box--flex-direction-row box--justify-content-space-between"
+        class="mm-box mm-box--display-flex mm-box--justify-content-space-between"
       >
         <div
-          class="box box--display-flex box--flex-direction-row box--align-items-center"
+          class="mm-box mm-box--display-flex mm-box--align-items-center"
         >
           <p
             class="box mm-text mm-text--body-sm box--flex-direction-row box--color-text-alternative"
@@ -130,18 +130,16 @@ exports[`AccountListItem renders AccountListItem component and shows account nam
         </div>
       </div>
     </div>
-    <div>
+    <button
-      <button
+      aria-label="Test Account Options"
-        aria-label="Test Account Options"
+      class="box mm-button-icon mm-button-icon--size-sm box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--color-icon-default box--background-color-transparent box--rounded-lg"
-        class="box mm-button-icon mm-button-icon--size-sm box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--color-icon-default box--background-color-transparent box--rounded-lg"
+      data-testid="account-list-item-menu-button"
-        data-testid="account-list-item-menu-button"
+    >
-      >
+      <span
-        <span
+        class="box mm-icon mm-icon--size-sm box--display-inline-block box--flex-direction-row box--color-inherit"
-          class="box mm-icon mm-icon--size-sm box--display-inline-block box--flex-direction-row box--color-inherit"
+        style="mask-image: url('./images/icons/more-vertical.svg');"
-          style="mask-image: url('./images/icons/more-vertical.svg');"
+      />
-        />
+    </button>
-      </button>
-    </div>
   </div>
 </div>
 `;

ui/components/multichain/account-list-item/account-list-item.js

@@ -1,4 +1,4 @@
-import React, { useState, useRef, useContext } from 'react';
+import React, { useState, useContext } from 'react';
 import PropTypes from 'prop-types';
 import classnames from 'classnames';
 
@@ -8,9 +8,9 @@ import { getRpcPrefsForCurrentProvider } from '../../../selectors';
 import { getURLHostName, shortenAddress } from '../../../helpers/utils/util';
 
 import { AccountListItemMenu } from '..';
-import Box from '../../ui/box/box';
 import {
   AvatarAccount,
+  Box,
   Text,
   AvatarFavicon,
   Tag,
@@ -24,13 +24,13 @@ import {
   Color,
   TextAlign,
   AlignItems,
-  DISPLAY,
   TextVariant,
-  FLEX_DIRECTION,
+  FlexDirection,
   BorderRadius,
   JustifyContent,
   Size,
   BorderColor,
+  Display,
 } from '../../../helpers/constants/design-system';
 import { HardwareKeyringNames } from '../../../../shared/constants/hardware-wallets';
 import { KeyringType } from '../../../../shared/constants/keyring';
@@ -75,9 +75,14 @@ export const AccountListItem = ({
 }) => {
   const t = useI18nContext();
   const [accountOptionsMenuOpen, setAccountOptionsMenuOpen] = useState(false);
-  const ref = useRef(false);
+  const [accountListItemMenuElement, setAccountListItemMenuElement] =
+    useState();
   const useBlockie = useSelector((state) => state.metamask.useBlockie);
 
+  const setAccountListItemMenuRef = (ref) => {
+    setAccountListItemMenuElement(ref);
+  };
+
   const keyring = useSelector((state) =>
     findKeyringForAddress(state, identity.address),
   );
@@ -91,7 +96,7 @@ export const AccountListItem = ({
 
   return (
     <Box
-      display={DISPLAY.FLEX}
+      display={Display.Flex}
       padding={4}
       backgroundColor={selected ? Color.primaryMuted : Color.transparent}
       className={classnames('multichain-account-list-item', {
@@ -125,13 +130,13 @@ export const AccountListItem = ({
         marginInlineEnd={2}
       ></AvatarAccount>
       <Box
-        display={DISPLAY.FLEX}
+        display={Display.Flex}
-        flexDirection={FLEX_DIRECTION.COLUMN}
+        flexDirection={FlexDirection.Column}
         className="multichain-account-list-item__content"
       >
-        <Box display={DISPLAY.FLEX} flexDirection={FLEX_DIRECTION.COLUMN}>
+        <Box display={Display.Flex} flexDirection={FlexDirection.Column}>
           <Box
-            display={DISPLAY.FLEX}
+            display={Display.Flex}
             justifyContent={JustifyContent.spaceBetween}
           >
             <Text
@@ -165,8 +170,8 @@ export const AccountListItem = ({
             <Text
               as="div"
               className="multichain-account-list-item__asset"
-              display={DISPLAY.FLEX}
+              display={Display.Flex}
-              flexDirection={FLEX_DIRECTION.ROW}
+              flexDirection={FlexDirection.Row}
               alignItems={AlignItems.center}
               ellipsis
               textAlign={TextAlign.End}
@@ -180,10 +185,10 @@ export const AccountListItem = ({
           </Box>
         </Box>
         <Box
-          display={DISPLAY.FLEX}
+          display={Display.Flex}
           justifyContent={JustifyContent.spaceBetween}
         >
-          <Box display={DISPLAY.FLEX} alignItems={AlignItems.center}>
+          <Box display={Display.Flex} alignItems={AlignItems.center}>
             {connectedAvatar ? (
               <AvatarFavicon
                 size={Size.XS}
@@ -219,13 +224,14 @@ export const AccountListItem = ({
           />
         ) : null}
       </Box>
-      <div ref={ref}>
+      <ButtonIcon
-        <ButtonIcon
+        ariaLabel={`${identity.name} ${t('options')}`}
-          ariaLabel={`${identity.name} ${t('options')}`}
+        iconName={IconName.MoreVertical}
-          iconName={IconName.MoreVertical}
+        size={IconSize.Sm}
-          size={IconSize.Sm}
+        ref={setAccountListItemMenuRef}
-          onClick={(e) => {
+        onClick={(e) => {
-            e.stopPropagation();
+          e.stopPropagation();
+          if (!accountOptionsMenuOpen) {
             trackEvent({
               event: MetaMetricsEventName.AccountDetailMenuOpened,
               category: MetaMetricsEventCategory.Navigation,
@@ -233,21 +239,20 @@ export const AccountListItem = ({
                 location: 'Account Options',
               },
             });
-            setAccountOptionsMenuOpen(true);
+          }
-          }}
+          setAccountOptionsMenuOpen(!accountOptionsMenuOpen);
-          data-testid="account-list-item-menu-button"
+        }}
-        />
+        data-testid="account-list-item-menu-button"
-        {accountOptionsMenuOpen ? (
+      />
-          <AccountListItemMenu
+      <AccountListItemMenu
-            anchorElement={ref.current}
+        anchorElement={accountListItemMenuElement}
-            blockExplorerUrlSubTitle={blockExplorerUrlSubTitle}
+        blockExplorerUrlSubTitle={blockExplorerUrlSubTitle}
-            identity={identity}
+        identity={identity}
-            onClose={() => setAccountOptionsMenuOpen(false)}
+        onClose={() => setAccountOptionsMenuOpen(false)}
-            isRemovable={keyring?.type !== KeyringType.hdKeyTree}
+        isOpen={accountOptionsMenuOpen}
-            closeMenu={closeMenu}
+        isRemovable={keyring?.type !== KeyringType.hdKeyTree}
-          />
+        closeMenu={closeMenu}
-        ) : null}
+      />
-      </div>
     </Box>
   );
 };

ui/components/multichain/account-list-item/account-list-item.test.js

@@ -68,7 +68,9 @@ describe('AccountListItem', () => {
     );
     expect(optionsButton).toBeInTheDocument();
     fireEvent.click(optionsButton);
-    expect(document.querySelector('.menu__background')).toBeInTheDocument();
+    expect(
+      document.querySelector('.multichain-account-list-item-menu__popover'),
+    ).toBeInTheDocument();
   });
 
   it('executes the action when the item is clicked', () => {

ui/components/multichain/multichain-components.scss

@@ -6,6 +6,7 @@
 **/
 @import 'address-copy-button/index';
 @import 'account-list-item/index';
+@import 'account-list-item-menu/index';
 @import 'account-list-menu/index';
 @import 'account-picker/index';
 @import 'app-header/app-header';

ui/components/ui/menu/menu-item.js

@@ -5,29 +5,35 @@ import classnames from 'classnames';
 import { Text, Icon, IconSize } from '../../component-library';
 import { TextVariant } from '../../../helpers/constants/design-system';
 
-const MenuItem = ({
+const MenuItem = React.forwardRef(
-  children,
+  (
-  className,
+    {
-  'data-testid': dataTestId,
+      children,
-  iconName,
+      className,
-  onClick,
+      'data-testid': dataTestId,
-  subtitle,
+      iconName,
-  disabled = false,
+      onClick,
-}) => (
+      subtitle,
-  <button
+      disabled = false,
-    className={classnames('menu-item', className)}
+    },
-    data-testid={dataTestId}
+    ref,
-    onClick={onClick}
+  ) => (
-    disabled={disabled}
+    <button
-  >
+      className={classnames('menu-item', className)}
-    {iconName ? (
+      data-testid={dataTestId}
-      <Icon name={iconName} size={IconSize.Sm} marginRight={2} />
+      onClick={onClick}
-    ) : null}
+      ref={ref}
-    <div>
+      disabled={disabled}
-      <div>{children}</div>
+    >
-      {subtitle ? <Text variant={TextVariant.bodyXs}>{subtitle}</Text> : null}
+      {iconName ? (
-    </div>
+        <Icon name={iconName} size={IconSize.Sm} marginRight={2} />
-  </button>
+      ) : null}
+      <div>
+        <div>{children}</div>
+        {subtitle ? <Text variant={TextVariant.bodyXs}>{subtitle}</Text> : null}
+      </div>
+    </button>
+  ),
 );
 
 MenuItem.propTypes = {
@@ -40,4 +46,6 @@ MenuItem.propTypes = {
   disabled: PropTypes.bool,
 };
 
+MenuItem.displayName = 'MenuItem';
+
 export default MenuItem;

ui/css/design-system/z-index.scss

@@ -11,3 +11,4 @@ $send-card-z-index: 20;
 $sidebar-z-index: 26;
 $sidebar-overlay-z-index: 25;
 $modal-z-index: 1050;
+$popover-in-modal-z-index: 1051;