feat(14507): improve error message for failed txn in activity details…

… view
MetaMask · Dec 19, 2024 · 66c9bb0 · 66c9bb0
1 parent c8a3c58
commit 66c9bb0
Show file tree

Hide file tree

Showing 9 changed files with 254 additions and 289 deletions.
diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json
diff --git a/ui/components/app/transaction-list-item-details/index.scss b/ui/components/app/transaction-list-item-details/index.scss
@@ -49,6 +49,8 @@
     display: flex;
     flex-direction: column;
     align-items: flex-end;
+    height: 42px;
+    justify-content: space-between;
 
     .btn-link {
       font-size: 12px;
@@ -62,9 +64,10 @@
   }
 
   &__operations {
-    margin: 0 0 16px 16px;
+    margin: 0 16px 16px 16px;
     display: flex;
-    justify-content: flex-end;
+    justify-content: space-between;
+    align-items: center;
   }
 
   &__header {

diff --git a/ui/components/app/transaction-list-item-details/transaction-list-item-details.component.js b/ui/components/app/transaction-list-item-details/transaction-list-item-details.component.js
@@ -13,9 +13,19 @@ import Tooltip from '../../ui/tooltip';
 import CancelButton from '../cancel-button';
 import Popover from '../../ui/popover';
 import { Box } from '../../component-library/box';
+import { Text } from '../../component-library/text';
+import {
+  BannerAlert,
+  BannerAlertSeverity,
+} from '../../component-library/banner-alert';
+import {
+  TextVariant,
+  ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
+  IconColor,
+  ///: END:ONLY_INCLUDE_IF
+} from '../../../helpers/constants/design-system';
 ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
-import { Icon, IconName, Text } from '../../component-library';
-import { IconColor } from '../../../helpers/constants/design-system';
+import { Icon, IconName } from '../../component-library';
 ///: END:ONLY_INCLUDE_IF
 import { SECOND } from '../../../../shared/constants/time';
 import { MetaMetricsEventCategory } from '../../../../shared/constants/metametrics';
@@ -55,6 +65,7 @@ export default class TransactionListItemDetails extends PureComponent {
     recipientNickname: PropTypes.string,
     transactionStatus: PropTypes.func,
     isCustomNetwork: PropTypes.bool,
+    showErrorBanner: PropTypes.bool,
     history: PropTypes.object,
     blockExplorerLinkText: PropTypes.object,
     ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
@@ -204,6 +215,7 @@ export default class TransactionListItemDetails extends PureComponent {
       onClose,
       recipientNickname,
       showCancel,
+      showErrorBanner,
       transactionStatus: TransactionStatus,
       blockExplorerLinkText,
       ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
@@ -220,6 +232,17 @@ export default class TransactionListItemDetails extends PureComponent {
       <Popover title={title} onClose={onClose}>
         <div className="transaction-list-item-details">
           <div className="transaction-list-item-details__operations">
+            {showErrorBanner && (
+              <BannerAlert severity={BannerAlertSeverity.Warning}>
+                <Text
+                  variant={TextVariant.bodyMd}
+                  as="h6"
+                  data-testid="transaction-list-item-details-banner-error-message"
+                >
+                  {t('transactionFailedBannerMessage')}
+                </Text>
+              </BannerAlert>
+            )}
             <div className="transaction-list-item-details__header-buttons">
               {showSpeedUp && (
                 <Button
@@ -246,7 +269,7 @@ export default class TransactionListItemDetails extends PureComponent {
                     className="transaction-list-item-details__header-button"
                     data-testid="rety-button"
                   >
-                    <i className="fa fa-sync"></i>
+                    <i className="fa fa-sync" />
                   </Button>
                 </Tooltip>
               )}
@@ -258,22 +281,18 @@ export default class TransactionListItemDetails extends PureComponent {
               data-testid="transaction-list-item-details-tx-status"
             >
               <div>{t('status')}</div>
-              <div>
-                <TransactionStatus />
-              </div>
+              <TransactionStatus />
             </div>
             <div className="transaction-list-item-details__tx-hash">
-              <div>
-                <Button
-                  type="link"
-                  onClick={this.handleBlockExplorerClick}
-                  disabled={!hash}
-                >
-                  {blockExplorerLinkText.firstPart === 'addBlockExplorer'
-                    ? t('addBlockExplorer')
-                    : t('viewOnBlockExplorer')}
-                </Button>
-              </div>
+              <Button
+                type="link"
+                onClick={this.handleBlockExplorerClick}
+                disabled={!hash}
+              >
+                {blockExplorerLinkText.firstPart === 'addBlockExplorer'
+                  ? t('addBlockExplorer')
+                  : t('viewOnBlockExplorer')}
+              </Button>
               <div>
                 <Tooltip
                   wrapperClassName="transaction-list-item-details__header-button"

diff --git a/...ponents/app/transaction-list-item-details/transaction-list-item-details.component.test.js b/...ponents/app/transaction-list-item-details/transaction-list-item-details.component.test.js
@@ -1,7 +1,7 @@
 import React from 'react';
 import configureMockStore from 'redux-mock-store';
 import thunk from 'redux-thunk';
-import { act, waitFor } from '@testing-library/react';
+import { waitFor } from '@testing-library/react';
 import { TransactionStatus } from '@metamask/transaction-controller';
 import { GAS_LIMITS } from '../../../../shared/constants/gas';
 import { renderWithProvider } from '../../../../test/lib/render-helpers';
@@ -54,7 +54,7 @@ const transactionGroup = {
   hasCancelled: false,
 };
 
-const render = async (overrideProps) => {
+const render = (overrideProps) => {
   const rpcPrefs = {
     blockExplorerUrl: 'https://customblockexplorer.com/',
   };
@@ -71,67 +71,65 @@ const render = async (overrideProps) => {
     senderAddress: '0x0dcd5d886577d5081b0c52e242ef29e70be3e7bc',
     tryReverseResolveAddress: jest.fn(),
     transactionGroup,
-    transactionStatus: () => <div></div>,
+    transactionStatus: () => <div />,
     blockExplorerLinkText,
     rpcPrefs,
     ...overrideProps,
   };
 
   const mockStore = configureMockStore([thunk])(mockState);
 
-  let result;
-
-  await act(
-    async () =>
-      (result = renderWithProvider(
-        <TransactionListItemDetails {...props} />,
-        mockStore,
-      )),
+  const result = renderWithProvider(
+    <TransactionListItemDetails {...props} />,
+    mockStore,
   );
 
   return result;
 };
 
 describe('TransactionListItemDetails Component', () => {
-  it('should render title with title prop', async () => {
-    const { queryByText } = await render();
+  describe('matches snapshot', () => {
+    it('for non-error details', async () => {
+      const { queryByText, queryByTestId } = render();
+      expect(queryByText('Test Transaction Details')).toBeInTheDocument();
+      expect(
+        queryByTestId('transaction-list-item-details-banner-error-message'),
+      ).not.toBeInTheDocument();
+    });
 
-    await waitFor(() => {
+    it('for error details', async () => {
+      const { queryByText, queryByTestId } = render({ showErrorBanner: true });
       expect(queryByText('Test Transaction Details')).toBeInTheDocument();
+      expect(
+        queryByTestId('transaction-list-item-details-banner-error-message'),
+      ).toBeInTheDocument();
     });
   });
 
-  describe('Retry button', () => {
-    it('should render retry button with showRetry prop', async () => {
-      const { queryByTestId } = await render({ showRetry: true });
-
+  describe('Action buttons', () => {
+    it('renders retry button with showRetry prop', async () => {
+      const { queryByTestId } = render({ showRetry: true });
       expect(queryByTestId('rety-button')).toBeInTheDocument();
     });
-  });
-
-  describe('Cancel button', () => {
-    it('should render cancel button with showCancel prop', async () => {
-      const { queryByTestId } = await render({ showCancel: true });
 
+    it('renders cancel button with showCancel prop', async () => {
+      const { queryByTestId } = render({ showCancel: true });
       expect(queryByTestId('cancel-button')).toBeInTheDocument();
     });
-  });
-
-  describe('Speedup button', () => {
-    it('should render speedup button with showSpeedUp prop', async () => {
-      const { queryByTestId } = await render({ showSpeedUp: true });
 
+    it('renders speedup button with showSpeedUp prop', async () => {
+      const { queryByTestId } = render({ showSpeedUp: true });
       expect(queryByTestId('speedup-button')).toBeInTheDocument();
     });
   });
 
   describe('Institutional', () => {
-    it('should render correctly if custodyTransactionDeepLink has a url', async () => {
+    it('renders correctly if custodyTransactionDeepLink has a url', async () => {
       mockGetCustodianTransactionDeepLink = jest
         .fn()
         .mockReturnValue({ url: 'https://url.com' });
 
-      await render({ showCancel: true });
+      render({ showCancel: true });
 
       await waitFor(() => {
         const custodianViewButton = document.querySelector(
@@ -143,7 +141,7 @@ describe('TransactionListItemDetails Component', () => {
       });
     });
 
-    it('should render correctly if transactionNote is provided', async () => {
+    it('renders correctly if transactionNote is provided', async () => {
       const newTransaction = {
         ...transaction,
         metadata: {
@@ -159,13 +157,11 @@ describe('TransactionListItemDetails Component', () => {
         initialTransaction: newTransaction,
       };
 
-      const { queryByText } = await render({
+      const { queryByText } = render({
         transactionGroup: newTransactionGroup,
       });
 
-      await waitFor(() => {
-        expect(queryByText('some note')).toBeInTheDocument();
-      });
+      expect(queryByText('some note')).toBeInTheDocument();
     });
   });
 });
diff --git a/ui/components/app/transaction-list-item/smart-transaction-list-item.component.js b/ui/components/app/transaction-list-item/smart-transaction-list-item.component.js
@@ -125,6 +125,7 @@ export default function SmartTransactionListItem({
               date={date}
               status={displayedStatusKey}
               statusOnly
+              shouldShowTooltip={false}
             />
           )}
         />

diff --git a/ui/components/app/transaction-list-item/transaction-list-item.component.js b/ui/components/app/transaction-list-item/transaction-list-item.component.js
@@ -459,6 +459,7 @@ function TransactionListItemInner({
             !hasCancelled &&
             !isBridgeTx
           }
+          showErrorBanner={Boolean(error)}
           transactionStatus={() => (
             <TransactionStatusLabel
               isPending={isPending}
@@ -467,6 +468,7 @@ function TransactionListItemInner({
               date={date}
               status={displayedStatusKey}
               statusOnly
+              shouldShowTooltip={false}
               ///: BEGIN:ONLY_INCLUDE_IF(build-mmi)
               custodyStatus={transactionGroup.primaryTransaction.custodyStatus}
               custodyStatusDisplayText={

diff --git a/...mponents/app/transaction-status-label/__snapshots__/transaction-status-label.test.js.snap b/...mponents/app/transaction-status-label/__snapshots__/transaction-status-label.test.js.snap
@@ -1,49 +1,49 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`TransactionStatusLabel Component should render CONFIRMED properly 1`] = `
+exports[`TransactionStatusLabel Component renders CONFIRMED properly and tooltip 1`] = `
 <div>
   <div
-    class="transaction-status-label transaction-status-label--confirmed"
+    class="transaction-status-label"
   >
     June 1
   </div>
 </div>
 `;
 
-exports[`TransactionStatusLabel Component should render PENDING properly 1`] = `
+exports[`TransactionStatusLabel Component renders PENDING properly and tooltip 1`] = `
 <div>
   <div
-    class="transaction-status-label transaction-status-label--pending transaction-status-label--pending"
+    class="transaction-status-label transaction-status-label--pending"
   >
     Pending
   </div>
 </div>
 `;
 
-exports[`TransactionStatusLabel Component should render QUEUED properly 1`] = `
+exports[`TransactionStatusLabel Component renders QUEUED properly and tooltip 1`] = `
 <div>
   <div
-    class="transaction-status-label transaction-status-label--queued transaction-status-label--queued"
+    class="transaction-status-label transaction-status-label--queued"
   >
     Queued
   </div>
 </div>
 `;
 
-exports[`TransactionStatusLabel Component should render SIGNING if status is approved 1`] = `
+exports[`TransactionStatusLabel Component renders SIGNING properly and tooltip 1`] = `
 <div>
   <div
-    class="transaction-status-label transaction-status-label--signing"
+    class="transaction-status-label"
   >
     Signing
   </div>
 </div>
 `;
 
-exports[`TransactionStatusLabel Component should render UNAPPROVED properly 1`] = `
+exports[`TransactionStatusLabel Component renders UNAPPROVED properly and tooltip 1`] = `
 <div>
   <div
-    class="transaction-status-label transaction-status-label--unapproved transaction-status-label--unapproved"
+    class="transaction-status-label transaction-status-label--unapproved"
   >
     Unapproved
   </div>