Fix #19439 - Allow Account Picker during Send flow (#19522)

* Fix #19439 - Allow Account Picker during Send flow * Disable any network change with confirmations pending * Disable the settings menu during an unconfirmed transaction
2024-12-22 17:33:23 +01:00 · 2023-06-14 13:49:14 -05:00 · 2023-06-14 13:49:14 -05:00 · 875bad125f
commit 875bad125f
parent f4f80c223e
8 changed files with 353 additions and 324 deletions
--- a/ui/components/multichain/app-header/snapshots/app-header.test.js.snap
+++ b/ui/components/multichain/app-header/snapshots/app-header.test.js.snap
@ -3,32 +3,9 @@
 exports[`App Header should match snapshot 1`] = `
 <div>
  <div
-    class="box multichain-app-header multichain-app-header-shadow box--display-flex box--flex-direction-row box--align-items-center box--width-full box--background-color-background-default"
+    class="box multichain-app-header-logo box--margin-2 box--display-none box--sm:display-flex box--flex-direction-row box--justify-content-center box--align-items-center"
+    data-testid="app-header-logo"
  >
-    <div
-      class="box multichain-app-header__lock-contents box--padding-2 box--display-flex box--gap-2 box--flex-direction-row box--justify-content-space-between box--align-items-center box--width-full box--background-color-background-default"
-    >
-      <div>
-        <button
-          class="box mm-picker-network multichain-app-header__contents__network-picker box--padding-right-4 box--padding-left-2 box--display-flex box--gap-2 box--flex-direction-row box--align-items-center box--background-color-background-alternative box--rounded-pill"
-          data-testid="network-display"
-        >
-          <div
-            class="box mm-text mm-avatar-base mm-avatar-base--size-xs mm-avatar-network mm-picker-network__avatar-network mm-text--body-xs mm-text--text-transform-uppercase 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"
-          >
-            G
-          </div>
-          <p
-            class="box mm-text mm-text--body-sm mm-text--ellipsis box--flex-direction-row box--color-text-default"
-          >
-            Goerli
-          </p>
-          <span
-            class="box mm-picker-network__arrow-down-icon mm-icon mm-icon--size-xs box--display-inline-block box--flex-direction-row box--color-icon-default"
-            style="mask-image: url('./images/icons/arrow-down.svg');"
-          />
-        </button>
-      </div>
    <button
      class="box app-header__logo-container app-header__logo-container--clickable box--flex-direction-row box--background-color-transparent"
      data-testid="app-header-logo"
@ -222,6 +199,119 @@ exports[`App Header should match snapshot 1`] = `
      />
    </button>
  </div>
+  <div
+    class="box multichain-app-header box--display-flex box--flex-direction-row box--align-items-center box--width-full box--background-color-background-alternative"
+  >
+    <div
+      class="box multichain-app-header__contents multichain-app-header-shadow box--padding-2 box--padding-right-4 box--padding-left-4 box--gap-2 box--flex-direction-row box--align-items-center box--width-full box--background-color-background-default box--display-flex"
+    >
+      <div>
+        <button
+          class="box mm-picker-network multichain-app-header__contents__network-picker box--margin-2 box--padding-right-4 box--padding-left-2 box--display-none box--sm:display-flex box--gap-2 box--flex-direction-row box--align-items-center box--background-color-background-alternative box--rounded-pill"
+          data-testid="network-display"
+          disabled=""
+        >
+          <div
+            class="box mm-text mm-avatar-base mm-avatar-base--size-xs mm-avatar-network mm-picker-network__avatar-network mm-text--body-xs mm-text--text-transform-uppercase 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"
+          >
+            G
+          </div>
+          <p
+            class="box mm-text mm-text--body-sm mm-text--ellipsis box--flex-direction-row box--color-text-default"
+          >
+            Goerli
+          </p>
+          <span
+            class="box mm-picker-network__arrow-down-icon mm-icon mm-icon--size-xs box--display-inline-block box--flex-direction-row box--color-icon-default"
+            style="mask-image: url('./images/icons/arrow-down.svg');"
+          />
+        </button>
+      </div>
+      <button
+        class="box mm-text mm-button-base mm-button-base--size-md mm-button-base--ellipsis multichain-account-picker mm-button-primary mm-text--body-md mm-text--ellipsis box--padding-right-4 box--padding-left-4 box--display-inline-flex box--flex-direction-row box--justify-content-center box--align-items-center box--color-primary-inverse box--background-color-transparent box--rounded-lg"
+        data-testid="account-menu-icon"
+      >
+        <span
+          class="box mm-text mm-text--inherit mm-text--ellipsis box--display-flex box--gap-2 box--flex-direction-row box--align-items-center box--color-primary-inverse"
+        >
+          <div
+            class="box mm-text mm-avatar-base mm-avatar-base--size-xs mm-avatar-account mm-text--body-xs mm-text--text-transform-uppercase 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-background-default box--border-style-solid box--border-width-1"
+          >
+            <div
+              class="mm-avatar-account__jazzicon"
+            >
+              <div
+                style="border-radius: 50px; overflow: hidden; padding: 0px; margin: 0px; width: 16px; height: 16px; display: inline-block; background: rgb(250, 58, 0);"
+              >
+                <svg
+                  height="16"
+                  width="16"
+                  x="0"
+                  y="0"
+                >
+                  <rect
+                    fill="#18CDF2"
+                    height="16"
+                    transform="translate(-0.52419675189697 -1.6521420347302493) rotate(328.9 8 8)"
+                    width="16"
+                    x="0"
+                    y="0"
+                  />
+                  <rect
+                    fill="#035E56"
+                    height="16"
+                    transform="translate(-9.149230854416022 5.2962309358743) rotate(176.2 8 8)"
+                    width="16"
+                    x="0"
+                    y="0"
+                  />
+                  <rect
+                    fill="#F26602"
+                    height="16"
+                    transform="translate(8.333921009111961 -7.102569861498541) rotate(468.9 8 8)"
+                    width="16"
+                    x="0"
+                    y="0"
+                  />
+                </svg>
+              </div>
+            </div>
+          </div>
+          <span
+            class="box mm-text mm-text--body-md mm-text--font-weight-bold mm-text--ellipsis box--flex-direction-row box--color-text-default"
+          >
+            Test Account
+          </span>
+          <span
+            class="box mm-icon mm-icon--size-sm box--display-inline-block box--flex-direction-row box--color-icon-default"
+            style="mask-image: url('./images/icons/arrow-down.svg');"
+          />
+        </span>
+      </button>
+      <div
+        class="box box--display-flex box--flex-direction-row box--justify-content-flex-end box--align-items-center"
+      >
+        <div
+          class="box box--display-flex box--gap-4 box--flex-direction-row"
+        >
+           
+          <div
+            class="box box--display-flex box--flex-direction-row box--justify-content-flex-end box--width-full"
+          >
+            <button
+              aria-label="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"
+              data-testid="account-options-menu-button"
+            >
+              <span
+                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');"
+              />
+            </button>
+          </div>
+        </div>
+      </div>
+    </div>
  </div>
 </div>
 `;
--- a/ui/components/multichain/app-header/app-header.js
+++ b/ui/components/multichain/app-header/app-header.js
@ -105,17 +105,17 @@ export const AppHeader = ({ location }) => {
  // Disable the network and account pickers if the user is in
  // a critical flow
  const sendStage = useSelector(getSendStage);
+  const isTransactionEditPage = [
+    SEND_STAGES.EDIT,
+    SEND_STAGES.DRAFT,
+    SEND_STAGES.ADD_RECIPIENT,
+  ].includes(sendStage);
  const isConfirmationPage = Boolean(
    matchPath(location.pathname, {
      path: CONFIRM_TRANSACTION_ROUTE,
      exact: false,
    }),
  );
-  const isTransactionEditPage = [
-    SEND_STAGES.EDIT,
-    SEND_STAGES.DRAFT,
-    SEND_STAGES.ADD_RECIPIENT,
-  ].includes(sendStage);
  const isSwapsPage = Boolean(
    matchPath(location.pathname, { path: SWAPS_ROUTE, exact: false }),
  );
@ -123,11 +123,18 @@ export const AppHeader = ({ location }) => {
    matchPath(location.pathname, { path: BUILD_QUOTE_ROUTE, exact: false }),
  );

-  const disablePickers =
-    isConfirmationPage ||
+  const hasUnapprovedTransactions = useSelector(
+    (state) => Object.keys(state.metamask.unapprovedTxs).length > 0,
+  );
+
+  const disableAccountPicker =
+    isConfirmationPage || (isSwapsPage && !isSwapsBuildQuotePage);
+
+  const disableNetworkPicker =
+    isSwapsPage ||
    isTransactionEditPage ||
-    (isSwapsPage && !isSwapsBuildQuotePage);
-  const disableNetworkPicker = isSwapsPage || disablePickers;
+    isConfirmationPage ||
+    hasUnapprovedTransactions;

  // Callback for network dropdown
  const networkOpenCallback = useCallback(() => {
@ -261,7 +268,7 @@ export const AppHeader = ({ location }) => {
                      },
                    });
                  }}
-                  disabled={disablePickers}
+                  disabled={disableAccountPicker}
                />
              ) : null}
              <Box
--- a/ui/components/multichain/app-header/app-header.test.js
+++ b/ui/components/multichain/app-header/app-header.test.js
@ -1,118 +1,34 @@
 import React from 'react';
-import configureStore from 'redux-mock-store';
-import { CHAIN_IDS } from '../../../../shared/constants/network';
+import configureStore from '../../../store/store';
 import { renderWithProvider } from '../../../../test/lib/render-helpers';
+import mockState from '../../../../test/data/mock-state.json';
 import { SEND_STAGES } from '../../../ducks/send';
 import { AppHeader } from '.';

-describe('App Header', () => {
-  it('should match snapshot', () => {
-    const mockState = {
+const render = (stateChanges = {}, location = jest.fn()) => {
+  const store = configureStore({
+    ...mockState,
    activeTab: {
-        title: 'Eth Sign Tests',
      origin: 'https://remix.ethereum.org',
-        protocol: 'https:',
-        url: 'https://remix.ethereum.org/',
-      },
-      metamask: {
-        providerConfig: {
-          chainId: CHAIN_IDS.GOERLI,
-        },
-        accounts: {
-          '0x7250739de134d33ec7ab1ee592711e15098c9d2d': {
-            address: '0x7250739de134d33ec7ab1ee592711e15098c9d2d',
-          },
-          '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5': {
-            address: '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5',
-          },
-        },
-        preferences: {
-          showTestNetworks: true,
-        },
-        selectedAddress: '0x7250739de134d33ec7ab1ee592711e15098c9d2d',
-        cachedBalances: {},
-        subjects: {
-          'https://remix.ethereum.org': {
-            permissions: {
-              eth_accounts: {
-                caveats: [
-                  {
-                    type: 'restrictReturnedAccounts',
-                    value: [
-                      '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5',
-                      '0x7250739de134d33ec7ab1ee592711e15098c9d2d',
-                    ],
-                  },
-                ],
-                date: 1586359844177,
-                id: '3aa65a8b-3bcb-4944-941b-1baa5fe0ed8b',
-                invoker: 'https://remix.ethereum.org',
-                parentCapability: 'eth_accounts',
-              },
-            },
-          },
-          'peepeth.com': {
-            permissions: {
-              eth_accounts: {
-                caveats: [
-                  {
-                    type: 'restrictReturnedAccounts',
-                    value: ['0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5'],
-                  },
-                ],
-                date: 1585676177970,
-                id: '840d72a0-925f-449f-830a-1aa1dd5ce151',
-                invoker: 'peepeth.com',
-                parentCapability: 'eth_accounts',
-              },
-            },
-          },
-        },
-        identities: {
-          '0x7250739de134d33ec7ab1ee592711e15098c9d2d': {
-            address: '0x7250739de134d33ec7ab1ee592711e15098c9d2d',
-            name: 'Really Long Name That Should Be Truncated',
-          },
-          '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5': {
-            address: '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5',
-            lastSelected: 1586359844192,
-            name: 'Account 1',
-          },
-        },
-        keyrings: [
-          {
-            accounts: [
-              '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5',
-              '0x7250739de134d33ec7ab1ee592711e15098c9d2d',
-            ],
-          },
-        ],
-        permissionHistory: {
-          'https://remix.ethereum.org': {
-            eth_accounts: {
-              accounts: {
-                '0x7250739de134d33ec7ab1ee592711e15098c9d2d': 1586359844192,
-                '0x8e5d75d60224ea0c33d0041e75de68b1c3cb6dd5': 1586359844192,
-              },
-              lastApproved: 1586359844192,
-            },
-          },
-        },
-      },
-      appState: {
-        onboardedInThisUISession: false,
-      },
-      send: {
-        stage: SEND_STAGES.INACTIVE,
    },
+    ...stateChanges,
+  });
+  return renderWithProvider(<AppHeader location={location} />, store);
 };

-    const mockStore = configureStore();
-    const store = mockStore(mockState);
-    const { container } = renderWithProvider(
-      <AppHeader location={{ pathname: '' }} />,
-      store,
-    );
+describe('App Header', () => {
+  it('should match snapshot', () => {
+    const { container } = render();
    expect(container).toMatchSnapshot();
  });
+
+  it('should disable the network picker during a send', () => {
+    const { getByTestId } = render({ send: { stage: SEND_STAGES.DRAFT } });
+    expect(getByTestId('network-display')).toBeDisabled();
+  });
+
+  it('should allow switching accounts during a send', () => {
+    const { getByTestId } = render({ send: { stage: SEND_STAGES.DRAFT } });
+    expect(getByTestId('account-menu-icon')).toBeEnabled();
+  });
 });
--- a/ui/components/multichain/global-menu/global-menu.js
+++ b/ui/components/multichain/global-menu/global-menu.js
@ -58,6 +58,10 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => {
  const history = useHistory();
  const metaMetricsId = useSelector(getMetaMetricsId);

+  const hasUnapprovedTransactions = useSelector(
+    (state) => Object.keys(state.metamask.unapprovedTxs).length > 0,
+  );
+
  ///: BEGIN:ONLY_INCLUDE_IN(snaps)
  const unreadNotificationsCount = useSelector(getUnreadNotificationsCount);
  ///: END:ONLY_INCLUDE_IN
@ -199,6 +203,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => {
      </MenuItem>
      <MenuItem
        iconName={IconName.Setting}
+        disabled={hasUnapprovedTransactions}
        onClick={() => {
          history.push(SETTINGS_ROUTE);
          trackEvent({
@ -210,6 +215,7 @@ export const GlobalMenu = ({ closeMenu, anchorElement }) => {
          });
          closeMenu();
        }}
+        data-testid="global-menu-settings"
      >
        {t('settings')}
      </MenuItem>
--- a/ui/components/multichain/global-menu/global-menu.test.js
+++ b/ui/components/multichain/global-menu/global-menu.test.js
@ -4,10 +4,11 @@ import configureStore from '../../../store/store';
 import mockState from '../../../../test/data/mock-state.json';
 import { GlobalMenu } from '.';

-const render = () => {
+const render = (metamaskStateChanges = {}) => {
  const store = configureStore({
    metamask: {
      ...mockState.metamask,
+      ...metamaskStateChanges,
    },
  });
  return renderWithProvider(
@ -52,6 +53,20 @@ describe('AccountListItem', () => {
    });
  });

+  it('disables the settings item when there is an active transaction', async () => {
+    const { getByTestId } = render();
+    await waitFor(() => {
+      expect(getByTestId('global-menu-settings')).toBeDisabled();
+    });
+  });
+
+  it('enables the settings item when there is no active transaction', async () => {
+    const { getByTestId } = render({ unapprovedTxs: {} });
+    await waitFor(() => {
+      expect(getByTestId('global-menu-settings')).toBeEnabled();
+    });
+  });
+
  it('expands metamask to tab when item is clicked', async () => {
    global.platform = { openExtensionInBrowser: jest.fn() };

--- a/ui/components/ui/menu/menu-item.js
+++ b/ui/components/ui/menu/menu-item.js
@ -12,11 +12,13 @@ const MenuItem = ({
  iconName,
  onClick,
  subtitle,
+  disabled = false,
 }) => (
  <button
    className={classnames('menu-item', className)}
    data-testid={dataTestId}
    onClick={onClick}
+    disabled={disabled}
  >
    {iconName ? (
      <Icon name={iconName} size={IconSize.Sm} marginRight={2} />
@ -35,14 +37,7 @@ MenuItem.propTypes = {
  iconName: PropTypes.string,
  onClick: PropTypes.func,
  subtitle: PropTypes.node,
-};
-
-MenuItem.defaultProps = {
-  className: undefined,
-  'data-testid': undefined,
-  iconName: undefined,
-  onClick: undefined,
-  subtitle: undefined,
+  disabled: PropTypes.bool,
 };

 export default MenuItem;
--- a/ui/components/ui/menu/menu.stories.js
+++ b/ui/components/ui/menu/menu.stories.js
@ -36,6 +36,12 @@ export const Anchored = () => {
        <MenuItem iconName={IconName.EyeSlash} onClick={action('Menu Item 3')}>
          Menu Item 3
        </MenuItem>
+        <MenuItem
+          iconName={IconName.AddSquare}
+          onClick={action('Disabled Item')}
+        >
+          Disabled Item
+        </MenuItem>
      </Menu>
    </>
  );
--- a/ui/pages/routes/routes.component.test.js
+++ b/ui/pages/routes/routes.component.test.js
@ -58,7 +58,7 @@ describe('Routes Component', () => {
    mockHideNetworkDropdown.mockClear();
  });
  describe('render during send flow', () => {
-    it('should render with network and account change disabled while adding recipient for send flow', () => {
+    it('should render with network change disabled while adding recipient for send flow', () => {
      const store = configureMockStore()({
        ...mockSendState,
        send: {
@ -68,25 +68,21 @@ describe('Routes Component', () => {
      });
      const { getByTestId } = renderWithProvider(<Routes />, store, ['/send']);

-      expect(getByTestId('account-menu-icon')).toBeDisabled();
-
      const networkDisplay = getByTestId('network-display');
      fireEvent.click(networkDisplay);
      expect(mockShowNetworkDropdown).not.toHaveBeenCalled();
    });
-    it('should render with network and account change disabled while user is in send page', () => {
+    it('should render with network change disabled while user is in send page', () => {
      const store = configureMockStore()({
        ...mockSendState,
      });
      const { getByTestId } = renderWithProvider(<Routes />, store, ['/send']);

-      expect(getByTestId('account-menu-icon')).toBeDisabled();
-
      const networkDisplay = getByTestId('network-display');
      fireEvent.click(networkDisplay);
      expect(mockShowNetworkDropdown).not.toHaveBeenCalled();
    });
-    it('should render with network and account change disabled while editing a send transaction', () => {
+    it('should render with network change disabled while editing a send transaction', () => {
      const store = configureMockStore()({
        ...mockSendState,
        send: {
@ -96,8 +92,6 @@ describe('Routes Component', () => {
      });
      const { getByTestId } = renderWithProvider(<Routes />, store, ['/send']);

-      expect(getByTestId('account-menu-icon')).toBeDisabled();
-
      const networkDisplay = getByTestId('network-display');
      fireEvent.click(networkDisplay);
      expect(mockShowNetworkDropdown).not.toHaveBeenCalled();